Re: run bpf prog w/o sockmap [was: bpf: Add bpf_link support for sk_msg and sk_skb progs]

[John Fastabend <john.fastabend@gmail.com>]

Yonghong Song wrote:
> 
> On 4/3/24 6:11 PM, John Fastabend wrote:
> > Martin KaFai Lau wrote:
> >> On 4/3/24 10:47 AM, John Fastabend wrote:
> >>> on my todo list, I want
> >>> to just remove the map notion and bind progs to socks directly.
> >> Run the bpf prog without the sockmap? +1, it would be nice.
> > Part of my motivation for doing this is almost all the bugs syzbot and
> > others find are related to removing sockets from the map. We never
> > do this in any of our code. Once a socket is in the map (added at
> > accept time) it stays there until TCP stack closes it.
> >
> > Also we have to make up some size for the map that somehow looks like
> > max number of concurrent sessions for the application. For many
> > server applicatoins (nginx, httpd, ...) we know this, but is a bit
> > artifically derived.
> >
> >>> but other than quick hacks I've never built such a thing nor ran it
> >>> in production.
> >> How do you see the interface will look like (e.g. attaching the bpf prog to a sk) ?
> > I would propse doing it directly with a helper/kfunc from the sockops
> > programs.
> >
> >    attach_sk_msg_prog(sk, sk_msg_prog)
> >    attach_sk_skb_prog(sk, sk_skb_prog)
> >
> >> It will be nice if the whole function (e.g. sk->sk_data_ready or may be some of
> >> the sk->sk_prot) can be implemented completely in bpf. I don't have a concrete
> >> use case for now but I think it will be powerful.
> > Perhaps a data_ready prog could also replace the ops?
> >
> >    attach_sk_data_ready(sk, sk_msg_data_ready)
> >
> > The attach_sk_data_ready could use pretty much the logic we have for
> > creating psocks but only replace the sk_data_ready callback.
> 
> sounds a good idea. Do we need to support detach function or atomic
> update function as well? Can each sk has multiple sk_msg_prog programs?

I've not found any use for multiple programs, detach functions, or updating
the psock once its created to be honest. Also why syzbot finds all the bugs
in this space because we unfortunately don't stress this area much. In the
original design I had fresh in my head building hardware load balancers and the
XDP redirect bits so a map seemed natural. Also we didn't have a lot of the
machinery we have now so went with the map. As I noted above the L7 LB
hasn't really got much traction on my side at least not yet.

In reality we've been using sk_msg and sk_skb progs attaching 1:1
with protocols and observing, auditing, adding/removing fields from
data streams.

I would probably suggest for first implementation of a sk msg attach without
maps I would just make it one prog no need for multiple programs and even
skip a detach function. Maybe there is some use for multiple programs but
we just have a single agent so it hasn't come up yet. Maybe similar to cgroups
though because we only have single prog in those at the moment.

Thanks.