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 78760239 for ; Tue, 2 Oct 2018 08:54:17 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 75ECC940080; Tue, 2 Oct 2018 06:54:15 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Tue, 2 Oct 2018 07:54:04 +0100 To: Jeff Guo , , , , , , , , , , , , , , CC: , , , 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> <0b707332-aedc-36b6-d7cb-ac1fd69d9364@intel.com> From: Andrew Rybchenko Message-ID: Date: Tue, 2 Oct 2018 09:53:20 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 MIME-Version: 1.0 In-Reply-To: <0b707332-aedc-36b6-d7cb-ac1fd69d9364@intel.com> Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24130.003 X-TM-AS-Result: No-14.492900-8.000000-10 X-TMASE-MatchedRID: 9d2LtCNB3NIOwH4pD14DsPHkpkyUphL9Ct59Uh3p/NW0rcU5V/oSe0NF JC6C2PHm8w+Ie/GWeQczf0oEV+yOAKR59JeT+K51ZwmQqXe/8sPGV2zyh/SpN8UmcSma304Tmyc Y/8v++6Q3/OfmOs3Txr9S9UHiL5NitRrZmY+hVK0flhDI6DvVlkyQ5fRSh2650zabL4+/4ct1gn ekL1c23n6rbXgHo+wchd0xaOehFSlvC3T5Ka3phwlPus/MSqC7gHzgwy8qV5qOQJOkHmszf2kwY Sl1W+8HSe/pmcWQNTIFJcPaPXKIsR7emFLR3eMKHcQQBuf4ZFtoipawfE0g6kuCjz4ggdtw/wTb MANBmxM7OJE/RLyfLlwcONb9dh7dWcbhowRxPdNoOA9kFf9sy+q9xwosX7inDC/Vm90If4WGY82 HNi0Wvva8+G6VxpdYgWg8WzvAdS0FrDF4OCzISTSn5RJdzqcI8rT9Xkv1QL07LF3pX3rdVLjxa5 EVBV1q585VzGMOFzABi3kqJOK62XnN0DN7HnFmyYy/8vjdx4ZFJ88CgryMLGNTrkVii9aCR3qvv 4gn6kTAMUJ7yaTqxt7dTKWhGrhM2JIdasvD7ENA5zIT/BBBpNZvnnw4lurlftwZ3X11IV0= X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--14.492900-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24130.003 X-MDID: 1538463256-kEy1GrcMaIao Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v2 2/4] eal: modify device event callback process func 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: Tue, 02 Oct 2018 06:54:17 -0000 On 10/2/18 7:45 AM, Jeff Guo wrote: > 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. Yes, it would be good. Thanks. > > >>> 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. It is OK to have line longer than 80 symbols in this case. As far as I know checkpatches.sh accepts it and does not generate any errors/warnings. >>> +} >>> 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; >>