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

Thomas Monjalon thomas at monjalon.net
Thu Sep 24 14:17:04 CEST 2020


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.

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




More information about the dev mailing list