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