On 21/03/2023 13.24, Toke Høiland-Jørgensen wrote:
Jesper Dangaard Brouer <jbrouer@xxxxxxxxxx> writes:
On 17/03/2023 22.21, Stanislav Fomichev wrote:
On 03/17, Jesper Dangaard Brouer wrote:
When driver doesn't implement a bpf_xdp_metadata kfunc the fallback
implementation returns EOPNOTSUPP, which indicate device driver doesn't
implement this kfunc.
Currently many drivers also return EOPNOTSUPP when the hint isn't
available, which is inconsistent from an API point of view. 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.
When describing the return values, adjust the function kernel-doc layout
to get proper rendering for the return values.
Signed-off-by: Jesper Dangaard Brouer <brouer@xxxxxxxxxx>
I don't remember whether the previous discussion ended in something?
IIRC Martin was preferring to use xdp-features for this instead?
IIRC Martin asked for a second vote/opinion to settle the vote.
The xdp-features use is orthogonal and this patch does not prohibit the
later implementation of xdp-features, to detect if driver doesn't
implement kfuncs via using global vars. Not applying this patch leaves
the API in an strange inconsistent state, because of an argument that in
the *future* we can use xdp-features to solve *one* of the discussed
use-cases for another return code.
I argued for a practical PTP use-case where not all frames contain the
PTP timestamp. This patch solve this use-case *now*, so I don't see why
we should stall solving this, because of a "future" feature we might
never get around to implement, which require the user to use global vars.
Personally I'm fine with having this convention, but I'm not sure how well
we'll be able to enforce them. (In general, I'm not a fan of userspace
changing it's behavior based on errno. If it's mostly for
debugging/development - seems ok)
We enforce the API by documenting the return behavior, like below. If a
driver violate this, then we will fix the driver code with a fixes tag.
My ask is simply let not have ambiguous return codes.
FWIW I don't get the opposition to this patch: having distinct return
codes strictly increases the amount of information that is available to
the caller. Even if some driver happens to use the "wrong" return code,
it's still an improvement for all the drivers that do the right thing
(and, well, we can fix broken drivers). And if a BPF program doesn't
care about the type of failure they can just ignore treat all error
codes the same; realistically, that is what most programs will do, but
that doesn't mean we can't provide the more-granular error codes to the
programs that do care.
My only concern with this patch is that it targets bpf-next and carries
no Fixes tag, so we'll end up with a kernel release that doesn't have
this change...
Good point, I'll send this patch against 'bpf' tree instead.
--Jesper
