netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] ipv4: Fix route deletion when nexthop info is not specified
@ 2022-11-24 21:09 Ido Schimmel
  2022-11-24 21:25 ` Nikolay Aleksandrov
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Ido Schimmel @ 2022-11-24 21:09 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, dsahern, razor, jonas.gorski,
	mlxsw, Ido Schimmel, stable

When the kernel receives a route deletion request from user space it
tries to delete a route that matches the route attributes specified in
the request.

If only prefix information is specified in the request, the kernel
should delete the first matching FIB alias regardless of its associated
FIB info. However, an error is currently returned when the FIB info is
backed by a nexthop object:

 # ip nexthop add id 1 via 192.0.2.2 dev dummy10
 # ip route add 198.51.100.0/24 nhid 1
 # ip route del 198.51.100.0/24
 RTNETLINK answers: No such process

Fix by matching on such a FIB info when legacy nexthop attributes are
not specified in the request. An earlier check already covers the case
where a nexthop ID is specified in the request.

Add tests that cover these flows. Before the fix:

 # ./fib_nexthops.sh -t ipv4_fcnal
 ...
 TEST: Delete route when not specifying nexthop attributes           [FAIL]

 Tests passed:  11
 Tests failed:   1

After the fix:

 # ./fib_nexthops.sh -t ipv4_fcnal
 ...
 TEST: Delete route when not specifying nexthop attributes           [ OK ]

 Tests passed:  12
 Tests failed:   0

No regressions in other tests:

 # ./fib_nexthops.sh
 ...
 Tests passed: 228
 Tests failed:   0

 # ./fib_tests.sh
 ...
 Tests passed: 186
 Tests failed:   0

Cc: stable@vger.kernel.org
Reported-by: Jonas Gorski <jonas.gorski@gmail.com>
Tested-by: Jonas Gorski <jonas.gorski@gmail.com>
Fixes: 493ced1ac47c ("ipv4: Allow routes to use nexthop objects")
Fixes: 6bf92d70e690 ("net: ipv4: fix route with nexthop object delete warning")
Fixes: 61b91eb33a69 ("ipv4: Handle attempt to delete multipath route when fib_info contains an nh reference")
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 net/ipv4/fib_semantics.c                    |  8 +++++---
 tools/testing/selftests/net/fib_nexthops.sh | 11 +++++++++++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index f721c308248b..19a662003eef 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -888,9 +888,11 @@ int fib_nh_match(struct net *net, struct fib_config *cfg, struct fib_info *fi,
 		return 1;
 	}
 
-	/* cannot match on nexthop object attributes */
-	if (fi->nh)
-		return 1;
+	if (fi->nh) {
+		if (cfg->fc_oif || cfg->fc_gw_family || cfg->fc_mp)
+			return 1;
+		return 0;
+	}
 
 	if (cfg->fc_oif || cfg->fc_gw_family) {
 		struct fib_nh *nh;
diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
index ee5e98204d3d..a47b26ab48f2 100755
--- a/tools/testing/selftests/net/fib_nexthops.sh
+++ b/tools/testing/selftests/net/fib_nexthops.sh
@@ -1228,6 +1228,17 @@ ipv4_fcnal()
 	run_cmd "$IP ro add 172.16.101.0/24 nhid 21"
 	run_cmd "$IP ro del 172.16.101.0/24 nexthop via 172.16.1.7 dev veth1 nexthop via 172.16.1.8 dev veth1"
 	log_test $? 2 "Delete multipath route with only nh id based entry"
+
+	run_cmd "$IP nexthop add id 22 via 172.16.1.6 dev veth1"
+	run_cmd "$IP ro add 172.16.102.0/24 nhid 22"
+	run_cmd "$IP ro del 172.16.102.0/24 dev veth1"
+	log_test $? 2 "Delete route when specifying only nexthop device"
+
+	run_cmd "$IP ro del 172.16.102.0/24 via 172.16.1.6"
+	log_test $? 2 "Delete route when specifying only gateway"
+
+	run_cmd "$IP ro del 172.16.102.0/24"
+	log_test $? 0 "Delete route when not specifying nexthop attributes"
 }
 
 ipv4_grp_fcnal()
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net] ipv4: Fix route deletion when nexthop info is not specified
  2022-11-24 21:09 [PATCH net] ipv4: Fix route deletion when nexthop info is not specified Ido Schimmel
@ 2022-11-24 21:25 ` Nikolay Aleksandrov
  2022-11-25  3:52 ` David Ahern
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Nikolay Aleksandrov @ 2022-11-24 21:25 UTC (permalink / raw)
  To: Ido Schimmel, netdev
  Cc: davem, kuba, pabeni, edumazet, dsahern, jonas.gorski, mlxsw, stable

On 24/11/2022 23:09, Ido Schimmel wrote:
> When the kernel receives a route deletion request from user space it
> tries to delete a route that matches the route attributes specified in
> the request.
> 
> If only prefix information is specified in the request, the kernel
> should delete the first matching FIB alias regardless of its associated
> FIB info. However, an error is currently returned when the FIB info is
> backed by a nexthop object:
> 
>  # ip nexthop add id 1 via 192.0.2.2 dev dummy10
>  # ip route add 198.51.100.0/24 nhid 1
>  # ip route del 198.51.100.0/24
>  RTNETLINK answers: No such process
> 
> Fix by matching on such a FIB info when legacy nexthop attributes are
> not specified in the request. An earlier check already covers the case
> where a nexthop ID is specified in the request.
> 
> Add tests that cover these flows. Before the fix:
> 
>  # ./fib_nexthops.sh -t ipv4_fcnal
>  ...
>  TEST: Delete route when not specifying nexthop attributes           [FAIL]
> 
>  Tests passed:  11
>  Tests failed:   1
> 
> After the fix:
> 
>  # ./fib_nexthops.sh -t ipv4_fcnal
>  ...
>  TEST: Delete route when not specifying nexthop attributes           [ OK ]
> 
>  Tests passed:  12
>  Tests failed:   0
> 
> No regressions in other tests:
> 
>  # ./fib_nexthops.sh
>  ...
>  Tests passed: 228
>  Tests failed:   0
> 
>  # ./fib_tests.sh
>  ...
>  Tests passed: 186
>  Tests failed:   0
> 
> Cc: stable@vger.kernel.org
> Reported-by: Jonas Gorski <jonas.gorski@gmail.com>
> Tested-by: Jonas Gorski <jonas.gorski@gmail.com>
> Fixes: 493ced1ac47c ("ipv4: Allow routes to use nexthop objects")
> Fixes: 6bf92d70e690 ("net: ipv4: fix route with nexthop object delete warning")
> Fixes: 61b91eb33a69 ("ipv4: Handle attempt to delete multipath route when fib_info contains an nh reference")
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> ---
>  net/ipv4/fib_semantics.c                    |  8 +++++---
>  tools/testing/selftests/net/fib_nexthops.sh | 11 +++++++++++
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 

Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>

> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index f721c308248b..19a662003eef 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -888,9 +888,11 @@ int fib_nh_match(struct net *net, struct fib_config *cfg, struct fib_info *fi,
>  		return 1;
>  	}
>  
> -	/* cannot match on nexthop object attributes */
> -	if (fi->nh)
> -		return 1;
> +	if (fi->nh) {
> +		if (cfg->fc_oif || cfg->fc_gw_family || cfg->fc_mp)
> +			return 1;
> +		return 0;
> +	}
>  
>  	if (cfg->fc_oif || cfg->fc_gw_family) {
>  		struct fib_nh *nh;
> diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
> index ee5e98204d3d..a47b26ab48f2 100755
> --- a/tools/testing/selftests/net/fib_nexthops.sh
> +++ b/tools/testing/selftests/net/fib_nexthops.sh
> @@ -1228,6 +1228,17 @@ ipv4_fcnal()
>  	run_cmd "$IP ro add 172.16.101.0/24 nhid 21"
>  	run_cmd "$IP ro del 172.16.101.0/24 nexthop via 172.16.1.7 dev veth1 nexthop via 172.16.1.8 dev veth1"
>  	log_test $? 2 "Delete multipath route with only nh id based entry"
> +
> +	run_cmd "$IP nexthop add id 22 via 172.16.1.6 dev veth1"
> +	run_cmd "$IP ro add 172.16.102.0/24 nhid 22"
> +	run_cmd "$IP ro del 172.16.102.0/24 dev veth1"
> +	log_test $? 2 "Delete route when specifying only nexthop device"
> +
> +	run_cmd "$IP ro del 172.16.102.0/24 via 172.16.1.6"
> +	log_test $? 2 "Delete route when specifying only gateway"
> +
> +	run_cmd "$IP ro del 172.16.102.0/24"
> +	log_test $? 0 "Delete route when not specifying nexthop attributes"
>  }
>  
>  ipv4_grp_fcnal()


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] ipv4: Fix route deletion when nexthop info is not specified
  2022-11-24 21:09 [PATCH net] ipv4: Fix route deletion when nexthop info is not specified Ido Schimmel
  2022-11-24 21:25 ` Nikolay Aleksandrov
@ 2022-11-25  3:52 ` David Ahern
  2022-11-25 10:07 ` Thorsten Leemhuis
  2022-11-29  1:00 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: David Ahern @ 2022-11-25  3:52 UTC (permalink / raw)
  To: Ido Schimmel, netdev
  Cc: davem, kuba, pabeni, edumazet, razor, jonas.gorski, mlxsw, stable

On 11/24/22 2:09 PM, Ido Schimmel wrote:
> When the kernel receives a route deletion request from user space it
> tries to delete a route that matches the route attributes specified in
> the request.
> 
> If only prefix information is specified in the request, the kernel
> should delete the first matching FIB alias regardless of its associated
> FIB info. However, an error is currently returned when the FIB info is
> backed by a nexthop object:
> 
>  # ip nexthop add id 1 via 192.0.2.2 dev dummy10
>  # ip route add 198.51.100.0/24 nhid 1
>  # ip route del 198.51.100.0/24
>  RTNETLINK answers: No such process
> 
> Fix by matching on such a FIB info when legacy nexthop attributes are
> not specified in the request. An earlier check already covers the case
> where a nexthop ID is specified in the request.
> 
> Add tests that cover these flows. Before the fix:
> 
>  # ./fib_nexthops.sh -t ipv4_fcnal
>  ...
>  TEST: Delete route when not specifying nexthop attributes           [FAIL]
> 
>  Tests passed:  11
>  Tests failed:   1
> 
> After the fix:
> 
>  # ./fib_nexthops.sh -t ipv4_fcnal
>  ...
>  TEST: Delete route when not specifying nexthop attributes           [ OK ]
> 
>  Tests passed:  12
>  Tests failed:   0
> 
> No regressions in other tests:
> 
>  # ./fib_nexthops.sh
>  ...
>  Tests passed: 228
>  Tests failed:   0
> 
>  # ./fib_tests.sh
>  ...
>  Tests passed: 186
>  Tests failed:   0
> 
> Cc: stable@vger.kernel.org
> Reported-by: Jonas Gorski <jonas.gorski@gmail.com>
> Tested-by: Jonas Gorski <jonas.gorski@gmail.com>
> Fixes: 493ced1ac47c ("ipv4: Allow routes to use nexthop objects")
> Fixes: 6bf92d70e690 ("net: ipv4: fix route with nexthop object delete warning")
> Fixes: 61b91eb33a69 ("ipv4: Handle attempt to delete multipath route when fib_info contains an nh reference")
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> ---
>  net/ipv4/fib_semantics.c                    |  8 +++++---
>  tools/testing/selftests/net/fib_nexthops.sh | 11 +++++++++++
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] ipv4: Fix route deletion when nexthop info is not specified
  2022-11-24 21:09 [PATCH net] ipv4: Fix route deletion when nexthop info is not specified Ido Schimmel
  2022-11-24 21:25 ` Nikolay Aleksandrov
  2022-11-25  3:52 ` David Ahern
@ 2022-11-25 10:07 ` Thorsten Leemhuis
  2022-11-29  1:00 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: Thorsten Leemhuis @ 2022-11-25 10:07 UTC (permalink / raw)
  To: Ido Schimmel, netdev
  Cc: davem, kuba, pabeni, edumazet, dsahern, razor, jonas.gorski,
	mlxsw, stable

Hi, this is your Linux kernel regression tracker.

On 24.11.22 22:09, Ido Schimmel wrote:
> When the kernel receives a route deletion request from user space it
> tries to delete a route that matches the route attributes specified in
> the request.
> [...]
 > Cc: stable@vger.kernel.org
> Reported-by: Jonas Gorski <jonas.gorski@gmail.com>

Many thx for taking care of this. There is one small thing to improve,
please add the following tags here:

Link:
https://lore.kernel.org/r/CAOiHx==ddZr6mvvbzgoAwwhJW76qGNVOcNsTG-6m79Ch%2B=aA5Q@mail.gmail.com/

To explain: Linus[1] and others considered proper link tags in cases
like important, as they allow anyone to look into the backstory weeks or
years from now. That why they should be placed here, as outlined by the
documentation[2]. I care personally, because these tags make my
regression tracking efforts a whole lot easier, as they allow my
tracking bot 'regzbot' to automatically connect reports with patches
posted or committed to fix tracked regressions.

Apropos regzbot, let me tell regzbot to monitor this thread:

#regzbot ^backmonitor:
https://lore.kernel.org/r/CAOiHx==ddZr6mvvbzgoAwwhJW76qGNVOcNsTG-6m79Ch%2B=aA5Q@mail.gmail.com/

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.

[1] for details, see:
https://lore.kernel.org/all/CAHk-=wjMmSZzMJ3Xnskdg4+GGz=5p5p+GSYyFBTh0f-DgvdBWg@mail.gmail.com/
https://lore.kernel.org/all/CAHk-=wgs38ZrfPvy=nOwVkVzjpM3VFU1zobP37Fwd_h9iAD5JQ@mail.gmail.com/
https://lore.kernel.org/all/CAHk-=wjxzafG-=J8oT30s7upn4RhBs6TX-uVFZ5rME+L5_DoJA@mail.gmail.com/

[2] see Documentation/process/submitting-patches.rst
(http://docs.kernel.org/process/submitting-patches.html) and
Documentation/process/5.Posting.rst
(https://docs.kernel.org/process/5.Posting.html)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] ipv4: Fix route deletion when nexthop info is not specified
  2022-11-24 21:09 [PATCH net] ipv4: Fix route deletion when nexthop info is not specified Ido Schimmel
                   ` (2 preceding siblings ...)
  2022-11-25 10:07 ` Thorsten Leemhuis
@ 2022-11-29  1:00 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-11-29  1:00 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: netdev, davem, kuba, pabeni, edumazet, dsahern, razor,
	jonas.gorski, mlxsw, stable

Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 24 Nov 2022 23:09:32 +0200 you wrote:
> When the kernel receives a route deletion request from user space it
> tries to delete a route that matches the route attributes specified in
> the request.
> 
> If only prefix information is specified in the request, the kernel
> should delete the first matching FIB alias regardless of its associated
> FIB info. However, an error is currently returned when the FIB info is
> backed by a nexthop object:
> 
> [...]

Here is the summary with links:
  - [net] ipv4: Fix route deletion when nexthop info is not specified
    https://git.kernel.org/netdev/net/c/d5082d386eee

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-11-29  1:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-24 21:09 [PATCH net] ipv4: Fix route deletion when nexthop info is not specified Ido Schimmel
2022-11-24 21:25 ` Nikolay Aleksandrov
2022-11-25  3:52 ` David Ahern
2022-11-25 10:07 ` Thorsten Leemhuis
2022-11-29  1:00 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).