[PATCH 2/7] turn Makefile more distribution friendly

[PATCH 2/7] turn Makefile more distribution friendly

[Pavel Šimerda]

From: Pavel Šimerda <psimerda@redhat.com>

Changes:

 * Accept directory settings from environment.
 * Remove redundant ROOTDIR variable.
 * Set KERNEL_INCLUDE default to '/usr/include'.
 * Use CFLAGS from environemnt.

Note: In the long term it might be better to improve the configure
script to generate those parts of the Makefile in a manner similar
to autoconf. It might be even practical to autotoolize the package.

Signed-off-by: Pavel Šimerda <psimerda@redhat.com>
---
 Makefile | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/Makefile b/Makefile
index 9dbb29f..18faee4 100644
--- a/Makefile
+++ b/Makefile
@@ -1,15 +1,15 @@
-ROOTDIR=$(DESTDIR)
-PREFIX=/usr
-LIBDIR=$(PREFIX)/lib
-SBINDIR=/sbin
-CONFDIR=/etc/iproute2
-DATADIR=$(PREFIX)/share
-DOCDIR=$(DATADIR)/doc/iproute2
-MANDIR=$(DATADIR)/man
-ARPDDIR=/var/lib/arpd
+PREFIX?=/usr
+LIBDIR?=$(PREFIX)/lib
+SBINDIR?=/sbin
+CONFDIR?=/etc/iproute2
+DATADIR?=$(PREFIX)/share
+DOCDIR?=$(DATADIR)/doc/iproute2
+MANDIR?=$(DATADIR)/man
+ARPDDIR?=/var/lib/arpd
+KERNEL_INCLUDE?=/usr/include
 
 # Path to db_185.h include
-DBM_INCLUDE:=$(ROOTDIR)/usr/include
+DBM_INCLUDE:=$(DESTDIR)/usr/include
 
 SHARED_LIBS = y
 
@@ -33,7 +33,7 @@ CCOPTS = -O2
 WFLAGS := -Wall -Wstrict-prototypes  -Wmissing-prototypes
 WFLAGS += -Wmissing-declarations -Wold-style-definition -Wformat=2
 
-CFLAGS = $(WFLAGS) $(CCOPTS) -I../include $(DEFINES)
+CFLAGS := $(WFLAGS) $(CCOPTS) -I../include $(DEFINES) $(CFLAGS)
 YACCFLAGS = -d -t -v
 
 SUBDIRS=lib ip tc bridge misc netem genl man
-- 
2.3.5

[PATCH 3/7] ip-xfrm: support 'proto any' with 'sport' and 'dport'

[Pavel Šimerda]

From: Pavel Šimerda <psimerda@redhat.com>

When creating an IPsec SA that sets 'proto any' (IPPROTO_IP) and
specifies 'sport' and 'dport' at the same time in selector, the
following error is issued:

"sport" and "dport" are invalid with proto=ip

However using IPPROTO_IP with ports is completely legal and necessary
when one wants to share the SA on both TCP and UDP. One of the
applications requiring sharing SAs is 3GPP IMS AKA authentication.

See also:

 * https://bugzilla.redhat.com/show_bug.cgi?id=497355

Reported-by: Jiří Klimeš <jklimes@redhat.com>
Signed-off-by: Pavel Šimerda <psimerda@redhat.com>
---
 ip/ipxfrm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ip/ipxfrm.c b/ip/ipxfrm.c
index 95f91a5..e685571 100644
--- a/ip/ipxfrm.c
+++ b/ip/ipxfrm.c
@@ -1339,6 +1339,7 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
 		case IPPROTO_UDP:
 		case IPPROTO_SCTP:
 		case IPPROTO_DCCP:
+		case IPPROTO_IP: /* to allow shared SA for different protocols */
 			break;
 		default:
 			fprintf(stderr, "\"sport\" and \"dport\" are invalid with PROTO value \"%s\"\n", strxf_proto(sel->proto));
-- 
2.3.5

[PATCH 4/7] cbq: fix find syntax in example

[Pavel Šimerda]

From: Pavel Šimerda <psimerda@redhat.com>

Without modification, using the example resulted in the following error:

[root@localhost sbin]# cbq restart
find: warning: you have specified the -maxdepth option after a
non-option argument (, but options are not positional (-maxdepth affects
tests specified before it as well as those specified after it).  Please
specify options before other arguments.

find: warning: you have specified the -maxdepth option after a
non-option argument (, but options are not positional (-maxdepth affects
tests specified before it as well as those specified after it).  Please
specify options before other arguments.

**CBQ: failed to compile CBQ configuration!

See also:

 * https://bugzilla.redhat.com/show_bug.cgi?id=539232

Reported-by: Mads Kiilerich <mads@kiilerich.com>
Signed-off-by: Pavel Šimerda <psimerda@redhat.com>
---
 examples/cbq.init-v0.7.3 | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/examples/cbq.init-v0.7.3 b/examples/cbq.init-v0.7.3
index 35a0a05..1bc0d44 100644
--- a/examples/cbq.init-v0.7.3
+++ b/examples/cbq.init-v0.7.3
@@ -578,14 +578,14 @@ cbq_show () {
 ### Check configuration and load DEVICES, DEVFIELDS and CLASSLIST from $1
 cbq_init () {
 	### Get a list of configured classes
-	CLASSLIST=`find $1 \( -type f -or -type l \) -name 'cbq-*' \
-		-not -name '*~' -maxdepth 1 -printf "%f\n"| sort`
+	CLASSLIST=`find $1 -maxdepth 1 \( -type f -or -type l \) -name 'cbq-*' \
+		-not -name '*~' -printf "%f\n"| sort`
 	[ -z "$CLASSLIST" ] &&
 		cbq_failure "no configuration files found in $1!"
 
 	### Gather all DEVICE fields from $1/cbq-*
-	DEVFIELDS=`find $1 \( -type f -or -type l \) -name 'cbq-*' \
-		  -not -name '*~' -maxdepth 1| xargs sed -n 's/#.*//; \
+	DEVFIELDS=`find $1 -maxdepth 1 \( -type f -or -type l \) -name 'cbq-*' \
+		  -not -name '*~' | xargs sed -n 's/#.*//; \
 		  s/[[:space:]]//g; /^DEVICE=[^,]*,[^,]*\(,[^,]*\)\?/ \
 		  { s/.*=//; p; }'| sort -u`
 	[ -z "$DEVFIELDS" ] &&
-- 
2.3.5

[PATCH 5/7] ip-route: don't hide routes with RTM_F_CLONED by default

[Pavel Šimerda]

From: Pavel Šimerda <psimerda@redhat.com>

Signed-off-by: Pavel Šimerda <psimerda@redhat.com>
---
 ip/iproute.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ip/iproute.c b/ip/iproute.c
index 024d401..46c24bb 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -144,7 +144,7 @@ static int filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
 	if (r->rtm_family == AF_INET6 && table != RT_TABLE_MAIN)
 		ip6_multiple_tables = 1;
 
-	if (filter.cloned == !(r->rtm_flags&RTM_F_CLONED))
+	if (filter.cloned && !(r->rtm_flags&RTM_F_CLONED))
 		return 0;
 
 	if (r->rtm_family == AF_INET6 && !ip6_multiple_tables) {
-- 
2.3.5

[PATCH 6/7] lnstat: dump to stdout, not stderr

[Pavel Šimerda]

From: Pavel Šimerda <psimerda@redhat.com>

See also:

 * https://bugzilla.redhat.com/show_bug.cgi?id=736332

Signed-off-by: Pavel Šimerda <psimerda@redhat.com>
---
 misc/lnstat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/misc/lnstat.c b/misc/lnstat.c
index 32b5cbe..e105b70 100644
--- a/misc/lnstat.c
+++ b/misc/lnstat.c
@@ -347,7 +347,7 @@ int main(int argc, char **argv)
 
 	switch (mode) {
 	case MODE_DUMP:
-		lnstat_dump(stderr, lnstat_files);
+		lnstat_dump(stdout, lnstat_files);
 		break;
 
 	case MODE_NORMAL:
-- 
2.3.5

[PATCH 7/7] lnstat: run indefinitely by default

[Pavel Šimerda]

From: Pavel Šimerda <psimerda@redhat.com>

See also:

 * https://bugzilla.redhat.com/show_bug.cgi?id=977845

Signed-off-by: Pavel Šimerda <psimerda@redhat.com>
---
 misc/lnstat.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/misc/lnstat.c b/misc/lnstat.c
index e105b70..0385cbb 100644
--- a/misc/lnstat.c
+++ b/misc/lnstat.c
@@ -249,7 +249,7 @@ int main(int argc, char **argv)
 		MODE_JSON,
 		MODE_NORMAL,
 	} mode = MODE_NORMAL;
-	unsigned long count = 1;
+	unsigned long count = 0;
 	struct table_hdr *header;
 	static struct field_params fp;
 	int num_req_files = 0;
@@ -362,7 +362,7 @@ int main(int argc, char **argv)
 		if (interval < 1 )
 			interval = 1;
 
-		for (i = 0; i < count; i++) {
+		for (i = 0; i < count || !count; ) {
 			lnstat_update(lnstat_files);
 			if (mode == MODE_JSON)
 				print_json(stdout, lnstat_files, &fp);
@@ -373,8 +373,10 @@ int main(int argc, char **argv)
 				print_line(stdout, lnstat_files, &fp);
 			}
 			fflush(stdout);
-			if (i < count - 1)
+			if (i < count - 1 || !count)
 				sleep(interval);
+			if (count)
+				++i;
 		}
 		break;
 	}
-- 
2.3.5

Re: [PATCH 2/7] turn Makefile more distribution friendly

[Stephen Hemminger]

On Mon, 13 Apr 2015 16:00:56 +0200
Pavel Šimerda <pavlix@pavlix.net> wrote:

> From: Pavel Šimerda <psimerda@redhat.com>
> 
> Changes:
> 
>  * Accept directory settings from environment.
>  * Remove redundant ROOTDIR variable.
>  * Set KERNEL_INCLUDE default to '/usr/include'.
>  * Use CFLAGS from environemnt.
> 
> Note: In the long term it might be better to improve the configure
> script to generate those parts of the Makefile in a manner similar
> to autoconf. It might be even practical to autotoolize the package.
> 
> Signed-off-by: Pavel Šimerda <psimerda@redhat.com>

I will take this part.
But don't want to start iproute2 down the autoconf/autotool sink hole.

Re: [PATCH 2/7] turn Makefile more distribution friendly

[Pavel Šimerda]

On 04/20/2015 06:55 PM, Stephen Hemminger wrote:
> On Mon, 13 Apr 2015 16:00:56 +0200
> Pavel Šimerda <pavlix@pavlix.net> wrote:
> 
>> From: Pavel Šimerda <psimerda@redhat.com>
>>
>> Changes:
>>
>>  * Accept directory settings from environment.
>>  * Remove redundant ROOTDIR variable.
>>  * Set KERNEL_INCLUDE default to '/usr/include'.
>>  * Use CFLAGS from environemnt.
>>
>> Note: In the long term it might be better to improve the configure
>> script to generate those parts of the Makefile in a manner similar
>> to autoconf. It might be even practical to autotoolize the package.
>>
>> Signed-off-by: Pavel Šimerda <psimerda@redhat.com>
> 
> I will take this part.
> But don't want to start iproute2 down the autoconf/autotool sink hole.

Thanks! The changes I submitted should generally be good enough for
distro maintainers to avoid Makefile modifications.

Cheers,

Pavel

Re: [PATCH 5/7] ip-route: don't hide routes with RTM_F_CLONED by default

[Stephen Hemminger]

On Mon, 13 Apr 2015 16:00:59 +0200
Pavel Šimerda <pavlix@pavlix.net> wrote:

> From: Pavel Šimerda <psimerda@redhat.com>
> 
> Signed-off-by: Pavel Šimerda <psimerda@redhat.com>
> ---
>  ip/iproute.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I understand your concern, and it probably was a poor design choice initially.

Since this will change the output for the default case, it will upset some
peoples expectations and potentially break scripts that screen scrape the output
of ip commands. Therefore I can't accept it at this time.

Sorry for the delay, but I thought someone else would add more comments.
Perhaps if you explained in more detail the motivation of why this is an
important problem I would reconsider change the behavior.

Re: [PATCH 5/7] ip-route: don't hide routes with RTM_F_CLONED by default

[David Miller]

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 4 May 2015 08:37:51 -0700

> On Mon, 13 Apr 2015 16:00:59 +0200
> Pavel Šimerda <pavlix@pavlix.net> wrote:
> 
>> From: Pavel Šimerda <psimerda@redhat.com>
>> 
>> Signed-off-by: Pavel Šimerda <psimerda@redhat.com>
>> ---
>>  ip/iproute.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> I understand your concern, and it probably was a poor design choice initially.
> 
> Since this will change the output for the default case, it will upset some
> peoples expectations and potentially break scripts that screen scrape the output
> of ip commands. Therefore I can't accept it at this time.
> 
> Sorry for the delay, but I thought someone else would add more comments.
> Perhaps if you explained in more detail the motivation of why this is an
> important problem I would reconsider change the behavior.

We definitely need to report routes created by caching/cloning separately
from the main FIB entries.

And I agree that even if we wanted to change behavior, the horse has
already left the barn on this one and therefore there is no way we can
change this now.

Re: [PATCH 5/7] ip-route: don't hide routes with RTM_F_CLONED by default

[Pavel Šimerda]

On 05/04/2015 08:37 PM, David Miller wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Mon, 4 May 2015 08:37:51 -0700
> 
>> On Mon, 13 Apr 2015 16:00:59 +0200
>> Pavel Šimerda <pavlix@pavlix.net> wrote:
>>
>>> From: Pavel Šimerda <psimerda@redhat.com>
>>>
>>> Signed-off-by: Pavel Šimerda <psimerda@redhat.com>
>>> ---
>>>  ip/iproute.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> I understand your concern, and it probably was a poor design choice initially.
>>
>> Since this will change the output for the default case, it will upset some
>> peoples expectations and potentially break scripts that screen scrape the output
>> of ip commands. Therefore I can't accept it at this time.
>>
>> Sorry for the delay, but I thought someone else would add more comments.
>> Perhaps if you explained in more detail the motivation of why this is an
>> important problem I would reconsider change the behavior.

Hi,

I have no specific concern right now.

> We definitely need to report routes created by caching/cloning separately
> from the main FIB entries.
> 
> And I agree that even if we wanted to change behavior, the horse has
> already left the barn on this one and therefore there is no way we can
> change this now.

For now I treat the patch as rejected by upstream and as I'm not aware
of a specific concern for Fedora, I'm also dropping the patch from the
development branch[1]. Thank you for accepting other patches.

My plan is to submit new patches for requests coming from Fedora and
RHEL distributions and also resubmit the manpage patches with new
changes and split by command/subcommand.

Cheers,

Pavel

[1]:
http://pkgs.fedoraproject.org/cgit/iproute.git/commit/?id=f0557f39740b5c18f07b2caf634eb073ba19484f