Hi Stanislav, On 27/07/2023 20:01, Stanislav Fomichev wrote: > On 07/27, Matthieu Baerts wrote: >> Hi Paul, Stanislav, >> >> On 18/07/2023 18:14, Paul Moore wrote: >>> On Tue, Jul 18, 2023 at 11:21 AM Geliang Tang <geliang.tang@xxxxxxxx> wrote: >>>> >>>> As is described in the "How to use MPTCP?" section in MPTCP wiki [1]: >>>> >>>> "Your app can create sockets with IPPROTO_MPTCP as the proto: >>>> ( socket(AF_INET, SOCK_STREAM, IPPROTO_MPTCP); ). Legacy apps can be >>>> forced to create and use MPTCP sockets instead of TCP ones via the >>>> mptcpize command bundled with the mptcpd daemon." >>>> >>>> But the mptcpize (LD_PRELOAD technique) command has some limitations >>>> [2]: >>>> >>>> - it doesn't work if the application is not using libc (e.g. GoLang >>>> apps) >>>> - in some envs, it might not be easy to set env vars / change the way >>>> apps are launched, e.g. on Android >>>> - mptcpize needs to be launched with all apps that want MPTCP: we could >>>> have more control from BPF to enable MPTCP only for some apps or all the >>>> ones of a netns or a cgroup, etc. >>>> - it is not in BPF, we cannot talk about it at netdev conf. >>>> >>>> So this patchset attempts to use BPF to implement functions similer to >>>> mptcpize. >>>> >>>> The main idea is add a hook in sys_socket() to change the protocol id >>>> from IPPROTO_TCP (or 0) to IPPROTO_MPTCP. >>>> >>>> [1] >>>> https://github.com/multipath-tcp/mptcp_net-next/wiki >>>> [2] >>>> https://github.com/multipath-tcp/mptcp_net-next/issues/79 >>>> >>>> v5: >>>> - add bpf_mptcpify helper. >>>> >>>> v4: >>>> - use lsm_cgroup/socket_create >>>> >>>> v3: >>>> - patch 8: char cmd[128]; -> char cmd[256]; >>>> >>>> v2: >>>> - Fix build selftests errors reported by CI >>>> >>>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/79 >>>> Signed-off-by: Geliang Tang <geliang.tang@xxxxxxxx> >>>> --- >>>> include/linux/bpf.h | 1 + >>>> include/linux/lsm_hook_defs.h | 2 +- >>>> include/linux/security.h | 6 +- >>>> include/uapi/linux/bpf.h | 7 + >>>> kernel/bpf/bpf_lsm.c | 2 + >>>> net/mptcp/bpf.c | 20 +++ >>>> net/socket.c | 4 +- >>>> security/apparmor/lsm.c | 8 +- >>>> security/security.c | 2 +- >>>> security/selinux/hooks.c | 6 +- >>>> tools/include/uapi/linux/bpf.h | 7 + >>>> .../testing/selftests/bpf/prog_tests/mptcp.c | 128 ++++++++++++++++-- >>>> tools/testing/selftests/bpf/progs/mptcpify.c | 17 +++ >>>> 13 files changed, 187 insertions(+), 23 deletions(-) >>>> create mode 100644 tools/testing/selftests/bpf/progs/mptcpify.c >>> >>> ... >>> >>>> diff --git a/security/security.c b/security/security.c >>>> index b720424ca37d..bbebcddce420 100644 >>>> --- a/security/security.c >>>> +++ b/security/security.c >>>> @@ -4078,7 +4078,7 @@ EXPORT_SYMBOL(security_unix_may_send); >>>> * >>>> * Return: Returns 0 if permission is granted. >>>> */ >>>> -int security_socket_create(int family, int type, int protocol, int kern) >>>> +int security_socket_create(int *family, int *type, int *protocol, int kern) >>>> { >>>> return call_int_hook(socket_create, 0, family, type, protocol, kern); >>>> } >>> >>> Using the LSM to change the protocol family is not something we want >>> to allow. I'm sorry, but you will need to take a different approach. >> >> @Paul: Thank you for your feedback. It makes sense and I understand. >> >> @Stanislav: Despite the fact the implementation was smaller and reusing >> more code, it looks like we cannot go in the direction you suggested. Do >> you think what Geliang suggested before in his v3 [1] can be accepted? >> >> (Note that the v3 is the same as the v1, only some fixes in the selftests.) > > We have too many hooks in networking, so something that doesn't add > a new one is preferable :-( Thank you for your reply and the explanation, I understand. > Moreover, we already have a 'socket init' hook, but it runs a bit late. Indeed. And we cannot move it before the creation of the socket. > Is existing cgroup/sock completely unworkable? Is it possible to > expose some new bpf_upgrade_socket_to(IPPROTO_MPTCP) kfunc which would > call some new net_proto_family->upgrade_to(IPPROTO_MPTCP) to do the surgery? > Or is it too hacky? I cannot judge if it is too hacky or not but if you think it would be OK, please tell us :) > Another option Alexei suggested is to add some fentry-like thing: > > noinline int update_socket_protocol(int protocol) > { > return protocol; > } > /* TODO: ^^^ add the above to mod_ret set */ > > int __sys_socket(int family, int type, int protocol) > { > ... > > protocol = update_socket_protocol(protocol); > > ... > } > > But it's also too problem specific it seems? And it's not cgroup-aware. It looks like it is what Geliang did in his v6. If it is the only acceptable solution, I guess we can do without cgroup support. We can continue the discussions in his v6 if that's easier. Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net
