On Thu, Mar 12, 2020 at 09:16:34AM +0000, Lorenz Bauer wrote:

On Thu, 12 Mar 2020 at 01:58, Alexei Starovoitov
[off-list ref] wrote:

quoted

we do store the socket FD into a sockmap, but returning new FD to that socket
feels weird. The user space suppose to hold those sockets. If it was bpf prog
that stored a socket then what does user space want to do with that foreign
socket? It likely belongs to some other process. Stealing it from other process
doesn't feel right.

For our BPF socket dispatch control plane this is true by design: all sockets
belong to another process. The privileged user space is the steward of these,
and needs to make sure traffic is steered to them. I agree that stealing them is
weird, but after all this is CAP_NET_ADMIN only. pidfd_getfd allows you to
really steal an fd from another process, so that cat is out of the bag ;)

but there it goes through ptrace checks and lsm hoooks, whereas here similar
security model cannot be enforced. bpf prog can put any socket into sockmap and
from bpf_lookup_elem side there is no way to figure out the owner task of the
socket to do ptrace checks. Just doing it all under CAP_NET_ADMIN is not a
great security answer.

Marek wrote a PoC control plane: https://github.com/majek/inet-tool
It is a CLI tool and not a service, so it can't hold on to any sockets.

You can argue that we should turn it into a service, but that leads to another
problem: there is no way of recovering these fds if the service crashes for
some reason. The only solution would be to restart all services, which in
our set up is the same as rebooting a machine really.

quoted

Sounds like the use case is to take sockets one by one from one map, allocate
another map and store them there? The whole process has plenty of races.

It doesn't have to race. Our user space can do the appropriate locking to ensure
that operations are atomic wrt. dispatching to sockets:

- lock
- read sockets from sockmap
- write sockets into new sockmap

but bpf side may still need to insert them into old.
you gonna solve it with a flag for the prog to stop doing its job?
Or the prog will know that it needs to put sockets into second map now?
It's really the same problem as with classic so_reuseport
which was solved with BPF_MAP_TYPE_REUSEPORT_SOCKARRAY.

quoted

I think it's better to tackle the problem from resize perspective. imo making it
something like sk_local_storage (which is already resizable pseudo map of
sockets) is a better way forward.

Resizing is only one aspect. We may also need to shuffle services around,
think "defragmentation", and I think there will be other cases as we gain more
experience with the control plane. Being able to recover fds from the sockmap
will make it more resilient. Adding a special API for every one of these cases
seems cumbersome.

I think sockmap needs a redesign. Consider that today all sockets can be in any
number of sk_local_storage pseudo maps. They are 'defragmented' and resizable.
I think plugging socket redirect to use sk_local_storage-like infra is the
answer.