[PATCH v1 2/2] ethdev: fix skip valid port in probing callback

Thomas Monjalon thomas at monjalon.net
Mon Jan 13 11:57:09 CET 2025


13/01/2025 10:35, lihuisong (C):
> 在 2025/1/13 16:16, Thomas Monjalon 写道:
> > 13/01/2025 03:55, Huisong Li:
> >> The event callback in application may use the macro RTE_ETH_FOREACH_DEV to
> >> iterate over all enabled ports to do something(like, verifying the port id
> >> validity) when receive a probing event. If the ethdev state of a port is
> >> not RTE_ETH_DEV_UNUSED, this port will be considered as a valid port.
> >>
> >> However, this state is set to RTE_ETH_DEV_ATTACHED after pushing probing
> >> event. It means that probing callback will skip this port. But this
> >> assignment can not move to front of probing notification. See
> >> commit be8cd210379a ("ethdev: fix port probing notification")
> >>
> >> So this patch has to add a new state, RTE_ETH_DEV_ALLOCATED. Set the ethdev
> >> state to RTE_ETH_DEV_ALLOCATED before pushing probing event and set it to
> >> RTE_ETH_DEV_ATTACHED after definitely probed. And this port is valid if its
> >> device state is 'ALLOCATED' or 'ATTACHED'.
> > If you do that, changing the definition of eth_dev_find_free_port()
> > you allow the application using a port before probing is finished.
> 
> Yes, it's not reasonable.
> 
> Thinking your comment twice, I feel that the root cause of this issue is 
> application want to check if the port id is valid.
> However, application just receive the new event from the device and the 
> port id of this device must be valid when report new event.
> So application can think the received new event is valid and don't need 
> to check, right?

Yes
Do you think it should be highlighted in the API doc?

We currently have this:
	RTE_ETH_EVENT_NEW,      /**< port is probed */


> If so I think this series can be dropped.
> > It is the same as changing the state to RTE_ETH_DEV_ATTACHED
> > before calling the event callback.
> >
> > So this is a NACK.
> >
> > Why do you need drivers to check the state of a notified device?
> > If it is RTE_ETH_EVENT_NEW, you know that's a new device,
> > there is nothing else to check.
> 
> It just modified the verification about RTE_ETH_DEV_UNUSED in the device 
> driver.
> Driver not need to know the event.

Sorry I was not clear.
I said "drivers", but it should be "apps & drivers" because they can both
register to the event RTE_ETH_EVENT_NEW.
In some situations, it is convenient for a driver to listen to new ports
(it was done for failsafe driver).




More information about the dev mailing list