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 56B631B59D for ; Wed, 27 Jun 2018 05:56:36 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Jun 2018 20:56:35 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,277,1526367600"; d="scan'208";a="240972334" Received: from jguo15x-mobl3.ccr.corp.intel.com (HELO [10.67.68.63]) ([10.67.68.63]) by fmsmga006.fm.intel.com with ESMTP; 26 Jun 2018 20:56:24 -0700 To: Matan Azrad , "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 , Mordechay Haimovsky , "harry.van.haaren@intel.com" , "qi.z.zhang@intel.com" , "shaopeng.he@intel.com" , "bernard.iremonger@intel.com" References: <1498711073-42917-1-git-send-email-jia.guo@intel.com> <1530027372-24233-1-git-send-email-jia.guo@intel.com> <1530027372-24233-4-git-send-email-jia.guo@intel.com> Cc: "jblunck@infradead.org" , "shreyansh.jain@nxp.com" , "dev@dpdk.org" , "helin.zhang@intel.com" From: "Guo, Jia" Message-ID: <94e4601e-0f20-2693-5672-64ede044c104@intel.com> Date: Wed, 27 Jun 2018 11:56:23 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH V3 4/4] app/testpmd: show example to handle hot unplug 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: Wed, 27 Jun 2018 03:56:38 -0000 hi, mantan On 6/27/2018 1:07 AM, Matan Azrad wrote: > Hi Jeff > > Continue session from last version + more comments\question. > > From: Jeff Guo >> Sent: Tuesday, June 26, 2018 6:36 PM >> To: 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 >> ; Mordechay Haimovsky ; >> Matan Azrad ; harry.van.haaren@intel.com; >> qi.z.zhang@intel.com; shaopeng.he@intel.com; bernard.iremonger@intel.com >> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; >> jia.guo@intel.com; helin.zhang@intel.com >> Subject: [PATCH V3 4/4] app/testpmd: show example to handle hot unplug >> >> Use testpmd for example, to show how an application smoothly handle failure >> when device being hot unplug. If app have enabled the device event monitor >> and register the hot plug event’s callback before running, once app detect the >> removal event, the callback would be called. It will first stop the packet >> forwarding, then stop the port, close the port, and finally detach the port to >> remove the device out from the device lists. >> >> Signed-off-by: Jeff Guo >> --- >> v3->v2: >> delete some unused check >> --- >> app/test-pmd/testpmd.c | 22 +++++++++++++++++----- >> 1 file changed, 17 insertions(+), 5 deletions(-) >> >> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index >> 24c1998..2ee5621 100644 >> --- a/app/test-pmd/testpmd.c >> +++ b/app/test-pmd/testpmd.c >> @@ -1951,9 +1951,10 @@ eth_dev_event_callback_unregister(void) >> void >> attach_port(char *identifier) >> { >> - portid_t pi = 0; >> unsigned int socket_id; >> >> + portid_t pi = rte_eth_dev_count_avail(); >> + > I don't understand this change... can you explain? think about if there are 2 or more device have been attached? The new device should not always add into port 0, right? >> printf("Attaching a new port...\n"); >> >> if (identifier == NULL) { >> @@ -2125,16 +2126,22 @@ check_all_ports_link_status(uint32_t port_mask) >> static void rmv_event_callback(void *arg) { >> + struct rte_eth_dev *dev; >> + >> int need_to_start = 0; >> int org_no_link_check = no_link_check; >> portid_t port_id = (intptr_t)arg; >> >> RTE_ETH_VALID_PORTID_OR_RET(port_id); >> + dev = &rte_eth_devices[port_id]; >> + >> + printf("removing device %s\n", dev->device->name); >> >> if (!test_done && port_is_forwarding(port_id)) { >> need_to_start = 1; >> stop_packet_forwarding(); >> } >> + > I don't think you need to change anything in this function. > You can add the print in the caller code. ok, i am fine for your point. >> no_link_check = 1; >> stop_port(port_id); >> no_link_check = org_no_link_check; > Suggestion for synchronization: > Don't register to ethdev RMV event if EAL hotplug is enabled. i think that what you propose might be a chose right now, and might need we think more about the better for all, but do you agree it is better split it in another fix patch, to let it patch focus on the feature propose and implement? >> @@ -2196,6 +2203,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); >> @@ -2206,9 +2216,12 @@ 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) { >> + printf("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", @@ - >> 2736,7 +2749,6 @@ main(int argc, char** argv) >> return -1; >> } >> eth_dev_event_callback_register(); >> - >> } >> >> if (start_port(RTE_PORT_ALL) != 0) >> -- >> 2.7.4