[dpdk-dev] [PATCH 3/3] app/testpmd: fix callback issue for hot-unplug

Jeff Guo jia.guo at intel.com
Wed Nov 7 08:30:21 CET 2018


matan

On 11/6/2018 2:36 PM, Matan Azrad wrote:
> Hi Jeff
>
>   From: Jeff Guo <jia.guo at intel.com>
>> Before detach device when device be hot-unplugged, the failure process in
>> user space and kernel space both need to be finished, such as eal interrupt
>> callback need to be inactive before the callback be unregistered when device
>> is being cleaned. This patch add rte alarm for device detaching, with that it
>> could finish interrupt callback soon and give time to let the failure process
>> done before detaching.
>>
>> Fixes: 2049c5113fe8 ("app/testpmd: use hotplug failure handler")
>> Signed-off-by: Jeff Guo <jia.guo at intel.com>
>> ---
>>   app/test-pmd/testpmd.c | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
>> 9c0edca..9c673cf 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -2620,7 +2620,18 @@ eth_dev_event_callback(const char
>> *device_name, enum rte_dev_event_type type,
>>   				device_name);
>>   			return;
>>   		}
>> -		rmv_event_callback((void *)(intptr_t)port_id);
>> +		/*
>> +		 * Before detach device, the hot-unplug failure process in
>> +		 * user space and kernel space both need to be finished,
>> +		 * such as eal interrupt callback need to be inactive before
>> +		 * the callback be unregistered when device is being cleaned.
>> +		 * So finished interrupt callback soon here and give time to
>> +		 * let the work done before detaching.
>> +		 */
>> +		if (rte_eal_alarm_set(100000,
>> +				rmv_event_callback, (void
>> *)(intptr_t)port_id))
>> +			RTE_LOG(ERR, EAL,
>> +				"Could not set up deferred device
>
> It looks me strange to use callback and alarm to remove a device:
> Why not to use callback and that is it?
>
> I think that it's better to let the EAL to detach the device after all the callbacks were done and not to do it by the user callback.
> So the application\callback owners just need to clean its resources with understanding that after the callback the device(and the callback itself) will be detached by the EAL.


Firstly, at the currently framework and solution, such as callback for 
RTE_ETH_EVENT_INTR_RMV, still need to use the deferred device removal, 
we tend to give the control of detaching device to the application, and 
the whole process is located on the user's callback. Notify app to 
detach device by callback but make it deferred, i think it is fine.

Secondly, the vfio is different with igb_uio for hot-unplug, it 
register/unregister hotplug interrupt callback for each device, so need 
to make  the callback done before unregister the callback.

So I think it should be considerate as an workaround here, before we 
find a better way.


>
>> removal\n");
>>   		break;
>>   	case RTE_DEV_EVENT_ADD:
>>   		RTE_LOG(ERR, EAL, "The device: %s has been added!\n",
>> --
>> 2.7.4


More information about the dev mailing list