Re: [PATCH v3 bpf-next 06/17] libbpf: xsk: use bpf_link

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

On Mon, Mar 22, 2021 at 10:47:09PM +0100, 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.
> >
> > Consider the scenario below:
> >
> > // load xdp prog and xskmap and add entry to xskmap at idx 10
> > $ sudo ./xdpsock -i ens801f0 -t -q 10
> >
> > // add entry to xskmap at idx 11
> > $ sudo ./xdpsock -i ens801f0 -t -q 11
> >
> > terminate one of the processes and another one is unable to work due to
> > the fact that the XDP prog was unloaded 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.
> >
> > Provide backward compatibility by checking if underlying system is
> > bpf_link capable. Do this by looking up/creating bpf_link on loopback
> > device. If it failed in any way, stick with netlink-based XDP prog.
> > Otherwise, use bpf_link-based logic.
> 
> So how is the caller supposed to know which of the cases happened?
> Presumably they need to do their own cleanup in that case? AFAICT you're
> changing the code to always clobber the existing XDP program on detach
> in the fallback case, which seems like a bit of an aggressive change? :)

Sorry Toke, I was offline yesterday.
Yeah once again I went too far and we shouldn't do:

bpf_set_link_xdp_fd(xsk->ctx->ifindex, -1, 0);

if xsk_lookup_bpf_maps(xsk) returned non-zero value which implies that the
underlying prog is not AF_XDP related.

closing prog_fd (and link_fd under the condition that system is bpf_link
capable) is enough for that case.

If we agree on that and there's nothing else that I missed, I'll send a
v4.

Thanks for review!

> 
> -Toke
>