[xdp-hints] Re: [PATCH bpf-next V3] xdp: bpf_xdp_metadata use EOPNOTSUPP for no driver support

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

On 21/02/2023 20.03, Martin KaFai Lau wrote:
> On 2/21/23 9:13 AM, Stanislav Fomichev wrote:
>> On Sat, Feb 18, 2023 at 7:34 AM Jesper Dangaard Brouer
>>>
>>> When driver doesn't implement a bpf_xdp_metadata kfunc the default
>>> implementation returns EOPNOTSUPP, which indicate device driver doesn't
>>> implement this kfunc.
>>>
>>> Currently many drivers also return EOPNOTSUPP when the hint isn't
>>> available. Instead change drivers to return ENODATA in these cases.
>>> There can be natural cases why a driver doesn't provide any hardware
>>> info for a specific hint, even on a frame to frame basis (e.g. PTP).
>>> Lets keep these cases as separate return codes.
> 
>> Long term probably still makes sense to export this info via 
>> xdp-features? >> Not sure how long we can 100% ensure EOPNOTSUPP vs ENODATA 
convention :-)
> 
> I am also not sure if it makes the xdp-hints adoption easier for other 
> drivers by enforcing ENODATA or what other return values a driver should 
> or should not return while EOPNOTSUPP is a more common errno to use. May 
> be the driver experts can prove me wrong here.

Which is why I suggested an errno (ENODEV) that drivers will not want to
use by accident.

> iiuc, it is for debugging if the bpf prog has been patched with the 
> driver's xdp kfunc. Others have suggested method like dumping the bpf 
> prog insn. It could also trace the driver xdp kfunc and see if it is 
> actually called. Why these won't work?

I regret talking about this as a debugging tool.  IMHO it have steered
the conversation in a wrong direction, sorry.  There are (obviously)
other metods for debugging this.

For me this is more about the API we are giving the BPF-programmer.

There can be natural cases why a driver doesn't provide any hardware
info for a specific hint.  The RX-timestamp is a good practical example,
as often only PTP packets will be timestamped by hardware.

I can write a BPF-prog that create a stats-map for counting
RX-timestamps, expecting to catch any PTP packets with timestamps.  The
problem is my stats-map cannot record the difference of EOPNOTSUPP vs
ENODATA.  Thus, the user of my RX-timestamps stats program can draw the
wrong conclusion, that there are no packets with (PTP) timestamps, when
this was actually a case of driver not implementing this.

I hope this simple stats example make is clearer that the BPF-prog can
make use of this info runtime.  It is simply a question of keeping these
cases as separate return codes. Is that too much to ask for from an API?

--Jesper