Re: [PATCH mptcp-next v13 4/4] selftests/bpf: Add mptcpify test

Geliang Tang
2023-08-15T10:08:06Z

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!"))
>
>