| Message ID | 49ff39b1f55c914847cd58678bae6282112db701.1608836260.git.gnault@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [net] ipv4: Ignore ECN bits for fib lookups in fib_compute_spec_dst() | expand |
From: Guillaume Nault <gnault@redhat.com> Date: Thu, 24 Dec 2020 20:01:09 +0100 > RT_TOS() only clears one of the ECN bits. Therefore, when > fib_compute_spec_dst() resorts to a fib lookup, it can return > different results depending on the value of the second ECN bit. > > For example, ECT(0) and ECT(1) packets could be treated differently. > > $ ip netns add ns0 > $ ip netns add ns1 > $ ip link add name veth01 netns ns0 type veth peer name veth10 netns ns1 > $ ip -netns ns0 link set dev lo up > $ ip -netns ns1 link set dev lo up > $ ip -netns ns0 link set dev veth01 up > $ ip -netns ns1 link set dev veth10 up > > $ ip -netns ns0 address add 192.0.2.10/24 dev veth01 > $ ip -netns ns1 address add 192.0.2.11/24 dev veth10 > > $ ip -netns ns1 address add 192.0.2.21/32 dev lo > $ ip -netns ns1 route add 192.0.2.10/32 tos 4 dev veth10 src 192.0.2.21 > $ ip netns exec ns1 sysctl -wq net.ipv4.icmp_echo_ignore_broadcasts=0 > > With TOS 4 and ECT(1), ns1 replies using source address 192.0.2.21 > (ping uses -Q to set all TOS and ECN bits): > > $ ip netns exec ns0 ping -c 1 -b -Q 5 192.0.2.255 > [...] > 64 bytes from 192.0.2.21: icmp_seq=1 ttl=64 time=0.544 ms > > But with TOS 4 and ECT(0), ns1 replies using source address 192.0.2.11 > because the "tos 4" route isn't matched: > > $ ip netns exec ns0 ping -c 1 -b -Q 6 192.0.2.255 > [...] > 64 bytes from 192.0.2.11: icmp_seq=1 ttl=64 time=0.597 ms > > After this patch the ECN bits don't affect the result anymore: > > $ ip netns exec ns0 ping -c 1 -b -Q 6 192.0.2.255 > [...] > 64 bytes from 192.0.2.21: icmp_seq=1 ttl=64 time=0.591 ms > > Fixes: 35ebf65e851c ("ipv4: Create and use fib_compute_spec_dst() helper.") > Signed-off-by: Guillaume Nault <gnault@redhat.com> Applied and queued up for -stable, thanks.
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c index cdf6ec5aa45d..84bb707bd88d 100644 --- a/net/ipv4/fib_frontend.c +++ b/net/ipv4/fib_frontend.c @@ -292,7 +292,7 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb) .flowi4_iif = LOOPBACK_IFINDEX, .flowi4_oif = l3mdev_master_ifindex_rcu(dev), .daddr = ip_hdr(skb)->saddr, - .flowi4_tos = RT_TOS(ip_hdr(skb)->tos), + .flowi4_tos = ip_hdr(skb)->tos & IPTOS_RT_MASK, .flowi4_scope = scope, .flowi4_mark = vmark ? skb->mark : 0, };
RT_TOS() only clears one of the ECN bits. Therefore, when fib_compute_spec_dst() resorts to a fib lookup, it can return different results depending on the value of the second ECN bit. For example, ECT(0) and ECT(1) packets could be treated differently. $ ip netns add ns0 $ ip netns add ns1 $ ip link add name veth01 netns ns0 type veth peer name veth10 netns ns1 $ ip -netns ns0 link set dev lo up $ ip -netns ns1 link set dev lo up $ ip -netns ns0 link set dev veth01 up $ ip -netns ns1 link set dev veth10 up $ ip -netns ns0 address add 192.0.2.10/24 dev veth01 $ ip -netns ns1 address add 192.0.2.11/24 dev veth10 $ ip -netns ns1 address add 192.0.2.21/32 dev lo $ ip -netns ns1 route add 192.0.2.10/32 tos 4 dev veth10 src 192.0.2.21 $ ip netns exec ns1 sysctl -wq net.ipv4.icmp_echo_ignore_broadcasts=0 With TOS 4 and ECT(1), ns1 replies using source address 192.0.2.21 (ping uses -Q to set all TOS and ECN bits): $ ip netns exec ns0 ping -c 1 -b -Q 5 192.0.2.255 [...] 64 bytes from 192.0.2.21: icmp_seq=1 ttl=64 time=0.544 ms But with TOS 4 and ECT(0), ns1 replies using source address 192.0.2.11 because the "tos 4" route isn't matched: $ ip netns exec ns0 ping -c 1 -b -Q 6 192.0.2.255 [...] 64 bytes from 192.0.2.11: icmp_seq=1 ttl=64 time=0.597 ms After this patch the ECN bits don't affect the result anymore: $ ip netns exec ns0 ping -c 1 -b -Q 6 192.0.2.255 [...] 64 bytes from 192.0.2.21: icmp_seq=1 ttl=64 time=0.591 ms Fixes: 35ebf65e851c ("ipv4: Create and use fib_compute_spec_dst() helper.") Signed-off-by: Guillaume Nault <gnault@redhat.com> --- net/ipv4/fib_frontend.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)