[RFC,bpf-next,v7,0/6] bpf: Force to MPTCP

Message

Geliang Tang July 29, 2023, 9:57 a.m. UTC

As is described in the "How to use MPTCP?" section in MPTCP wiki [1]:

"Your app should 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 to 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

v7:
 - add __weak and __diag_* for update_socket_protocol.

v6:
 - add update_socket_protocol.

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

Geliang Tang (6):
  net: socket: add update_socket_protocol hook
  bpf: Register mptcp modret set
  selftests/bpf: Add mptcpify program
  selftests/bpf: use random netns name for mptcp
  selftests/bpf: add two mptcp netns helpers
  selftests/bpf: Add mptcpify selftest

 net/mptcp/bpf.c                               |  17 +++
 net/socket.c                                  |  26 ++++
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 125 ++++++++++++++++--
 tools/testing/selftests/bpf/progs/mptcpify.c  |  25 ++++
 4 files changed, 184 insertions(+), 9 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/mptcpify.c

Comments

Alexei Starovoitov Aug. 1, 2023, 12:43 a.m. UTC | #1

On Sat, Jul 29, 2023 at 05:57:21PM +0800, Geliang Tang wrote:
> 
> The main idea is to add a hook in sys_socket() to change the protocol id
> from IPPROTO_TCP (or 0) to IPPROTO_MPTCP.

I still think it's a hack, but its blast radius is nicely contained.
And since I cannot propose any better I'm ok with it.

Patches 1-2 can be squashed into one.
Just like patches 3-6 as a single patch for selftests.

But before proceeding I'd like an explicit ack from netdev maintainers.

Geliang Tang Aug. 2, 2023, 2:19 a.m. UTC | #2

On Mon, Jul 31, 2023 at 05:43:23PM -0700, Alexei Starovoitov wrote:
> On Sat, Jul 29, 2023 at 05:57:21PM +0800, Geliang Tang wrote:
> > 
> > The main idea is to add a hook in sys_socket() to change the protocol id
> > from IPPROTO_TCP (or 0) to IPPROTO_MPTCP.
> 
> I still think it's a hack, but its blast radius is nicely contained.
> And since I cannot propose any better I'm ok with it.
> 
> Patches 1-2 can be squashed into one.
> Just like patches 3-6 as a single patch for selftests.

Thanks Alexei. I'll squash patch 1 and patch 2 into one, and squash patch 3
and patch 6 into one for selftests. But I prefer to keep patch 4 and patch 5
as is, since they were implemented in different times for different purposes.
They were merged into MPTCP repo on May 17 for "run MPTCP sched tests in a
dedicated ns" [1].

[1]
https://patchwork.kernel.org/project/mptcp/cover/[e-mail]/

-Geliang

> 
> But before proceeding I'd like an explicit ack from netdev maintainers.

Alexei Starovoitov Aug. 2, 2023, 2:23 a.m. UTC | #3

On Tue, Aug 1, 2023 at 7:19 PM Geliang Tang <[e-mail]> wrote:
>
> On Mon, Jul 31, 2023 at 05:43:23PM -0700, Alexei Starovoitov wrote:
> > On Sat, Jul 29, 2023 at 05:57:21PM +0800, Geliang Tang wrote:
> > >
> > > The main idea is to add a hook in sys_socket() to change the protocol id
> > > from IPPROTO_TCP (or 0) to IPPROTO_MPTCP.
> >
> > I still think it's a hack, but its blast radius is nicely contained.
> > And since I cannot propose any better I'm ok with it.
> >
> > Patches 1-2 can be squashed into one.
> > Just like patches 3-6 as a single patch for selftests.
>
> Thanks Alexei. I'll squash patch 1 and patch 2 into one, and squash patch 3
> and patch 6 into one for selftests. But I prefer to keep patch 4 and patch 5
> as is, since they were implemented in different times for different purposes.
> They were merged into MPTCP repo on May 17 for "run MPTCP sched tests in a
> dedicated ns" [1].

since they were sent to a different tree than don't send them here.
git will not like that during the merge window.

Paolo Abeni Aug. 2, 2023, 8:03 a.m. UTC | #4

On Mon, 2023-07-31 at 17:43 -0700, Alexei Starovoitov wrote:

> I still think it's a hack, but its blast radius is nicely contained.
> And since I cannot propose any better I'm ok with it.
> 
> Patches 1-2 can be squashed into one.
> Just like patches 3-6 as a single patch for selftests.
> 
> But before proceeding I'd like an explicit ack from netdev maintainers.

Just to state the obvious, I carry my personal bias on this topic due
to my background ;)

My perspective is quite similar to Alexei's one: the solution is not
extremely elegant, but is very self-contained; it looks viable to me.

WRT the specific code, I think the additional checks on the 'protocol'
value after the 'update_socket_protocol()' call should be dropped: the
user space can already provide an arbitrary value there and the later
code deal with that.

Cheers,

Paolo

Matthieu Baerts Aug. 3, 2023, 7:40 a.m. UTC | #5

Hi Alexei,

On 02/08/2023 04:23, Alexei Starovoitov wrote:
> On Tue, Aug 1, 2023 at 7:19 PM Geliang Tang <[e-mail]> wrote:
>>
>> On Mon, Jul 31, 2023 at 05:43:23PM -0700, Alexei Starovoitov wrote:
>>> On Sat, Jul 29, 2023 at 05:57:21PM +0800, Geliang Tang wrote:
>>>>
>>>> The main idea is to add a hook in sys_socket() to change the protocol id
>>>> from IPPROTO_TCP (or 0) to IPPROTO_MPTCP.
>>>
>>> I still think it's a hack, but its blast radius is nicely contained.
>>> And since I cannot propose any better I'm ok with it.
>>>
>>> Patches 1-2 can be squashed into one.
>>> Just like patches 3-6 as a single patch for selftests.
>>
>> Thanks Alexei. I'll squash patch 1 and patch 2 into one, and squash patch 3
>> and patch 6 into one for selftests. But I prefer to keep patch 4 and patch 5
>> as is, since they were implemented in different times for different purposes.
>> They were merged into MPTCP repo on May 17 for "run MPTCP sched tests in a
>> dedicated ns" [1].
> 
> since they were sent to a different tree than don't send them here.
> git will not like that during the merge window.

Thank you for the suggestion but that's OK to have these patches applied
in BPF tree because on MPTCP side, they have been applied in a "devel"
branch which is rebased on top of -net and net-next everyday. In other
words, we don't use it in pull requests and it is fine to apply these
patches in bpf-next or elsewhere.

Cheers,
Matt