From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yuanhan Liu Subject: Re: [PATCH 7/9] net/virtio: rationalize setting of Rx/Tx handlers Date: Thu, 7 Sep 2017 16:13:30 +0800 Message-ID: <20170907081330.GZ9736@yliu-home> 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> <20170906144010.weoqqjzdmvnj7lwy@neon> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org, maxime.coquelin@redhat.com, stephen@networkplumber.org, stable@dpdk.org To: Olivier MATZ Return-path: Received: from mail-pf0-f182.google.com (mail-pf0-f182.google.com [209.85.192.182]) by dpdk.org (Postfix) with ESMTP id 60E2D7D4E for ; Thu, 7 Sep 2017 10:13:40 +0200 (CEST) Received: by mail-pf0-f182.google.com with SMTP id e199so16650037pfh.3 for ; Thu, 07 Sep 2017 01:13:40 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20170906144010.weoqqjzdmvnj7lwy@neon> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Sep 06, 2017 at 04:40:12PM +0200, Olivier MATZ wrote: > 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. Yes, you are right. --yliu