From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Blunck Subject: Re: [PATCH 2/2] Move non-PCI related eth_dev initialization to rte_eth_dev_allocate() Date: Thu, 17 Nov 2016 17:47:38 +0100 Message-ID: References: <1479392685-19608-1-git-send-email-jblunck@infradead.org> <1479392685-19608-2-git-send-email-jblunck@infradead.org> <5260eda3-89ed-ba1f-284e-5861bac2c724@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: dev@dpdk.org To: Ferruh Yigit Return-path: Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by dpdk.org (Postfix) with ESMTP id B09393777 for ; Thu, 17 Nov 2016 17:47:39 +0100 (CET) Received: by mail-wm0-f65.google.com with SMTP id m203so22893611wma.3 for ; Thu, 17 Nov 2016 08:47:39 -0800 (PST) In-Reply-To: <5260eda3-89ed-ba1f-284e-5861bac2c724@intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Nov 17, 2016 at 4:46 PM, Ferruh Yigit wrote: > On 11/17/2016 2:24 PM, Jan Blunck wrote: >> This moves the non-PCI related initialization of the link state interrupt >> callback list and the setting of the default MTU to rte_eth_dev_allocate() >> so that drivers only need to set non-default values. >> >> Signed-off-by: Jan Blunck >> --- >> drivers/net/bonding/rte_eth_bond_api.c | 2 -- >> drivers/net/cxgbe/cxgbe_main.c | 2 -- >> drivers/net/mlx4/mlx4.c | 2 -- >> drivers/net/mlx5/mlx5.c | 3 --- >> drivers/net/null/rte_eth_null.c | 2 -- >> drivers/net/ring/rte_eth_ring.c | 2 -- >> drivers/net/vhost/rte_eth_vhost.c | 2 -- >> lib/librte_ether/rte_ethdev.c | 12 ++++-------- >> 8 files changed, 4 insertions(+), 23 deletions(-) > > I think following also redundant and can be removed: > app/test/virtual_pmd.c: > 604: TAILQ_INIT(&(eth_dev->link_intr_cbs)); > > <...> > >> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c >> index 12af4b1..f58a995 100644 >> --- a/lib/librte_ether/rte_ethdev.c >> +++ b/lib/librte_ether/rte_ethdev.c > > What do you think doing same thing for rte_cryptodev.c J > Thanks for the review Ferruh. I'll fixup the patches and resend. I'm currently looking in the rte_bus and rte_eth_dev stuff. If nobody volunteers to do the changes for cryptodev I can take a look at some later point. Thanks, Jan >> @@ -215,6 +215,10 @@ rte_eth_dev_allocate(const char *name) >> memset(eth_dev->data, 0, sizeof(*eth_dev->data)); >> snprintf(eth_dev->data->name, sizeof(eth_dev->data->name), "%s", name); >> eth_dev->data->port_id = port_id; >> + eth_dev->data->rx_mbuf_alloc_failed = 0; > > This is no more required, because of memset > >> + eth_dev->data->mtu = ETHER_MTU; >> + TAILQ_INIT(&(eth_dev->link_intr_cbs)); >> + >> eth_dev->attached = DEV_ATTACHED; >> eth_dev_last_created_port = port_id; >> nb_ports++; >> @@ -261,14 +265,6 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv, >> eth_dev->pci_dev = pci_dev; >> eth_dev->driver = eth_drv; >> >> - /* init user callbacks */ >> - TAILQ_INIT(&(eth_dev->link_intr_cbs)); >> - >> - /* >> - * Set the default MTU. >> - */ >> - eth_dev->data->mtu = ETHER_MTU; >> - >> /* Invoke PMD device initialization function */ >> diag = (*eth_drv->eth_dev_init)(eth_dev); >> if (diag == 0) >> >