On Fri, Nov 15, 2024 at 3:07 AM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: > > Hi Ryan > > I'll take a more detailed look at your patch later, but wanted to add > a few smallish comment now, see below: > > > Ryan Wilson <ryantimwilson@xxxxxxxxx> writes: > > On Thu, Nov 14, 2024 at 4:52 PM Alexei Starovoitov > > <alexei.starovoitov@xxxxxxxxx> wrote: > >> > >> On Thu, Nov 14, 2024 at 9:07 AM Ryan Wilson <ryantimwilson@xxxxxxxxx> wrote: > >> > > >> > Currently, network devices only support a single XDP program. However, > >> > there are use cases for multiple XDP programs per device. For example, > >> > at Meta, we have XDP programs for firewalls, DDOS and logging that must > >> > all run in a specific order. To work around the lack of multi-program > >> > support, a single daemon loads all programs and uses bpf_tail_call() > >> > in a loop to jump to each program contained in a BPF map. > >> > >> The support for multiple XDP progs per netdev is long overdue. > >> Thank you for working on this! > > +1 on this! > > > [...] > > > Note for real drivers, we do not hit this code. This is how it works > > for real drivers: > > - When installing a BPF program on a driver, we call the driver's > > ndo_bpf() callback function with command = XDP_QUERY_MPROG_SUPPORT. If > > this returns 0, then mprog is supported. Otherwise, mprog is not > > supported. > > We already have feature flags for XDP, so why not just make mprog > support a feature flag instead of the query thing? It probably should be > anyway, so it can also be reported to userspace. Oh wow can't believe I missed the feature flag API. Yes, I'll use this in v2 instead. Thanks for the suggestion! And if it's exposed to userspace, users no longer need to guess if their driver supports mprog or not - although hopefully this is an intermediary state and the mprog migration for all drivers will be relatively quick and painless. > > >> I think it will remove this branch and XDP performance will remain > >> the same ? > >> Benchmarking on real NIC matters, of course. > > > > Good point. I will migrate a real driver and add XDP benchmarking > > numbers to v2. > > Yes, please, looking forward to seeing benchmark numbers! > > -Toke >
