[xdp-hints] Re: [PATCH bpf-next V1] xdp: bpf_xdp_metadata use NODEV for no device support

[Jesper Dangaard Brouer <jbrouer@redhat.com>]

On 15/02/2023 18.11, Alexander Lobakin wrote:
> From: Zaremba, Larysa <larysa.zaremba@intel.com>
> 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@redhat.com>
>>> ---
> [...]
> 
> Thanks,
> Olek
>