From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f53.google.com (mail-wg0-f53.google.com [74.125.82.53]) by dpdk.org (Postfix) with ESMTP id B7B58B64D for ; Tue, 17 Feb 2015 01:56:48 +0100 (CET) Received: by mail-wg0-f53.google.com with SMTP id a1so15693806wgh.12 for ; Mon, 16 Feb 2015 16:56:48 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=KK8lfBYUyIyli7B2tml9LXCD/o72WF+cmAXb3KumEaY=; b=UuZ0y67yxS5fgCuUjtkNYxkocmSv5CYBt+LO8eTRiiCjZwweT8bTknw0FvZB3Psk2f n+wKyERYh5ZXSsiMTdFVgqvLLEL+Enb6jHMlINW6n/4k/sLILBEY1wPAGZln1fEXyQpg MyE6wACsGaZJL6pYxzvEWKsMxAfShTmTtHxzjevp5uQygch1Rp3DTH2BcEHCq9Ow2SUe izw1KyULmRTg9BhCHTEYaBMvMa/2PpXLMD5dZDHYYgg+YDq1QQizff7TXMq+cmGiH8cu j3/GFGSjHMFt57TGj+x6DXOlmBggUVV8RppO+cnj5IJvbwjfodAKfnL7Zndlli3smBKF oSuA== X-Gm-Message-State: ALoCoQn+Kfb3EhtZuA+LR57IwuG+deX9e/nm4Ejkg7X96zCU/qr2uLD/OPxvj3qbZlyBvczoF73W X-Received: by 10.180.10.131 with SMTP id i3mr52919457wib.54.1424134608578; Mon, 16 Feb 2015 16:56:48 -0800 (PST) Received: from xps13.localnet (guy78-1-82-235-116-147.fbx.proxad.net. [82.235.116.147]) by mx.google.com with ESMTPSA id ch6sm25088045wjc.3.2015.02.16.16.56.47 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 16 Feb 2015 16:56:47 -0800 (PST) From: Thomas Monjalon To: Tetsuya Mukawa Date: Tue, 17 Feb 2015 01:56:16 +0100 Message-ID: <22631413.IesiHpntxK@xps13> Organization: 6WIND User-Agent: KMail/4.14.4 (Linux/3.18.4-1-ARCH; KDE/4.14.4; x86_64; ; ) In-Reply-To: <1424060073-23484-7-git-send-email-mukawa@igel.co.jp> References: <1423470639-15744-2-git-send-email-mukawa@igel.co.jp> <1424060073-23484-1-git-send-email-mukawa@igel.co.jp> <1424060073-23484-7-git-send-email-mukawa@igel.co.jp> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v8 06/14] eal, ethdev: Add a function and function pointers to close ether device X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Feb 2015 00:56:49 -0000 2015-02-16 13:14, Tetsuya Mukawa: > The patch adds function pointer to rte_pci_driver and eth_driver > structure. These function pointers are used when ports are detached. > Also, the patch adds rte_eth_dev_uninit(). So far, it's not called > by anywhere, but it will be called when port hotplug function is > implemented. > > v6: > - Fix rte_eth_dev_uninit() to handle a return value of uninit > function of PMD. > v4: > - Add parameter checking. > - Change function names. > > Signed-off-by: Tetsuya Mukawa > --- > lib/librte_eal/common/include/rte_pci.h | 7 +++++ > lib/librte_ether/rte_ethdev.c | 47 +++++++++++++++++++++++++++++++++ > lib/librte_ether/rte_ethdev.h | 24 +++++++++++++++++ > 3 files changed, 78 insertions(+) > > diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h > index 4814cd7..87ca4cf 100644 > --- a/lib/librte_eal/common/include/rte_pci.h > +++ b/lib/librte_eal/common/include/rte_pci.h > @@ -189,12 +189,19 @@ struct rte_pci_driver; > typedef int (pci_devinit_t)(struct rte_pci_driver *, struct rte_pci_device *); > > /** > + * Uninitialisation function for the driver called during hotplugging. > + */ > +typedef int (pci_devuninit_t)( > + struct rte_pci_driver *, struct rte_pci_device *); > + > +/** > * A structure describing a PCI driver. > */ > struct rte_pci_driver { > TAILQ_ENTRY(rte_pci_driver) next; /**< Next in list. */ > const char *name; /**< Driver name. */ > pci_devinit_t *devinit; /**< Device init. function. */ > + pci_devuninit_t *devuninit; /**< Device uninit function. */ > struct rte_pci_id *id_table; /**< ID table, NULL terminated. */ > uint32_t drv_flags; /**< Flags contolling handling of device. */ > }; > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index 2463d18..58d8072 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -326,6 +326,52 @@ rte_eth_dev_init(struct rte_pci_driver *pci_drv, > return diag; > } > > +static int > +rte_eth_dev_uninit(struct rte_pci_driver *pci_drv, > + struct rte_pci_device *pci_dev) There's something strange here: this is an uninit of an ethdev but the parameter is a pci_dev. The driver parameter shouldn't be needed. > +{ > + struct eth_driver *eth_drv; > + struct rte_eth_dev *eth_dev; > + char ethdev_name[RTE_ETH_NAME_MAX_LEN]; > + int ret; > + > + if ((pci_drv == NULL) || (pci_dev == NULL)) > + return -EINVAL; > + > + /* Create unique Ethernet device name using PCI address */ > + snprintf(ethdev_name, RTE_ETH_NAME_MAX_LEN, "%d:%d.%d", > + pci_dev->addr.bus, pci_dev->addr.devid, > + pci_dev->addr.function); You should call a function common to init and uninit to generate the same unique name. > + > + eth_dev = rte_eth_dev_allocated(ethdev_name); > + if (eth_dev == NULL) > + return -ENODEV; > + > + eth_drv = (struct eth_driver *)pci_drv; > + > + /* Invoke PMD device uninit function */ > + if (*eth_drv->eth_dev_uninit) { > + ret = (*eth_drv->eth_dev_uninit)(eth_drv, eth_dev); > + if (ret) > + return ret; > + } > + > + /* free ether device */ > + rte_eth_dev_free(eth_dev); > + > + /* init user callbacks */ > + TAILQ_INIT(&(eth_dev->callbacks)); Please comment more why you are resetting callbacks. An init in an uninit function seems weird ;) > + > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) > + rte_free(eth_dev->data->dev_private); > + > + eth_dev->pci_dev = NULL; > + eth_dev->driver = NULL; > + eth_dev->data = NULL; > + > + return 0; > +} > + > /** > * Register an Ethernet [Poll Mode] driver. > * > @@ -344,6 +390,7 @@ void > rte_eth_driver_register(struct eth_driver *eth_drv) > { > eth_drv->pci_drv.devinit = rte_eth_dev_init; > + eth_drv->pci_drv.devuninit = rte_eth_dev_uninit; > rte_eal_pci_register(ð_drv->pci_drv); > } > > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h > index fbe7ac1..91d9e86 100644 > --- a/lib/librte_ether/rte_ethdev.h > +++ b/lib/librte_ether/rte_ethdev.h > @@ -1678,6 +1678,27 @@ typedef int (*eth_dev_init_t)(struct eth_driver *eth_drv, > > /** > * @internal > + * Finalization function of an Ethernet driver invoked for each matching > + * Ethernet PCI device detected during the PCI closing phase. > + * > + * @param eth_drv > + * The pointer to the [matching] Ethernet driver structure supplied by > + * the PMD when it registered itself. > + * @param eth_dev > + * The *eth_dev* pointer is the address of the *rte_eth_dev* structure > + * associated with the matching device and which have been [automatically] > + * allocated in the *rte_eth_devices* array. > + * @return > + * - 0: Success, the device is properly finalized by the driver. > + * In particular, the driver MUST free the *dev_ops* pointer > + * of the *eth_dev* structure. > + * - <0: Error code of the device initialization failure. > + */ > +typedef int (*eth_dev_uninit_t)(struct eth_driver *eth_drv, > + struct rte_eth_dev *eth_dev); > + > +/** > + * @internal > * The structure associated with a PMD Ethernet driver. > * > * Each Ethernet driver acts as a PCI driver and is represented by a generic > @@ -1687,11 +1708,14 @@ typedef int (*eth_dev_init_t)(struct eth_driver *eth_drv, > * > * - The *eth_dev_init* function invoked for each matching PCI device. > * > + * - The *eth_dev_uninit* function invoked for each matching PCI device. > + * > * - The size of the private data to allocate for each matching device. > */ > struct eth_driver { > struct rte_pci_driver pci_drv; /**< The PMD is also a PCI driver. */ > eth_dev_init_t eth_dev_init; /**< Device init function. */ > + eth_dev_uninit_t eth_dev_uninit; /**< Device uninit function. */ > unsigned int dev_private_size; /**< Size of device private data. */ > }; > >