Re: [PATCH bpf-next 1/3] libbpf: xsk: use bpf_link

["Björn Töpel" <bjorn.topel@intel.com>]

On 2021-02-15 18:07, Toke Høiland-Jørgensen wrote:
> Maciej Fijalkowski <maciej.fijalkowski@intel.com> writes:
> 
>> Currently, if there are multiple xdpsock instances running on a single
>> interface and in case one of the instances is terminated, the rest of
>> them are left in an inoperable state due to the fact of unloaded XDP
>> prog from interface.
>>
>> To address that, step away from setting bpf prog in favour of bpf_link.
>> This means that refcounting of BPF resources will be done automatically
>> by bpf_link itself.
>>
>> When setting up BPF resources during xsk socket creation, check whether
>> bpf_link for a given ifindex already exists via set of calls to
>> bpf_link_get_next_id -> bpf_link_get_fd_by_id -> bpf_obj_get_info_by_fd
>> and comparing the ifindexes from bpf_link and xsk socket.
> 
> One consideration here is that bpf_link_get_fd_by_id() is a privileged
> operation (privileged as in CAP_SYS_ADMIN), so this has the side effect
> of making AF_XDP privileged as well. Is that the intention?
>

We're already using, e.g., bpf_map_get_fd_by_id() which has that
as well. So we're assuming that for XDP setup already!

> Another is that the AF_XDP code is in the process of moving to libxdp
> (see in-progress PR [0]), and this approach won't carry over as-is to
> that model, because libxdp has to pin the bpf_link fds.
>

I was assuming there were two modes of operations for AF_XDP in libxdp.
One which is with the multi-program support (which AFAIK is why the
pinning is required), and one "like the current libbpf" one. For the
latter Maciej's series would be a good fit, no?

> However, in libxdp we can solve the original problem in a different way,
> and in fact I already suggested to Magnus that we should do this (see
> [1]); so one way forward could be to address it during the merge in
> libxdp? It should be possible to address the original issue (two
> instances of xdpsock breaking each other when they exit), but
> applications will still need to do an explicit unload operation before
> exiting (i.e., the automatic detach on bpf_link fd closure will take
> more work, and likely require extending the bpf_link kernel support)...
>

I'd say it's depending on the libbpf 1.0/libxdp merge timeframe. If
we're months ahead, then I'd really like to see this in libbpf until the
merge. However, I'll leave that for Magnus/you to decide!

Bottom line; I'd *really* like bpf_link behavior (process scoped) for
AF_XDP sooner than later! ;-)


Thanks for the input!
Björn


> -Toke
> 
> [0] https://github.com/xdp-project/xdp-tools/pull/92
> [1] https://github.com/xdp-project/xdp-tools/pull/92#discussion_r576204719
>