From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id D2FAA1B456 for ; Thu, 4 Oct 2018 15:57:40 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Oct 2018 06:57:39 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,340,1534834800"; d="scan'208";a="85727723" Received: from jguo15x-mobl.ccr.corp.intel.com (HELO [10.249.174.2]) ([10.249.174.2]) by FMSMGA003.fm.intel.com with ESMTP; 04 Oct 2018 06:53:12 -0700 To: "Iremonger, Bernard" , "stephen@networkplumber.org" , "Richardson, Bruce" , "Yigit, Ferruh" , "Ananyev, Konstantin" , "gaetan.rivet@6wind.com" , "Wu, Jingjing" , "thomas@monjalon.net" , "motih@mellanox.com" , "matan@mellanox.com" , "Van Haaren, Harry" , "Zhang, Qi Z" , "He, Shaopeng" , "arybchenko@solarflare.com" , "Lu, Wenzhuo" , "Burakov, Anatoly" , "jerin.jacob@caviumnetworks.com" Cc: "jblunck@infradead.org" , "shreyansh.jain@nxp.com" , "dev@dpdk.org" , "Zhang, Helin" References: <1498711073-42917-1-git-send-email-jia.guo@intel.com> <1538634659-91335-1-git-send-email-jia.guo@intel.com> <1538634659-91335-8-git-send-email-jia.guo@intel.com> <8CEF83825BEC744B83065625E567D7C260D0C830@IRSMSX107.ger.corp.intel.com> From: Jeff Guo Message-ID: <0139ad2b-004d-dd0e-425a-edc5a4a3045b@intel.com> Date: Thu, 4 Oct 2018 21:53:10 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.0 MIME-Version: 1.0 In-Reply-To: <8CEF83825BEC744B83065625E567D7C260D0C830@IRSMSX107.ger.corp.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Subject: Re: [dpdk-dev] [PATCH v13 7/7] app/testpmd: use hotplug failure handler 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: Thu, 04 Oct 2018 13:57:41 -0000 thanks for your kindly review. On 10/4/2018 6:31 PM, Iremonger, Bernard wrote: > Hi Jeff, > >> -----Original Message----- >> From: Guo, Jia >> Sent: Thursday, October 4, 2018 7:31 AM >> To: stephen@networkplumber.org; Richardson, Bruce >> ; Yigit, Ferruh ; Ananyev, >> Konstantin ; gaetan.rivet@6wind.com; Wu, >> Jingjing ; thomas@monjalon.net; >> motih@mellanox.com; matan@mellanox.com; Van Haaren, Harry >> ; Zhang, Qi Z ; He, >> Shaopeng ; Iremonger, Bernard >> ; arybchenko@solarflare.com; Lu, Wenzhuo >> ; Burakov, Anatoly ; >> jerin.jacob@caviumnetworks.com >> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo, Jia >> ; Zhang, Helin >> Subject: [PATCH v13 7/7] app/testpmd: use hotplug failure handler >> >> This patch use testpmd for example, to show how an app smoothly handle >> failure when device be hot-unplug. Except that app should enabled the device >> event monitor and register the hotplug event’s callback, it also need enable >> hotplug handle mechanism before running. Once app detect the removal event, >> the hot-unplug callback would be called. It will first stop the packet forwarding, >> then stop the port, close the port, and finally detach the port to clean the device >> and release the resources. >> >> Signed-off-by: Jeff Guo >> --- >> v13->v12: >> delete needless helper in app. >> --- >> app/test-pmd/testpmd.c | 86 +++++++++++++++++++++++-------------------------- >> - >> 1 file changed, 40 insertions(+), 46 deletions(-) >> >> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index >> 001f0e5..f3f8e44 100644 >> --- a/app/test-pmd/testpmd.c >> +++ b/app/test-pmd/testpmd.c >> @@ -434,9 +434,6 @@ static int eth_event_callback(portid_t port_id, static >> void eth_dev_event_callback(char *device_name, >> enum rte_dev_event_type type, >> void *param); >> -static int eth_dev_event_callback_register(void); >> -static int eth_dev_event_callback_unregister(void); >> - >> >> /* >> * Check if all the ports are started. >> @@ -1954,39 +1951,6 @@ reset_port(portid_t pid) >> printf("Done\n"); >> } >> >> -static int >> -eth_dev_event_callback_register(void) >> -{ >> - int ret; >> - >> - /* register the device event callback */ >> - ret = rte_dev_event_callback_register(NULL, >> - eth_dev_event_callback, NULL); >> - if (ret) { >> - printf("Failed to register device event callback\n"); >> - return -1; >> - } >> - >> - return 0; >> -} >> - >> - >> -static int >> -eth_dev_event_callback_unregister(void) >> -{ >> - int ret; >> - >> - /* unregister the device event callback */ >> - ret = rte_dev_event_callback_unregister(NULL, >> - eth_dev_event_callback, NULL); >> - if (ret < 0) { >> - printf("Failed to unregister device event callback\n"); >> - return -1; >> - } >> - >> - return 0; >> -} >> - >> void >> attach_port(char *identifier) >> { >> @@ -2093,14 +2057,25 @@ pmd_test_exit(void) >> >> if (hot_plug) { >> ret = rte_dev_event_monitor_stop(); >> - if (ret) >> + if (ret) { >> RTE_LOG(ERR, EAL, >> "fail to stop device event monitor."); >> + return; >> + } >> >> - ret = eth_dev_event_callback_unregister(); >> - if (ret) >> + ret = rte_dev_event_callback_unregister(NULL, >> + eth_dev_event_callback, NULL); >> + if (ret < 0) { >> + printf("fail to unregister device event callback.\n"); > Better to use RTE_LOG() instead of printf(). > >> + return; >> + } >> + >> + ret = rte_dev_hotplug_handle_disable(); >> + if (ret) { >> RTE_LOG(ERR, EAL, >> - "fail to unregister all event callbacks."); >> + "fail to disable hotplug handling.\n"); >> + return; >> + } >> } >> >> printf("\nBye...\n"); >> @@ -2244,6 +2219,9 @@ static void >> eth_dev_event_callback(char *device_name, enum rte_dev_event_type type, >> __rte_unused void *arg) >> { >> + uint16_t port_id; >> + int ret; >> + >> if (type >= RTE_DEV_EVENT_MAX) { >> fprintf(stderr, "%s called upon invalid event %d\n", >> __func__, type); >> @@ -2254,9 +2232,13 @@ eth_dev_event_callback(char *device_name, enum >> rte_dev_event_type type, >> case RTE_DEV_EVENT_REMOVE: >> RTE_LOG(ERR, EAL, "The device: %s has been removed!\n", >> device_name); >> - /* TODO: After finish failure handle, begin to stop >> - * packet forward, stop port, close port, detach port. >> - */ >> + ret = rte_eth_dev_get_port_by_name(device_name, &port_id); >> + if (ret) { >> + RTE_LOG(ERR, EAL, "can not get port by device %s!\n", >> + device_name); >> + return; >> + } >> + rmv_event_callback((void *)(intptr_t)port_id); >> break; >> case RTE_DEV_EVENT_ADD: >> RTE_LOG(ERR, EAL, "The device: %s has been added!\n", @@ - >> 2779,14 +2761,26 @@ main(int argc, char** argv) >> init_config(); >> >> if (hot_plug) { >> - /* enable hot plug monitoring */ >> + ret = rte_dev_hotplug_handle_enable(); >> + if (ret) { >> + RTE_LOG(ERR, EAL, >> + "fail to enable hotplug handling."); >> + return -1; >> + } >> + >> ret = rte_dev_event_monitor_start(); >> if (ret) { >> - rte_errno = EINVAL; >> + RTE_LOG(ERR, EAL, >> + "fail to start device event monitoring."); >> return -1; >> } >> - eth_dev_event_callback_register(); >> >> + ret = rte_dev_event_callback_register(NULL, >> + eth_dev_event_callback, NULL); >> + if (ret) { >> + printf("faile to register device event callback\n"); > Better to use RTE_LOG() instead of peintf(). Note type in message, "faile" should be "failed" > >> + return -1; >> + } >> } >> >> if (start_port(RTE_PORT_ALL) != 0) >> -- >> 2.7.4