[dpdk-dev] [PATCH v2] eal: fix positive error codes from probe/remove
david.marchand at redhat.com
Fri Jun 7 10:32:37 CEST 2019
On Thu, Jun 6, 2019 at 12:03 PM Ilya Maximets <i.maximets at samsung.com>
> According to API, 'rte_dev_probe()' and 'rte_dev_remove()' must
> return 0 or negative error code. Bus code returns positive values
> if device wasn't recognized by any driver, so the result of
> 'bus->plug/unplug()' must be converted. 'local_dev_probe()' and
> 'local_dev_remove()' also has their internal API, so the conversion
> should be done there.
> Positive on remove means that device not found by driver.
For backports, it is safer to add the check on > 0.
The patch looks good to me.
Reviewed-by: David Marchand <david.marchand at redhat.com>
But I have some comments on the current state of the code.
After inspecting the eal and buses, this problem is not supposed to happen
on the rte_dev_remove path.
rte_dev_remove() ensures that it calls local_dev_remove() after checking
that the device is attached to a driver (see the check on
- When handling a detach operation in the primary process
signal all other secondary processes to detach right away.
Then we do a bus/device lookup.
Then we call the bus unplug.
Would not it be better to check the device exists _and_ check if the device
is attached to a driver in the primary process before calling other
- Calling unplug on a device that is not attached is a bit weird to me, all
the more so that we have rte_dev_probed().
But there might be users calling directly the bus unplug api and not the
Does this enter the ABI stability perimeter?
If not, I would be for changing unplug api so that we only deal with 0 or <
0 on remove path.
On the plug side, is there a reason why we do not check for
rte_dev_probed() and let the bus replies that the device is already probed?
Does it have something to do with representors ?
- On the plug side again, can't we have an indication from the buses that
they have a driver that can handle the device rather than this odd (and
historical) > 0 return code?
This should not change the current behavior, just make the code a bit
easier to understand.
I know you are travelling, so this can wait anyway.
More information about the dev