Thread (120 messages) 120 messages, 12 authors, 2020-05-13

Re: [PATCH bpf-next 1/4] xdp: Support specifying expected existing program when attaching XDP

From: Andrii Nakryiko <hidden>
Date: 2020-03-31 20:19:24
Also in: bpf

On Tue, Mar 31, 2020 at 8:00 AM Toke Høiland-Jørgensen [off-list ref] wrote:
Daniel Borkmann [off-list ref] writes:
quoted
On 3/31/20 12:13 PM, Toke Høiland-Jørgensen wrote:
quoted
Andrii Nakryiko [off-list ref] writes:
quoted
quoted
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help