[PATCH v2] net: route: Fix vrf dst_entry ref count false increasing

[PATCH v2] net: route: Fix vrf dst_entry ref count false increasing

[linmiaohe]

From: Suanming.Mou <mousuanming@huawei.com>

When config ip in default vrf same as the ip in specified
vrf, fib_lookup will return the route from table local
even if the in device is an enslaved l3mdev. Then the
dst_entry will hold the vrf device rather than loopback
device in local_input of function ip_route_input_slow.
So vrf dst_entry is false increased by route from table
local because device passed to rt_dst_alloc is in device
rather than fib result device.

Here is reproduce step:
1.enslave enp4s0 to vrf2, and config ip address:
ip link add vrf2 type vrf table 1
ip link set vrf2 up
ip link set enp4s0 master vrf2
ip addr ad 125.1.1.1/16 dev enp4s0

2.config same ip in default vrf:
ip addr ad 125.1.1.1/16 dev enp6s0

3.config peer and ping:
ip vrf exec vrf2 ping 125.1.1.2 -c 3

4.del vrf2 link:
ip link del vrf2

System hang with del vrf2 ops and "unregister_netdevice:
waiting for vrf2 to become free. Usage count = 1" occur.

Reported-by: Hui Wang <wanghui104@huawei.com>
Signed-off-by: Suanming.Mou <mousuanming@huawei.com>
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 net/ipv4/route.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 6fdf1c195d8e..74def8710ae8 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2077,6 +2077,11 @@ out:	return err;
 			}
 			do_cache = true;
 		}
+		/* Use fib res nh_dev as local input device because enslaved
+		 * l3mdev may hit route from other rule table. Dst_entry
+		 * should hold right device.
+		 */
+		dev = FIB_RES_DEV(*res);
 	}

 	rth = rt_dst_alloc(l3mdev_master_dev_rcu(dev) ? : net->loopback_dev,
-- 
2.21.GIT

Re: [PATCH v2] net: route: Fix vrf dst_entry ref count false increasing

[David Ahern]

On 5/4/19 7:13 AM, linmiaohe wrote:
> From: Suanming.Mou <mousuanming@huawei.com>
> 
> When config ip in default vrf same as the ip in specified
> vrf, fib_lookup will return the route from table local
> even if the in device is an enslaved l3mdev. Then the

you need to move the local rule with a preference of 0 after the l3mdev
rule.

Re: [PATCH v2] net: route: Fix vrf dst_entry ref count false increasing

[linmiaohe]

On 2019/5/4 22:59, David Ahern wrote:
> On 5/4/19 7:13 AM, linmiaohe wrote:
>> From: Suanming.Mou <mousuanming@huawei.com>
>>
>> When config ip in default vrf same as the ip in specified
>> vrf, fib_lookup will return the route from table local
>> even if the in device is an enslaved l3mdev. Then the
> 
> you need to move the local rule with a preference of 0 after the l3mdev
> rule.
> 
> 

Move the local rule after l3mdev rule can get rid of this problem. And
even if this happend, we can delete the same ip address in default vrf
to fix it.
But I think maybe it's still a problem because other rule with default
vrf out device holds the specified vrf device. It looks unreasonable.

Many Thanks.

Re: [PATCH v2] net: route: Fix vrf dst_entry ref count false increasing

[David Ahern]

On 5/4/19 8:11 PM, linmiaohe wrote:
> 
> 
> On 2019/5/4 22:59, David Ahern wrote:
>> On 5/4/19 7:13 AM, linmiaohe wrote:
>>> From: Suanming.Mou <mousuanming@huawei.com>
>>>
>>> When config ip in default vrf same as the ip in specified
>>> vrf, fib_lookup will return the route from table local
>>> even if the in device is an enslaved l3mdev. Then the
>>
>> you need to move the local rule with a preference of 0 after the l3mdev
>> rule.
>>
>>
> 
> Move the local rule after l3mdev rule can get rid of this problem. And
> even if this happend, we can delete the same ip address in default vrf
> to fix it.
> But I think maybe it's still a problem because other rule with default
> vrf out device holds the specified vrf device. It looks unreasonable.
> 
> Many Thanks.
> 

VRF is implemented using policy routing. If you do not move the local
rule below the l3mdev rule, you are doing a lookup in the local table
first, then vrf table, then main table. Doing the local table first can
result in false hits - like the case of duplicate IP addresses in
default VRF and a VRF. In short, it is just wrong.

Looking at the VRF documentation in the kernel tree I do see such a
comment is missing, but I do mention in all of the VRF tutorials such as
this one (see slide 79):

http://schd.ws/hosted_files/ossna2017/fe/vrf-tutorial-oss.pdf

Re: [PATCH v2] net: route: Fix vrf dst_entry ref count false increasing

[linmiaohe]

On 2019/5/5 10:56, David Ahern wrote:
> On 5/4/19 8:11 PM, linmiaohe wrote:
>>
>>
>> On 2019/5/4 22:59, David Ahern wrote:
>>> On 5/4/19 7:13 AM, linmiaohe wrote:
>>>> From: Suanming.Mou <mousuanming@huawei.com>
>>>>
>>>> When config ip in default vrf same as the ip in specified
>>>> vrf, fib_lookup will return the route from table local
>>>> even if the in device is an enslaved l3mdev. Then the
>>>
>>> you need to move the local rule with a preference of 0 after the l3mdev
>>> rule.
>>>
>>>
>>
>> Move the local rule after l3mdev rule can get rid of this problem. And
>> even if this happend, we can delete the same ip address in default vrf
>> to fix it.
>> But I think maybe it's still a problem because other rule with default
>> vrf out device holds the specified vrf device. It looks unreasonable.
>>
>> Many Thanks.
>>
> 
> VRF is implemented using policy routing. If you do not move the local
> rule below the l3mdev rule, you are doing a lookup in the local table
> first, then vrf table, then main table. Doing the local table first can
> result in false hits - like the case of duplicate IP addresses in
> default VRF and a VRF. In short, it is just wrong.
> 
> Looking at the VRF documentation in the kernel tree I do see such a
> comment is missing, but I do mention in all of the VRF tutorials such as
> this one (see slide 79):
> 
> http://schd.ws/hosted_files/ossna2017/fe/vrf-tutorial-oss.pdf
> 
> .
> 

Thanks for your patience. As it's just like the case of duplicate
IP addresses in default VRF and a VRF and can be fix by hand, we
needn't this patch anymore.

Best regards.