On 15/02/2023 18.11, Alexander Lobakin wrote:
From: Zaremba, Larysa <larysa.zaremba@xxxxxxxxx>
Date: Wed, 15 Feb 2023 16:45:18 +0100
On Wed, Feb 15, 2023 at 11:09:36AM +0100, Jesper Dangaard Brouer wrote:
With our XDP-hints kfunc approach, where individual drivers overload the
default implementation, it can be hard for API users to determine
whether or not the current device driver have this kfunc available.
Change the default implementations to use an errno (ENODEV), that
drivers shouldn't return, to make it possible for BPF runtime to
determine if bpf kfunc for xdp metadata isn't implemented by driver.
I think it diverts ENODEV usage from its original purpose too much.
Can you suggest a errno that is a better fit?
Maybe providing information in dmesg would be a better solution?
IMHO we really don't want to print any information in this code path, as
this is being executed as part of the BPF-prog. This will lead to
unfortunate latency issues. Also considering the packet rates this need
to operate at.
+1, -%ENODEV shouldn't be used here. It stands for "no device", for
example the driver probing core doesn't treat it as an error or that
something is not supported (rather than there's no device installed
in a slot / on a bus etc.).
I wanted to choose something that isn't natural for a device driver
developer to choose as a return code. I choose the "no device", because
the "device" driver doesn't implement this.
The important part is help ourselves (and support) to reliably determine
if a device driver implements this kfunc or not. I'm not married to the
specific errno.
I hit this issue myself, when developing these kfuncs for igc. I was
constantly loading and unloading the driver while developing this. And
my kfunc would return -EOPNOTSUPP in some cases, and I couldn't
understand why my code changes was not working, but in reality I was
hitting the default kfunc implementation as it wasn't the correct
version of the driver I had loaded. It would in practice have save me
time while developing...
Please suggest a better errno if the color is important to you.
This is intended to ease supporting and troubleshooting setups. E.g.
when users on mailing list report -19 (ENODEV) as an error, then we can
immediately tell them their kernel is too old.
Do you mean driver being too old, not kernel?
Sure I guess, I do mean the driver version.
I guess you are thinking in the lines of Intel customers compiling Intel
out-of-tree kernel modules, this will also be practical and ease
troubleshooting for Intel support teams.
Signed-off-by: Jesper Dangaard Brouer <brouer@xxxxxxxxxx>
---
[...]
Thanks,
Olek
