[PATCH net 0/2] udp6: allow SO_MARK ctrl msg to affect routing

STALE1750d

8 messages, 4 authors, 2021-11-01 · open the first message on its own page

[PATCH net 0/2] udp6: allow SO_MARK ctrl msg to affect routing

From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-10-29 15:51:39

Hi!

Looks like SO_MARK from cmsg does not affect routing policy.
This seems accidental.

I opted for net because of the discrepancy between IPv4
and IPv6, but it never worked and doesn't cause crashes..

Jakub Kicinski (2):
  udp6: allow SO_MARK ctrl msg to affect routing
  selftests: udp: test for passing SO_MARK as cmsg

 net/ipv6/udp.c                              |  2 +-
 tools/testing/selftests/net/.gitignore      |  1 +
 tools/testing/selftests/net/Makefile        |  2 +
 tools/testing/selftests/net/cmsg_so_mark.c  | 67 +++++++++++++++++++++
 tools/testing/selftests/net/cmsg_so_mark.sh | 61 +++++++++++++++++++
 5 files changed, 132 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/net/cmsg_so_mark.c
 create mode 100755 tools/testing/selftests/net/cmsg_so_mark.sh


base-commit: 6de6e46d27ef386feecdbea56b3bfd6c3b3bc1f9
-- 
2.31.1

[PATCH net 1/2] udp6: allow SO_MARK ctrl msg to affect routing

From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-10-29 15:51:40

Commit c6af0c227a22 ("ip: support SO_MARK cmsg")
added propagation of SO_MARK from cmsg to skb->mark.
For IPv4 and raw sockets the mark also affects route
lookup, but in case of IPv6 the flow info is
initialized before cmsg is parsed.

Fixes: c6af0c227a22 ("ip: support SO_MARK cmsg")
Reported-and-tested-by: Xintong Hu <redacted>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/ipv6/udp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 8d785232b479..be6dc64ece29 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1435,7 +1435,6 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	if (!fl6.flowi6_oif)
 		fl6.flowi6_oif = np->sticky_pktinfo.ipi6_ifindex;
 
-	fl6.flowi6_mark = ipc6.sockc.mark;
 	fl6.flowi6_uid = sk->sk_uid;
 
 	if (msg->msg_controllen) {
@@ -1471,6 +1470,7 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	ipc6.opt = opt;
 
 	fl6.flowi6_proto = sk->sk_protocol;
+	fl6.flowi6_mark = ipc6.sockc.mark;
 	fl6.daddr = *daddr;
 	if (ipv6_addr_any(&fl6.saddr) && !ipv6_addr_any(&np->saddr))
 		fl6.saddr = np->saddr;
-- 
2.31.1

[PATCH net 2/2] selftests: udp: test for passing SO_MARK as cmsg

From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-10-29 15:51:40

Before fix:
|  Case IPv6 rejection returned 0, expected 1
|FAIL - 1/4 cases failed

With the fix:
| OK

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
--
CC: shuah@kernel.org
CC: linux-kselftest@vger.kernel.org
---
 tools/testing/selftests/net/.gitignore      |  1 +
 tools/testing/selftests/net/Makefile        |  2 +
 tools/testing/selftests/net/cmsg_so_mark.c  | 67 +++++++++++++++++++++
 tools/testing/selftests/net/cmsg_so_mark.sh | 61 +++++++++++++++++++
 4 files changed, 131 insertions(+)
 create mode 100644 tools/testing/selftests/net/cmsg_so_mark.c
 create mode 100755 tools/testing/selftests/net/cmsg_so_mark.sh
diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index 501550501216..7581a7348e1b 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -35,3 +35,4 @@ test_unix_oob
 gro
 ioam6_parser
 toeplitz
+cmsg_so_mark
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 492b273743b4..f56b652d5cc6 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -28,6 +28,7 @@ TEST_PROGS += veth.sh
 TEST_PROGS += ioam6.sh
 TEST_PROGS += gro.sh
 TEST_PROGS += gre_gso.sh
+TEST_PROGS += cmsg_so_mark.sh
 TEST_PROGS_EXTENDED := in_netns.sh
 TEST_GEN_FILES =  socket nettest
 TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy reuseport_addr_any
@@ -44,6 +45,7 @@ TEST_GEN_FILES += gro
 TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
 TEST_GEN_PROGS += reuseport_dualstack reuseaddr_conflict tls
 TEST_GEN_FILES += toeplitz
+TEST_GEN_FILES += cmsg_so_mark
 
 TEST_FILES := settings
 
diff --git a/tools/testing/selftests/net/cmsg_so_mark.c b/tools/testing/selftests/net/cmsg_so_mark.c
new file mode 100644
index 000000000000..27f2804892a7
--- /dev/null
+++ b/tools/testing/selftests/net/cmsg_so_mark.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <errno.h>
+#include <netdb.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <linux/types.h>
+#include <sys/socket.h>
+
+int main(int argc, const char **argv)
+{
+	char cbuf[CMSG_SPACE(sizeof(__u32))];
+	struct addrinfo hints, *ai;
+	struct cmsghdr *cmsg;
+	struct iovec iov[1];
+	struct msghdr msg;
+	int mark;
+	int err;
+	int fd;
+
+	if (argc != 4) {
+		fprintf(stderr, "Usage: %s <dst_ip> <port> <mark>\n", argv[0]);
+		return 1;
+	}
+	mark = atoi(argv[3]);
+
+	memset(&hints, 0, sizeof(hints));
+	hints.ai_family = AF_UNSPEC;
+	hints.ai_socktype = SOCK_DGRAM;
+
+	ai = NULL;
+	err = getaddrinfo(argv[1], argv[2], &hints, &ai);
+	if (err) {
+		fprintf(stderr, "Can't resolve address: %s\n", strerror(errno));
+		return 1;
+	}
+
+	fd = socket(ai->ai_family, SOCK_DGRAM, IPPROTO_UDP);
+	if (fd < 0) {
+		fprintf(stderr, "Can't open socket: %s\n", strerror(errno));
+		freeaddrinfo(ai);
+		return 1;
+	}
+
+	iov[0].iov_base = "bla";
+	iov[0].iov_len = 4;
+
+	msg.msg_name = ai->ai_addr;
+	msg.msg_namelen = ai->ai_addrlen;
+	msg.msg_iov = iov;
+	msg.msg_iovlen = 1;
+	msg.msg_control = cbuf;
+	msg.msg_controllen = sizeof(cbuf);
+
+	cmsg = CMSG_FIRSTHDR(&msg);
+	cmsg->cmsg_level = SOL_SOCKET;
+	cmsg->cmsg_type = SO_MARK;
+	cmsg->cmsg_len = CMSG_LEN(sizeof(__u32));
+	*(__u32 *)CMSG_DATA(cmsg) = mark;
+
+	err = sendmsg(fd, &msg, 0);
+
+	close(fd);
+	freeaddrinfo(ai);
+	return err != 4;
+}
diff --git a/tools/testing/selftests/net/cmsg_so_mark.sh b/tools/testing/selftests/net/cmsg_so_mark.sh
new file mode 100755
index 000000000000..19c6aab8d0e9
--- /dev/null
+++ b/tools/testing/selftests/net/cmsg_so_mark.sh
@@ -0,0 +1,61 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+NS=ns
+IP4=172.16.0.1/24
+TGT4=172.16.0.2
+IP6=2001:db8:1::1/64
+TGT6=2001:db8:1::2
+MARK=1000
+
+cleanup()
+{
+    ip netns del $NS
+}
+
+trap cleanup EXIT
+
+# Namespaces
+ip netns add $NS
+
+# Connectivity
+ip -netns $NS link add type dummy
+ip -netns $NS link set dev dummy0 up
+ip -netns $NS addr add $IP4 dev dummy0
+ip -netns $NS addr add $IP6 dev dummy0
+
+ip -netns $NS rule add fwmark $MARK lookup 300
+ip -6 -netns $NS rule add fwmark $MARK lookup 300
+ip -netns $NS route add prohibit any table 300
+ip -6 -netns $NS route add prohibit any table 300
+
+# Test
+BAD=0
+TOTAL=0
+
+check_result() {
+    ((TOTAL++))
+    if [ $1 -ne $2 ]; then
+	echo "  Case $3 returned $1, expected $2"
+	((BAD++))
+    fi
+}
+
+ip netns exec $NS ./cmsg_so_mark $TGT4 1234 $((MARK + 1))
+check_result $? 0 "IPv4 pass"
+ip netns exec $NS ./cmsg_so_mark $TGT6 1234 $((MARK + 1))
+check_result $? 0 "IPv6 pass"
+
+ip netns exec $NS ./cmsg_so_mark $TGT4 1234 $MARK
+check_result $? 1 "IPv4 rejection"
+ip netns exec $NS ./cmsg_so_mark $TGT6 1234 $MARK
+check_result $? 1 "IPv6 rejection"
+
+# Summary
+if [ $BAD -ne 0 ]; then
+    echo "FAIL - $BAD/$TOTAL cases failed"
+    exit 1
+else
+    echo "OK"
+    exit 0
+fi
-- 
2.31.1

Re: [PATCH net 1/2] udp6: allow SO_MARK ctrl msg to affect routing

From: David Ahern <hidden>
Date: 2021-10-29 18:22:53

On 10/29/21 9:51 AM, Jakub Kicinski wrote:
Commit c6af0c227a22 ("ip: support SO_MARK cmsg")
added propagation of SO_MARK from cmsg to skb->mark.
For IPv4 and raw sockets the mark also affects route
lookup, but in case of IPv6 the flow info is
initialized before cmsg is parsed.

Fixes: c6af0c227a22 ("ip: support SO_MARK cmsg")
Reported-and-tested-by: Xintong Hu <redacted>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/ipv6/udp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: David Ahern <dsahern@kernel.org>

Re: [PATCH net 2/2] selftests: udp: test for passing SO_MARK as cmsg

From: David Ahern <hidden>
Date: 2021-10-29 18:24:36

On 10/29/21 9:51 AM, Jakub Kicinski wrote:
Before fix:
|  Case IPv6 rejection returned 0, expected 1
|FAIL - 1/4 cases failed

With the fix:
| OK

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
--
CC: shuah@kernel.org
CC: linux-kselftest@vger.kernel.org
---
 tools/testing/selftests/net/.gitignore      |  1 +
 tools/testing/selftests/net/Makefile        |  2 +
 tools/testing/selftests/net/cmsg_so_mark.c  | 67 +++++++++++++++++++++
 tools/testing/selftests/net/cmsg_so_mark.sh | 61 +++++++++++++++++++
 4 files changed, 131 insertions(+)
 create mode 100644 tools/testing/selftests/net/cmsg_so_mark.c
 create mode 100755 tools/testing/selftests/net/cmsg_so_mark.sh
Reviewed-by: David Ahern <dsahern@kernel.org>

Re: [PATCH net 2/2] selftests: udp: test for passing SO_MARK as cmsg

From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: 2021-10-30 01:15:23

On Fri, Oct 29, 2021 at 2:24 PM David Ahern [off-list ref] wrote:
On 10/29/21 9:51 AM, Jakub Kicinski wrote:
quoted
Before fix:
|  Case IPv6 rejection returned 0, expected 1
|FAIL - 1/4 cases failed

With the fix:
| OK

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
--
CC: shuah@kernel.org
CC: linux-kselftest@vger.kernel.org
---
 tools/testing/selftests/net/.gitignore      |  1 +
 tools/testing/selftests/net/Makefile        |  2 +
 tools/testing/selftests/net/cmsg_so_mark.c  | 67 +++++++++++++++++++++
 tools/testing/selftests/net/cmsg_so_mark.sh | 61 +++++++++++++++++++
 4 files changed, 131 insertions(+)
 create mode 100644 tools/testing/selftests/net/cmsg_so_mark.c
 create mode 100755 tools/testing/selftests/net/cmsg_so_mark.sh
Reviewed-by: David Ahern <dsahern@kernel.org>
Reviewed-by: Willem de Bruijn <willemb@google.com>

Re: [PATCH net 1/2] udp6: allow SO_MARK ctrl msg to affect routing

From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: 2021-10-30 01:24:06

On Fri, Oct 29, 2021 at 2:22 PM David Ahern [off-list ref] wrote:
On 10/29/21 9:51 AM, Jakub Kicinski wrote:
quoted
Commit c6af0c227a22 ("ip: support SO_MARK cmsg")
added propagation of SO_MARK from cmsg to skb->mark.
For IPv4 and raw sockets the mark also affects route
lookup, but in case of IPv6 the flow info is
initialized before cmsg is parsed.

Fixes: c6af0c227a22 ("ip: support SO_MARK cmsg")
Reported-and-tested-by: Xintong Hu <redacted>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/ipv6/udp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: David Ahern <dsahern@kernel.org>
Reviewed-by: Willem de Bruijn <willemb@google.com>

Thanks!

Re: [PATCH net 0/2] udp6: allow SO_MARK ctrl msg to affect routing

From: patchwork-bot+netdevbpf@kernel.org
Date: 2021-11-01 13:20:10

Hello:

This series was applied to netdev/net.git (master)
by David S. Miller [off-list ref]:

On Fri, 29 Oct 2021 08:51:33 -0700 you wrote:
Hi!

Looks like SO_MARK from cmsg does not affect routing policy.
This seems accidental.

I opted for net because of the discrepancy between IPv4
and IPv6, but it never worked and doesn't cause crashes..

[...]
Here is the summary with links:
  - [net,1/2] udp6: allow SO_MARK ctrl msg to affect routing
    https://git.kernel.org/netdev/net/c/42dcfd850e51
  - [net,2/2] selftests: udp: test for passing SO_MARK as cmsg
    https://git.kernel.org/netdev/net/c/b0ced8f290fb

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help