[V3,4/4] app/testpmd: show example to handle hot unplug

Message ID 1530027372-24233-4-git-send-email-jia.guo@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [V3,1/4] bus/pci: handle device hot unplug |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Guo, Jia June 26, 2018, 3:36 p.m. UTC
  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 <jia.guo@intel.com>
---
v3->v2:
delete some unused check
---
 app/test-pmd/testpmd.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)
  

Comments

Matan Azrad June 26, 2018, 5:07 p.m. UTC | #1
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
> <thomas@monjalon.net>; Mordechay Haimovsky <motih@mellanox.com>;
> Matan Azrad <matan@mellanox.com>; 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 <jia.guo@intel.com>
> ---
> 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?

>  	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.

>  	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.

> @@ -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
  
Guo, Jia June 27, 2018, 3:56 a.m. UTC | #2
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
>> <thomas@monjalon.net>; Mordechay Haimovsky <motih@mellanox.com>;
>> Matan Azrad <matan@mellanox.com>; 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 <jia.guo@intel.com>
>> ---
>> 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
  
Matan Azrad June 27, 2018, 6:05 a.m. UTC | #3
Hi Guo

From: Guo, Jia
> Sent: Wednesday, June 27, 2018 6:56 AM
> To: Matan Azrad <matan@mellanox.com>; 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 <thomas@monjalon.net>;
> Mordechay Haimovsky <motih@mellanox.com>; 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;
> helin.zhang@intel.com
> Subject: Re: [PATCH V3 4/4] app/testpmd: show example to handle hot unplug
> 
> 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
> >> <thomas@monjalon.net>; Mordechay Haimovsky <motih@mellanox.com>;
> >> Matan Azrad <matan@mellanox.com>; 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 <jia.guo@intel.com>
> >> ---
> >> 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?

I think you miss here something, you are getting the port id from ethdev, you are just passing a pointer to get it.
I think you should remove this change too.

> 
> >>   	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?

So, Are you suggesting to insert a bug and then to fix it ?:)

My suggestion:
Add a prior patch to depend the ethdev RMV event by a user parameter (can be your hotplug parameter and should be true by default).
In this patch add one more mode to the parameter to enable hotplug by the EAL. 

So finally the options of hotplug parameter can be:
0 - for no hotplug handle.
1 - ethdev hotplug (should be the default)
2 - EAL hotplug

What do you think?

> >> @@ -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
  
Guo, Jia June 29, 2018, 10:26 a.m. UTC | #4
matan


On 6/27/2018 2:05 PM, Matan Azrad wrote:
> Hi Guo
>
> From: Guo, Jia
>> Sent: Wednesday, June 27, 2018 6:56 AM
>> To: Matan Azrad <matan@mellanox.com>; 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 <thomas@monjalon.net>;
>> Mordechay Haimovsky <motih@mellanox.com>; 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;
>> helin.zhang@intel.com
>> Subject: Re: [PATCH V3 4/4] app/testpmd: show example to handle hot unplug
>>
>> 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
>>>> <thomas@monjalon.net>; Mordechay Haimovsky <motih@mellanox.com>;
>>>> Matan Azrad <matan@mellanox.com>; 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 <jia.guo@intel.com>
>>>> ---
>>>> 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?
> I think you miss here something, you are getting the port id from ethdev, you are just passing a pointer to get it.
> I think you should remove this change too.

ok, seems i am missing something, let me check.

>>>>    	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?
> So, Are you suggesting to insert a bug and then to fix it ?:)
>
> My suggestion:
> Add a prior patch to depend the ethdev RMV event by a user parameter (can be your hotplug parameter and should be true by default).
> In this patch add one more mode to the parameter to enable hotplug by the EAL.
>
> So finally the options of hotplug parameter can be:
> 0 - for no hotplug handle.
> 1 - ethdev hotplug (should be the default)
> 2 - EAL hotplug
>
> What do you think?

sure, i think you absolutely know i don't want to add any bug here :) 
just want to make it more focus and clear.
your propose looks fine by me. good idea, thanks.
please check my v4 patch set.

>>>> @@ -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
  

Patch

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();
+
 	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();
 	}
+
 	no_link_check = 1;
 	stop_port(port_id);
 	no_link_check = org_no_link_check;
@@ -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)