Skip to contentSkip to conversation

ipv6: Fix socket connection with DSCP fib-rules.

PATCH Applied Guillaume Nault 4 participants 10 messages Feb 8, 2023 – Feb 10, 2023

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

cover ipv6: Fix socket connection with DSCP fib-rules.

The "flowlabel" field of struct flowi6 is used to store both the actual flow label and the DS Field (or Traffic Class). However the .connect handlers of datagram and TCP sockets don't set the DS Field part when doing their route lookup. This breaks fib-rules that match on DSCP.

Guillaume Nault (3):
ipv6: Fix datagram socket connect with DSCP.
ipv6: Fix tcp socket connect with DSCP.
selftests: fib_rule_tests: Test UDP and TCP connections with DSCP
rules.

 net/ipv6/datagram.c                           |   2 +-
 net/ipv6/tcp_ipv6.c                           |   1 +
 tools/testing/selftests/net/fib_rule_tests.sh | 128 +++++++++++++++++-
 tools/testing/selftests/net/nettest.c         |  51 ++++++-
 4 files changed, 179 insertions(+), 3 deletions(-)
Signature · 1 line
2.30.2
#1 lore raw

PATCH 1/3 ipv6: Fix datagram socket connection with DSCP.

Take into account the IPV6_TCLASS socket option (DSCP) in ip6_datagram_flow_key_init(). Otherwise fib6_rule_match() can't properly match the DSCP value, resulting in invalid route lookup.

For example:

ip route add unreachable table main 2001:db8::10/124

ip route add table 100 2001:db8::10/124 dev eth0 ip -6 rule add dsfield 0x04 table 100

echo test | socat - UDP6:[2001:db8::11]:54321,ipv6-tclass=0x04

Without this patch, socat fails at connect() time ("No route to host") because the fib-rule doesn't jump to table 100 and the lookup ends up being done in the main table.

Fixes 2cc67cc731d9 ("[IPV6] ROUTE: Routing by Traffic Class.")
---
 net/ipv6/datagram.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ipv6/datagram.c +1 -1
index e624497fa992..9b6818453afe 100644
@@ -51,7 +51,7 @@ static void ip6_datagram_flow_key_init(struct flowi6 *fl6, struct sock *sk)
 	fl6->flowi6_mark = sk->sk_mark;
 	fl6->fl6_dport = inet->inet_dport;
 	fl6->fl6_sport = inet->inet_sport;
-	fl6->flowlabel = np->flow_label;
+	fl6->flowlabel = ip6_make_flowinfo(np->tclass, np->flow_label);
 	fl6->flowi6_uid = sk->sk_uid;
 
 	if (!oif)
Signature · 1 line
2.30.2
Reviewed-by on v1 1/3
#2 lore raw

Re: [PATCH net 1/3] ipv6: Fix datagram socket connection with DSCP.

Quoted 9 lines · Guillaume Nault

Take into account the IPV6_TCLASS socket option (DSCP) in ip6_datagram_flow_key_init(). Otherwise fib6_rule_match() can't properly match the DSCP value, resulting in invalid route lookup.

Fixes 2cc67cc731d9 ("[IPV6] ROUTE: Routing by Traffic Class.")
maintainer Reviewed-by on v1 1/3
#3 lore raw

Re: [PATCH net 1/3] ipv6: Fix datagram socket connection with DSCP.

In reply to Guillaume Nault
Quoted 17 lines · Guillaume Nault

Take into account the IPV6_TCLASS socket option (DSCP) in ip6_datagram_flow_key_init(). Otherwise fib6_rule_match() can't properly match the DSCP value, resulting in invalid route lookup.

For example:

ip route add unreachable table main 2001:db8::10/124

ip route add table 100 2001:db8::10/124 dev eth0 ip -6 rule add dsfield 0x04 table 100

echo test | socat - UDP6:[2001:db8::11]:54321,ipv6-tclass=0x04

Without this patch, socat fails at connect() time ("No route to host") because the fib-rule doesn't jump to table 100 and the lookup ends up being done in the main table.

Quoted 6 lines
Fixes 2cc67cc731d9 ("[IPV6] ROUTE: Routing by Traffic Class.")
---
 net/ipv6/datagram.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
#4 lore raw

PATCH 2/3 ipv6: Fix tcp socket connection with DSCP.

In reply to Guillaume Nault

Take into account the IPV6_TCLASS socket option (DSCP) in tcp_v6_connect(). Otherwise fib6_rule_match() can't properly match the DSCP value, resulting in invalid route lookup.

For example:

ip route add unreachable table main 2001:db8::10/124

ip route add table 100 2001:db8::10/124 dev eth0 ip -6 rule add dsfield 0x04 table 100

echo test | socat - TCP6:[2001:db8::11]:54321,ipv6-tclass=0x04

Without this patch, socat fails at connect() time ("No route to host") because the fib-rule doesn't jump to table 100 and the lookup ends up being done in the main table.

Fixes 2cc67cc731d9 ("[IPV6] ROUTE: Routing by Traffic Class.")
---
 net/ipv6/tcp_ipv6.c | 1 +
 1 file changed, 1 insertion(+)
net/ipv6/tcp_ipv6.c +1 -0
index 11b736a76bd7..0d25e813288d 100644
@@ -272,6 +272,7 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
 	fl6.flowi6_proto = IPPROTO_TCP;
 	fl6.daddr = sk->sk_v6_daddr;
 	fl6.saddr = saddr ? *saddr : np->saddr;
+	fl6.flowlabel = ip6_make_flowinfo(np->tclass, np->flow_label);
 	fl6.flowi6_oif = sk->sk_bound_dev_if;
 	fl6.flowi6_mark = sk->sk_mark;
 	fl6.fl6_dport = usin->sin6_port;
Signature · 1 line
2.30.2
Reviewed-by on v1 2/3
#5 lore raw

Re: [PATCH net 2/3] ipv6: Fix tcp socket connection with DSCP.

Quoted 10 lines · Guillaume Nault

Take into account the IPV6_TCLASS socket option (DSCP) in tcp_v6_connect(). Otherwise fib6_rule_match() can't properly match the DSCP value, resulting in invalid route lookup.

being done in the main table.

Fixes 2cc67cc731d9 ("[IPV6] ROUTE: Routing by Traffic Class.")

---

Thanks.

maintainer Reviewed-by on v1 2/3
#6 lore raw

Re: [PATCH net 2/3] ipv6: Fix tcp socket connection with DSCP.

In reply to Guillaume Nault
Quoted 17 lines · Guillaume Nault

Take into account the IPV6_TCLASS socket option (DSCP) in tcp_v6_connect(). Otherwise fib6_rule_match() can't properly match the DSCP value, resulting in invalid route lookup.

For example:

ip route add unreachable table main 2001:db8::10/124

ip route add table 100 2001:db8::10/124 dev eth0 ip -6 rule add dsfield 0x04 table 100

echo test | socat - TCP6:[2001:db8::11]:54321,ipv6-tclass=0x04

Without this patch, socat fails at connect() time ("No route to host") because the fib-rule doesn't jump to table 100 and the lookup ends up being done in the main table.

Quoted 6 lines
Fixes 2cc67cc731d9 ("[IPV6] ROUTE: Routing by Traffic Class.")
---
 net/ipv6/tcp_ipv6.c | 1 +
 1 file changed, 1 insertion(+)
#7 lore raw

PATCH 3/3 selftests: fib_rule_tests: Test UDP and TCP connections with DSCP rules.

In reply to Guillaume Nault

Add the fib_rule6_send and fib_rule4_send tests to verify that DSCP values are properly taken into account when UDP or TCP sockets try to connect().

Tests are done with nettest, which needs a new option to specify the DS Field value of the socket being tested. This new option is named '-Q', in reference to the similar option used by ping.

---
 tools/testing/selftests/net/fib_rule_tests.sh | 128 +++++++++++++++++-
 tools/testing/selftests/net/nettest.c         |  51 ++++++-
 2 files changed, 177 insertions(+), 2 deletions(-)
tools/testing/selftests/net/fib_rule_tests.sh +127 -1
index c245476fa29d..63c3eaec8d30 100755
@@ -10,8 +10,10 @@ ret=0
 
 PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
 IP="ip -netns testns"
+IP_PEER="ip -netns peerns"
 
 RTABLE=100
+RTABLE_PEER=101
 GW_IP4=192.51.100.2
 SRC_IP=192.51.100.3
 GW_IP6=2001:db8:1::2
@@ -20,7 +22,9 @@ SRC_IP6=2001:db8:1::3
 DEV_ADDR=192.51.100.1
 DEV_ADDR6=2001:db8:1::1
 DEV=dummy0
-TESTS="fib_rule6 fib_rule4"
+TESTS="fib_rule6 fib_rule4 fib_rule6_connect fib_rule4_connect"
+
+SELFTEST_PATH=""
 
 log_test()
 {
@@ -52,6 +56,31 @@ log_section()
 	echo "######################################################################"
 }
 
+check_nettest()
+{
+	if which nettest > /dev/null 2>&1; then
+		return 0
+	fi
+
+	# Add the selftest directory to PATH if not already done
+	if [ "${SELFTEST_PATH}" = "" ]; then
+		SELFTEST_PATH="$(dirname $0)"
+		PATH="${PATH}:${SELFTEST_PATH}"
+
+		# Now retry with the new path
+		if which nettest > /dev/null 2>&1; then
+			return 0
+		fi
+
+		if [ "${ret}" -eq 0 ]; then
+			ret="${ksft_skip}"
+		fi
+		echo "nettest not found (try 'make -C ${SELFTEST_PATH} nettest')"
+	fi
+
+	return 1
+}
+
 setup()
 {
 	set -e
@@ -72,6 +101,39 @@ cleanup()
 	ip netns del testns
 }
 
+setup_peer()
+{
+	set -e
+
+	ip netns add peerns
+	$IP_PEER link set dev lo up
+
+	ip link add name veth0 netns testns type veth \
+		peer name veth1 netns peerns
+	$IP link set dev veth0 up
+	$IP_PEER link set dev veth1 up
+
+	$IP address add 192.0.2.10 peer 192.0.2.11/32 dev veth0
+	$IP_PEER address add 192.0.2.11 peer 192.0.2.10/32 dev veth1
+
+	$IP address add 2001:db8::10 peer 2001:db8::11/128 dev veth0 nodad
+	$IP_PEER address add 2001:db8::11 peer 2001:db8::10/128 dev veth1 nodad
+
+	$IP_PEER address add 198.51.100.11/32 dev lo
+	$IP route add table $RTABLE_PEER 198.51.100.11/32 via 192.0.2.11
+
+	$IP_PEER address add 2001:db8::1:11/128 dev lo
+	$IP route add table $RTABLE_PEER 2001:db8::1:11/128 via 2001:db8::11
+
+	set +e
+}
+
+cleanup_peer()
+{
+	$IP link del dev veth0
+	ip netns del peerns
+}
+
 fib_check_iproute_support()
 {
 	ip rule help 2>&1 | grep -q $1
@@ -190,6 +252,37 @@ fib_rule6_test()
 	fi
 }
 
+# Verify that the IPV6_TCLASS option of UDPv6 and TCPv6 sockets is properly
+# taken into account when connecting the socket and when sending packets.
+fib_rule6_connect_test()
+{
+	local dsfield
+
+	if ! check_nettest; then
+		echo "SKIP: Could not run test without nettest tool"
+		return
+	fi
+
+	setup_peer
+	$IP -6 rule add dsfield 0x04 table $RTABLE_PEER
+
+	# Combine the base DS Field value (0x04) with all possible ECN values
+	# (Not-ECT: 0, ECT(1): 1, ECT(0): 2, CE: 3).
+	# The ECN bits shouldn't influence the result of the test.
+	for dsfield in 0x04 0x05 0x06 0x07; do
+		nettest -q -6 -B -t 5 -N testns -O peerns -U -D \
+			-Q "${dsfield}" -l 2001:db8::1:11 -r 2001:db8::1:11
+		log_test $? 0 "rule6 dsfield udp connect (dsfield ${dsfield})"
+
+		nettest -q -6 -B -t 5 -N testns -O peerns -Q "${dsfield}" \
+			-l 2001:db8::1:11 -r 2001:db8::1:11
+		log_test $? 0 "rule6 dsfield tcp connect (dsfield ${dsfield})"
+	done
+
+	$IP -6 rule del dsfield 0x04 table $RTABLE_PEER
+	cleanup_peer
+}
+
 fib_rule4_del()
 {
 	$IP rule del $1
@@ -296,6 +389,37 @@ fib_rule4_test()
 	fi
 }
 
+# Verify that the IP_TOS option of UDPv4 and TCPv4 sockets is properly taken
+# into account when connecting the socket and when sending packets.
+fib_rule4_connect_test()
+{
+	local dsfield
+
+	if ! check_nettest; then
+		echo "SKIP: Could not run test without nettest tool"
+		return
+	fi
+
+	setup_peer
+	$IP -4 rule add dsfield 0x04 table $RTABLE_PEER
+
+	# Combine the base DS Field value (0x04) with all possible ECN values
+	# (Not-ECT: 0, ECT(1): 1, ECT(0): 2, CE: 3).
+	# The ECN bits shouldn't influence the result of the test.
+	for dsfield in 0x04 0x05 0x06 0x07; do
+		nettest -q -B -t 5 -N testns -O peerns -D -U -Q "${dsfield}" \
+			-l 198.51.100.11 -r 198.51.100.11
+		log_test $? 0 "rule4 dsfield udp connect (dsfield ${dsfield})"
+
+		nettest -q -B -t 5 -N testns -O peerns -Q "${dsfield}" \
+			-l 198.51.100.11 -r 198.51.100.11
+		log_test $? 0 "rule4 dsfield tcp connect (dsfield ${dsfield})"
+	done
+
+	$IP -4 rule del dsfield 0x04 table $RTABLE_PEER
+	cleanup_peer
+}
+
 run_fibrule_tests()
 {
 	log_section "IPv4 fib rule"
@@ -345,6 +469,8 @@ do
 	case $t in
 	fib_rule6_test|fib_rule6)		fib_rule6_test;;
 	fib_rule4_test|fib_rule4)		fib_rule4_test;;
+	fib_rule6_connect_test|fib_rule6_connect)	fib_rule6_connect_test;;
+	fib_rule4_connect_test|fib_rule4_connect)	fib_rule4_connect_test;;
 
 	help) echo "Test names: $TESTS"; exit 0;;
 
tools/testing/selftests/net/nettest.c +50 -1

99 lines not on this page · raw message

Signature · 1 line
2.30.2
maintainer Reviewed-by on v1 3/3
#8 lore raw

Re: [PATCH net 3/3] selftests: fib_rule_tests: Test UDP and TCP connections with DSCP rules.

Quoted 8 lines · Guillaume Nault

Add the fib_rule6_send and fib_rule4_send tests to verify that DSCP values are properly taken into account when UDP or TCP sockets try to connect().

Tests are done with nettest, which needs a new option to specify the DS Field value of the socket being tested. This new option is named '-Q', in reference to the similar option used by ping.

Quoted 6 lines
---
 tools/testing/selftests/net/fib_rule_tests.sh | 128 +++++++++++++++++-
 tools/testing/selftests/net/nettest.c         |  51 ++++++-
 2 files changed, 177 insertions(+), 2 deletions(-)
#9 lore raw
In reply to Guillaume Nault
This series was applied to netdev/net.git (master) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net.git (master) by Jakub Kicinski <[email protected]>:

Quoted 5 lines · On Wed, 8 Feb 2023 18:13:56 +0100 you wrote:

The "flowlabel" field of struct flowi6 is used to store both the actual flow label and the DS Field (or Traffic Class). However the .connect handlers of datagram and TCP sockets don't set the DS Field part when doing their route lookup. This breaks fib-rules that match on DSCP.

Quoted 7 lines

Guillaume Nault (3):
ipv6: Fix datagram socket connect with DSCP.
ipv6: Fix tcp socket connect with DSCP.
selftests: fib_rule_tests: Test UDP and TCP connections with DSCP
rules.

[...]

Here is the summary with links:
- [net,1/3] ipv6: Fix datagram socket connection with DSCP.

    https://git.kernel.org/netdev/net/c/e010ae08c71f
  - [net,2/3] ipv6: Fix tcp socket connection with DSCP.
    https://git.kernel.org/netdev/net/c/8230680f36fd
  - [net,3/3] selftests: fib_rule_tests: Test UDP and TCP connections with DSCP rules.
    https://git.kernel.org/netdev/net/c/c21a20d9d102

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html