* [PATCH mptcp-next v13 1/4] bpf: Add update_socket_protocol hook
2023-08-12 2:54 [PATCH mptcp-next v13 0/4] bpf: Force to MPTCP Geliang Tang
@ 2023-08-12 2:54 ` Geliang Tang
2023-08-12 2:54 ` [PATCH mptcp-next v13 2/4] selftests/bpf: Add two mptcp netns helpers Geliang Tang
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Geliang Tang @ 2023-08-12 2:54 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Florent Revest,
Brendan Jackman, Matthieu Baerts, Mat Martineau, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, John Johansen,
Paul Moore, James Morris, Serge E. Hallyn, Stephen Smalley,
Eric Paris, Mykola Lysenko, Shuah Khan, Simon Horman
Cc: Geliang Tang, bpf, netdev, mptcp, linux-security-module, selinux,
linux-kselftest, Yonghong Song
Add a hook named update_socket_protocol in __sys_socket(), for bpf
progs to attach to and update socket protocol. One user case is to
force legacy TCP apps to create and use MPTCP sockets instead of
TCP ones.
Define a fmod_ret set named bpf_mptcp_fmodret_ids, add the hook
update_socket_protocol into this set, and register it in
bpf_mptcp_kfunc_init().
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/79
Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/bpf.c | 15 +++++++++++++++
net/socket.c | 26 +++++++++++++++++++++++++-
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index 5a0a84ad94af..8a16672b94e2 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -19,3 +19,18 @@ struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
return NULL;
}
+
+BTF_SET8_START(bpf_mptcp_fmodret_ids)
+BTF_ID_FLAGS(func, update_socket_protocol)
+BTF_SET8_END(bpf_mptcp_fmodret_ids)
+
+static const struct btf_kfunc_id_set bpf_mptcp_fmodret_set = {
+ .owner = THIS_MODULE,
+ .set = &bpf_mptcp_fmodret_ids,
+};
+
+static int __init bpf_mptcp_kfunc_init(void)
+{
+ return register_btf_fmodret_id_set(&bpf_mptcp_fmodret_set);
+}
+late_initcall(bpf_mptcp_kfunc_init);
diff --git a/net/socket.c b/net/socket.c
index 5d4e37595e9a..fdb5233bf560 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1657,12 +1657,36 @@ struct file *__sys_socket_file(int family, int type, int protocol)
return sock_alloc_file(sock, flags, NULL);
}
+/* A hook for bpf progs to attach to and update socket protocol.
+ *
+ * A static noinline declaration here could cause the compiler to
+ * optimize away the function. A global noinline declaration will
+ * keep the definition, but may optimize away the callsite.
+ * Therefore, __weak is needed to ensure that the call is still
+ * emitted, by telling the compiler that we don't know what the
+ * function might eventually be.
+ *
+ * __diag_* below are needed to dismiss the missing prototype warning.
+ */
+
+__diag_push();
+__diag_ignore_all("-Wmissing-prototypes",
+ "A fmod_ret entry point for BPF programs");
+
+__weak noinline int update_socket_protocol(int family, int type, int protocol)
+{
+ return protocol;
+}
+
+__diag_pop();
+
int __sys_socket(int family, int type, int protocol)
{
struct socket *sock;
int flags;
- sock = __sys_socket_create(family, type, protocol);
+ sock = __sys_socket_create(family, type,
+ update_socket_protocol(family, type, protocol));
if (IS_ERR(sock))
return PTR_ERR(sock);
--
2.35.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH mptcp-next v13 2/4] selftests/bpf: Add two mptcp netns helpers
2023-08-12 2:54 [PATCH mptcp-next v13 0/4] bpf: Force to MPTCP Geliang Tang
2023-08-12 2:54 ` [PATCH mptcp-next v13 1/4] bpf: Add update_socket_protocol hook Geliang Tang
@ 2023-08-12 2:54 ` Geliang Tang
2023-08-12 2:54 ` [PATCH mptcp-next v13 3/4] selftests/bpf: Fix error checks of mptcp open_and_load Geliang Tang
2023-08-12 2:54 ` [PATCH mptcp-next v13 4/4] selftests/bpf: Add mptcpify test Geliang Tang
3 siblings, 0 replies; 8+ messages in thread
From: Geliang Tang @ 2023-08-12 2:54 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Florent Revest,
Brendan Jackman, Matthieu Baerts, Mat Martineau, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, John Johansen,
Paul Moore, James Morris, Serge E. Hallyn, Stephen Smalley,
Eric Paris, Mykola Lysenko, Shuah Khan, Simon Horman
Cc: Geliang Tang, bpf, netdev, mptcp, linux-security-module, selinux,
linux-kselftest, Yonghong Song
Add two netns helpers for mptcp tests: create_netns() and
cleanup_netns(). Use them in test_base().
These new helpers will be re-used in the following commits
introducing new tests.
Acked-by: Yonghong Song <yonghong.song@linux.dev>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
.../testing/selftests/bpf/prog_tests/mptcp.c | 31 +++++++++++++------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index cd0c42fff7c0..76afb5191772 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -22,6 +22,24 @@ struct mptcp_storage {
char ca_name[TCP_CA_NAME_MAX];
};
+static struct nstoken *create_netns(void)
+{
+ SYS(fail, "ip netns add %s", NS_TEST);
+ SYS(fail, "ip -net %s link set dev lo up", NS_TEST);
+
+ return open_netns(NS_TEST);
+fail:
+ return NULL;
+}
+
+static void cleanup_netns(struct nstoken *nstoken)
+{
+ if (nstoken)
+ close_netns(nstoken);
+
+ SYS_NOFAIL("ip netns del %s &> /dev/null", NS_TEST);
+}
+
static int verify_tsk(int map_fd, int client_fd)
{
int err, cfd = client_fd;
@@ -147,11 +165,8 @@ static void test_base(void)
if (!ASSERT_GE(cgroup_fd, 0, "test__join_cgroup"))
return;
- SYS(fail, "ip netns add %s", NS_TEST);
- SYS(fail, "ip -net %s link set dev lo up", NS_TEST);
-
- nstoken = open_netns(NS_TEST);
- if (!ASSERT_OK_PTR(nstoken, "open_netns"))
+ nstoken = create_netns();
+ if (!ASSERT_OK_PTR(nstoken, "create_netns"))
goto fail;
/* without MPTCP */
@@ -174,11 +189,7 @@ static void test_base(void)
close(server_fd);
fail:
- if (nstoken)
- close_netns(nstoken);
-
- SYS_NOFAIL("ip netns del " NS_TEST " &> /dev/null");
-
+ cleanup_netns(nstoken);
close(cgroup_fd);
}
--
2.35.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH mptcp-next v13 3/4] selftests/bpf: Fix error checks of mptcp open_and_load
2023-08-12 2:54 [PATCH mptcp-next v13 0/4] bpf: Force to MPTCP Geliang Tang
2023-08-12 2:54 ` [PATCH mptcp-next v13 1/4] bpf: Add update_socket_protocol hook Geliang Tang
2023-08-12 2:54 ` [PATCH mptcp-next v13 2/4] selftests/bpf: Add two mptcp netns helpers Geliang Tang
@ 2023-08-12 2:54 ` Geliang Tang
2023-08-12 2:54 ` [PATCH mptcp-next v13 4/4] selftests/bpf: Add mptcpify test Geliang Tang
3 siblings, 0 replies; 8+ messages in thread
From: Geliang Tang @ 2023-08-12 2:54 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Florent Revest,
Brendan Jackman, Matthieu Baerts, Mat Martineau, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, John Johansen,
Paul Moore, James Morris, Serge E. Hallyn, Stephen Smalley,
Eric Paris, Mykola Lysenko, Shuah Khan, Simon Horman
Cc: Geliang Tang, bpf, netdev, mptcp, linux-security-module, selinux,
linux-kselftest, Yonghong Song
Return libbpf_get_error(), instead of -EIO, for the error from
mptcp_sock__open_and_load().
Load success means prog_fd and map_fd are always valid. So drop these
unneeded ASSERT_GE checks for them in mptcp run_test().
Acked-by: Yonghong Song <yonghong.song@linux.dev>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
tools/testing/selftests/bpf/prog_tests/mptcp.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 76afb5191772..3d3999067e27 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -118,24 +118,14 @@ static int run_test(int cgroup_fd, int server_fd, bool is_mptcp)
sock_skel = mptcp_sock__open_and_load();
if (!ASSERT_OK_PTR(sock_skel, "skel_open_load"))
- return -EIO;
+ return libbpf_get_error(sock_skel);
err = mptcp_sock__attach(sock_skel);
if (!ASSERT_OK(err, "skel_attach"))
goto out;
prog_fd = bpf_program__fd(sock_skel->progs._sockops);
- if (!ASSERT_GE(prog_fd, 0, "bpf_program__fd")) {
- err = -EIO;
- goto out;
- }
-
map_fd = bpf_map__fd(sock_skel->maps.socket_storage_map);
- if (!ASSERT_GE(map_fd, 0, "bpf_map__fd")) {
- err = -EIO;
- goto out;
- }
-
err = bpf_prog_attach(prog_fd, cgroup_fd, BPF_CGROUP_SOCK_OPS, 0);
if (!ASSERT_OK(err, "bpf_prog_attach"))
goto out;
--
2.35.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH mptcp-next v13 4/4] selftests/bpf: Add mptcpify test
2023-08-12 2:54 [PATCH mptcp-next v13 0/4] bpf: Force to MPTCP Geliang Tang
` (2 preceding siblings ...)
2023-08-12 2:54 ` [PATCH mptcp-next v13 3/4] selftests/bpf: Fix error checks of mptcp open_and_load Geliang Tang
@ 2023-08-12 2:54 ` Geliang Tang
2023-08-15 6:23 ` Martin KaFai Lau
3 siblings, 1 reply; 8+ messages in thread
From: Geliang Tang @ 2023-08-12 2:54 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Florent Revest,
Brendan Jackman, Matthieu Baerts, Mat Martineau, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, John Johansen,
Paul Moore, James Morris, Serge E. Hallyn, Stephen Smalley,
Eric Paris, Mykola Lysenko, Shuah Khan, Simon Horman
Cc: Geliang Tang, bpf, netdev, mptcp, linux-security-module, selinux,
linux-kselftest, Yonghong Song
Implement a new test program mptcpify: if the family is AF_INET or
AF_INET6, the type is SOCK_STREAM, and the protocol ID is 0 or
IPPROTO_TCP, set it to IPPROTO_MPTCP. It will be hooked in
update_socket_protocol().
Extend the MPTCP test base, add a selftest test_mptcpify() for the
mptcpify case. Open and load the mptcpify test prog to mptcpify the
TCP sockets dynamically, then use start_server() and connect_to_fd()
to create a TCP socket, but actually what's created is an MPTCP
socket, which can be verified through 'getsockopt(SOL_PROTOCOL)'
and 'nstat' commands.
Acked-by: Yonghong Song <yonghong.song@linux.dev>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
.../testing/selftests/bpf/prog_tests/mptcp.c | 102 ++++++++++++++++++
tools/testing/selftests/bpf/progs/mptcpify.c | 20 ++++
2 files changed, 122 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/mptcpify.c
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 3d3999067e27..a6bbc734a876 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -2,13 +2,19 @@
/* Copyright (c) 2020, Tessares SA. */
/* Copyright (c) 2022, SUSE. */
+#include <netinet/in.h>
#include <test_progs.h>
#include "cgroup_helpers.h"
#include "network_helpers.h"
#include "mptcp_sock.skel.h"
+#include "mptcpify.skel.h"
#define NS_TEST "mptcp_ns"
+#ifndef IPPROTO_MPTCP
+#define IPPROTO_MPTCP 262
+#endif
+
#ifndef TCP_CA_NAME_MAX
#define TCP_CA_NAME_MAX 16
#endif
@@ -183,8 +189,104 @@ static void test_base(void)
close(cgroup_fd);
}
+static void send_byte(int fd)
+{
+ char b = 0x55;
+
+ ASSERT_EQ(write(fd, &b, sizeof(b)), 1, "send single byte");
+}
+
+static int verify_mptcpify(int server_fd)
+{
+ socklen_t optlen;
+ char cmd[256];
+ int protocol;
+ int err = 0;
+
+ optlen = sizeof(protocol);
+ if (!ASSERT_OK(getsockopt(server_fd, SOL_SOCKET, SO_PROTOCOL, &protocol, &optlen),
+ "getsockopt(SOL_PROTOCOL)"))
+ return -1;
+
+ if (!ASSERT_EQ(protocol, IPPROTO_MPTCP, "protocol isn't MPTCP"))
+ err++;
+
+ /* Output of nstat:
+ *
+ * #kernel
+ * MPTcpExtMPCapableSYNACKRX 1 0.0
+ */
+ snprintf(cmd, sizeof(cmd),
+ "ip netns exec %s nstat -asz %s | awk '%s' | grep -q '%s'",
+ NS_TEST, "MPTcpExtMPCapableSYNACKRX",
+ "NR==1 {next} {print $2}", "1");
+ if (!ASSERT_OK(system(cmd), "No MPTcpExtMPCapableSYNACKRX found!"))
+ err++;
+
+ return err;
+}
+
+static int run_mptcpify(int cgroup_fd)
+{
+ int server_fd, client_fd, err = 0;
+ struct mptcpify *mptcpify_skel;
+
+ mptcpify_skel = mptcpify__open_and_load();
+ if (!ASSERT_OK_PTR(mptcpify_skel, "skel_open_load"))
+ return libbpf_get_error(mptcpify_skel);
+
+ err = mptcpify__attach(mptcpify_skel);
+ if (!ASSERT_OK(err, "skel_attach"))
+ goto out;
+
+ /* without MPTCP */
+ server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0);
+ if (!ASSERT_GE(server_fd, 0, "start_server")) {
+ err = -EIO;
+ goto out;
+ }
+
+ client_fd = connect_to_fd(server_fd, 0);
+ if (!ASSERT_GE(client_fd, 0, "connect to fd")) {
+ err = -EIO;
+ goto close_server;
+ }
+
+ send_byte(client_fd);
+ err = verify_mptcpify(server_fd);
+
+ close(client_fd);
+close_server:
+ close(server_fd);
+out:
+ mptcpify__destroy(mptcpify_skel);
+ return err;
+}
+
+static void test_mptcpify(void)
+{
+ struct nstoken *nstoken = NULL;
+ int cgroup_fd;
+
+ cgroup_fd = test__join_cgroup("/mptcpify");
+ if (!ASSERT_GE(cgroup_fd, 0, "test__join_cgroup"))
+ return;
+
+ nstoken = create_netns();
+ if (!ASSERT_OK_PTR(nstoken, "create_netns"))
+ goto fail;
+
+ ASSERT_OK(run_mptcpify(cgroup_fd), "run_mptcpify");
+
+fail:
+ cleanup_netns(nstoken);
+ close(cgroup_fd);
+}
+
void test_mptcp(void)
{
if (test__start_subtest("base"))
test_base();
+ if (test__start_subtest("mptcpify"))
+ test_mptcpify();
}
diff --git a/tools/testing/selftests/bpf/progs/mptcpify.c b/tools/testing/selftests/bpf/progs/mptcpify.c
new file mode 100644
index 000000000000..53301ae8a8f7
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/mptcpify.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023, SUSE. */
+
+#include "vmlinux.h"
+#include <bpf/bpf_tracing.h>
+#include "bpf_tracing_net.h"
+
+char _license[] SEC("license") = "GPL";
+
+SEC("fmod_ret/update_socket_protocol")
+int BPF_PROG(mptcpify, int family, int type, int protocol)
+{
+ if ((family == AF_INET || family == AF_INET6) &&
+ type == SOCK_STREAM &&
+ (!protocol || protocol == IPPROTO_TCP)) {
+ return IPPROTO_MPTCP;
+ }
+
+ return protocol;
+}
--
2.35.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH mptcp-next v13 4/4] selftests/bpf: Add mptcpify test
2023-08-12 2:54 ` [PATCH mptcp-next v13 4/4] selftests/bpf: Add mptcpify test Geliang Tang
@ 2023-08-15 6:23 ` Martin KaFai Lau
2023-08-15 10:08 ` Geliang Tang
0 siblings, 1 reply; 8+ messages in thread
From: Martin KaFai Lau @ 2023-08-15 6:23 UTC (permalink / raw)
To: Geliang Tang
Cc: bpf, netdev, mptcp, linux-security-module, selinux,
linux-kselftest, Yonghong Song, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Florent Revest, Brendan Jackman, Matthieu Baerts, Mat Martineau,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley, Eric Paris, Mykola Lysenko, Shuah Khan,
Simon Horman
On 8/11/23 7:54 PM, Geliang Tang wrote:
> +static int verify_mptcpify(int server_fd)
> +{
> + socklen_t optlen;
> + char cmd[256];
> + int protocol;
> + int err = 0;
> +
> + optlen = sizeof(protocol);
> + if (!ASSERT_OK(getsockopt(server_fd, SOL_SOCKET, SO_PROTOCOL, &protocol, &optlen),
> + "getsockopt(SOL_PROTOCOL)"))
> + return -1;
> +
> + if (!ASSERT_EQ(protocol, IPPROTO_MPTCP, "protocol isn't MPTCP"))
> + err++;
> +
> + /* Output of nstat:
> + *
> + * #kernel
> + * MPTcpExtMPCapableSYNACKRX 1 0.0
> + */
> + snprintf(cmd, sizeof(cmd),
> + "ip netns exec %s nstat -asz %s | awk '%s' | grep -q '%s'",
> + NS_TEST, "MPTcpExtMPCapableSYNACKRX",
> + "NR==1 {next} {print $2}", "1");
Is the mp-capable something that the regular mptcp user want to learn from a fd
also? Does it have a simpler way like to learn this, eg. getsockopt(fd,
SOL_MPTCP, MPTCP_xxx), instead of parsing text output?
> + if (!ASSERT_OK(system(cmd), "No MPTcpExtMPCapableSYNACKRX found!"))
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH mptcp-next v13 4/4] selftests/bpf: Add mptcpify test
2023-08-15 6:23 ` Martin KaFai Lau
@ 2023-08-15 10:08 ` Geliang Tang
2023-08-15 18:57 ` Martin KaFai Lau
0 siblings, 1 reply; 8+ messages in thread
From: Geliang Tang @ 2023-08-15 10:08 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Florent Revest,
Brendan Jackman, Matthieu Baerts, Mat Martineau, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, John Johansen,
Paul Moore, James Morris, Serge E. Hallyn, Stephen Smalley,
Eric Paris, Mykola Lysenko, Shuah Khan, Simon Horman, bpf, netdev,
mptcp, linux-security-module, selinux, linux-kselftest
On Mon, Aug 14, 2023 at 11:23:49PM -0700, Martin KaFai Lau wrote:
> On 8/11/23 7:54 PM, Geliang Tang wrote:
> > +static int verify_mptcpify(int server_fd)
> > +{
> > + socklen_t optlen;
> > + char cmd[256];
> > + int protocol;
> > + int err = 0;
> > +
> > + optlen = sizeof(protocol);
> > + if (!ASSERT_OK(getsockopt(server_fd, SOL_SOCKET, SO_PROTOCOL, &protocol, &optlen),
> > + "getsockopt(SOL_PROTOCOL)"))
> > + return -1;
> > +
> > + if (!ASSERT_EQ(protocol, IPPROTO_MPTCP, "protocol isn't MPTCP"))
> > + err++;
> > +
> > + /* Output of nstat:
> > + *
> > + * #kernel
> > + * MPTcpExtMPCapableSYNACKRX 1 0.0
> > + */
> > + snprintf(cmd, sizeof(cmd),
> > + "ip netns exec %s nstat -asz %s | awk '%s' | grep -q '%s'",
> > + NS_TEST, "MPTcpExtMPCapableSYNACKRX",
> > + "NR==1 {next} {print $2}", "1");
>
> Is the mp-capable something that the regular mptcp user want to learn from a
> fd also? Does it have a simpler way like to learn this, eg. getsockopt(fd,
> SOL_MPTCP, MPTCP_xxx), instead of parsing text output?
Thanks Martin. Yes, you're right. A better one is using getsockopt
(MPTCP_INFO) to get the mptcpi_flags, then test the FALLBACK bit to make
sure this MPTCP connection didn't fallback. This is, in other word, this
MPTCP connection has been established correctly. Something like this:
+ optlen = sizeof(info);
+ if (!ASSERT_OK(getsockopt(fd, SOL_MPTCP, MPTCP_INFO, &info, &optlen),
+ "getsockopt(MPTCP_INFO)"))
+ return -1;
+
+ if (!ASSERT_FALSE(info.mptcpi_flags & MPTCP_INFO_FLAG_FALLBACK,
+ "MPTCP fallback"))
+ err++;
It's necessary to add this further check after the MPTCP protocol check
using getsockopt(SOL_PROTOCOL). Since in some cases, the MPTCP protocol
check is not enough. Say, if we change TCP protocol into MPTCP using
"cgroup/sock_create", the hook of BPF_CGROUP_RUN_PROG_INET_SOCK in
inet_create(), this place is too late to change the protocol. Although
sk->sk_protocol is set to MPTCP correctly, and the MPTCP protocol check
using getsockopt(SOL_PROTOCOL) will pass. This MPTCP connection will
fallback to TCP connection. So this further check is needed.
-Geliang
>
> > + if (!ASSERT_OK(system(cmd), "No MPTcpExtMPCapableSYNACKRX found!"))
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH mptcp-next v13 4/4] selftests/bpf: Add mptcpify test
2023-08-15 10:08 ` Geliang Tang
@ 2023-08-15 18:57 ` Martin KaFai Lau
0 siblings, 0 replies; 8+ messages in thread
From: Martin KaFai Lau @ 2023-08-15 18:57 UTC (permalink / raw)
To: Geliang Tang
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Florent Revest, Brendan Jackman,
Matthieu Baerts, Mat Martineau, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, John Johansen, Paul Moore,
James Morris, Serge E. Hallyn, Stephen Smalley, Eric Paris,
Mykola Lysenko, Shuah Khan, Simon Horman, bpf, netdev, mptcp,
linux-security-module, selinux, linux-kselftest
On 8/15/23 3:08 AM, Geliang Tang wrote:
> On Mon, Aug 14, 2023 at 11:23:49PM -0700, Martin KaFai Lau wrote:
>> On 8/11/23 7:54 PM, Geliang Tang wrote:
>>> +static int verify_mptcpify(int server_fd)
>>> +{
>>> + socklen_t optlen;
>>> + char cmd[256];
>>> + int protocol;
>>> + int err = 0;
>>> +
>>> + optlen = sizeof(protocol);
>>> + if (!ASSERT_OK(getsockopt(server_fd, SOL_SOCKET, SO_PROTOCOL, &protocol, &optlen),
>>> + "getsockopt(SOL_PROTOCOL)"))
>>> + return -1;
>>> +
>>> + if (!ASSERT_EQ(protocol, IPPROTO_MPTCP, "protocol isn't MPTCP"))
>>> + err++;
>>> +
>>> + /* Output of nstat:
>>> + *
>>> + * #kernel
>>> + * MPTcpExtMPCapableSYNACKRX 1 0.0
>>> + */
>>> + snprintf(cmd, sizeof(cmd),
>>> + "ip netns exec %s nstat -asz %s | awk '%s' | grep -q '%s'",
>>> + NS_TEST, "MPTcpExtMPCapableSYNACKRX",
>>> + "NR==1 {next} {print $2}", "1");
>>
>> Is the mp-capable something that the regular mptcp user want to learn from a
>> fd also? Does it have a simpler way like to learn this, eg. getsockopt(fd,
>> SOL_MPTCP, MPTCP_xxx), instead of parsing text output?
>
> Thanks Martin. Yes, you're right. A better one is using getsockopt
> (MPTCP_INFO) to get the mptcpi_flags, then test the FALLBACK bit to make
> sure this MPTCP connection didn't fallback. This is, in other word, this
> MPTCP connection has been established correctly. Something like this:
>
> + optlen = sizeof(info);
> + if (!ASSERT_OK(getsockopt(fd, SOL_MPTCP, MPTCP_INFO, &info, &optlen),
> + "getsockopt(MPTCP_INFO)"))
> + return -1;
> +
> + if (!ASSERT_FALSE(info.mptcpi_flags & MPTCP_INFO_FLAG_FALLBACK,
> + "MPTCP fallback"))
> + err++;
>
> It's necessary to add this further check after the MPTCP protocol check
> using getsockopt(SOL_PROTOCOL). Since in some cases, the MPTCP protocol
> check is not enough. Say, if we change TCP protocol into MPTCP using
> "cgroup/sock_create", the hook of BPF_CGROUP_RUN_PROG_INET_SOCK in
> inet_create(), this place is too late to change the protocol. Although
> sk->sk_protocol is set to MPTCP correctly, and the MPTCP protocol check
> using getsockopt(SOL_PROTOCOL) will pass. This MPTCP connection will
> fallback to TCP connection. So this further check is needed.
If I read it correctly, it sounds like the "ip netns... nstat.... awk...grep..."
can be replaced with the getsockopt(MPTCP_INFO)? If that is the case, could you
respin one more time to do getsockopt(MPTCP_INFO) instead? I would like the test
to avoid parsing text output and have less dependency on external binary/library
if possible (On top of 'ip', the above uses nstat, awk, grep...). This will make
the bpf CI and other developers' environment tricky to maintain. eg. fwiw,
although unrelated to the commands used above, my dev environment suddenly like
to give this text output when I run "e"grep: "egrep: warning: egrep is
obsolescent; using grep -E"
>>
>>> + if (!ASSERT_OK(system(cmd), "No MPTcpExtMPCapableSYNACKRX found!"))
>>
>>
^ permalink raw reply [flat|nested] 8+ messages in thread