Re: bpf: ability to attach freplace to multiple parents

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> It's a different link.
> For fentry/fexit/freplace the link is pair:
>   // target           ...         bpf_prog
> (target_prog_fd_or_vmlinux, fentry_exit_replace_prog_fd).
>
> So for xdp case we will have:
> root_link = (eth0_ifindex, dispatcher_prog_fd) // dispatcher prog attached to eth0
> link1 = (dispatcher_prog_fd, xdp_firewall1_fd) // 1st extension prog attached to dispatcher
> link2 = (dispatcher_prog_fd, xdp_firewall2_fd) // 2nd extension prog attached to dispatcher
>
> Now libxdp wants to update the dispatcher prog.
> It generates new dispatcher prog with more placeholder entries or new policy:
> new_dispatcher_prog_fd.
> It's not attached anywhere.
> Then libxdp calls new bpf_raw_tp_open() api I'm proposing above to create:
> link3 = (new_dispatcher_prog_fd, xdp_firewall1_fd)
> link4 = (new_dispatcher_prog_fd, xdp_firewall2_fd)
> Now we have two firewalls attached to both old dispatcher prog and new dispatcher prog.
> Both firewalls are executing via old dispatcher prog that is active.
> Now libxdp calls:
> bpf_link_udpate(root_link, dispatcher_prog_fd, new_dispatcher_prog_fd)
> which atomically replaces old dispatcher prog with new dispatcher prog in eth0.
> The traffic keeps flowing into both firewalls. No packets lost.
> But now it goes through new dipsatcher prog.
> libxdp can now:
> close(dispatcher_prog_fd);
> close(link1);
> close(link2);
> Closing (and destroying two links) will remove old dispatcher prog
> from linked list in xdp_firewall1_prog->aux->linked_prog_list and from
> xdp_firewall2_prog->aux->linked_prog_list.
> Notice that there is no need to explicitly detach old dispatcher prog from eth0.
> link_update() did it while replacing it with new dispatcher prog.

Yeah, this was the flow I had in mind already. However, what I meant was
that *from the PoV of an application consuming the link fd*, this would
lead to dangling links.

I.e., an application does:

app1_link_fd = libxdp_install_prog(prog1);

and stores link_fd somewhere (just holds on to it, or pins it
somewhere).

Then later, another application does:

app2_link_fd = libxdp_install_prog(prog2);

but this has the side-effect of replacing the dispatcher, so
app1_link_fd is now no longer valid.

This can be worked around, of course (e.g., just return the prog_fd and
hide any link_fd details inside the library), but if the point of
bpf_link is that the application could hold on to it and use it for
subsequent replacements, that would be nice to have for consumers of the
library as well, no?

-Toke