NACK/Cmnt: [SRU][B][PATCH 0/1] vrf: fix refcnt leak with vxlan slaves

[Tim Gardner]

I also noted that the patch still says 'cherry-picked'. Any time a patch 
is modified you need to change 'cherry-picked' to 'backported'. Your 
description of the backport changes is good, though.

rtg

On 10/5/21 2:16 AM, Stefan Bader wrote:
> On 04.10.21 20:37, Luke Nowakowski-Krijger wrote:
>> This is a backport request by Nicolas Dichtel <nicolas.dichtel at 6wind.com>
>>
>> BugLink: https://bugs.launchpad.net/bugs/1945180
>>
>> [Impact]
>>
>> There are cases, where deleting a VRF device can hang waiting for the 
>> refcnt
>> to drop to 0, with the message:
>>    unregister_netdevice: waiting for vrf1 to become free. Usage count = 1
>>
>> This is fixed upstream with commit b87b04f5019e
>> ("ipv4: Fix device used for dst_alloc with local routes"), included in 
>> linux
>> v5.13. The original patch, which has introduced the bug, is included in
>> linux v4.10.
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b87b04f5019e 
>>
>>
>> [Backports]
>>
>> Removed updates to tools/testing/selftests/net/fib_tests.sh as they do 
>> not
>> exist for bionic. Also changed the struct referenced in the patch from
>> fib_nh_common to fib_nh as well as the associated accesses to the
>> struct.
>>
>> [Test Case]
>>
>> Reproduced the bug with the upstream test case which also is the test in
>> the original patch added to fib_tests.sh.
>> Confirmed that after the patch was applied the test case does not hang
>> and successfully removes VRF device.
>>
>> [Regression Potential]
>>
>> Relatively straightforward in that it links a new dst to a vrf device
>> isntead of the loopback if there is a valid nexthop device.
>>
>> David Ahern (1):
>>    ipv4: Fix device used for dst_alloc with local routes
>>
>>   net/ipv4/route.c | 15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)
>>
> 
> This is not about the patches themselves. Just the fact that they are 
> sent as separate threads. This is one request with the same change, just 
> for Bionic it needs some adjustments. Which is one things that is much 
> simpler to compare when the versions are grouped together. Also it is 
> simpler to check whether all series have been covered. And finally make 
> it more likely to get applied around the same time.
> 
> To do this, you would prepare one patch with --cover-email and the other 
> with --start-number which is +1 from the first set, Here it would be 2.
> Then the subjects should be adjusted. I would propose
> 
> [SRU][B/F][PATCH 0/1] ....
> +- [SRU][B][PATCH 1/1] ...
> +- [SRU][F][PATCH 1/1] ...
> 
> 
> -Stefan
> 
> 

-- 
-----------
Tim Gardner
Canonical, Inc