From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id 8BD731B3EE for ; Mon, 9 Jul 2018 15:14:45 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (webmail.solarflare.com [12.187.104.26]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1-us3.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id AB5C4B40068; Mon, 9 Jul 2018 13:14:43 +0000 (UTC) Received: from [192.168.1.16] (85.187.13.33) by ocex03.SolarFlarecom.com (10.20.40.36) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Mon, 9 Jul 2018 06:14:33 -0700 To: Jeff Guo , , , , , , , , , , , , , , , CC: , , , References: <1530787185-5915-1-git-send-email-jia.guo@intel.com> <1531136777-9815-1-git-send-email-jia.guo@intel.com> <1531136777-9815-2-git-send-email-jia.guo@intel.com> From: Andrew Rybchenko Message-ID: Date: Mon, 9 Jul 2018 16:14:25 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <1531136777-9815-2-git-send-email-jia.guo@intel.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Originating-IP: [85.187.13.33] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ocex03.SolarFlarecom.com (10.20.40.36) X-MDID: 1531142084-9q7zLQYbqv84 Subject: Re: [dpdk-dev] [PATCH v3 1/4] ethdev: Add eal device event callback X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Jul 2018 13:14:45 -0000 On 09.07.2018 14:46, Jeff Guo wrote: > Implement a eal device event callback "rte_eth_dev_event_callback" > in ethdev, it could let pmd driver have chance to manage the eal > device event, such as process hotplug event.  > > Signed-off-by: Jeff Guo > --- > v3->v2: > add new callback in ethdev > --- > doc/guides/rel_notes/release_18_08.rst | 8 ++++++++ > lib/librte_ethdev/rte_ethdev.c | 37 ++++++++++++++++++++++++++++++++++ > lib/librte_ethdev/rte_ethdev_driver.h | 20 ++++++++++++++++++ > 3 files changed, 65 insertions(+) > > diff --git a/doc/guides/rel_notes/release_18_08.rst b/doc/guides/rel_notes/release_18_08.rst > index bc01242..2326058 100644 > --- a/doc/guides/rel_notes/release_18_08.rst > +++ b/doc/guides/rel_notes/release_18_08.rst > @@ -46,6 +46,14 @@ New Features > Flow API support has been added to CXGBE Poll Mode Driver to offload > flows to Chelsio T5/T6 NICs. > > +* **Added eal device event callback in ethdev for hotplug.** > + > + Implement a eal device event callback in ethdev, it could let pmd driver "pmd driver" sounds strange since PMD stands for poll-mode driver. > + have chance to manage the eal device event, such as process hotplug event. > + > + * ``rte_eth_dev_event_callback`` for driver use to register it and process > + eal device event. > + > > API Changes > ----------- > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c > index a9977df..36f218a 100644 > --- a/lib/librte_ethdev/rte_ethdev.c > +++ b/lib/librte_ethdev/rte_ethdev.c > @@ -4518,6 +4518,43 @@ rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da) > return result; > } > > +void __rte_experimental > +rte_eth_dev_event_callback(char *device_name, enum rte_dev_event_type type, > + void *arg) > +{ > + struct rte_eth_dev *eth_dev = (struct rte_eth_dev *)arg; > + > + if (type >= RTE_DEV_EVENT_MAX) { > + fprintf(stderr, "%s called upon invalid event %d\n", > + __func__, type); > + fflush(stderr); I'd like to understand why fprintf() is used here for logging instead of rte_log mechanisms. Also if we really want the log, may be it make sense to move the if to default case below. > + } > + > + switch (type) { > + case RTE_DEV_EVENT_REMOVE: > + ethdev_log(INFO, "The device: %s has been removed!\n", > + device_name); > + > + if (!device_name || !eth_dev) > + return; > + > + if (!(eth_dev->data->dev_flags & RTE_ETH_EVENT_INTR_RMV)) > + return; > + > + if (!strcmp(device_name, eth_dev->device->name)) Do we really need to check it? The callback is registered for devices with such name, so it should be always true. May be it is OK to double-check I just want to be sure that I understand it properly. > + _rte_eth_dev_callback_process(eth_dev, > + RTE_ETH_EVENT_INTR_RMV, > + NULL); > + break; > + case RTE_DEV_EVENT_ADD: > + ethdev_log(INFO, "The device: %s has been added!\n", > + device_name); > + break; > + default: > + break; > + } > +} > + > RTE_INIT(ethdev_init_log); > static void > ethdev_init_log(void) > diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h > index c9c825e..fed5afa 100644 > --- a/lib/librte_ethdev/rte_ethdev_driver.h > +++ b/lib/librte_ethdev/rte_ethdev_driver.h > @@ -82,6 +82,26 @@ int rte_eth_dev_release_port(struct rte_eth_dev *eth_dev); > void _rte_eth_dev_reset(struct rte_eth_dev *dev); > > /** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice. > + * > + * Implement a rte eth eal device event callbacks for the specific device. > + * > + * @param device_name > + * Pointer to the name of the rte device. Is it name of the device which generates the event? If so, it should be highlighted. > + * @param event > + * Eal device event type. > + * @param ret_param > + * To pass data back to user application. > + * > + * @return > + * void > + */ > +void __rte_experimental > +rte_eth_dev_event_callback(char *device_name, > + enum rte_dev_event_type event, void *cb_arg); > + > +/** > * @internal Executes all the user application registered callbacks for > * the specific device. It is for DPDK internal user only. User > * application should not call it directly.