[dpdk-dev] [PATCH 19/20] app/testpmd: reset port status on close notification

[Ferruh Yigit]

On 9/24/2020 1:17 PM, Thomas Monjalon wrote:
> 24/09/2020 14:07, Ferruh Yigit:
>> On 9/23/2020 9:32 PM, Thomas Monjalon wrote:
>>> 23/09/2020 18:45, Ferruh Yigit:
>>>> On 9/13/2020 11:07 PM, Thomas Monjalon wrote:
>>>>> Since rte_eth_dev_release_port() is called on all port close operations,
>>>>> the event RTE_ETH_EVENT_DESTROY can be reliably used for resetting
>>>>> the port status on the application side.
>>>>>
>>>>> The intermediate state RTE_PORT_HANDLING is removed in close_port()
>>>>> because a port can also be closed by a PMD in a device remove operation.
>>>>>
>>>>> In case multiple ports are closed, calling remove_invalid_ports()
>>>>> only once is enough.
>>>>>
>>>>> Signed-off-by: Thomas Monjalon <thomas at monjalon.net>
>>>>
>>>> <...>
>>>>
>>>>> @@ -3118,6 +3093,13 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
>>>>>     				rmv_port_callback, (void *)(intptr_t)port_id))
>>>>>     			fprintf(stderr, "Could not set up deferred device removal\n");
>>>>>     		break;
>>>>> +	case RTE_ETH_EVENT_DESTROY:
>>>>> +		if (rte_atomic16_cmpset(&(ports[port_id].port_status),
>>>>> +					RTE_PORT_STOPPED,
>>>>> +					RTE_PORT_CLOSED) == 0)
>>>>> +			printf("Port %d cannot be set to closed\n", port_id);
>>>>> +		printf("Port %u is closed\n", port_id);
>>>>> +		break;
>>>>
>>>> This is failing if a port closed without application port stop command,
>>>> PMD may be doing port stop within the close function but since
>>>> application didn't give the stop command, the port status is not
>>>> 'RTE_PORT_STOPPED', hence 'port_status' is not updated correctly.
>>>
>>> Do you think we should give up with the atomic state transition,
>>> and just assign the state as closed?
>>>
>>
>> It can be better, if the DESTROY event received it should be in closed
>> state.
> 
> I don't understand your proposal.
> Note that we are not managing ethdev states here.
> Despite the misleading RTE prefix, it is a testpmd state.
> 

I am aware this is testpmd state, but basically saying same thing, force 
the 'closed' state.

>> Right now device can be hot removed while running, or it may be closed
>> before it has been stopped, both cases state transition assumption from
>> stopped->closed will be wrong, and final state will be wrong.
> 
> I think we should force the state, not matter what it was before.
> 
>