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

[Geliang Tang <geliang.tang@suse.com>]

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