On Tue, Mar 31, 2020 at 8:00 AM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: > > Daniel Borkmann <daniel@xxxxxxxxxxxxx> writes: > > > On 3/31/20 12:13 PM, Toke Høiland-Jørgensen wrote: > >> Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> writes: > >> > >>>>> So you install your libxdp-based firewalls and are happy. Then you > >>>>> decide to install this awesome packet analyzer, which doesn't know > >>>>> about libxdp yet. Suddenly, you get all packets analyzer, but no more > >>>>> firewall, until users somehow notices that it's gone. Or firewall > >>>>> periodically checks that it's still runinng. Both not great, IMO, but > >>>>> might be acceptable for some users, I guess. But imagine all the > >>>>> confusion for user, especially if he doesn't give a damn about XDP and > >>>>> other buzzwords, but only needs a reliable firewall :) > >>>> > >>>> Yes, whereas if the firewall is using bpf_link, then the packet analyser > >>>> will be locked out and can't do its thing. Either way you end up with a > >>>> broken application; it's just moving the breakage. In the case of > >>> > >>> Hm... In one case firewall installation reported success and stopped > >>> working afterwards with no notification and user having no clue. In > >>> another, packet analyzer refused to start and reported error to user. > >>> Let's agree to disagree that those are not at all equivalent. To me > >>> silent failure is so much worse, than application failing to start in > >>> the first place. > > > > I sort of agree with both of you that either case is not great. The silent > > override we currently have is not great since it can be evicted at any time > > but also bpf_link to lock-out other programs at XDP layer is not great either > > since there is also huge potential to break existing programs. It's probably > > best to discuss on an actual proposal to see the concrete semantics, but my > > concerns, assuming I didn't misunderstand or got confused on something along > > the way (if so, please let me know), currently are: > > I think you're summarising the issues well, with perhaps one thing > missing: The goal is to enable multi-prog execution, i.e., execute two > programs in sequence. So, when things work correctly the flow should be: > > App1, loading prog1: > - get current program from $IFACE > - current program is NULL: > -> build dispatcher(prog1) > -> load dispatcher onto $IFACE with UPDATE_IF_NOEXIST flag > -> success > > Then, app2 loading prog2: > - get current program from $IFACE > - current program is dispatcher(prog1): > -> build new dispatcher(prog1,prog2) > -> atomically replace old dispatcher with new one > -> success > > As long as app1 and app2 agree on what a dispatcher looks like, and how > to update it, they can cooperatively install themselves in the chain, as > long as there's a way to resolve the race between reading and updating > the state in the kernel. > > However, if they *don't* agree on how to build the dispatcher and run in > sequence, they are fundamentally incompatible. Which also means that > multi-prog operation is going to be incompatible with any application > that was written before it was implemented. The only way to avoid that > is to provide the multi-prog support in the kernel, in a way that is > compatible with the old API. I'm not sure if this is even possible; but > I certainly got a very emphatic NACK on any attempt to implement the > support in the kernel when I posted my initial patch back in the fall. > > Also, to your point about needing a specific library: I've been saying > "using the same library" because I think that is the most likely way to > get applications to agree. But really, what's needed is more like a > protocol; there could in theory be several independent implementations > that interoperate. However, I don't see a way to make things compatible > with applications that don't follow that protocol; we only get to pick > the failure mode (and those failure modes I think you summarised quite > well). Well, for once we agree with Toke in this thread (regarding last two paragraphs) :) > > -Toke >
