[PATCH iproute2] lib utils: fix family during af_bit_len calculation

[PATCH iproute2] lib utils: fix family during af_bit_len calculation

[roopa]

From: Roopa Prabhu <roopa@cumulusnetworks.com>

commit f3a2ddc124e0 ("lib utils: Use helpers to get AF bit/byte len")
used a wrong family or family of zero in the default case
during af_bit_len calculation causing ip route commands to
fail with below error

Error: an inet prefix is expected rather than "10.0.2.14/24".

Reported-by: Sven-Haegar Koch <haegar@sdinet.de>
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 lib/utils.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/utils.c b/lib/utils.c
index 9cda268..0d08a86 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -477,7 +477,7 @@ int get_prefix_1(inet_prefix *dst, char *arg, int family)
 
 	err = get_addr_1(dst, arg, family);
 	if (err == 0) {
-		dst->bitlen = af_bit_len(family);
+		dst->bitlen = af_bit_len(dst->family);
 
 		if (slash) {
 			if (get_netmask(&plen, slash+1, 0)
-- 
1.7.10.4

Re: [PATCH iproute2] lib utils: fix family during af_bit_len calculation

[Stephen Hemminger]

On Tue, 17 Mar 2015 19:18:28 -0700
roopa@cumulusnetworks.com wrote:

> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> 
> commit f3a2ddc124e0 ("lib utils: Use helpers to get AF bit/byte len")
> used a wrong family or family of zero in the default case
> during af_bit_len calculation causing ip route commands to
> fail with below error
> 
> Error: an inet prefix is expected rather than "10.0.2.14/24".
> 
> Reported-by: Sven-Haegar Koch <haegar@sdinet.de>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>

Applied, thanks everybody

Re: [PATCH iproute2] lib utils: fix family during af_bit_len calculation

[Eric Dumazet]

On Tue, 2015-03-24 at 15:05 -0700, Stephen Hemminger wrote:
> On Tue, 17 Mar 2015 19:18:28 -0700
> roopa@cumulusnetworks.com wrote:
> 
> > From: Roopa Prabhu <roopa@cumulusnetworks.com>
> > 
> > commit f3a2ddc124e0 ("lib utils: Use helpers to get AF bit/byte len")
> > used a wrong family or family of zero in the default case
> > during af_bit_len calculation causing ip route commands to
> > fail with below error
> > 
> > Error: an inet prefix is expected rather than "10.0.2.14/24".
> > 
> > Reported-by: Sven-Haegar Koch <haegar@sdinet.de>
> > Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> 
> Applied, thanks everybody

okay... but... still not good.

Could someone add some basic regression tests on ss command ?

Vadim, can you take a look at this problem for example ?

root@edumazet-glaptop2:/usr/src/iproute2# ss --version ; ss -t dst 1.2.3.4
ss utility, iproute2-ss131122
State      Recv-Q Send-Q                                                             Local Address:Port                                                                 Peer Address:Port   

(This was the expected result, and an old ss command is working)

But with current git tree we now have :

root@edumazet-glaptop2:/usr/src/iproute2# misc/ss --version ; misc/ss -t dst 1.2.3.4 | head -4
ss utility, iproute2-ss150210
State      Recv-Q Send-Q Local Address:Port                 Peer Address:Port                
ESTAB      0      0      172.29.161.170:37914                74.125.201.138:https                
ESTAB      0      0      172.29.161.170:60388                74.125.69.189:https                
ESTAB      0      0      172.29.161.170:42870                74.125.69.188:5228                 

Meaning filter does not work anymore.

A second problem I noticed are these, although they are not new.

root@edumazet-glaptop2:/usr/src/iproute2# misc/ss -ta dst ::1
Error: an inet prefix is expected rather than ":".
Cannot parse dst/src address.

root@edumazet-glaptop2:/usr/src/iproute2# misc/ss -ta dst ::ffff:1.2.3.4
Error: "1.2.3.4" does not look like a port.
Cannot parse dst/src address.


Thanks so much guys.

Re: [PATCH iproute2] lib utils: fix family during af_bit_len calculation

[Vadim Kochan]

On Wed, Mar 25, 2015 at 06:32:32AM -0700, Eric Dumazet wrote:
> On Tue, 2015-03-24 at 15:05 -0700, Stephen Hemminger wrote:
> > On Tue, 17 Mar 2015 19:18:28 -0700
> > roopa@cumulusnetworks.com wrote:
> > 
> > > From: Roopa Prabhu <roopa@cumulusnetworks.com>
> > > 
> > > commit f3a2ddc124e0 ("lib utils: Use helpers to get AF bit/byte len")
> > > used a wrong family or family of zero in the default case
> > > during af_bit_len calculation causing ip route commands to
> > > fail with below error
> > > 
> > > Error: an inet prefix is expected rather than "10.0.2.14/24".
> > > 
> > > Reported-by: Sven-Haegar Koch <haegar@sdinet.de>
> > > Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> > 
> > Applied, thanks everybody
> 
> okay... but... still not good.
> 
> Could someone add some basic regression tests on ss command ?
> 
> Vadim, can you take a look at this problem for example ?
> 
> root@edumazet-glaptop2:/usr/src/iproute2# ss --version ; ss -t dst 1.2.3.4
> ss utility, iproute2-ss131122
> State      Recv-Q Send-Q                                                             Local Address:Port                                                                 Peer Address:Port   
> 
> (This was the expected result, and an old ss command is working)
> 
> But with current git tree we now have :
> 
> root@edumazet-glaptop2:/usr/src/iproute2# misc/ss --version ; misc/ss -t dst 1.2.3.4 | head -4
> ss utility, iproute2-ss150210
> State      Recv-Q Send-Q Local Address:Port                 Peer Address:Port                
> ESTAB      0      0      172.29.161.170:37914                74.125.201.138:https                
> ESTAB      0      0      172.29.161.170:60388                74.125.69.189:https                
> ESTAB      0      0      172.29.161.170:42870                74.125.69.188:5228                 
> 
> Meaning filter does not work anymore.
> 
> A second problem I noticed are these, although they are not new.
> 
> root@edumazet-glaptop2:/usr/src/iproute2# misc/ss -ta dst ::1
> Error: an inet prefix is expected rather than ":".
> Cannot parse dst/src address.
> 
> root@edumazet-glaptop2:/usr/src/iproute2# misc/ss -ta dst ::ffff:1.2.3.4
> Error: "1.2.3.4" does not look like a port.
> Cannot parse dst/src address.
> 
> 
> Thanks so much guys.
> 
> 

I will try to look at these issues today later.

Regards,

Re: [PATCH iproute2] lib utils: fix family during af_bit_len calculation

[Vadim Kochan]

> okay... but... still not good.
> 
> Could someone add some basic regression tests on ss command ?
> 
> Vadim, can you take a look at this problem for example ?
> 
> root@edumazet-glaptop2:/usr/src/iproute2# ss --version ; ss -t dst 1.2.3.4
> ss utility, iproute2-ss131122
> State      Recv-Q Send-Q                                                             Local Address:Port                                                                 Peer Address:Port   
> 
> (This was the expected result, and an old ss command is working)
> 
> But with current git tree we now have :
> 
> root@edumazet-glaptop2:/usr/src/iproute2# misc/ss --version ; misc/ss -t dst 1.2.3.4 | head -4
> ss utility, iproute2-ss150210
> State      Recv-Q Send-Q Local Address:Port                 Peer Address:Port                
> ESTAB      0      0      172.29.161.170:37914                74.125.201.138:https                
> ESTAB      0      0      172.29.161.170:60388                74.125.69.189:https                
> ESTAB      0      0      172.29.161.170:42870                74.125.69.188:5228                 
> 

I reproduced this once, but after not any more, I even tried to setup
tcp servers with same address but with no luck, would you please dump
stats into file by -D option and send it, in case if you will catch the
issue ?

> Meaning filter does not work anymore.
> 
> A second problem I noticed are these, although they are not new.
> 
> root@edumazet-glaptop2:/usr/src/iproute2# misc/ss -ta dst ::1
> Error: an inet prefix is expected rather than ":".
> Cannot parse dst/src address.
> 
> root@edumazet-glaptop2:/usr/src/iproute2# misc/ss -ta dst ::ffff:1.2.3.4
> Error: "1.2.3.4" does not look like a port.
> Cannot parse dst/src address.
> 
> 
> Thanks so much guys.
> 
> 

Re: [PATCH iproute2] lib utils: fix family during af_bit_len calculation

[Eric Dumazet]

On Wed, 2015-03-25 at 18:13 +0200, Vadim Kochan wrote:

> I reproduced this once, but after not any more, I even tried to setup
> tcp servers with same address but with no luck, would you please dump
> stats into file by -D option and send it, in case if you will catch the
> issue ?

Yeah, this is strange, because after a small change, and a revert, I
could not reproduce it anymore.

Something fishy... maybe a Makefile bug ....

Re: [PATCH iproute2] lib utils: fix family during af_bit_len calculation

[Vadim Kochan]

On Wed, Mar 25, 2015 at 10:07:27AM -0700, Eric Dumazet wrote:
> On Wed, 2015-03-25 at 18:13 +0200, Vadim Kochan wrote:
> 
> > I reproduced this once, but after not any more, I even tried to setup
> > tcp servers with same address but with no luck, would you please dump
> > stats into file by -D option and send it, in case if you will catch the
> > issue ?
> 
> Yeah, this is strange, because after a small change, and a revert, I
> could not reproduce it anymore.
> 
> Something fishy... maybe a Makefile bug ....
> 
> 

Seems this issue is already fixed by this patch from Roopa, because when I
switched to the commit before the fix then I reproduced the issue. I
meant the issue related to :

    ss -t dst 1.2.3.4

Regards,

Re: [PATCH iproute2] lib utils: fix family during af_bit_len calculation

[Eric Dumazet]

On Thu, 2015-03-26 at 16:04 +0200, Vadim Kochan wrote:
> On Wed, Mar 25, 2015 at 10:07:27AM -0700, Eric Dumazet wrote:
> > On Wed, 2015-03-25 at 18:13 +0200, Vadim Kochan wrote:
> > 
> > > I reproduced this once, but after not any more, I even tried to setup
> > > tcp servers with same address but with no luck, would you please dump
> > > stats into file by -D option and send it, in case if you will catch the
> > > issue ?
> > 
> > Yeah, this is strange, because after a small change, and a revert, I
> > could not reproduce it anymore.
> > 
> > Something fishy... maybe a Makefile bug ....
> > 
> > 
> 
> Seems this issue is already fixed by this patch from Roopa, because when I
> switched to the commit before the fix then I reproduced the issue. I
> meant the issue related to :
> 
>     ss -t dst 1.2.3.4

Yes, but I had my git tree up to date, and misc/ss rebuilt.

Let's say it was some kind of user error.

Re: [PATCH iproute2] lib utils: fix family during af_bit_len calculation

[Vadim Kochan]

On Thu, Mar 26, 2015 at 09:34:05AM -0700, Eric Dumazet wrote:
> On Thu, 2015-03-26 at 16:04 +0200, Vadim Kochan wrote:
> > On Wed, Mar 25, 2015 at 10:07:27AM -0700, Eric Dumazet wrote:
> > > On Wed, 2015-03-25 at 18:13 +0200, Vadim Kochan wrote:
> > > 
> > > > I reproduced this once, but after not any more, I even tried to setup
> > > > tcp servers with same address but with no luck, would you please dump
> > > > stats into file by -D option and send it, in case if you will catch the
> > > > issue ?
> > > 
> > > Yeah, this is strange, because after a small change, and a revert, I
> > > could not reproduce it anymore.
> > > 
> > > Something fishy... maybe a Makefile bug ....
> > > 
> > > 
> > 
> > Seems this issue is already fixed by this patch from Roopa, because when I
> > switched to the commit before the fix then I reproduced the issue. I
> > meant the issue related to :
> > 
> >     ss -t dst 1.2.3.4
> 
> Yes, but I had my git tree up to date, and misc/ss rebuilt.
> 
> Let's say it was some kind of user error.
> 
> 

I assume that may be if you did rebuild only misc/ss then probably it
used the old lib/utils.o which still had buggy func for parsing address ?