diff mbox series

[iproute2] treat "default" and "all"/"any" parameters differenty

Message ID 3ded09a4-54f9-9d82-d952-33527a6a3e58@msu.ru
State Superseded, archived
Delegated to: stephen hemminger
Headers show
Series [iproute2] treat "default" and "all"/"any" parameters differenty | expand

Commit Message

Alexander Zubkov March 13, 2018, 8:19 p.m. UTC 
Debian maintainer found that basic command:
	# ip route flush all
No longer worked as expected which breaks user scripts and
expectations. It no longer flushed all IPv4 routes.

Recently behaviour of "default" prefix parameter was corrected. But at
the same time behaviour of "all"/"any" was altered too, because they
were the same branch of the code. As those parameters mean different,
they need to be treated differently in code too. This patch reflects
the difference.

Reported-by: Luca Boccassi <bluca@debian.org>
Signed-off-by: Alexander Zubkov <green@msu.ru>
---
  lib/utils.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)


--
1.9.1

Comments

Luca Boccassi March 13, 2018, 9:45 p.m. UTC | #1 
On Tue, 2018-03-13 at 21:19 +0100, Alexander Zubkov wrote:
> Debian maintainer found that basic command:
> 	# ip route flush all
> No longer worked as expected which breaks user scripts and
> expectations. It no longer flushed all IPv4 routes.
> 
> Recently behaviour of "default" prefix parameter was corrected. But
> at
> the same time behaviour of "all"/"any" was altered too, because they
> were the same branch of the code. As those parameters mean different,
> they need to be treated differently in code too. This patch reflects
> the difference.
> 
> Reported-by: Luca Boccassi <bluca@debian.org>
> Signed-off-by: Alexander Zubkov <green@msu.ru>
> ---
>   lib/utils.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/utils.c b/lib/utils.c
> index 9fa5220..b289d9c 100644
> --- a/lib/utils.c
> +++ b/lib/utils.c
> @@ -658,7 +658,8 @@ int get_prefix_1(inet_prefix *dst, char *arg,
> int 
> family)
>   		dst->family = family;
>   		dst->bytelen = 0;
>   		dst->bitlen = 0;
> -		dst->flags |= PREFIXLEN_SPECIFIED;
> +		if (strcmp(arg, "default") == 0)
> +			dst->flags |= PREFIXLEN_SPECIFIED;
>   		return 0;
>   	}
> 
> --
> 1.9.1

Tested-by: Luca Boccassi <bluca@debian.org>

Yep solves the problem, ls all/any now work as before. Thanks!
Stephen Hemminger March 16, 2018, 8:37 p.m. UTC | #2 
On Tue, 13 Mar 2018 21:19:45 +0100
Alexander Zubkov <green@msu.ru> wrote:

> Debian maintainer found that basic command:
> 	# ip route flush all
> No longer worked as expected which breaks user scripts and
> expectations. It no longer flushed all IPv4 routes.
> 
> Recently behaviour of "default" prefix parameter was corrected. But at
> the same time behaviour of "all"/"any" was altered too, because they
> were the same branch of the code. As those parameters mean different,
> they need to be treated differently in code too. This patch reflects
> the difference.
> 
> Reported-by: Luca Boccassi <bluca@debian.org>
> Signed-off-by: Alexander Zubkov <green@msu.ru>
> ---
>   lib/utils.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/utils.c b/lib/utils.c
> index 9fa5220..b289d9c 100644
> --- a/lib/utils.c
> +++ b/lib/utils.c
> @@ -658,7 +658,8 @@ int get_prefix_1(inet_prefix *dst, char *arg, int 
> family)
>   		dst->family = family;
>   		dst->bytelen = 0;
>   		dst->bitlen = 0;
> -		dst->flags |= PREFIXLEN_SPECIFIED;
> +		if (strcmp(arg, "default") == 0)
> +			dst->flags |= PREFIXLEN_SPECIFIED;
>   		return 0;
>   	}
> 
> --
> 1.9.1
> 

Which code is this a patch against? the current master or followup to my revert
of the original patch?
Alexander Zubkov March 16, 2018, 8:59 p.m. UTC | #3 
Hi Stephen,

This patch is against v4.15.0 only. It will not fit the current master, because there were changes it that areas of code since then. I have made some patch for master too and trying to discuss it with Serhey, who made other changes there. He has not answered yet. If there are no answer for some more time, I'll consider that fix as suitable and send it as a proper patch to the list. That will not require reverse patch too.

16.03.2018, 21:37, "Stephen Hemminger" <stephen@networkplumber.org>:
> On Tue, 13 Mar 2018 21:19:45 +0100
> Alexander Zubkov <green@msu.ru> wrote:
>
>>  Debian maintainer found that basic command:
>>          # ip route flush all
>>  No longer worked as expected which breaks user scripts and
>>  expectations. It no longer flushed all IPv4 routes.
>>
>>  Recently behaviour of "default" prefix parameter was corrected. But at
>>  the same time behaviour of "all"/"any" was altered too, because they
>>  were the same branch of the code. As those parameters mean different,
>>  they need to be treated differently in code too. This patch reflects
>>  the difference.
>>
>>  Reported-by: Luca Boccassi <bluca@debian.org>
>>  Signed-off-by: Alexander Zubkov <green@msu.ru>
>>  ---
>>    lib/utils.c | 3 ++-
>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>
>>  diff --git a/lib/utils.c b/lib/utils.c
>>  index 9fa5220..b289d9c 100644
>>  --- a/lib/utils.c
>>  +++ b/lib/utils.c
>>  @@ -658,7 +658,8 @@ int get_prefix_1(inet_prefix *dst, char *arg, int
>>  family)
>>                    dst->family = family;
>>                    dst->bytelen = 0;
>>                    dst->bitlen = 0;
>>  - dst->flags |= PREFIXLEN_SPECIFIED;
>>  + if (strcmp(arg, "default") == 0)
>>  + dst->flags |= PREFIXLEN_SPECIFIED;
>>                    return 0;
>>            }
>>
>>  --
>>  1.9.1
>
> Which code is this a patch against? the current master or followup to my revert
> of the original patch?
diff mbox series

Patch

diff --git a/lib/utils.c b/lib/utils.c
index 9fa5220..b289d9c 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -658,7 +658,8 @@  int get_prefix_1(inet_prefix *dst, char *arg, int 
family)
  		dst->family = family;
  		dst->bytelen = 0;
  		dst->bitlen = 0;
-		dst->flags |= PREFIXLEN_SPECIFIED;
+		if (strcmp(arg, "default") == 0)
+			dst->flags |= PREFIXLEN_SPECIFIED;
  		return 0;
  	}