Thread (15 messages) 15 messages, 4 authors, 2021-11-22

RE: [RFC PATCH bpf-next 0/8] XDP_REDIRECT_XSK and Batched AF_XDP Rx

From: Loftus, Ciara <hidden>
Date: 2021-11-19 15:48:36
Also in: bpf

"Loftus, Ciara" [off-list ref] writes:
quoted
quoted
I'm fine with adding a new helper, but I don't like introducing a new
XDP_REDIRECT_XSK action, which requires updating ALL the drivers.

With XDP_REDIRECT infra we beleived we didn't need to add more
XDP-action code to drivers, as we multiplex/add new features by
extending the bpf_redirect_info.
In this extreme performance case, it seems the this_cpu_ptr "lookup" of
bpf_redirect_info is the performance issue itself.

Could you experiement with different approaches that modify
xdp_do_redirect() to handle if new helper bpf_redirect_xsk was called,
prior to this_cpu_ptr() call.
(Thus, avoiding to introduce a new XDP-action).
Thanks for your feedback Jesper.
I understand the hesitation of adding a new action. If we can achieve the
same improvement without
quoted
introducing a new action I would be very happy!
Without new the action we'll need a new way to indicate that the
bpf_redirect_xsk helper was
quoted
called. Maybe another new field in the netdev alongside the xsk_refcnt. Or
else extend
quoted
bpf_redirect_info - if we find a new home for it that it's too costly to
access.
quoted
Thanks for your suggestions. I'll experiment as you suggested and
report back.
I'll add a +1 to the "let's try to solve this without a new return code" :)

Also, I don't think we need a new helper either; the bpf_redirect()
helper takes a flags argument, so we could just use ifindex=0,
flags=DEV_XSK or something like that.
The advantage of a new helper is that we can access the netdev 
struct from it and check if there's a valid xsk stored in it, before
returning XDP_REDIRECT without the xskmap lookup. However,
I think your suggestion could work too. We would just
have to delay the check until xdp_do_redirect. At this point
though, if there isn't a valid xsk we might have to drop the packet
instead of falling back to the xskmap.
Also, I think the batching in the driver idea can be generalised: we
just need to generalise the idea of "are all these packets going to the
same place" and have a batched version of xdp_do_redirect(), no? The
other map types do batching internally already, though, so I'm wondering
why batching in the driver helps XSK?
With the current infrastructure figuring out if "all the packets are going
to the same place" looks like an expensive operation which could undo
the benefits of the batching that would come after it. We would need
to run the program N=batch_size times, store the actions and
bpf_redirect_info for each run and perform a series of compares. The new
action really helped here because it could easily indicate if all the
packets in a batch were going to the same place. But I understand it's
not an option. Maybe if we can mitigate the cost of accessing the
bpf_redirect_info as Jesper suggested, we can use a flag in it to signal
what the new action was signalling.
I'm not familiar with how the other map types and how they handle
batching so I will look into that.

Appreciate your feedback. I have a few avenues to explore.

Ciara
-Toke
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help