[PATCH net] ipv6: fix the check before getting the cookie in rt6_get_cookie

[PATCH net] ipv6: fix the check before getting the cookie in rt6_get_cookie

[Xin Long]

In Jianlin's testing, netperf was broken with 'Connection reset by peer',
as the cookie check failed in rt6_check() and ip6_dst_check() always
returned NULL.

It's caused by Commit 93531c674315 ("net/ipv6: separate handling of FIB
entries from dst based routes"), where the cookie can be got only when
'c1'(see below) for setting dst_cookie whereas rt6_check() is called
when !'c1' for checking dst_cookie, as we can see in ip6_dst_check().

Since in ip6_dst_check() both rt6_dst_from_check() (c1) and rt6_check()
(!c1) will check the 'from' cookie, this patch is to remove the c1 check
in rt6_get_cookie(), so that the dst_cookie can always be set properly.

c1:
  (rt->rt6i_flags & RTF_PCPU || unlikely(!list_empty(&rt->rt6i_uncached)))

Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
Reported-by: Jianlin Shi <jishi@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/ip6_fib.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 525f701..d6d936c 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -263,8 +263,7 @@ static inline u32 rt6_get_cookie(const struct rt6_info *rt)
 	rcu_read_lock();
 
 	from = rcu_dereference(rt->from);
-	if (from && (rt->rt6i_flags & RTF_PCPU ||
-	    unlikely(!list_empty(&rt->rt6i_uncached))))
+	if (from)
 		fib6_get_cookie_safe(from, &cookie);
 
 	rcu_read_unlock();
-- 
2.1.0

Re: [PATCH net] ipv6: fix the check before getting the cookie in rt6_get_cookie

[David Miller]

From: Xin Long <lucien.xin@gmail.com>
Date: Sun,  2 Jun 2019 19:10:46 +0800

> In Jianlin's testing, netperf was broken with 'Connection reset by peer',
> as the cookie check failed in rt6_check() and ip6_dst_check() always
> returned NULL.
> 
> It's caused by Commit 93531c674315 ("net/ipv6: separate handling of FIB
> entries from dst based routes"), where the cookie can be got only when
> 'c1'(see below) for setting dst_cookie whereas rt6_check() is called
> when !'c1' for checking dst_cookie, as we can see in ip6_dst_check().
> 
> Since in ip6_dst_check() both rt6_dst_from_check() (c1) and rt6_check()
> (!c1) will check the 'from' cookie, this patch is to remove the c1 check
> in rt6_get_cookie(), so that the dst_cookie can always be set properly.
> 
> c1:
>   (rt->rt6i_flags & RTF_PCPU || unlikely(!list_empty(&rt->rt6i_uncached)))
> 
> Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Applied and queued up for -stable.

Re: [PATCH net] ipv6: fix the check before getting the cookie in rt6_get_cookie

[David Ahern]

On 6/2/19 5:10 AM, Xin Long wrote:
> In Jianlin's testing, netperf was broken with 'Connection reset by peer',
> as the cookie check failed in rt6_check() and ip6_dst_check() always
> returned NULL.

Any particular test or setup that is causing the reset? I do not see
that problem in general.

Re: [PATCH net] ipv6: fix the check before getting the cookie in rt6_get_cookie

[Xin Long]

On Thu, Jun 6, 2019 at 11:35 PM David Ahern <dsahern@gmail.com> wrote:
>
> On 6/2/19 5:10 AM, Xin Long wrote:
> > In Jianlin's testing, netperf was broken with 'Connection reset by peer',
> > as the cookie check failed in rt6_check() and ip6_dst_check() always
> > returned NULL.
>
> Any particular test or setup that is causing the reset? I do not see
> that problem in general.
>
by running the script below, netperf ends up with:

Connection reset by peer
netperf: remote error 104


--------------------------

#!/bin/bash

ip netns add client
ip netns add server
ip netns add route

ip link add veth0_c type veth peer name veth0_cr
ip link add veth0_s type veth peer name veth0_sr
ip link set veth0_c netns client
ip link set veth0_s netns server
ip link set veth0_cr netns route
ip link set veth0_sr netns route

ip netns exec client ip link set lo up
ip netns exec client ip link set veth0_c up

ip netns exec server ip link set lo up
ip netns exec server ip link set veth0_s up

ip netns exec route ip link set lo up
ip netns exec route ip link set veth0_cr up
ip netns exec route ip link set veth0_sr up

ip netns exec client ip addr add 2000::1/64 dev veth0_c
ip netns exec client ip addr add 10.10.0.1/24 dev veth0_c
ip netns exec route ip addr add 2000::a/64 dev veth0_cr
ip netns exec route ip addr add 10.10.0.254/24 dev veth0_cr

ip netns exec server ip addr add 2001::1/64 dev veth0_s
ip netns exec server ip addr add 10.10.1.1/24 dev veth0_s
ip netns exec route ip addr add 2001::a/64 dev veth0_sr
ip netns exec route ip addr add 10.10.1.254/24 dev veth0_sr

ip netns exec client ip route add default via 10.10.0.254
ip netns exec client ip -6 route add default via 2000::a

ip netns exec server ip route add default via 10.10.1.254
ip netns exec server ip -6 route add default via 2001::a

ip netns exec route sysctl -w net.ipv4.conf.all.forwarding=1
ip netns exec route sysctl -w net.ipv6.conf.all.forwarding=1

ip netns exec client ping 10.10.1.1 -c 2
ip netns exec client ping6 2001::1 -c 2

ip netns exec route ip link set veth0_sr mtu 1400
ip netns exec server ip link set veth0_s mtu 1400

ip netns exec server netserver -d

ip netns exec client ping6 2001::1 -c 1 -s 1400 -M do
ip netns exec client ip route get 2001::1

ip netns exec client ip link set veth0_c mtu 1300
ip netns exec client ip route get 2001::1

ip netns exec client ip link set veth0_c mtu 1600
ip netns exec client ip route get 2001::1

ip netns exec client ping6 2001::1 -c 1 -s 1400 -M do
ip netns exec client ip route get 2001::1

ip netns exec client netperf -6 -H 2001::1 -t SCTP_STREAM -l 30 -- -m 16k &
#ip netns exec client netperf -6 -H 2001::1 -t TCP_STREAM -l 30 -- -m 16k &
#ip netns exec client netperf -6 -H 2001::1 -t UDP_STREAM -l 30 -- -R 1 &
pid=$!

for i in `seq 1 5`
do
ip netns exec client ip link set veth0_c mtu 1300
sleep 1
ip netns exec client ip link set veth0_c mtu 1600
sleep 1
done

wait $pid

Re: [PATCH net] ipv6: fix the check before getting the cookie in rt6_get_cookie

[David Ahern]

On 6/6/19 10:10 AM, Xin Long wrote:
> On Thu, Jun 6, 2019 at 11:35 PM David Ahern <dsahern@gmail.com> wrote:
>>
>> On 6/2/19 5:10 AM, Xin Long wrote:
>>> In Jianlin's testing, netperf was broken with 'Connection reset by peer',
>>> as the cookie check failed in rt6_check() and ip6_dst_check() always
>>> returned NULL.
>>
>> Any particular test or setup that is causing the reset? I do not see
>> that problem in general.
>>
> by running the script below, netperf ends up with:
> 
> Connection reset by peer
> netperf: remote error 104
> 

No luck reproducing on 5.1 or 5.2. I get the theory though - resetting
the mtu to change exceptions. Interesting test case to add under selftests