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. >> 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
