Re: [PATCH v2] iproute2: build nsid-name cache only for commands that need it

[Vadim Kochan]

On Fri, Sep 16, 2016 at 2:21 PM, Anton Aksola <aa...@iki.fi> wrote:
> Ok, I will post a new patch version. Should tests be posted in a separate
> patch?
>
> 2016-09-16 12:44 GMT+03:00 Nicolas Dichtel <nicolas.dich...@6wind.com>:
>>
>> Le 16/09/2016 à 11:23, Vadim Kochan a écrit :
>> [snip]
>> > Would it be useful to add test for this case into testsuite/ ?
>> Yes, it's a good idea.
>>
>> Regards,
>> Nicolas
>
>

Anton, I just looked into tests after when I did post here. I am not
sure it will be trivial,
currently tests are running within separated network namespace by
default (which I did) via
'unshare' tool, and now I see that it is better to call it explicitly
from the each test case. So
I am not sure netns related  tests might be valid if they will be ran
after 'unshare -n', if yes - then there is no problem, otherwise
it needs to be fixed somehow - I will try to do this.

Regards,
Vadim Kochan

Re: [PATCH v2] iproute2: build nsid-name cache only for commands that need it

[Vadim Kochan]

On Fri, Sep 16, 2016 at 12:13 PM, Nicolas Dichtel
<nicolas.dich...@6wind.com> wrote:
> Le 16/09/2016 à 09:22, Anton Aksola a écrit :
>> The calling of netns_map_init() before command parsing introduced
>> a performance issue with large number of namespaces.
>>
>> As commands such as add, del and exec do not need to iterate through
>> /var/run/netns it would be good not no build the cache before executing
>> these commands.
>>
>> Example:
>> unpatched:
>> time seq 1 1000 | xargs -n 1 ip netns add
>>
>> real0m16.832s
>> user0m1.350s
>> sys0m15.029s
>>
>> patched:
>> time seq 1 1000 | xargs -n 1 ip netns add
>>
>> real0m3.859s
>> user0m0.132s
>> sys0m3.205s
>>
>> Signed-off-by: Anton Aksola <aa...@iki.fi>
>> ---
> There is still some differences:
> $ cat test.batch
> netns add foo
> netns set foo 1234
> netns list-id
>
> Before your patch:
> $ ip -b test.batch
> nsid 1234 (iproute2 netns name: foo)
>
> After your patch:
> $ ip -b test.batch
> nsid 1234

Would it be useful to add test for this case into testsuite/ ?

Regards,
Vadim Kochan

Re: [iproute PATCH] ipaddress: Allow listing addresses by type

[Vadim Kochan]

On Wed, Jun 01, 2016 at 09:57:27PM +0200, Phil Sutter wrote:
> Not sure why this was limited to ip-link before. It is semantically
> equal to the 'master' keyword, which is not restricted at all.
> 
Hi Phil,

I don't remember why I did so, but you are right, it might be
used for 'ip addr' too.

> Cc: Vadim Kochan <vadi...@gmail.com>
> Signed-off-by: Phil Sutter <p...@nwl.cc>
> ---
>  ip/ipaddress.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index df363b070d5de..1b5ee838ef3fe 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -1613,7 +1613,7 @@ static int ipaddr_list_flush_or_save(int argc, char 
> **argv, int action)
>   if (!ifindex)
>   invarg("Device does not exist\n", *argv);
>   filter.master = ifindex;
> - } else if (do_link && strcmp(*argv, "type") == 0) {
> + } else if (strcmp(*argv, "type") == 0) {
>   NEXT_ARG();
>   filter.kind = *argv;
>   } else {
> -- 
> 2.8.2
> 

Re: ss filter problem

[Vadim Kochan]

Hi Phil,

On Tue, Mar 29, 2016 at 09:32:42PM +0200, Phil Sutter wrote:
> Hi,
> 
> I am trying to fix a bug in ss filter code, but feel quite lost right
> now. The issue is this:
> 
> | ss -nl -f inet '( sport = :22 )'
> 
> prints not only listening sockets (as requested by -l flag), but
> established ones as well (reproduce by opening ssh connection to
> 127.0.0.1 before calling above).
> 
> In contrast, the following both don't show the established sockets:
> 
> | ss -nl '( sport = :22 )'
> | ss -nl -f inet
> 
> My investigation led me to see that current_filter.states is altered
> after ssfilter_parse() returns, and using gdb with a watchpoint I was
> able to identify parse_hostcond() to be the bad guy: In line 1560, it
> calls filter_af_set() after checking for fam != AF_UNSPEC (which is the
> case, since fam = preferred_family and the latter is changed to AF_INET
> when parsing '-f inet' parameter).

Yes, after removing of fam != AF_UNSPEC body - it works, because
it does not overwrite specified states (-l) from command line, but I
can't say what it may affect else, I will try to investigate it better.

> 
> This whole jumping back and forth confuses me quite effectively. Since
> you did some fixes in the past already, are you possibly able to point
> out where/how this tiny mess has to be fixed?
> 
> I guess in an ideal world we would translate '-l' to 'state listen', '-f
> inet' to 'src inet:*' and pass everything ANDed together to
> ssfilter_parse(). Or maybe that would make things even worse. ;)
> 
> Cheers, Phil

I thought I fixed & tested well ss filter, but seems it would be good to
have good automation testing.

Regards,
Vadim Kochan

Re: [PATCH iproute2] ip route get: change exit to return to support batch commands

[Vadim Kochan]

On Thu, Oct 15, 2015 at 08:13:23PM -0700, roopa wrote:
> On 10/15/15, 7:38 PM, David Ahern wrote:
> > Hi Roopa:
> >
> > On 10/15/15 4:23 PM, Roopa Prabhu wrote:
> >> From: Roopa Prabhu 
> >>
> >> replace exit with return -2 on rtnl_talk failure
> >>
> >> Signed-off-by: Roopa Prabhu 
> >> ---
> >>   ip/iproute.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/ip/iproute.c b/ip/iproute.c
> >> index da25548..b137f55 100644
> >> --- a/ip/iproute.c
> >> +++ b/ip/iproute.c
> >> @@ -1643,7 +1643,7 @@ static int iproute_get(int argc, char **argv)
> >>   req.r.rtm_family = AF_INET;
> >>
> >>   if (rtnl_talk(, , , sizeof(req)) < 0)
> >> -exit(2);
> >> +return -2;
> >>
> >>   if (connected && !from_ok) {
> >>   struct rtmsg *r = NLMSG_DATA();
> >>
> >
> > Why return -2 vs exit(2)? What does the change mean to a user or the 
> > functionality of ip?
> Stephen has documented this someplace. I have seen it before. I now forget 
> where. Right now i am just following
> the rest of the code.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi,

IMHO I think it would be good to have #define's for these return values.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH iproute2] man tc-htb: Fix HRB -> HTB typo

[Vadim Kochan]

From: Vadim Kochan <vadi...@gmail.com>

Changed HRB -> HTB.

Signed-off-by: Vadim Kochan <vadi...@gmail.com>
---
 man/man8/tc-htb.8 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/man8/tc-htb.8 b/man/man8/tc-htb.8
index d196ecd..95f25de 100644
--- a/man/man8/tc-htb.8
+++ b/man/man8/tc-htb.8
@@ -48,7 +48,7 @@ Shaping works as documented in
 .B tc-tbf (8).
 
 .SH CLASSIFICATION
-Within the one HRB instance many classes may exist. Each of these classes
+Within the one HTB instance many classes may exist. Each of these classes
 contains another qdisc, by default 
 .BR tc-pfifo (8).
 
-- 
2.4.2

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH iproute2] configure: Check for Berkeley DB for arpd compilation

[Vadim Kochan]

From: Vadim Kochan <vadi...@gmail.com>

Add check for Berkeley DB header & lib before compile arpd util.

Signed-off-by: Vadim Kochan <vadi...@gmail.com>
---
 configure | 29 +++--
 misc/Makefile |  6 +-
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 3ae4c95..d2540b0 100755
--- a/configure
+++ b/configure
@@ -289,12 +289,34 @@ check_mnl()
if ${PKG_CONFIG} libmnl --exists
then
echo "HAVE_MNL:=y" >>Config
-   echo -n "yes"
+   echo "yes"
else
-   echo -n "no"
+   echo "no"
fi
 }
 
+check_berkeley_db()
+{
+cat >$TMPDIR/dbtest.c <
+#include 
+#include 
+int main(int argc, char **argv) {
+   dbopen("/tmp/xxx_test_db.db", O_CREAT|O_RDWR, 0644, DB_HASH, NULL);
+   return 0;
+}
+EOF
+$CC -I$INCLUDE -o $TMPDIR/dbtest $TMPDIR/dbtest.c -ldb >/dev/null 2>&1
+if [ $? -eq 0 ]
+then
+   echo "HAVE_BERKELEY_DB:=y" >>Config
+   echo "yes"
+else
+   echo "no"
+fi
+rm -f $TMPDIR/dbtest.c $TMPDIR/dbtest
+}
+
 echo "# Generated config based on" $INCLUDE >Config
 check_toolchain
 
@@ -328,6 +350,9 @@ check_elf
 echo -n "libmnl support: "
 check_mnl
 
+echo -n "Berkeley DB: "
+check_berkeley_db
+
 echo
 echo -n "docs:"
 check_docs
diff --git a/misc/Makefile b/misc/Makefile
index 6185217..389c1b0 100644
--- a/misc/Makefile
+++ b/misc/Makefile
@@ -1,10 +1,14 @@
 SSOBJ=ss.o ssfilter.o
 LNSTATOBJ=lnstat.o lnstat_util.o
 
-TARGETS=ss nstat ifstat rtacct arpd lnstat
+TARGETS=ss nstat ifstat rtacct lnstat
 
 include ../Config
 
+ifeq ($(HAVE_BERKELEY_DB),y)
+   TARGETS += arpd
+endif
+
 ifeq ($(HAVE_SELINUX),y)
LDLIBS += $(shell pkg-config --libs libselinux)
CFLAGS += $(shell pkg-config --cflags libselinux) -DHAVE_SELINUX
-- 
2.4.2

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: iproute2 tunnel name parsing

[Vadim Kochan]

On Thu, Sep 17, 2015 at 09:55:29PM +0200, Wilhelm Wijkander wrote:
> Hi,
> 
> I'm trying to create a sit tunnel called "hel": ip tun add hel mode
> sit remote 10.200.0.2 local 10.200.1.2 ttl 255, however it seems like
> this is interpreted as the help argument and I get the usage text. Is
> there a way to escape names that I've missed, or is this an error
> somewhere in argv parsing?
> 
> (I'm not subscribed, so a cc would be appreciated)
> Thanks,
> Wilhelm
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Wilhelm,

You can use 'name' before 'hel' like:

$ ip tun add name hel mode sit remote 10.200.0.2 local 10.200.1.2 ttl 255

and it should work, actually I just tried and it works.

Regards,
Vadim Kochan
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH iproute2] man ip-link: Fix wording in VLAN reorder_hdr explanation

[Vadim Kochan]

From: Vadim Kochan <vadi...@gmail.com>

Signed-off-by: Vadim Kochan <vadi...@gmail.com>
---
 man/man8/ip-link.8.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 1896eb6..4928249 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -327,7 +327,7 @@ physical device (if this device does not support VLAN 
offloading), the similar
 on the RX direction - by default the packet will be untagged before being
 received by VLAN device. Reordering allows to accelerate tagging on egress and
 to hide VLAN header on ingress so the packet looks like regular Ethernet 
packet,
-at the same time it might be confusing while the packet sniffing as the VLAN 
header
+at the same time it might be confusing for packet capture as the VLAN header
 does not exist within the packet.
 
 VLAN offloading can be checked by
-- 
2.4.2

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] man ip-link: Add more explanation about vlan reordering

[Vadim Kochan]

On Wed, Aug 26, 2015 at 04:27:48PM +0100, Jeremy Harris wrote:
> On 17/08/15 20:22, Vadim Kochan wrote:
> > +.BR reorder_hdr " is " on
> > +then VLAN header will be not inserted immediately but only before passing 
> > to the
> > +physical device (if this device does not support VLAN offloading), the 
> > similar
> > +on the RX direction - by default the packet will be untagged before being
> > +received by VLAN device. Reordering allows to accelerate tagging on egress 
> > and
> > +to hide VLAN header on ingress so the packet looks like regular Ethernet 
> > packet,
> > +at the same time it might be confusing while the packet sniffing as the 
> > VLAN header
>   ^
> 
> Does not read well.  "for packet capture" perhaps?
> -- 
> Jeremy
> 
> 

Hi Jeremy,

Thanks for you comment, I have sent a patch, let me know if it is correct now.
And sorry for so late response.

Thanks,
Vadim Kochan
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] man ip-link: Add little explanations about VLAN qos map

[Vadim Kochan]

From: Vadim Kochan vadi...@gmail.com

Add little more info about how to manually set priority by iptables,
and some little clarifications about ingress/egress QoS mapping.

Signed-off-by: Vadim Kochan vadi...@gmail.com
---
 man/man8/ip-link.8.in | 27 ---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index b9137fb..2283b71 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -349,10 +349,30 @@ where phy_dev is the physical device to which VLAN 
device is bound.
 - specifies whether the VLAN device state is bound to the physical device 
state.
 
 .BI ingress-qos-map  QOS-MAP 
-- defines a mapping between priority code points on incoming frames.  The 
format is FROM:TO with multiple mappings separated by spaces.
+- defines a mapping of VLAN header prio field to the Linux internal packet
+priority on incoming frames. The format is FROM:TO with multiple mappings
+separated by spaces.
 
 .BI egress-qos-map  QOS-MAP 
-- the same as ingress-qos-map but for outgoing frames.
+- defines a mapping of Linux internal packet priority to VLAN header prio field
+but for outgoing frames. The format is the same as for ingress-qos-map.
+.in +4
+
+Linux packet priority can be set by
+.BR iptables (8):
+.in +4
+.sp
+.B iptables
+-t mangle -A POSTROUTING [...] -j CLASSIFY --set-class 0:4
+.sp
+.in -4
+and this 4 priority can be used in the egress qos mapping to set VLAN prio 
5:
+.sp
+.in +4
+.B ip
+link set veth0.10 type vlan egress 4:5
+.in -4
+.in -4
 .in -8
 
 .TP
@@ -1090,7 +1110,8 @@ IEEE 802.15.4 device wpan0.
 .br
 .BR ip (8),
 .BR ip-netns (8),
-.BR ethtool (8)
+.BR ethtool (8),
+.BR iptables (8)
 
 .SH AUTHOR
 Original Manpage by Michail Litvak m...@owl.openwall.com
-- 
2.4.2

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: iproute2: Behavioural Bug?

[Vadim Kochan]

On Mon, Aug 24, 2015 at 02:00:29PM +0530, Akshat Kakkar wrote:
 When I am trying to delete a single tc filter (i.e. specifying its
 handle), it is deleting all the
 filters with the same priority/preference. i.e. it is ignoring the
 handle specified.
 
 But, When I am doing similar activity in hashtable 800: it is deleting only 
 the
 specified filter, i.e. it is behaving as expected.
 
 I am unable to comprehend the reason for this difference in behaviour.
 
 Infact, in kernel 2.6.32 all is working as expected. However, in
 kernel 3.1 and 4.1 it is having the behaviour as mentioned above.
 
 For example, following set of commands  create a hashtable 15: and add
 2 filters to it.
 
 tc filter add dev eth0 parent 1:0 prio 5 handle 15: protocol ip u32 divisor 
 256
 tc filter add dev eth0 protocol ip parent 1: prio 5 handle 15:2:2 u32
 ht 15:2: match ip src 10.0.0.2 flowid 1:10
 tc filter add dev eth0 protocol ip parent 1: prio 5 handle 15:2:3 u32
 ht 15:2: match ip src 10.0.0.3 flowid 1:10
 
 Now following command DELETES ALL THE FILTERS, though it should only
 delete FILTER 15:2:3 !
 tc filter del dev eth0 protocol ip parent 1: prio 5 handle 15:2:3 u32
 
 O/p of tc filter show eth0 is this case is blank. As all filters are deleted.
 
 
 However, similar commands when executed for hashtable 800: is deleting
 only the specified filter
 tc filter add dev eth0 protocol ip parent 1: prio 5 handle 800:0:2 u32
 ht 800:0: match ip src 10.0.0.2 flowid 1:10
 tc filter add dev eth0 protocol ip parent 1: prio 5 handle 800:0:3 u32
 ht 800:0: match ip src 10.0.0.3 flowid 1:10
 
 tc filter del dev eth0 protocol ip parent 1: prio 5 handle 800:0:2 u32
 
 Above mentioned command only deletes single filter.
 O/p of tc filter show eth0 is 2nd case is
 
 filter parent 1: protocol ip pref 5 u32
 filter parent 1: protocol ip pref 5 u32 fh 800: ht divisor 1
 filter parent 1: protocol ip pref 5 u32 fh 800::3 order 3 key ht 800
 bkt 0 flowid 1:10
   match 0a03/ at 12
 --
 To unsubscribe from this list: send the line unsubscribe netdev in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi,

Thats what I got using this script where I copied your commands:

--
#!/bin/bash

DEV=dummy0

ip link del $DEV 2 /dev/null

ip link add dev $DEV type dummy

tc qdisc add dev $DEV root handle 1: htb

tc filter add dev $DEV parent 1:0 prio 5 handle 15: protocol ip u32 divisor 256
tc filter add dev $DEV protocol ip parent 1: prio 5 handle 15:2:2 u32 ht 15:2: 
match ip src 10.0.0.2 flowid 1:10
tc filter add dev $DEV protocol ip parent 1: prio 5 handle 15:2:3 u32 ht 15:2: 
match ip src 10.0.0.3 flowid 1:10

tc filter del dev $DEV protocol ip parent 1: prio 5 handle 15:2:3 u32

tc filter show dev $DEV
# -

Result is:

filter parent 1: protocol ip pref 5 u32 
filter parent 1: protocol ip pref 5 u32 fh 15: ht divisor 256 
filter parent 1: protocol ip pref 5 u32 fh 15:2:2 order 2 key ht 15 bkt 2 
flowid 1:10 
  match 0a02/ at 12
filter parent 1: protocol ip pref 5 u32 fh 800: ht divisor 1 

Some additional info:

# tc -V
tc utility, iproute2-ss150413

# uname -a
Linux angus-think 4.0.4-2-ARCH #1 SMP PREEMPT Fri May 22 03:05:23 UTC 2015 
x86_64 GNU/Linux

Regards,
Vadim Kochan
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] man ip-link: Add more explanation about vlan reordering

[Vadim Kochan]

From: Vadim Kochan vadi...@gmail.com

Add more explanation about VLAN reordering and what it affects.

Signed-off-by: Vadim Kochan vadi...@gmail.com
---
 man/man8/ip-link.8.in | 29 +++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index df2fcce..b9137fb 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -313,7 +313,31 @@ the following additional arguments are supported:
 - specifies the VLAN Identifer to use. Note that numbers with a leading  0  
or  0x  are interpreted as octal or hexadeimal, respectively.
 
 .BR reorder_hdr  {  on  |  off  } 
-- specifies whether ethernet headers are reordered or not.
+- specifies whether ethernet headers are reordered or not (default is
+.BR on ).
+
+.in +4
+If
+.BR reorder_hdr  is  on
+then VLAN header will be not inserted immediately but only before passing to 
the
+physical device (if this device does not support VLAN offloading), the similar
+on the RX direction - by default the packet will be untagged before being
+received by VLAN device. Reordering allows to accelerate tagging on egress and
+to hide VLAN header on ingress so the packet looks like regular Ethernet 
packet,
+at the same time it might be confusing while the packet sniffing as the VLAN 
header
+does not exist within the packet.
+
+VLAN offloading can be checked by
+.BR ethtool (8):
+.in +4
+.sp
+.B ethtool -k
+phy_dev |
+.RB grep  tx-vlan-offload
+.sp
+.in -4
+where phy_dev is the physical device to which VLAN device is bound.
+.in -4
 
 .BR gvrp  {  on  |  off  } 
 - specifies whether this VLAN should be registered using GARP VLAN 
Registration Protocol.
@@ -1065,7 +1089,8 @@ IEEE 802.15.4 device wpan0.
 .SH SEE ALSO
 .br
 .BR ip (8),
-.BR ip-netns (8)
+.BR ip-netns (8),
+.BR ethtool (8)
 
 .SH AUTHOR
 Original Manpage by Michail Litvak m...@owl.openwall.com
-- 
2.4.2

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH iproute2] man ss: Fix explanation when no options specified

[Vadim Kochan]

From: Vadim Kochan vadi...@gmail.com

Really by default ss dumps not only TCP sockets but any kind of socket
which is in ESTABLISHED state (TCP/UDP/UNIX).

Signed-off-by: Vadim Kochan vadi...@gmail.com
Reported-by: Miha Marolt mi...@beyondsemi.com
---
 man/man8/ss.8 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/man8/ss.8 b/man/man8/ss.8
index b7fbaef..6afbabb 100644
--- a/man/man8/ss.8
+++ b/man/man8/ss.8
@@ -13,7 +13,7 @@ It can display more TCP and state informations than other 
tools.
 
 .SH OPTIONS
 When no option is used ss displays a list of 
-open non-listening TCP sockets that have established connection.
+open non-listening sockets (e.g. TCP/UNIX/UDP) that have established 
connection.
 .TP
 .B \-h, \-\-help
 Show summary of options.
-- 
2.4.2

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH iproute2 v2] ss: Fix crash when dump stats from /proc with '-p'

[Vadim Kochan]

From: Vadim Kochan vadi...@gmail.com

It really partially reverts:

ec4d0d8a9def35 (ss: Replace unixstat struct by new sockstat struct)

but adds few fields (name  peer_name) from removed unixstat to sockstat struct 
to easy
return original code.

Fixes: ec4d0d8a9def35 (ss: Replace unixstat struct by new sockstat struct)
Reported-by: Marc Dietrich marvi...@gmx.de
Signed-off-by: Vadim Kochan vadi...@gmail.com
---
v2:
Get rid of check for NULL before free(...) func.

 misc/ss.c | 71 ---
 1 file changed, 31 insertions(+), 40 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 03f92fa..320 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -676,18 +676,6 @@ static int get_slabstat(struct slabstat *s)
return 0;
 }
 
-static inline void sock_addr_set_str(inet_prefix *prefix, char **ptr)
-{
-memcpy(prefix-data, ptr, sizeof(char *));
-}
-
-static inline char *sock_addr_get_str(const inet_prefix *prefix)
-{
-char *tmp ;
-memcpy(tmp, prefix-data, sizeof(char *));
-return tmp;
-}
-
 static unsigned long long cookie_sk_get(const uint32_t *cookie)
 {
return (((unsigned long long)cookie[1]  31)  1) | cookie[0];
@@ -739,6 +727,8 @@ struct sockstat
int refcnt;
unsigned intiface;
unsigned long long  sk;
+   char *name;
+   char *peer_name;
 };
 
 struct dctcpstat
@@ -1063,9 +1053,9 @@ static int inet2_addr_match(const inet_prefix *a, const 
inet_prefix *p,
 
 static int unix_match(const inet_prefix *a, const inet_prefix *p)
 {
-   char *addr = sock_addr_get_str(a);
-   char *pattern = sock_addr_get_str(p);
-
+   char *addr, *pattern;
+   memcpy(addr, a-data, sizeof(addr));
+   memcpy(pattern, p-data, sizeof(pattern));
if (pattern == NULL)
return 1;
if (addr == NULL)
@@ -1081,7 +1071,8 @@ static int run_ssfilter(struct ssfilter *f, struct 
sockstat *s)
 static int low, high=65535;
 
if (s-local.family == AF_UNIX) {
-   char *p = sock_addr_get_str(s-local);
+   char *p;
+   memcpy(p, s-local.data, sizeof(p));
return p == NULL || (p[0] == '@'  strlen(p) == 6 
 strspn(p+1, 0123456789abcdef) == 
5);
}
@@ -1401,7 +1392,7 @@ void *parse_hostcond(char *addr, bool is_port)
addr+=5;
p = strdup(addr);
a.addr.bitlen = 8*strlen(p);
-   sock_addr_set_str(a.addr, p);
+   memcpy(a.addr.data, p, sizeof(p));
fam = AF_UNIX;
goto out;
}
@@ -2508,12 +2499,9 @@ static void unix_list_free(struct sockstat *list)
 {
while (list) {
struct sockstat *s = list;
-   char *name = sock_addr_get_str(s-local);
-
list = list-next;
 
-   if (name)
-   free(name);
+   free(s-name);
free(s);
}
 }
@@ -2556,7 +2544,7 @@ static bool unix_use_proc(void)
 static void unix_stats_print(struct sockstat *list, struct filter *f)
 {
struct sockstat *s;
-   char *local, *peer;
+   char *peer;
char *ctx_buf = NULL;
bool use_proc = unix_use_proc();
char port_name[30] = {};
@@ -2567,8 +2555,9 @@ static void unix_stats_print(struct sockstat *list, 
struct filter *f)
if (unix_type_skip(s, f))
continue;
 
-   local = sock_addr_get_str(s-local);
-   peer  = *;
+   peer = *;
+   if (s-peer_name)
+   peer = s-peer_name;
 
if (s-rport  use_proc) {
struct sockstat *p;
@@ -2581,24 +2570,26 @@ static void unix_stats_print(struct sockstat *list, 
struct filter *f)
if (!p) {
peer = ?;
} else {
-   peer = sock_addr_get_str(p-local);
-   peer = peer ? : *;
+   peer = p-name ? : *;
}
}
 
if (use_proc  f-f) {
+   struct sockstat st;
+   st.local.family = AF_UNIX;
+   st.remote.family = AF_UNIX;
+   memcpy(st.local.data, s-name, sizeof(s-name));
if (strcmp(peer, *) == 0)
-   memset(s-remote.data, 0, sizeof(char *));
+   memset(st.remote.data, 0, sizeof(peer));
else
-   sock_addr_set_str(s-remote, peer);
-
-   if (run_ssfilter(f-f, s) == 0)
+   memcpy(st.remote.data, peer, sizeof(peer

[PATCH iproute2] ss: Fix crash when dump stats from /proc with '-p'

[Vadim Kochan]

From: Vadim Kochan vadi...@gmail.com

It really partially reverts:

ec4d0d8a9def35 (ss: Replace unixstat struct by new sockstat struct)

but adds few fields (name  peer_name) from removed unixstat to sockstat struct 
to easy
return original code.

Fixes: ec4d0d8a9def35 (ss: Replace unixstat struct by new sockstat struct)
Reported-by: Marc Dietrich marvi...@gmx.de
Signed-off-by: Vadim Kochan vadi...@gmail.com
---
 misc/ss.c | 73 ---
 1 file changed, 32 insertions(+), 41 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 03f92fa..ed7960f 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -676,18 +676,6 @@ static int get_slabstat(struct slabstat *s)
return 0;
 }
 
-static inline void sock_addr_set_str(inet_prefix *prefix, char **ptr)
-{
-memcpy(prefix-data, ptr, sizeof(char *));
-}
-
-static inline char *sock_addr_get_str(const inet_prefix *prefix)
-{
-char *tmp ;
-memcpy(tmp, prefix-data, sizeof(char *));
-return tmp;
-}
-
 static unsigned long long cookie_sk_get(const uint32_t *cookie)
 {
return (((unsigned long long)cookie[1]  31)  1) | cookie[0];
@@ -739,6 +727,8 @@ struct sockstat
int refcnt;
unsigned intiface;
unsigned long long  sk;
+   char *name;
+   char *peer_name;
 };
 
 struct dctcpstat
@@ -1063,9 +1053,9 @@ static int inet2_addr_match(const inet_prefix *a, const 
inet_prefix *p,
 
 static int unix_match(const inet_prefix *a, const inet_prefix *p)
 {
-   char *addr = sock_addr_get_str(a);
-   char *pattern = sock_addr_get_str(p);
-
+   char *addr, *pattern;
+   memcpy(addr, a-data, sizeof(addr));
+   memcpy(pattern, p-data, sizeof(pattern));
if (pattern == NULL)
return 1;
if (addr == NULL)
@@ -1081,7 +1071,8 @@ static int run_ssfilter(struct ssfilter *f, struct 
sockstat *s)
 static int low, high=65535;
 
if (s-local.family == AF_UNIX) {
-   char *p = sock_addr_get_str(s-local);
+   char *p;
+   memcpy(p, s-local.data, sizeof(p));
return p == NULL || (p[0] == '@'  strlen(p) == 6 
 strspn(p+1, 0123456789abcdef) == 
5);
}
@@ -1401,7 +1392,7 @@ void *parse_hostcond(char *addr, bool is_port)
addr+=5;
p = strdup(addr);
a.addr.bitlen = 8*strlen(p);
-   sock_addr_set_str(a.addr, p);
+   memcpy(a.addr.data, p, sizeof(p));
fam = AF_UNIX;
goto out;
}
@@ -2508,12 +2499,9 @@ static void unix_list_free(struct sockstat *list)
 {
while (list) {
struct sockstat *s = list;
-   char *name = sock_addr_get_str(s-local);
-
list = list-next;
-
-   if (name)
-   free(name);
+   if (s-name)
+   free(s-name);
free(s);
}
 }
@@ -2556,7 +2544,7 @@ static bool unix_use_proc(void)
 static void unix_stats_print(struct sockstat *list, struct filter *f)
 {
struct sockstat *s;
-   char *local, *peer;
+   char *peer;
char *ctx_buf = NULL;
bool use_proc = unix_use_proc();
char port_name[30] = {};
@@ -2567,8 +2555,9 @@ static void unix_stats_print(struct sockstat *list, 
struct filter *f)
if (unix_type_skip(s, f))
continue;
 
-   local = sock_addr_get_str(s-local);
-   peer  = *;
+   peer = *;
+   if (s-peer_name)
+   peer = s-peer_name;
 
if (s-rport  use_proc) {
struct sockstat *p;
@@ -2581,24 +2570,26 @@ static void unix_stats_print(struct sockstat *list, 
struct filter *f)
if (!p) {
peer = ?;
} else {
-   peer = sock_addr_get_str(p-local);
-   peer = peer ? : *;
+   peer = p-name ? : *;
}
}
 
if (use_proc  f-f) {
+   struct sockstat st;
+   st.local.family = AF_UNIX;
+   st.remote.family = AF_UNIX;
+   memcpy(st.local.data, s-name, sizeof(s-name));
if (strcmp(peer, *) == 0)
-   memset(s-remote.data, 0, sizeof(char *));
+   memset(st.remote.data, 0, sizeof(peer));
else
-   sock_addr_set_str(s-remote, peer);
-
-   if (run_ssfilter(f-f, s) == 0)
+   memcpy(st.remote.data, peer, sizeof(peer));
+   if (run_ssfilter(f-f, st) == 0

Re: [PATCH iproute2] ss: Fix crash when dump stats from /proc with '-p'

[Vadim Kochan]

On Mon, Jul 20, 2015 at 01:43:40PM -0700, Stephen Hemminger wrote:
 On Mon, 20 Jul 2015 22:46:23 +0300
 Vadim Kochan vadi...@gmail.com wrote:
 
  +   if (s-name)
  +   free(s-name);
 
 Please don't add unnecessary conditional.
   free(NULL) is defined to do nothing.

OK, I will re-send, this was from original version.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ss -p segfaults

[Vadim Kochan]

On Wed, Jul 15, 2015 at 06:52:49PM +, Rustad, Mark D wrote:
  On Jul 15, 2015, at 9:49 AM, Rustad, Mark D mark.d.rus...@intel.com wrote:
  
  On Jul 15, 2015, at 8:12 AM, Vadim Kochan vadi...@gmail.com wrote:
  Would you please check this fix ?
  
  diff --git a/misc/ss.c b/misc/ss.c
  index 03f92fa..3a826e4 100644
  --- a/misc/ss.c
  +++ b/misc/ss.c
  @@ -683,8 +683,8 @@ static inline void sock_addr_set_str(inet_prefix 
  *prefix, char **ptr)
  
  static inline char *sock_addr_get_str(const inet_prefix *prefix)
  {
  -char *tmp ;
  -memcpy(tmp, prefix-data, sizeof(char *));
  +char *tmp;
  +memcpy(tmp, prefix-data[0], sizeof(char *));
 return tmp;
  }
  
  That surely is not a fix! The destination of the memcpy is the address of 
  an uninitialized stack variable! Both versions are equally bad.
 
 I probably over-reacted, but using memcpy to access a pointer in this way is 
 just ugly. For one thing, it circumvents any sanity-checking that the 
 compiler can do. And changing the prefix-data to prefix-data[0] should be 
 exactly the same thing and therefore should not fix anything. Anyway, never 
 mind that.
 
 Looking at more of the code, it looks to me like the the string pointer in 
 data can sometimes point to a literal string instead of allocated memory when 
 proc is in use. Free would not be happy with that. Look at the use of 
 variable peer in function unix_stats_print.
 
Yes that right, I am already looking on this ...
 --
 Mark Rustad, Networking Division, Intel Corporation
 
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ss -p segfaults

[Vadim Kochan]

On Wed, Jul 15, 2015 at 09:57:51PM +0300, Vadim Kochan wrote:
 On Wed, Jul 15, 2015 at 06:52:49PM +, Rustad, Mark D wrote:
   On Jul 15, 2015, at 9:49 AM, Rustad, Mark D mark.d.rus...@intel.com 
   wrote:
   
   On Jul 15, 2015, at 8:12 AM, Vadim Kochan vadi...@gmail.com wrote:
   Would you please check this fix ?
   
   diff --git a/misc/ss.c b/misc/ss.c
   index 03f92fa..3a826e4 100644
   --- a/misc/ss.c
   +++ b/misc/ss.c
   @@ -683,8 +683,8 @@ static inline void sock_addr_set_str(inet_prefix 
   *prefix, char **ptr)
   
   static inline char *sock_addr_get_str(const inet_prefix *prefix)
   {
   -char *tmp ;
   -memcpy(tmp, prefix-data, sizeof(char *));
   +char *tmp;
   +memcpy(tmp, prefix-data[0], sizeof(char *));
  return tmp;
   }
   
   That surely is not a fix! The destination of the memcpy is the address of 
   an uninitialized stack variable! Both versions are equally bad.
  
  I probably over-reacted, but using memcpy to access a pointer in this way 
  is just ugly. For one thing, it circumvents any sanity-checking that the 
  compiler can do. And changing the prefix-data to prefix-data[0] should 
  be exactly the same thing and therefore should not fix anything. Anyway, 
  never mind that.
  
  Looking at more of the code, it looks to me like the the string pointer in 
  data can sometimes point to a literal string instead of allocated memory 
  when proc is in use. Free would not be happy with that. Look at the use of 
  variable peer in function unix_stats_print.
  
 Yes that right, I am already looking on this ...
  --
  Mark Rustad, Networking Division, Intel Corporation
  

I did partially revert of the buggy commit and it does not crash, but I will do
more testing, and after will send the patch and will try to prepare some
test scripts for ss.

The crash appears only if to dump processes info from /proc, which might
be caused that netlink stats returned error, probably by wrong request
(not supported attribute or flag ?).
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ss -p segfaults

[Vadim Kochan]

On Wed, Jul 15, 2015 at 04:09:03PM +0200, Marc Dietrich wrote:
 Hi,
 
 ss -p segfaults here with some kind of memory corruption:
 
 *** Error in `/work/iproute2/misc/ss': free(): invalid pointer: 
 0x00623000 ***
 === Backtrace: =
 /lib64/libc.so.6(+0x71c6d)[0x77885c6d]
 /lib64/libc.so.6(+0x771be)[0x7788b1be]
 /lib64/libc.so.6(+0x7799b)[0x7788b99b]
 /work/iproute2/misc/ss[0x403de1]
 /work/iproute2/misc/ss[0x408247]
 /work/iproute2/misc/ss[0x403295]
 /lib64/libc.so.6(__libc_start_main+0xf0)[0x77834790]
 /work/iproute2/misc/ss[0x4037f9]
 === Memory map: 
 0040-00416000 r-xp  00:33 4207305
 /work/iproute2/misc/ss
 00616000-00617000 r--p 00016000 00:33 4207305
 /work/iproute2/misc/ss
 00617000-0061b000 rw-p 00017000 00:33 4207305
 /work/iproute2/misc/ss
 0061b000-0065f000 rw-p  00:00 0  
 [heap]
 76f6d000-76f83000 r-xp  00:21 16154175   
 /lib64/libgcc_s.so.1
 76f83000-77182000 ---p 00016000 00:21 16154175   
 /lib64/libgcc_s.so.1
 77182000-77183000 r--p 00015000 00:21 16154175   
 /lib64/libgcc_s.so.1
 77183000-77184000 rw-p 00016000 00:21 16154175   
 /lib64/libgcc_s.so.1
 77184000-7719c000 r-xp  00:21 16694826   
 /lib64/libpthread-2.21.so
 7719c000-7739b000 ---p 00018000 00:21 16694826   
 /lib64/libpthread-2.21.so
 7739b000-7739c000 r--p 00017000 00:21 16694826   
 /lib64/libpthread-2.21.so
 7739c000-7739d000 rw-p 00018000 00:21 16694826   
 /lib64/libpthread-2.21.so
 7739d000-773a1000 rw-p  00:00 0 
 773a1000-773a4000 r-xp  00:21 16694804   
 /lib64/libdl-2.21.so
 773a4000-775a3000 ---p 3000 00:21 16694804   
 /lib64/libdl-2.21.so
 775a3000-775a4000 r--p 2000 00:21 16694804   
 /lib64/libdl-2.21.so
 775a4000-775a5000 rw-p 3000 00:21 16694804   
 /lib64/libdl-2.21.so
 775a5000-77613000 r-xp  00:21 16153198   
 /usr/lib64/libpcre.so.1.2.5
 77613000-77812000 ---p 0006e000 00:21 16153198   
 /usr/lib64/libpcre.so.1.2.5
 77812000-77813000 r--p 0006d000 00:21 16153198   
 /usr/lib64/libpcre.so.1.2.5
 77813000-77814000 rw-p 0006e000 00:21 16153198   
 /usr/lib64/libpcre.so.1.2.5
 77814000-779ad000 r-xp  00:21 16694798   
 /lib64/libc-2.21.so
 779ad000-77bac000 ---p 00199000 00:21 16694798   
 /lib64/libc-2.21.so
 77bac000-77bb1000 r--p 00198000 00:21 16694798   
 /lib64/libc-2.21.so
 77bb1000-77bb3000 rw-p 0019d000 00:21 16694798   
 /lib64/libc-2.21.so
 77bb3000-77bb7000 rw-p  00:00 0 
 77bb7000-77bd8000 r-xp  00:21 16155991   
 /lib64/libselinux.so.1
 77bd8000-77dd7000 ---p 00021000 00:21 16155991   
 /lib64/libselinux.so.1
 77dd7000-77dd8000 r--p 0002 00:21 16155991   
 /lib64/libselinux.so.1
 77dd8000-77dd9000 rw-p 00021000 00:21 16155991   
 /lib64/libselinux.so.1
 77dd9000-77ddb000 rw-p  00:00 0 
 77ddb000-77dfc000 r-xp  00:21 16694791   
 /lib64/ld-2.21.so
 77fb5000-77fba000 rw-p  00:00 0 
 77ff5000-77ff8000 rw-p  00:00 0 
 77ff8000-77ffa000 r--p  00:00 0  
 [vvar]
 77ffa000-77ffc000 r-xp  00:00 0  
 [vdso]
 77ffc000-77ffd000 r--p 00021000 00:21 16694791   
 /lib64/ld-2.21.so
 77ffd000-77ffe000 rw-p 00022000 00:21 16694791   
 /lib64/ld-2.21.so
 77ffe000-77fff000 rw-p  00:00 0 
 7ffdd000-7000 rw-p  00:00 0  
 [stack]
 ff60-ff601000 r-xp  00:00 0  
 [vsyscall]
 
 Program received signal SIGABRT, Aborted.
 0x77847638 in raise () from /lib64/libc.so.6
 Missing separate debuginfos, use: zypper install libgcc_s1-
 debuginfo-5.1.1+r224716-1.2.x86_64 libpcre1-debuginfo-8.37-1.18.x86_64 
 libselinux1-debuginfo-2.3-5.18.x86_64
 (gdb) bt full
 #0  0x77847638 in raise () from /lib64/libc.so.6
 No symbol table info available.
 #1  0x77848a1a in abort () from /lib64/libc.so.6
 No symbol table info available.
 #2  0x77885c72 in __libc_message () from /lib64/libc.so.6
 No symbol table info available.
 #3  0x7788b1be in malloc_printerr () from /lib64/libc.so.6
 No symbol table info available.
 #4  0x7788b99b in _int_free () 

Re: ss -p segfaults

[Vadim Kochan]

On Wed, Jul 15, 2015 at 04:09:03PM +0200, Marc Dietrich wrote:
 Hi,
 
 ss -p segfaults here with some kind of memory corruption:
 
 *** Error in `/work/iproute2/misc/ss': free(): invalid pointer: 
 0x00623000 ***
 === Backtrace: =
 /lib64/libc.so.6(+0x71c6d)[0x77885c6d]
 /lib64/libc.so.6(+0x771be)[0x7788b1be]
 /lib64/libc.so.6(+0x7799b)[0x7788b99b]
 /work/iproute2/misc/ss[0x403de1]
 /work/iproute2/misc/ss[0x408247]
 /work/iproute2/misc/ss[0x403295]
 /lib64/libc.so.6(__libc_start_main+0xf0)[0x77834790]
 /work/iproute2/misc/ss[0x4037f9]
 === Memory map: 
 0040-00416000 r-xp  00:33 4207305
 /work/iproute2/misc/ss
 00616000-00617000 r--p 00016000 00:33 4207305
 /work/iproute2/misc/ss
 00617000-0061b000 rw-p 00017000 00:33 4207305
 /work/iproute2/misc/ss
 0061b000-0065f000 rw-p  00:00 0  
 [heap]
 76f6d000-76f83000 r-xp  00:21 16154175   
 /lib64/libgcc_s.so.1
 76f83000-77182000 ---p 00016000 00:21 16154175   
 /lib64/libgcc_s.so.1
 77182000-77183000 r--p 00015000 00:21 16154175   
 /lib64/libgcc_s.so.1
 77183000-77184000 rw-p 00016000 00:21 16154175   
 /lib64/libgcc_s.so.1
 77184000-7719c000 r-xp  00:21 16694826   
 /lib64/libpthread-2.21.so
 7719c000-7739b000 ---p 00018000 00:21 16694826   
 /lib64/libpthread-2.21.so
 7739b000-7739c000 r--p 00017000 00:21 16694826   
 /lib64/libpthread-2.21.so
 7739c000-7739d000 rw-p 00018000 00:21 16694826   
 /lib64/libpthread-2.21.so
 7739d000-773a1000 rw-p  00:00 0 
 773a1000-773a4000 r-xp  00:21 16694804   
 /lib64/libdl-2.21.so
 773a4000-775a3000 ---p 3000 00:21 16694804   
 /lib64/libdl-2.21.so
 775a3000-775a4000 r--p 2000 00:21 16694804   
 /lib64/libdl-2.21.so
 775a4000-775a5000 rw-p 3000 00:21 16694804   
 /lib64/libdl-2.21.so
 775a5000-77613000 r-xp  00:21 16153198   
 /usr/lib64/libpcre.so.1.2.5
 77613000-77812000 ---p 0006e000 00:21 16153198   
 /usr/lib64/libpcre.so.1.2.5
 77812000-77813000 r--p 0006d000 00:21 16153198   
 /usr/lib64/libpcre.so.1.2.5
 77813000-77814000 rw-p 0006e000 00:21 16153198   
 /usr/lib64/libpcre.so.1.2.5
 77814000-779ad000 r-xp  00:21 16694798   
 /lib64/libc-2.21.so
 779ad000-77bac000 ---p 00199000 00:21 16694798   
 /lib64/libc-2.21.so
 77bac000-77bb1000 r--p 00198000 00:21 16694798   
 /lib64/libc-2.21.so
 77bb1000-77bb3000 rw-p 0019d000 00:21 16694798   
 /lib64/libc-2.21.so
 77bb3000-77bb7000 rw-p  00:00 0 
 77bb7000-77bd8000 r-xp  00:21 16155991   
 /lib64/libselinux.so.1
 77bd8000-77dd7000 ---p 00021000 00:21 16155991   
 /lib64/libselinux.so.1
 77dd7000-77dd8000 r--p 0002 00:21 16155991   
 /lib64/libselinux.so.1
 77dd8000-77dd9000 rw-p 00021000 00:21 16155991   
 /lib64/libselinux.so.1
 77dd9000-77ddb000 rw-p  00:00 0 
 77ddb000-77dfc000 r-xp  00:21 16694791   
 /lib64/ld-2.21.so
 77fb5000-77fba000 rw-p  00:00 0 
 77ff5000-77ff8000 rw-p  00:00 0 
 77ff8000-77ffa000 r--p  00:00 0  
 [vvar]
 77ffa000-77ffc000 r-xp  00:00 0  
 [vdso]
 77ffc000-77ffd000 r--p 00021000 00:21 16694791   
 /lib64/ld-2.21.so
 77ffd000-77ffe000 rw-p 00022000 00:21 16694791   
 /lib64/ld-2.21.so
 77ffe000-77fff000 rw-p  00:00 0 
 7ffdd000-7000 rw-p  00:00 0  
 [stack]
 ff60-ff601000 r-xp  00:00 0  
 [vsyscall]
 
 Program received signal SIGABRT, Aborted.
 0x77847638 in raise () from /lib64/libc.so.6
 Missing separate debuginfos, use: zypper install libgcc_s1-
 debuginfo-5.1.1+r224716-1.2.x86_64 libpcre1-debuginfo-8.37-1.18.x86_64 
 libselinux1-debuginfo-2.3-5.18.x86_64
 (gdb) bt full
 #0  0x77847638 in raise () from /lib64/libc.so.6
 No symbol table info available.
 #1  0x77848a1a in abort () from /lib64/libc.so.6
 No symbol table info available.
 #2  0x77885c72 in __libc_message () from /lib64/libc.so.6
 No symbol table info available.
 #3  0x7788b1be in malloc_printerr () from /lib64/libc.so.6
 No symbol table info available.
 #4  0x7788b99b in _int_free () 

Re: [BUG] $ ss -a incorrectly displays raw sockets as udp sockets

[Vadim Kochan]

On Fri, Jul 10, 2015 at 09:09:46AM +0200, Miha Marolt wrote:
 
 
 On 07/09/2015 05:15 PM, Vadim Kochan wrote:
 On Thu, Jul 09, 2015 at 05:09:27PM +0200, Miha Marolt wrote:
 
 On 07/09/2015 04:57 PM, Nikolay Aleksandrov wrote:
 On 07/09/2015 04:55 PM, Vadim Kochan wrote:
 On Thu, Jul 09, 2015 at 04:50:06PM +0200, Nikolay Aleksandrov wrote:
 On 07/09/2015 04:13 PM, Miha Marolt wrote:
 Hi!
 
 I hope this is the right place to reports bugs. I apologize if it isn't.
 
 I have written a C program (see below for source code) that opens a raw 
 socket on CentOS 7.1 Linux and binds it to some address (it doesn't use 
 the port that I supplied, but that is not the point here). The 
 netstat program correctly recognizes the socket as raw, while ss 
 program says it is udp. Here are the relevant lines from the ss and 
 netstat commands:
 
 $ netstat -an
 raw0  0 127.0.0.1:6 0.0.0.0:* 7
 
 $ ./ss -an
 udpUNCONN 21569  0  127.0.0.1:6 *:*
 
 Here is the version information
 
 $ netstat --version  # From CentOS 7.1.
 net-tools 2.10-alpha
 
 $ ./ss --version  # Built from git.
 ss utility, iproute2-ss150626
 
 
 C source follows. If you store it in main.c, then compile it with $ 
 gcc main.c -o main and then run it by executing $ sudo ./main.
 
 #include arpa/inet.h
 #include assert.h
 #include sys/socket.h
 #include stdio.h
 #include unistd.h
 
 
 int main(void)
 {
  // Create a raw socket.
  int sock = socket(AF_INET, SOCK_RAW, IPPROTO_TCP);
  if (sock == -1) { perror(NULL); goto exc_cleanup; }
 
  // Bind socket to an address.
  struct sockaddr_in addr;
  addr.sin_family = AF_INET;
  inet_pton(AF_INET, 127.0.0.1, addr.sin_addr);
  addr.sin_port = htons(27183);
 
  int rc = bind(sock, (struct sockaddr*)addr, sizeof(addr));
  if (rc != 0) { perror(NULL); goto exc_cleanup; }
 
  // Wait until user presses ENTER.
  printf(\nPress ENTER to quit the program.\n);
  getchar();
 
 exc_cleanup:
  assert(!close(sock));
 }
 
 
 Best regards,
 Miha
 Hi,
 I think this was changed by commit:
 8250bc9ff4e5 (ss: Unify inet sockets output)
 Because dgram_show_line() is used for both UDP and RAW sockets
 IPPROTO_UDP is used for both now and proto_name() returns udp.
 CCed the patch author and attached a possible solution.
 
 Cheers,
   Nik
 diff --git a/misc/ss.c b/misc/ss.c
 index 870cad185341..4de77e92c319 100644
 --- a/misc/ss.c
 +++ b/misc/ss.c
 @@ -1554,6 +1554,8 @@ out:
   static char *proto_name(int protocol)
   {
  switch (protocol) {
 +case IPPROTO_RAW:
 +return raw;
  case IPPROTO_UDP:
  return udp;
  case IPPROTO_TCP:
 @@ -2398,7 +2400,7 @@ static int dgram_show_line(char *line, const 
 struct filter *f, int family)
  if (n  9)
  opt[0] = 0;
 -inet_stats_print(s, IPPROTO_UDP);
 +inet_stats_print(s, dg_proto == UDP_PROTO ? IPPROTO_UDP : 
 IPPROTO_RAW);
  if (show_details  opt[0])
  printf( opt:\%s\, opt);
 Yeah, it fixed the issue, just tested the fix.
 
 Great, I'll submit it in a minute.
 Thanks for testing!
 Thank you very much for the fast fix!
 
 I've found another bug: command $ ss doesn't display only tcp sockets as
 the man page ss(8) says (file man/man8/ss.8 in iproute2 git repo). It also
 displays e.g. unix sockets. The man page says When no option is used ss
 displays a list of open non-listening TCP sockets that have established
 connection..
 
 
 Regards,
 Miha
 That seems OK, I see that unix stream sockets are displayed which are
 connection oriented, and UDP can also have established state.
 

 According to the man page, **only** TCP sockets should be listed by $ ss.
 In reality, also Unix and UDP sockets are listed, so there is a discrepancy
 between what man page says and what actually happens.

Yes, you are right man page says so, but seems such behaviour is for a
long time and may be it is better to change man page rather than change
the default behaviour.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [BUG] $ ss -a incorrectly displays raw sockets as udp sockets

[Vadim Kochan]

On Thu, Jul 09, 2015 at 05:09:27PM +0200, Miha Marolt wrote:
 
 
 On 07/09/2015 04:57 PM, Nikolay Aleksandrov wrote:
 On 07/09/2015 04:55 PM, Vadim Kochan wrote:
 On Thu, Jul 09, 2015 at 04:50:06PM +0200, Nikolay Aleksandrov wrote:
 On 07/09/2015 04:13 PM, Miha Marolt wrote:
 Hi!
 
 I hope this is the right place to reports bugs. I apologize if it isn't.
 
 I have written a C program (see below for source code) that opens a raw 
 socket on CentOS 7.1 Linux and binds it to some address (it doesn't use 
 the port that I supplied, but that is not the point here). The netstat 
 program correctly recognizes the socket as raw, while ss program says 
 it is udp. Here are the relevant lines from the ss and netstat commands:
 
 $ netstat -an
 raw0  0 127.0.0.1:6 0.0.0.0:* 7
 
 $ ./ss -an
 udpUNCONN 21569  0  127.0.0.1:6 *:*
 
 Here is the version information
 
 $ netstat --version  # From CentOS 7.1.
 net-tools 2.10-alpha
 
 $ ./ss --version  # Built from git.
 ss utility, iproute2-ss150626
 
 
 C source follows. If you store it in main.c, then compile it with $ 
 gcc main.c -o main and then run it by executing $ sudo ./main.
 
 #include arpa/inet.h
 #include assert.h
 #include sys/socket.h
 #include stdio.h
 #include unistd.h
 
 
 int main(void)
 {
  // Create a raw socket.
  int sock = socket(AF_INET, SOCK_RAW, IPPROTO_TCP);
  if (sock == -1) { perror(NULL); goto exc_cleanup; }
 
  // Bind socket to an address.
  struct sockaddr_in addr;
  addr.sin_family = AF_INET;
  inet_pton(AF_INET, 127.0.0.1, addr.sin_addr);
  addr.sin_port = htons(27183);
 
  int rc = bind(sock, (struct sockaddr*)addr, sizeof(addr));
  if (rc != 0) { perror(NULL); goto exc_cleanup; }
 
  // Wait until user presses ENTER.
  printf(\nPress ENTER to quit the program.\n);
  getchar();
 
 exc_cleanup:
  assert(!close(sock));
 }
 
 
 Best regards,
 Miha
 
 Hi,
 I think this was changed by commit:
 8250bc9ff4e5 (ss: Unify inet sockets output)
 Because dgram_show_line() is used for both UDP and RAW sockets
 IPPROTO_UDP is used for both now and proto_name() returns udp.
 CCed the patch author and attached a possible solution.
 
 Cheers,
   Nik
 diff --git a/misc/ss.c b/misc/ss.c
 index 870cad185341..4de77e92c319 100644
 --- a/misc/ss.c
 +++ b/misc/ss.c
 @@ -1554,6 +1554,8 @@ out:
   static char *proto_name(int protocol)
   {
switch (protocol) {
 +  case IPPROTO_RAW:
 +  return raw;
case IPPROTO_UDP:
return udp;
case IPPROTO_TCP:
 @@ -2398,7 +2400,7 @@ static int dgram_show_line(char *line, const struct 
 filter *f, int family)
if (n  9)
opt[0] = 0;
 -  inet_stats_print(s, IPPROTO_UDP);
 +  inet_stats_print(s, dg_proto == UDP_PROTO ? IPPROTO_UDP : IPPROTO_RAW);
if (show_details  opt[0])
printf( opt:\%s\, opt);
 Yeah, it fixed the issue, just tested the fix.
 
 Great, I'll submit it in a minute.
 Thanks for testing!
 
 Thank you very much for the fast fix!
 
 I've found another bug: command $ ss doesn't display only tcp sockets as
 the man page ss(8) says (file man/man8/ss.8 in iproute2 git repo). It also
 displays e.g. unix sockets. The man page says When no option is used ss
 displays a list of open non-listening TCP sockets that have established
 connection..
 
 
 Regards,
 Miha

That seems OK, I see that unix stream sockets are displayed which are
connection oriented, and UDP can also have established state.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [BUG] $ ss -a incorrectly displays raw sockets as udp sockets

[Vadim Kochan]

On Thu, Jul 09, 2015 at 04:50:06PM +0200, Nikolay Aleksandrov wrote:
 On 07/09/2015 04:13 PM, Miha Marolt wrote:
  Hi!
  
  I hope this is the right place to reports bugs. I apologize if it isn't.
  
  I have written a C program (see below for source code) that opens a raw 
  socket on CentOS 7.1 Linux and binds it to some address (it doesn't use the 
  port that I supplied, but that is not the point here). The netstat 
  program correctly recognizes the socket as raw, while ss program says 
  it is udp. Here are the relevant lines from the ss and netstat commands:
  
  $ netstat -an
  raw0  0 127.0.0.1:6 0.0.0.0:* 7
  
  $ ./ss -an
  udpUNCONN 21569  0  127.0.0.1:6 *:*
  
  Here is the version information
  
  $ netstat --version  # From CentOS 7.1.
  net-tools 2.10-alpha
  
  $ ./ss --version  # Built from git.
  ss utility, iproute2-ss150626
  
  
  C source follows. If you store it in main.c, then compile it with $ gcc 
  main.c -o main and then run it by executing $ sudo ./main.
  
  #include arpa/inet.h
  #include assert.h
  #include sys/socket.h
  #include stdio.h
  #include unistd.h
  
  
  int main(void)
  {
  // Create a raw socket.
  int sock = socket(AF_INET, SOCK_RAW, IPPROTO_TCP);
  if (sock == -1) { perror(NULL); goto exc_cleanup; }
  
  // Bind socket to an address.
  struct sockaddr_in addr;
  addr.sin_family = AF_INET;
  inet_pton(AF_INET, 127.0.0.1, addr.sin_addr);
  addr.sin_port = htons(27183);
  
  int rc = bind(sock, (struct sockaddr*)addr, sizeof(addr));
  if (rc != 0) { perror(NULL); goto exc_cleanup; }
  
  // Wait until user presses ENTER.
  printf(\nPress ENTER to quit the program.\n);
  getchar();
  
  exc_cleanup:
  assert(!close(sock));
  }
  
  
  Best regards,
  Miha
 
 
 Hi,
 I think this was changed by commit:
 8250bc9ff4e5 (ss: Unify inet sockets output)
 Because dgram_show_line() is used for both UDP and RAW sockets
 IPPROTO_UDP is used for both now and proto_name() returns udp.
 CCed the patch author and attached a possible solution.
 
 Cheers,
  Nik

 diff --git a/misc/ss.c b/misc/ss.c
 index 870cad185341..4de77e92c319 100644
 --- a/misc/ss.c
 +++ b/misc/ss.c
 @@ -1554,6 +1554,8 @@ out:
  static char *proto_name(int protocol)
  {
   switch (protocol) {
 + case IPPROTO_RAW:
 + return raw;
   case IPPROTO_UDP:
   return udp;
   case IPPROTO_TCP:
 @@ -2398,7 +2400,7 @@ static int dgram_show_line(char *line, const struct 
 filter *f, int family)
   if (n  9)
   opt[0] = 0;
  
 - inet_stats_print(s, IPPROTO_UDP);
 + inet_stats_print(s, dg_proto == UDP_PROTO ? IPPROTO_UDP : IPPROTO_RAW);
  
   if (show_details  opt[0])
   printf( opt:\%s\, opt);

Yeah, it fixed the issue, just tested the fix.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [BUG] $ ss -a incorrectly displays raw sockets as udp sockets

[Vadim Kochan]

On Thu, Jul 09, 2015 at 05:21:10PM +0200, Eric Dumazet wrote:
 On Thu, 2015-07-09 at 17:14 +0200, Eric Dumazet wrote:
 
  
  If I checkout iproute2 tree to db08bdb816d337102c5486744008db9c9faa43bf
  (before buggy commit) we indeed had this result :
  
  # ./ss -an | grep 127.0.0.1
  rawUNCONN 213486 0  127.0.0.1:6 *:* 
   
 
 Maybe following patch would fix the issue.
 
 diff --git a/misc/ss.c b/misc/ss.c
 index 03f92fa69270..9b7dad00290c 100644
 --- a/misc/ss.c
 +++ b/misc/ss.c
 @@ -1552,6 +1552,8 @@ out:
  static char *proto_name(int protocol)
  {
   switch (protocol) {
 + case 0:
 + return raw;
   case IPPROTO_UDP:
   return udp;
   case IPPROTO_TCP:
 @@ -2416,7 +2418,7 @@ static int dgram_show_line(char *line, const struct 
 filter *f, int family)
   if (n  9)
   opt[0] = 0;
  
 - inet_stats_print(s, IPPROTO_UDP);
 + inet_stats_print(s, 0);
  
   if (show_details  opt[0])
   printf( opt:\%s\, opt);
 
 

Yes, it fixes the issue too.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH iproute2] tests: Add output testing

[Vadim Kochan]

From: Vadim Kochan vadi...@gmail.com

Added possibility to check command output by grep from the testing
script.

Now TMP_OUT  TMP_ERR are passed from Makefile and changed to
STD_ERR  STD_OUT.

Also changed some existing tests to make output testing.

Signed-off-by: Vadim Kochan vadi...@gmail.com
---
 testsuite/Makefile   |  4 ++
 testsuite/lib/generic.sh | 80 
 testsuite/tests/ip/link/new_link.t   |  4 ++
 testsuite/tests/ip/route/add_default_route.t | 23 +++-
 4 files changed, 88 insertions(+), 23 deletions(-)

diff --git a/testsuite/Makefile b/testsuite/Makefile
index 4b945b0..2027650 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -52,6 +52,9 @@ endif
@for i in $(IPVERS); do \
o=`echo $$i | sed -e 's/iproute2\///'`; \
echo -n Running $@ [$$o/`uname -r`]: ; \
+   TMP_ERR=`mktemp /tmp/tc_testsuite.XX`; \
+   TMP_OUT=`mktemp /tmp/tc_testsuite.XX`; \
+   STD_ERR=$$TMP_ERR STD_OUT=$$TMP_OUT \
TC=$$i/tc/tc IP=$$i/ip/ip DEV=$(DEV) IPVER=$@ 
SNAME=$$i \
ERRF=$(RESULTS_DIR)/$@.$$o.err $(KENV) $(PREFIX) tests/$@  
$(RESULTS_DIR)/$@.$$o.out; \
if [ $$? = 127 ]; then \
@@ -61,5 +64,6 @@ endif
else \
echo PASS; \
fi; \
+   rm $$TMP_ERR $$TMP_OUT; \
dmesg  $(RESULTS_DIR)/$@.$$o.dmesg; \
done
diff --git a/testsuite/lib/generic.sh b/testsuite/lib/generic.sh
index 3473cc1..b7de704 100644
--- a/testsuite/lib/generic.sh
+++ b/testsuite/lib/generic.sh
@@ -30,57 +30,49 @@ ts_tc()
 {
SCRIPT=$1; shift
DESC=$1; shift
-   TMP_ERR=`mktemp /tmp/tc_testsuite.XX` || exit
-   TMP_OUT=`mktemp /tmp/tc_testsuite.XX` || exit
 
-   $TC $@ 2 $TMP_ERR  $TMP_OUT
+   $TC $@ 2 $STD_ERR  $STD_OUT
 
-   if [ -s $TMP_ERR ]; then
+   if [ -s $STD_ERR ]; then
ts_err ${SCRIPT}: ${DESC} failed:
ts_err command: $TC $@
ts_err stderr output:
-   ts_err_cat $TMP_ERR
-   if [ -s $TMP_OUT ]; then
+   ts_err_cat $STD_ERR
+   if [ -s $STD_OUT ]; then
ts_err stdout output:
-   ts_err_cat $TMP_OUT
+   ts_err_cat $STD_OUT
fi
-   elif [ -s $TMP_OUT ]; then
+   elif [ -s $STD_OUT ]; then
echo ${SCRIPT}: ${DESC} succeeded with output:
-   cat $TMP_OUT
+   cat $STD_OUT
else
echo ${SCRIPT}: ${DESC} succeeded
fi
-
-   rm $TMP_ERR $TMP_OUT
 }
 
 ts_ip()
 {
SCRIPT=$1; shift
DESC=$1; shift
-   TMP_ERR=`mktemp /tmp/tc_testsuite.XX` || exit
-   TMP_OUT=`mktemp /tmp/tc_testsuite.XX` || exit
 
-   $IP $@ 2 $TMP_ERR  $TMP_OUT
+   $IP $@ 2 $STD_ERR  $STD_OUT
 RET=$?
 
-   if [ -s $TMP_ERR ] || [ $RET != 0 ]; then
+   if [ -s $STD_ERR ] || [ $RET != 0 ]; then
ts_err ${SCRIPT}: ${DESC} failed:
ts_err command: $IP $@
ts_err stderr output:
-   ts_err_cat $TMP_ERR
-   if [ -s $TMP_OUT ]; then
+   ts_err_cat $STD_ERR
+   if [ -s $STD_OUT ]; then
ts_err stdout output:
-   ts_err_cat $TMP_OUT
+   ts_err_cat $STD_OUT
fi
-   elif [ -s $TMP_OUT ]; then
+   elif [ -s $STD_OUT ]; then
echo ${SCRIPT}: ${DESC} succeeded with output:
-   cat $TMP_OUT
+   cat $STD_OUT
else
echo ${SCRIPT}: ${DESC} succeeded
fi
-
-   rm $TMP_ERR $TMP_OUT
 }
 
 ts_qdisc_available()
@@ -97,3 +89,47 @@ rand_dev()
 {
 echo dev-$(tr -dc [:alpha:]  /dev/urandom | head -c 6)
 }
+
+pr_failed()
+{
+   echo  [FAILED]
+   ts_err matching failed
+}
+
+pr_success()
+{
+   echo  [SUCCESS]
+}
+
+test_on()
+{
+   echo -n test on: \$1\
+   if cat $STD_OUT | grep -qE $1
+   then
+   pr_success
+   else
+   pr_failed
+   fi
+}
+
+test_on_not()
+{
+   echo -n test on: \$1\
+   if cat $STD_OUT | grep -vqE $1
+   then
+   pr_success
+   else
+   pr_failed
+   fi
+}
+
+test_lines_count()
+{
+   echo -n test on lines count ($1): 
+   if cat $STD_OUT | wc -l | grep -q $1
+   then
+   pr_success
+   else
+   pr_failed
+   fi
+}
diff --git a/testsuite/tests/ip/link/new_link.t 
b/testsuite/tests/ip/link/new_link.t
index 549ff25..699adbc 100755
--- a/testsuite/tests/ip/link/new_link.t
+++ b/testsuite/tests/ip/link/new_link.t
@@ -7,5 +7,9 @@ ts_log [Testing add/del virtual links]
 NEW_DEV=$(rand_dev)
 
 ts_ip $0 Add $NEW_DEV dummy interface  link add dev $NEW_DEV

Re: Possible issue in iproute2 package

[Vadim Kochan]

Hi Jose,

On Thu, May 28, 2015 at 09:12:15PM +, Guzman Mosqueda, Jose R wrote:
 
 Hi all
 
 I'm Jose Guzman from a security team at Intel.
 We're using iproute2 in a GNU-Linux project and I'm analyzing the code
 to try to find possible issues/gaps/risks.
 Since I'm not too familiar with the package yet I have a question about
 a particular piece of code that could result in a memory corruption:
 
 Version: 4.0.0
 File: misc/ss.c
 Function: static void tcp_show_info(...)
 Line: ~1903
 Description: There is a memory allocation for a s.cong_alg variable:
 s.cong_alg = malloc(strlen(cong_attr + 1));
 The length is calculated about next position of the starting character.
 But next line there is a copy of the whole content:
 strcpy(s.cong_alg, cong_attr);
 I think there is a mistake and it should be something like:
 s.cong_alg = malloc(strlen(cong_attr) + 1);
I think strdup can be used here.
I will send a patch.

Thank You!
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH iproute2] configure: Check for libmnl

[Vadim Kochan]

From: Vadim Kochan vadi...@gmail.com

Indicate existence of libmnl which is required by tipc.

Signed-off-by: Vadim Kochan vadi...@gmail.com
---
 configure | 16 
 1 file changed, 16 insertions(+)

diff --git a/configure b/configure
index f1325df..1605464 100755
--- a/configure
+++ b/configure
@@ -284,6 +284,17 @@ check_selinux()
fi
 }
 
+check_mnl()
+{
+   if ${PKG_CONFIG} libmnl --exists
+   then
+   echo HAVE_MNL:=y Config
+   echo -n yes
+   else
+   echo -n no
+   fi
+}
+
 echo # Generated config based on $INCLUDE Config
 check_toolchain
 
@@ -313,5 +324,10 @@ check_selinux
 echo -n ELF support: 
 check_elf
 
+echo -n libmnl support: 
+check_mnl
+echo  (required by tipc)
+
 echo -e \nDocs
 check_docs
+echo 
-- 
2.3.1

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH iproute2] ss: Fix allocation of cong control alg name

[Vadim Kochan]

From: Vadim Kochan vadi...@gmail.com

Use strdup instead of malloc, and get rid of bad strcpy.

Signed-off-by: Vadim Kochan vadi...@gmail.com
---
 misc/ss.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 347e3a1..a719466 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -1908,8 +1908,7 @@ static void tcp_show_info(const struct nlmsghdr *nlh, 
struct inet_diag_msg *r,
 
if (tb[INET_DIAG_CONG]) {
const char *cong_attr = 
rta_getattr_str(tb[INET_DIAG_CONG]);
-   s.cong_alg = malloc(strlen(cong_attr + 1));
-   strcpy(s.cong_alg, cong_attr);
+   s.cong_alg = strdup(cong_attr);
}
 
if (TCPI_HAS_OPT(info, TCPI_OPT_WSCALE)) {
-- 
2.3.1

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH iproute2] ss: Fix allocation of cong control alg name

[Vadim Kochan]

On Fri, May 29, 2015 at 04:04:05AM -0700, Eric Dumazet wrote:
 On Fri, 2015-05-29 at 13:30 +0300, Vadim Kochan wrote:
  From: Vadim Kochan vadi...@gmail.com
  
  Use strdup instead of malloc, and get rid of bad strcpy.
  
  Signed-off-by: Vadim Kochan vadi...@gmail.com
  ---
   misc/ss.c | 3 +--
   1 file changed, 1 insertion(+), 2 deletions(-)
  
  diff --git a/misc/ss.c b/misc/ss.c
  index 347e3a1..a719466 100644
  --- a/misc/ss.c
  +++ b/misc/ss.c
  @@ -1908,8 +1908,7 @@ static void tcp_show_info(const struct nlmsghdr *nlh, 
  struct inet_diag_msg *r,
   
  if (tb[INET_DIAG_CONG]) {
  const char *cong_attr = 
  rta_getattr_str(tb[INET_DIAG_CONG]);
  -   s.cong_alg = malloc(strlen(cong_attr + 1));
  -   strcpy(s.cong_alg, cong_attr);
  +   s.cong_alg = strdup(cong_attr);
  }
   
  if (TCPI_HAS_OPT(info, TCPI_OPT_WSCALE)) {
 
 I doubt TCP_CA_NAME_MAX will ever change in the kernel : 16 bytes.
 
 Its typically cubic and less than 8 bytes.
 
 Using 8 bytes to point to a malloc(8) is a waste.
 
 Please remove the memory allocation, or store the pointer, since
 tcp_show_info() does the malloc()/free() before return.
 
 diff --git a/misc/ss.c b/misc/ss.c
 index 347e3a1..9fe229f 100644
 --- a/misc/ss.c
 +++ b/misc/ss.c
 @@ -755,7 +755,7 @@ struct tcpstat
   int timer;
   int timeout;
   int probes;
 - char*cong_alg;
 + charcong_alg[16];
   double  rto, ato, rtt, rttvar;
   int qack, cwnd, ssthresh, backoff;
   double  send_bps;
 @@ -1664,7 +1664,7 @@ static void tcp_stats_print(struct tcpstat *s)
   printf( ecnseen);
   if (s-has_fastopen_opt)
   printf( fastopen);
 - if (s-cong_alg)
 + if (s-cong_alg[0])
   printf( %s, s-cong_alg);
   if (s-has_wscale_opt)
   printf( wscale:%d,%d, s-snd_wscale, s-rcv_wscale);
 @@ -1906,11 +1906,10 @@ static void tcp_show_info(const struct nlmsghdr *nlh, 
 struct inet_diag_msg *r,
   s.has_fastopen_opt = TCPI_HAS_OPT(info, 
 TCPI_OPT_SYN_DATA);
   }
  
 - if (tb[INET_DIAG_CONG]) {
 - const char *cong_attr = 
 rta_getattr_str(tb[INET_DIAG_CONG]);
 - s.cong_alg = malloc(strlen(cong_attr + 1));
 - strcpy(s.cong_alg, cong_attr);
 - }
 + if (tb[INET_DIAG_CONG])
 + strncpy(s.cong_alg,
 + rta_getattr_str(tb[INET_DIAG_CONG]),
 + sizeof(s.cong_alg) - 1);
  
   if (TCPI_HAS_OPT(info, TCPI_OPT_WSCALE)) {
   s.has_wscale_opt  = true;
 @@ -1984,8 +1983,6 @@ static void tcp_show_info(const struct nlmsghdr *nlh, 
 struct inet_diag_msg *r,
   tcp_stats_print(s);
   if (s.dctcp)
   free(s.dctcp);
 - if (s.cong_alg)
 - free(s.cong_alg);
   }
  }
  
 
 

Thanks!

Should I put you in From tag or in Signed-off-by ?
Or your diff might be used from this email thread ?
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH iproute2 v2] ss: Fix allocation of cong control alg name

[Vadim Kochan]

From: Vadim Kochan vadi...@gmail.com

Used 16 char array for cong alg name instead of malloc.

Fixes: 8250bc9ff4e5 (ss: Unify inet sockets output)
Reported-by: Jose R. Guzman Mosqueda jose.r.guzman.mosqu...@intel.com
Signed-off-by: Vadim Kochan vadi...@gmail.com
---
v2:
   Used 16 byte array for cong alg name instead of malloc
suggested by Eric Dumazet eric.duma...@gmail.com

 misc/ss.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 347e3a1..0bab8a2 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -755,7 +755,7 @@ struct tcpstat
int timer;
int timeout;
int probes;
-   char*cong_alg;
+   charcong_alg[16];
double  rto, ato, rtt, rttvar;
int qack, cwnd, ssthresh, backoff;
double  send_bps;
@@ -1664,7 +1664,7 @@ static void tcp_stats_print(struct tcpstat *s)
printf( ecnseen);
if (s-has_fastopen_opt)
printf( fastopen);
-   if (s-cong_alg)
+   if (s-cong_alg[0])
printf( %s, s-cong_alg);
if (s-has_wscale_opt)
printf( wscale:%d,%d, s-snd_wscale, s-rcv_wscale);
@@ -1907,9 +1907,9 @@ static void tcp_show_info(const struct nlmsghdr *nlh, 
struct inet_diag_msg *r,
}
 
if (tb[INET_DIAG_CONG]) {
-   const char *cong_attr = 
rta_getattr_str(tb[INET_DIAG_CONG]);
-   s.cong_alg = malloc(strlen(cong_attr + 1));
-   strcpy(s.cong_alg, cong_attr);
+   strncpy(s.cong_alg,
+   rta_getattr_str(tb[INET_DIAG_CONG]),
+   sizeof(s.cong_alg) - 1);
}
 
if (TCPI_HAS_OPT(info, TCPI_OPT_WSCALE)) {
@@ -1984,8 +1984,6 @@ static void tcp_show_info(const struct nlmsghdr *nlh, 
struct inet_diag_msg *r,
tcp_stats_print(s);
if (s.dctcp)
free(s.dctcp);
-   if (s.cong_alg)
-   free(s.cong_alg);
}
 }
 
-- 
2.3.1

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH iproute2] man ip-link: Remove extra GROUP explanation

[Vadim Kochan]

From: Vadim Kochan vadi...@gmail.com

Remove double explanation of GROUP option from 'ip link set' section.

Signed-off-by: Vadim Kochan vadi...@gmail.com
---
 man/man8/ip-link.8.in | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 823246f..714aab4 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -714,12 +714,6 @@ tool can be used. But it allows to change network 
namespace only for physical de
 give the device a symbolic name for easy reference.
 
 .TP
-.BI group  GROUP
-specify the group the device belongs to.
-The available groups are listed in file
-.BR @SYSCONFDIR@/group .
-
-.TP
 .BI vf  NUM
 specify a Virtual Function device to be configured. The associated PF device
 must be specified using the
@@ -867,7 +861,7 @@ specifies which help of link type to dislpay.
 .SS
 .I GROUP
 may be a number or a string from the file
-.B /etc/iproute2/group
+.B @SYSCONFDIR@/group
 which can be manually filled.
 
 .SH EXAMPLES
-- 
2.3.1

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH iproute2 1/2] tests: Run each test in network namespace

[Vadim Kochan]

From: Vadim Kochan vadi...@gmail.com

Changed to forcely running each test in network
namespace to do not affect on current network setup.

Signed-off-by: Vadim Kochan vadi...@gmail.com
---
 testsuite/Makefile | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/testsuite/Makefile b/testsuite/Makefile
index a2c8a2d..4b945b0 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -1,9 +1,11 @@
 ## -- Config --
 DEV := lo
-PREFIX := sudo -E
+PREFIX := sudo -E unshare -n
 RESULTS_DIR := results
 ## -- End Config --
 
+HAVE_UNSHARED_UTIL := $(shell unshare --version 2 /dev/null)
+
 rwildcard=$(wildcard $1$2) $(foreach d,$(wildcard $1*),$(call 
rwildcard,$d/,$2))
 
 TESTS := $(patsubst tests/%,%,$(call rwildcard,tests/,*.t))
@@ -38,6 +40,9 @@ distclean: clean
echo Entering iproute2  cd iproute2  $(MAKE) distclean  cd ..;
 
 $(TESTS): clean
+ifeq (,$(HAVE_UNSHARED_UTIL))
+   $(error Please install util-linux tools to run tests in separated 
network namespace)
+endif
@mkdir -p $(RESULTS_DIR)

@for d in $(TESTS_DIR); do \
-- 
2.3.1

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: iproute2 regression -- ss -u returns an empty list

[Vadim Kochan]

On Tue, Apr 28, 2015 at 03:22:33AM +0200, Mihai Moldovan wrote:
 Hi,
 
 Following up a bug report I received at
 http://bugs.x2go.org/cgi-bin/bugreport.cgi?bug=799 it looks like the current
 version iproute2's ss utility always returns an empty result set whenever
 specifying the -u flag.
 
 The problem has been bisected to 9db7bf15e22b6a1b8bc09c4a1e29571cbca55c94 and 
 is
 also affecting version 3.19.0, but most notably 4.0.0+.
 
 I'm not quite sure that's the correct commit, but it does definitely look 
 related.
 
 Even ss -xu, which previously returned a list of UNIX and UDP sockets now
 returns nothing on my systems.
 
 
 Any help with that matter is greatly appreciated.
 
 
 
 Mihai
 


Hi Mihai,

Seems I have a fixed version, I tested it localy with different
option combinations, and now there should be no issues when specifying
different families.

Would you be able to test it ? How I can provide a fixed version to you -
by patch or I can push to my github and give you a link ?

Thanks,
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: iproute2 regression -- ss -u returns an empty list

[Vadim Kochan]

On Tue, Apr 28, 2015 at 06:33:18AM +0200, Mihai Moldovan wrote:
 On 28.04.2015 06:05 AM, Vadim Kochan wrote: So by default ss prints socket 
 with
 CONNECTED state and even UDP can be
  in the CONNECTED state on the Linux,
 
 UDP can be in a CONNECTED state... kay. That's probably a conntrack thing.
 
 
  so you can specify 'ss -ua' (may be
  some explanation should be added to the ss man page) which
  should print UDP sockets in the any state, AFAIK it was the default
  behaviour before my changes.
 
 Could this please be reverted? It's breaking other stuff that uses ss.
 
 Also: ss -lxua returns an empty list, too. Previously, the families seem to 
 have
 been OR'd, while they are now ANDed?
 
 
 
 Mihai
 

I will try to fix this ...
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: iproute2 regression -- ss -u returns an empty list

[Vadim Kochan]

On Tue, Apr 28, 2015 at 07:07:56AM +0200, Mihai Moldovan wrote:
 On 28.04.2015 06:47 AM, Vadim Kochan wrote:
  On Tue, Apr 28, 2015 at 06:33:18AM +0200, Mihai Moldovan wrote:
  On 28.04.2015 06:05 AM, Vadim Kochan wrote: So by default ss prints
  socket with CONNECTED state and even UDP can be
  in the CONNECTED state on the Linux,
  
  UDP can be in a CONNECTED state... kay. That's probably a conntrack 
  thing.
  
  
  so you can specify 'ss -ua' (may be some explanation should be added to 
  the ss man page) which should print UDP sockets in the any state, AFAIK 
  it was the default behaviour before my changes.
  
  Could this please be reverted? It's breaking other stuff that uses ss.
  
  Also: ss -lxua returns an empty list, too. Previously, the families seem
  to have been OR'd, while they are now ANDed?
  
  
  
  Mihai
  
  
  I will try to fix this ...
 
 Thanks!
 
 To be a bit more verbose about the rationale:
 
 Changing the default behavior is generally a bad idea, unless strictly 
 necessary
 (e.g., if the previous default behavior was buggy anyway), because it may 
 break

Yes the previous behaviour was buggy ... And that was the reason.

 other software. Please try introducing behavioral changes via new options.
 
 That way, dependencies will still work fine and anyone who wants to use the 
 new
 behavior can explicitly do so via a switch.
 
 I could work around this by checking ss' version, but that turns out to be a
 pain, too, because ss -V reports something like this:
 
 ss utility, iproute2-ss150413
 
 Not exactly a friendly version like 3.19.0 or 4.0.0 to check against...
 
 
 
 Mihai
 
 


--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH iproute2] tc util: Fix possible buffer overflow when print class id

[Vadim Kochan]

From: Vadim Kochan vadi...@gmail.com

Use correct handle buffer length.

Signed-off-by: Vadim Kochan vadi...@gmail.com
---
 tc/tc_util.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/tc/tc_util.c b/tc/tc_util.c
index 1d3153d..dc2b70f 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -128,30 +128,31 @@ ok:
return 0;
 }
 
-int print_tc_classid(char *buf, int len, __u32 h)
+int print_tc_classid(char *buf, int blen, __u32 h)
 {
-   char handle[40] = {};
+   SPRINT_BUF(handle) = {};
+   int hlen = SPRINT_BSIZE - 1;
 
if (h == TC_H_ROOT)
sprintf(handle, root);
else if (h == TC_H_UNSPEC)
-   snprintf(handle, len, none);
+   snprintf(handle, hlen, none);
else if (TC_H_MAJ(h) == 0)
-   snprintf(handle, len, :%x, TC_H_MIN(h));
+   snprintf(handle, hlen, :%x, TC_H_MIN(h));
else if (TC_H_MIN(h) == 0)
-   snprintf(handle, len, %x:, TC_H_MAJ(h)  16);
+   snprintf(handle, hlen, %x:, TC_H_MAJ(h)  16);
else
-   snprintf(handle, len, %x:%x, TC_H_MAJ(h)  16, TC_H_MIN(h));
+   snprintf(handle, hlen, %x:%x, TC_H_MAJ(h)  16, TC_H_MIN(h));
 
if (use_names) {
char clname[IDNAME_MAX] = {};
 
if (id_to_name(cls_names, h, clname))
-   snprintf(buf, len, %s#%s, clname, handle);
+   snprintf(buf, blen, %s#%s, clname, handle);
else
-   snprintf(buf, len, %s, handle);
+   snprintf(buf, blen, %s, handle);
} else {
-   snprintf(buf, len, %s, handle);
+   snprintf(buf, blen, %s, handle);
}
 
return 0;
-- 
2.3.1

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html