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

From: Toke Høiland-Jørgensen <[email-masked]>
Date: Thu, 16 Feb 2023 15:13:07 +0100

Larysa Zaremba <[email-masked]> writes:

> On Wed, Feb 15, 2023 at 06:50:10PM +0100, Jesper Dangaard Brouer wrote:
>> 
>> On 15/02/2023 18.11, Alexander Lobakin wrote:
>> > From: Zaremba, Larysa <[email-masked]>
>> > 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?
>
> EOPNOTSUPP fits just fine.

An alternative to changing the return code of the default kfuncs is also
to just not have the driver functions themselves use that error code? :)

>> > > 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.
>
> I meant printing messages at bpf program load time...
> When driver functions are patched-in, you have all the information you may need 
> to inform user, if the default implementation for a particular function is used 
> instead.

If you dump the byte code with bpftool (using `bpftool prog dump xlated`), the
name of the function being called will be in the output, which is also a
way to detect if the driver kfunc is being called...

-Toke