Thread (37 messages) 37 messages, 7 authors, 2021-02-23

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

From: Toke Høiland-Jørgensen <hidden>
Date: 2021-02-16 10:38:35
Also in: bpf

John Fastabend [off-list ref] writes:
Toke Høiland-Jørgensen wrote:
quoted
John Fastabend [off-list ref] writes:
quoted
quoted
quoted
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!
Did I miss some thread? What does this mean libbpf 1.0/libxdp merge?
The idea is to keep libbpf focused on bpf, and move the AF_XDP stuff to
libxdp (so the socket stuff in xsk.h). We're adding the existing code
wholesale, and keeping API compatibility during the move, so all that's
needed is adding -lxdp when compiling. And obviously the existing libbpf
code isn't going anywhere until such a time as there's a general
backwards compatibility-breaking deprecation in libbpf (which I believe
Andrii is planning to do in an upcoming and as-of-yet unannounced v1.0
release).
OK, I would like to keep the basic XDP pieces in libbpf though. For example
bpf_program__attach_xdp(). This way we don't have one lib to attach
everything, but XDP.
The details are still TDB; for now, we're just merging in the XSK code
to the libxdp repo. I expect Andrii to announce his plans for the rest
soonish. I wouldn't expect basic things like that to go away, though :)
quoted
While integrating the XSK code into libxdp we're trying to make it
compatible with the rest of the library (i.e., multi-prog). Hence my
preference to avoid introducing something that makes this harder :)

-Toke
OK that makes sense to me thanks. But, I'm missing something (maybe its
obvious to everyone else?).

When you load an XDP program you should get a reference to it. And then
XDP program should never be unloaded until that id is removed right? It
may or may not have an xsk map. Why does adding/removing programs from
an associated map have any impact on the XDP program? That seems like
the buggy part to me. No other map behaves this way as far as I can
tell. Now if the program with the XDP reference closes without pinning
the map or otherwise doing something with it, sure the map gets destroyed
and any xsk sockets are lost.
The original bug comes from the XSK code abstracting away the fact that
an AF_XDP socket needs an XDP program on the interface to work; so if
none exists, the library will just load a program that redirects into
the socket. Which breaks since the xdpsock example application is trying
to be nice and clean up after itself, by removing the XDP program when
it's done with the socket, thus breaking any other programs using that
XDP program. So this patch introduces proper synchronisation on both add
and remove of the XDP program...

-Toke
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help