From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Guo Subject: Re: [PATCH v2 2/4] eal: modify device event callback process func Date: Tue, 2 Oct 2018 12:45:35 +0800 Message-ID: <0b707332-aedc-36b6-d7cb-ac1fd69d9364@intel.com> References: <1534503091-31910-1-git-send-email-jia.guo@intel.com> <1538316988-128382-1-git-send-email-jia.guo@intel.com> <1538316988-128382-3-git-send-email-jia.guo@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: jblunck@infradead.org, shreyansh.jain@nxp.com, dev@dpdk.org, helin.zhang@intel.com To: Andrew Rybchenko , stephen@networkplumber.org, bruce.richardson@intel.com, ferruh.yigit@intel.com, konstantin.ananyev@intel.com, gaetan.rivet@6wind.com, jingjing.wu@intel.com, thomas@monjalon.net, motih@mellanox.com, matan@mellanox.com, harry.van.haaren@intel.com, qi.z.zhang@intel.com, shaopeng.he@intel.com, bernard.iremonger@intel.com, anatoly.burakov@intel.com Return-path: Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 3E408239 for ; Tue, 2 Oct 2018 06:45:43 +0200 (CEST) In-Reply-To: Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" andrew, On 10/1/2018 5:46 PM, Andrew Rybchenko wrote: > On 9/30/18 5:16 PM, Jeff Guo wrote: >> This patch modify the device event callback process function name to be >> more explicit, change the variable to be const and exposure the API out >> from private eal. The bus drivers and eal device would directly use this >> API to process device event callback. > > Sorry, but from the description it is unclear what has changed to > make it necessary. > the reason is that not only eal device help will use the callback, but also vfio bus will use the callback to handle hot-unplug. So exposure these API to let vfio call it. I will add detail more at next version if need. >> Signed-off-by: Jeff Guo >> --- >> v2->v1: >> change the rte_dev_event_callback_prcess from internal to external api >> for bus or app usage. >> --- >> app/test-pmd/testpmd.c | 4 ++-- >> lib/librte_eal/bsdapp/eal/eal_dev.c | 8 ++++++++ >> lib/librte_eal/common/eal_common_dev.c | 5 +++-- >> lib/librte_eal/common/eal_private.h | 12 ------------ >> lib/librte_eal/common/include/rte_dev.h | 18 +++++++++++++++++- >> lib/librte_eal/linuxapp/eal/eal_dev.c | 2 +- >> lib/librte_eal/rte_eal_version.map | 1 + >> 7 files changed, 32 insertions(+), 18 deletions(-) >> >> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c >> index bfef483..1313100 100644 >> --- a/app/test-pmd/testpmd.c >> +++ b/app/test-pmd/testpmd.c >> @@ -431,7 +431,7 @@ static void check_all_ports_link_status(uint32_t port_mask); >> static int eth_event_callback(portid_t port_id, >> enum rte_eth_event_type type, >> void *param, void *ret_param); >> -static void eth_dev_event_callback(char *device_name, >> +static void eth_dev_event_callback(const char *device_name, >> enum rte_dev_event_type type, >> void *param); >> static int eth_dev_event_callback_register(void); >> @@ -2249,7 +2249,7 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param, >> >> /* This function is used by the interrupt thread */ >> static void >> -eth_dev_event_callback(char *device_name, enum rte_dev_event_type type, >> +eth_dev_event_callback(const char *device_name, enum rte_dev_event_type type, >> __rte_unused void *arg) >> { >> uint16_t port_id; >> diff --git a/lib/librte_eal/bsdapp/eal/eal_dev.c b/lib/librte_eal/bsdapp/eal/eal_dev.c >> index ae1c558..374e737 100644 >> --- a/lib/librte_eal/bsdapp/eal/eal_dev.c >> +++ b/lib/librte_eal/bsdapp/eal/eal_dev.c >> @@ -33,3 +33,11 @@ rte_dev_hotplug_handle_disable(void) >> RTE_LOG(ERR, EAL, "Device event is not supported for FreeBSD\n"); >> return -1; >> } >> + >> +void __rte_experimental >> +rte_dev_event_callback_process(const char *device_name, >> + enum rte_dev_event_type event) >> +{ >> + RTE_LOG(ERR, EAL, "Device event callback process is not supported " >> + "for FreeBSD\n"); > > Consider to avoid split of log message to simplify search using grep. > i think i got your point, but the 80 limitation a  line and the clearly meaning of the log should be the 1 prior to be considerate, right? It should be considerate but it is inevitable. >> +} >> diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c >> index 678dbca..2d610a4 100644 >> --- a/lib/librte_eal/common/eal_common_dev.c >> +++ b/lib/librte_eal/common/eal_common_dev.c >> @@ -342,8 +342,9 @@ rte_dev_event_callback_unregister(const char *device_name, >> return ret; >> } >> >> -void >> -dev_callback_process(char *device_name, enum rte_dev_event_type event) >> +void __rte_experimental >> +rte_dev_event_callback_process(const char *device_name, >> + enum rte_dev_event_type event) >> { >> struct dev_event_callback *cb_lst; >> >> diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h >> index 637f20d..47e8a33 100644 >> --- a/lib/librte_eal/common/eal_private.h >> +++ b/lib/librte_eal/common/eal_private.h >> @@ -259,18 +259,6 @@ struct rte_bus *rte_bus_find_by_device_name(const char *str); >> int rte_mp_channel_init(void); >> >> /** >> - * 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. >> - * >> - * @param device_name >> - * The device name. >> - * @param event >> - * the device event type. >> - */ >> -void dev_callback_process(char *device_name, enum rte_dev_event_type event); >> - >> -/** >> * @internal >> * Parse a device string and store its information in an >> * rte_devargs structure. >> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h >> index ff580a0..58fab43 100644 >> --- a/lib/librte_eal/common/include/rte_dev.h >> +++ b/lib/librte_eal/common/include/rte_dev.h >> @@ -39,7 +39,7 @@ struct rte_dev_event { >> char *devname; /**< device name */ >> }; >> >> -typedef void (*rte_dev_event_cb_fn)(char *device_name, >> +typedef void (*rte_dev_event_cb_fn)(const char *device_name, >> enum rte_dev_event_type event, >> void *cb_arg); >> >> @@ -438,6 +438,22 @@ rte_dev_event_callback_unregister(const char *device_name, >> * @warning >> * @b EXPERIMENTAL: this API may change without prior notice >> * >> + * Executes all the user application registered callbacks for >> + * the specific device. >> + * >> + * @param device_name >> + * The device name. >> + * @param event >> + * the device event type. >> + */ >> +void __rte_experimental >> +rte_dev_event_callback_process(const char *device_name, >> + enum rte_dev_event_type event); >> + >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change without prior notice >> + * >> * Start the device event monitoring. >> * >> * @return >> diff --git a/lib/librte_eal/linuxapp/eal/eal_dev.c b/lib/librte_eal/linuxapp/eal/eal_dev.c >> index 9f9e1cf..01e3a04 100644 >> --- a/lib/librte_eal/linuxapp/eal/eal_dev.c >> +++ b/lib/librte_eal/linuxapp/eal/eal_dev.c >> @@ -271,7 +271,7 @@ dev_uev_handler(__rte_unused void *param) >> return; >> } >> } >> - dev_callback_process(uevent.devname, uevent.type); >> + rte_dev_event_callback_process(uevent.devname, uevent.type); >> } >> } >> >> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map >> index a3255aa..b96da50 100644 >> --- a/lib/librte_eal/rte_eal_version.map >> +++ b/lib/librte_eal/rte_eal_version.map >> @@ -277,6 +277,7 @@ EXPERIMENTAL { >> rte_class_register; >> rte_class_unregister; >> rte_ctrl_thread_create; >> + rte_dev_event_callback_process; >> rte_dev_event_callback_register; >> rte_dev_event_callback_unregister; >> rte_dev_event_monitor_start; >