> "Loftus, Ciara" <ciara.loftus@xxxxxxxxx> writes: > > >> 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 > > 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 > > called. Maybe another new field in the netdev alongside the xsk_refcnt. Or > else extend > > bpf_redirect_info - if we find a new home for it that it's too costly to > access. > > 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
