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 <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? EOPNOTSUPP fits just fine. > > > > 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. > > > > > +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 > > >
