[PATCH v2 mptcp-next 1/2] mptcp: getsockopt: add support for IP_TOS

[PATCH v2 mptcp-next 1/2] mptcp: getsockopt: add support for IP_TOS

[Florian Westphal]

earlier patch added IP_TOS setsockopt support, this allows to get
the value set by earlier setsockopt.

Extends mptcp_put_int_option to handle u8 input/output by
adding required cast.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/mptcp/sockopt.c | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 11cda8629993..44e0a37c567c 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -1053,15 +1053,24 @@ static int mptcp_put_int_option(struct mptcp_sock *msk, char __user *optval,
 
 	if (get_user(len, optlen))
 		return -EFAULT;
-
-	len = min_t(unsigned int, len, sizeof(int));
 	if (len < 0)
 		return -EINVAL;
 
-	if (put_user(len, optlen))
-		return -EFAULT;
-	if (copy_to_user(optval, &val, len))
-		return -EFAULT;
+	if (len < sizeof(int) && len > 0 && val >= 0 && val <= 255) {
+		unsigned char ucval = (unsigned char)val;
+
+		len = 1;
+		if (put_user(len, optlen))
+			return -EFAULT;
+		if (copy_to_user(optval, &ucval, 1))
+			return -EFAULT;
+	} else {
+		len = min_t(unsigned int, len, sizeof(int));
+		if (put_user(len, optlen))
+			return -EFAULT;
+		if (copy_to_user(optval, &val, len))
+			return -EFAULT;
+	}
 
 	return 0;
 }
@@ -1082,6 +1091,19 @@ static int mptcp_getsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 	return -EOPNOTSUPP;
 }
 
+static int mptcp_getsockopt_v4(struct mptcp_sock *msk, int optname,
+			       char __user *optval, int __user *optlen)
+{
+	struct sock *sk = (void *)msk;
+
+	switch (optname) {
+	case IP_TOS:
+		return mptcp_put_int_option(msk, optval, optlen, inet_sk(sk)->tos);
+	}
+
+	return -EOPNOTSUPP;
+}
+
 static int mptcp_getsockopt_sol_mptcp(struct mptcp_sock *msk, int optname,
 				      char __user *optval, int __user *optlen)
 {
@@ -1117,6 +1139,8 @@ int mptcp_getsockopt(struct sock *sk, int level, int optname,
 	if (ssk)
 		return tcp_getsockopt(ssk, level, optname, optval, option);
 
+	if (level == SOL_IP)
+		return mptcp_getsockopt_v4(msk, optname, optval, option);
 	if (level == SOL_TCP)
 		return mptcp_getsockopt_sol_tcp(msk, optname, optval, option);
 	if (level == SOL_MPTCP)
-- 
2.32.0

[PATCH v2 mptcp-next 2/2] selftests: mptcp: check IP_TOS in/out are the same

[Florian Westphal]

Check that getsockopt(IP_TOS) returns what setsockopt(IP_TOS) did set
right before.

Also check that socklen_t == 0 and -1 input values match those
of normal tcp sockets.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 .../selftests/net/mptcp/mptcp_sockopt.c       | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
index 417b11cafafe..5b87e162e704 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
@@ -4,6 +4,7 @@
 
 #include <assert.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <limits.h>
 #include <string.h>
 #include <stdarg.h>
@@ -13,6 +14,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <strings.h>
+#include <time.h>
 #include <unistd.h>
 
 #include <sys/socket.h>
@@ -594,6 +596,44 @@ static int server(int pipefd)
 	return 0;
 }
 
+static void test_ip_tos_sockopt(int fd)
+{
+	uint8_t tos_in, tos_out;
+	socklen_t s;
+	int r;
+
+	tos_in = rand() & 0xfc;
+	r = setsockopt(fd, SOL_IP, IP_TOS, &tos_in, sizeof(tos_out));
+	if (r != 0)
+		die_perror("setsockopt IP_TOS");
+
+	tos_out = 0;
+	s = sizeof(tos_out);
+	r = getsockopt(fd, SOL_IP, IP_TOS, &tos_out, &s);
+	if (r != 0)
+		die_perror("getsockopt IP_TOS");
+
+	if (tos_in != tos_out)
+		xerror("tos %x != %x socklen_t %d\n", tos_in, tos_out, s);
+
+	if (s != 1)
+		xerror("tos should be 1 byte");
+
+	s = 0;
+	r = getsockopt(fd, SOL_IP, IP_TOS, &tos_out, &s);
+	if (r != 0)
+		die_perror("getsockopt IP_TOS 0");
+	if (s != 0)
+		xerror("expect socklen_t == 0");
+
+	s = -1;
+	r = getsockopt(fd, SOL_IP, IP_TOS, &tos_out, &s);
+	if (r != -1 && errno != EINVAL)
+		die_perror("getsockopt IP_TOS did not indicate -EINVAL");
+	if (s != -1)
+		xerror("expect socklen_t == -1");
+}
+
 static int client(int pipefd)
 {
 	int fd = -1;
@@ -611,6 +651,8 @@ static int client(int pipefd)
 		xerror("Unknown pf %d\n", pf);
 	}
 
+	test_ip_tos_sockopt(fd);
+
 	connect_one_server(fd, pipefd);
 
 	return 0;
@@ -642,6 +684,23 @@ static int rcheck(int wstatus, const char *what)
 	return 111;
 }
 
+static void init_rng(void)
+{
+	int fd = open("/dev/urandom", O_RDONLY);
+
+	if (fd >= 0) {
+		unsigned int foo;
+
+		/* can't fail */
+		(void)read(fd, &foo, sizeof(foo));
+
+		close(fd);
+		srand(foo);
+	} else {
+		srand(time(NULL));
+	}
+}
+
 int main(int argc, char *argv[])
 {
 	int e1, e2, wstatus;
@@ -650,6 +709,8 @@ int main(int argc, char *argv[])
 
 	parse_opts(argc, argv);
 
+	init_rng();
+
 	e1 = pipe(pipefds);
 	if (e1 < 0)
 		die_perror("pipe");
-- 
2.32.0

Re: [PATCH v2 mptcp-next 2/2] selftests: mptcp: check IP_TOS in/out are the same

[Matthieu Baerts]

Hi Florian,

On 22/11/2021 16:26, Florian Westphal wrote:
> Check that getsockopt(IP_TOS) returns what setsockopt(IP_TOS) did set
> right before.
> 
> Also check that socklen_t == 0 and -1 input values match those
> of normal tcp sockets.

Thank you for these additional tests!

Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>

> +	tos_in = rand() & 0xfc;
> +	r = setsockopt(fd, SOL_IP, IP_TOS, &tos_in, sizeof(tos_out));
> +	if (r != 0)
> +		die_perror("setsockopt IP_TOS");

Funny, knowing this was coming from you, when I read "die_perror()" I
blocked and asked myself: but why did he pick German words for the
function name? But no, "(p)error" is not a German word, only "die"...
oh ok...

> +static void init_rng(void)
> +{
> +	int fd = open("/dev/urandom", O_RDONLY);
> +
> +	if (fd >= 0) {
> +		unsigned int foo;
> +
> +		/* can't fail */
> +		(void)read(fd, &foo, sizeof(foo));
> +
> +		close(fd);
> +		srand(foo);
> +	} else {
> +		srand(time(NULL));
> +	}
> +}

Do you think we need to modify the other ones in MPTCP selftests?

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Re: [PATCH v2 mptcp-next 2/2] selftests: mptcp: check IP_TOS in/out are the same

[Florian Westphal]

Matthieu Baerts <matthieu.baerts@tessares.net> wrote:
> > +static void init_rng(void)
> > +{
> > +	int fd = open("/dev/urandom", O_RDONLY);
> > +
> > +	if (fd >= 0) {
> > +		unsigned int foo;
> > +
> > +		/* can't fail */
> > +		(void)read(fd, &foo, sizeof(foo));
> > +
> > +		close(fd);
> > +		srand(foo);
> > +	} else {
> > +		srand(time(NULL));
> > +	}
> > +}
> 
> Do you think we need to modify the other ones in MPTCP selftests?

Probably best to add a followup patch that adds internal.h helpers
to reduce this copypastry.

Re: selftests: mptcp: check IP_TOS in/out are the same: Build Failure

[MPTCP CI]

Hi Florian,

Thank you for your modifications, that's great!

But sadly, our CI spotted some issues with it when trying to build it.

You can find more details there:

  https://patchwork.kernel.org/project/mptcp/patch/20211122152651.28466-2-fw@strlen.de/
  https://github.com/multipath-tcp/mptcp_net-next/actions/runs/1494450178

Status: failure
Initiator: matttbe
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/65da757c6e14

Feel free to reply to this email if you cannot access logs, if you need
some support to fix the error, if this doesn't seem to be caused by your
modifications or if the error is a false positive one.

Cheers,
MPTCP GH Action bot

Re: selftests: mptcp: check IP_TOS in/out are the same: Build Failure

[Matthieu Baerts]

Hi Florian,

On 23/11/2021 12:11, MPTCP CI wrote:
> Hi Florian,
> 
> Thank you for your modifications, that's great!
> 
> But sadly, our CI spotted some issues with it when trying to build it.

My bad, I applied the patches manually because Patchew was not able to
do that but I applied them on the wrong branch.

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Re: [PATCH v2 mptcp-next 2/2] selftests: mptcp: check IP_TOS in/out are the same

[Matthieu Baerts]

Hi Florian,

On 22/11/2021 16:26, Florian Westphal wrote:
> Check that getsockopt(IP_TOS) returns what setsockopt(IP_TOS) did set
> right before.
> 
> Also check that socklen_t == 0 and -1 input values match those
> of normal tcp sockets.

Again, thank you for the patches!

Now in our tree (feat. for next):

- 920d96a81032: mptcp: getsockopt: add support for IP_TOS

- 2a48dce13e02: selftests: mptcp: check IP_TOS in/out are the same

- Results: c0fa7477e85c..4d2c9db8443e

Builds and tests are now in progress:



https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20211123T154225

https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Re: [PATCH v2 mptcp-next 1/2] mptcp: getsockopt: add support for IP_TOS

[Matthieu Baerts]

Hi Florian,

On 22/11/2021 16:26, Florian Westphal wrote:
> earlier patch added IP_TOS setsockopt support, this allows to get
> the value set by earlier setsockopt.
> 
> Extends mptcp_put_int_option to handle u8 input/output by
> adding required cast.

(apparently I missed sending this yesterday)

Good idea to make it similar to what is in ip_sockglue.c!

Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net