From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 1ED5C100F; Wed, 6 Sep 2017 16:40:30 +0200 (CEST) Received: from lfbn-1-18623-73.w90-103.abo.wanadoo.fr ([90.103.154.73] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1dpbaU-0003BN-2F; Wed, 06 Sep 2017 16:46:04 +0200 Received: by droids-corp.org (sSMTP sendmail emulation); Wed, 06 Sep 2017 16:40:12 +0200 Date: Wed, 6 Sep 2017 16:40:12 +0200 From: Olivier MATZ To: Yuanhan Liu Cc: dev@dpdk.org, maxime.coquelin@redhat.com, stephen@networkplumber.org, stable@dpdk.org Message-ID: <20170906144010.weoqqjzdmvnj7lwy@neon> References: <20170831134015.1383-1-olivier.matz@6wind.com> <20170831134015.1383-8-olivier.matz@6wind.com> <20170901091916.GT9736@yliu-home> <20170901095216.kqx7lwdiktq7y5zs@neon> <20170901123106.GW9736@yliu-home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170901123106.GW9736@yliu-home> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-stable] [PATCH 7/9] net/virtio: rationalize setting of Rx/Tx handlers X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Sep 2017 14:40:31 -0000 On Fri, Sep 01, 2017 at 08:31:06PM +0800, Yuanhan Liu wrote: > On Fri, Sep 01, 2017 at 11:52:17AM +0200, Olivier MATZ wrote: > > > > @@ -1534,7 +1554,6 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev) > > > > RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr_mrg_rxbuf)); > > > > > > > > eth_dev->dev_ops = &virtio_eth_dev_ops; > > > > - eth_dev->tx_pkt_burst = &virtio_xmit_pkts; > > > > > > > > if (rte_eal_process_type() == RTE_PROC_SECONDARY) { > > > > if (!hw->virtio_user_dev) { > > > > @@ -1544,12 +1563,8 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev) > > > > } > > > > > > > > virtio_set_vtpci_ops(hw); > > > > - if (hw->use_simple_rxtx) { > > > > - eth_dev->tx_pkt_burst = virtio_xmit_pkts_simple; > > > > - eth_dev->rx_pkt_burst = virtio_recv_pkts_vec; > > > > - } else { > > > > - rx_func_get(eth_dev); > > > > - } > > > > + set_rxtx_funcs(eth_dev); > > > > > > No need to invoke it here? > > > > I wanted to stay consistent with previous code. > > > > I'm not very used to work with secondary processes, so I'm not 100% it > > is ok, but in my understanding, in that case the configuration is done > > first by the primary process, and the secondary the pointers to the > > rx/tx funcs. I suppose their value can be different than primary ones. > > It probably needs some testing, but I think it should be okay, because > the rx/tx funcs will always be updated at dev_start in this patch. I still have one doubt about this: looking in examples/multi_process/symmetric_mp/main.c, I can see that rte_eth_dev_start() is only called on the primary process. So if I remove set_rxtx_funcs() from eth_virtio_dev_init(), it looks that the rx functions won't be initialized. Again, I'm not a user of multi_proc, so I may be wrong, but I think it would be safer to keep it. Olivier