netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* iproute2: git pull request from debian repo.
@ 2007-10-19 12:27 Andreas Henriksson
  2007-10-19 13:06 ` maximilian attems
  0 siblings, 1 reply; 10+ messages in thread
From: Andreas Henriksson @ 2007-10-19 12:27 UTC (permalink / raw)
  To: shemminger; +Cc: netdev, andreas

Hello!

We now have our brand new git repo set up for the debian package. We have
created a branch called "patches" where we are going to put stuff that we
hope will get merged upstream.

I'm sending out this pull request to get confirmation about if we are on the
right track ...

The repo url is git://git.debian.org/git/collab-maint/pkg-iproute.git
and the branch you're supposed to look at is "patches", as I mentioned.

(gitweb available at:
http://git.debian.org/?p=collab-maint/pkg-iproute.git;a=summary )

It currently only contains two fixes:
- trivial syntax fix in the ss(8) manpage, by Justin Pryzby.
  (Should be a no-brainer.)
- ip addr flush loop fix (from ubuntu), by Daniel Silverstone
  (Equivalent to the treatment "ip neigh flush" previously got.
   This obsoletes the patch from Alexander Wirt which was dropped
   from the last patch submission.)

The patches branch is currently based on the v2.6.23-071016 release, and we
intend to rebase it when you do a new release.

Please notify me about anything I've set up wrong (and helpful hints on how
to do it right, would be deeply appreciated since I'm a complete git newbie).
I would also like to ask if this pull request should be followed up by
posting the patches to the mailing list or if a public git repo is considered
enough? If you want me to also post the individual patches, I'd be very happy
if you have some helpful hints on how to most easily export just the ones in
the tree which doesn't come from the upstream branch and send them out...

Have a nice weekend!

-- 
Regards,
Andreas Henriksson

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

* Re: iproute2: git pull request from debian repo.
  2007-10-19 12:27 iproute2: git pull request from debian repo Andreas Henriksson
@ 2007-10-19 13:06 ` maximilian attems
  2007-10-19 14:48   ` Andreas Henriksson
  0 siblings, 1 reply; 10+ messages in thread
From: maximilian attems @ 2007-10-19 13:06 UTC (permalink / raw)
  To: Andreas Henriksson; +Cc: shemminger, netdev

On Fri, Oct 19, 2007 at 02:27:53PM +0200, Andreas Henriksson wrote:
> Hello!
> 
> We now have our brand new git repo set up for the debian package. We have
> created a branch called "patches" where we are going to put stuff that we
> hope will get merged upstream.
> 
> I'm sending out this pull request to get confirmation about if we are on the
> right track ...
> 
> The repo url is git://git.debian.org/git/collab-maint/pkg-iproute.git
> and the branch you're supposed to look at is "patches", as I mentioned.

on klibc i'm asked to attach git log -p on a pull request so
that the patches can be reviewed, please do so too.
 
also apprecitated would be the output of
git log master..patches | git shortlog

> It currently only contains two fixes:
> - trivial syntax fix in the ss(8) manpage, by Justin Pryzby.
>   (Should be a no-brainer.)
> - ip addr flush loop fix (from ubuntu), by Daniel Silverstone
>   (Equivalent to the treatment "ip neigh flush" previously got.
>    This obsoletes the patch from Alexander Wirt which was dropped
>    from the last patch submission.)
> 

happy weekend

-- 
maks

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

* Re: iproute2: git pull request from debian repo.
  2007-10-19 13:06 ` maximilian attems
@ 2007-10-19 14:48   ` Andreas Henriksson
  2007-10-21 17:48     ` Patrick McHardy
  0 siblings, 1 reply; 10+ messages in thread
From: Andreas Henriksson @ 2007-10-19 14:48 UTC (permalink / raw)
  To: maximilian attems; +Cc: shemminger, netdev

Hello maks!

Thanks for your help. I had to tweak the commands a bit to git it working. I
hope I did it right..

On Fri, Oct 19, 2007 at 03:06:16PM +0200, maximilian attems wrote:
> 
> on klibc i'm asked to attach git log -p on a pull request so
> that the patches can be reviewed, please do so too.

git log upstream..patches -p 

>  
> also apprecitated would be the output of
> git log master..patches | git shortlog
> 

git log upstream..patches | git shortlog

I'm using the branching scheme madcoder described here:
http://lists.madduck.net/pipermail/vcs-pkg/2007-October/000021.html

Unfortunately this includes the extra cherry-picked patches from
upstream-repo/master, but I guess we'll just have to cut those out
manually or make sure that we do our submission right after each
upstream release (when we rebase).

Thanks again.

shortlog:
--------------------------------------------------

Daniel Silverstone (1):
      Avoid infinite loop in ip addr flush.

Justin Pryzby (1):
      ss(8) manpage formatting breaks EXAMPLE


git log -p:
--------------------------------------------------

commit 1e9304759ec9f1fd0d80a52cbb7d08decbe8c9a0
Author: Daniel Silverstone <daniel.silverstone@ubuntu.com>
Date:   Fri Oct 19 13:32:24 2007 +0200

    Avoid infinite loop in ip addr flush.
    
    Fix "ip addr flush" the same way "ip neigh flush" was previously fixed,
    by bailing out if the flush hasn't completed after MAX_ROUNDS (10) tries.

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 518d8cd..ff9e318 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -34,6 +34,8 @@
 #include "ll_map.h"
 #include "ip_common.h"
 
+#define MAX_ROUNDS 10
+
 static struct
 {
 	int ifindex;
@@ -667,7 +669,7 @@ int ipaddr_list_or_flush(int argc, char **argv, int flush)
 		filter.flushp = 0;
 		filter.flushe = sizeof(flushb);
 
-		for (;;) {
+		while (round < MAX_ROUNDS) {
 			if (rtnl_wilddump_request(&rth, filter.family, RTM_GETADDR) < 0) {
 				perror("Cannot send dump request");
 				exit(1);
@@ -694,6 +696,8 @@ int ipaddr_list_or_flush(int argc, char **argv, int flush)
 				fflush(stdout);
 			}
 		}
+		fprintf(stderr, "*** Flush remains incomplete after %d rounds. ***\n", MAX_ROUNDS); fflush(stderr);
+		return 1;
 	}
 
 	if (filter.family != AF_PACKET) {

commit 874e14c54be75cfe2ae8b9261eef64194750807b
Author: Justin Pryzby <justinpryzby@users.sourceforge.net>
Date:   Thu Oct 18 16:06:08 2007 +0200

    ss(8) manpage formatting breaks EXAMPLE
    
    Remove not needed quotes around path which breaks manpage formatting.
    Spotted and original patch by Justin Pryzby, fixed by Andreas Henriksson.
    
    http://bugs.debian.org/443071

diff --git a/man/man8/ss.8 b/man/man8/ss.8
index f732319..0c5bf24 100644
--- a/man/man8/ss.8
+++ b/man/man8/ss.8
@@ -107,7 +107,7 @@ Display all UDP sockets.
 .B ss -o state established '( dport = :ssh or sport = :ssh )'
 Display all established ssh connections.
 .TP
-.B ss -x src \"/tmp/.X11-unix/*\"
+.B ss -x src /tmp/.X11-unix/*
 Find all local processes connected to X server.
 .TP
 .B ss -o state fin-wait-1 '( sport = :http or sport = :https )' dst 193.233.7/24


-- 
Regards,
Andreas Henriksson

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

* Re: iproute2: git pull request from debian repo.
  2007-10-19 14:48   ` Andreas Henriksson
@ 2007-10-21 17:48     ` Patrick McHardy
  2007-10-23 13:56       ` Andreas Henriksson
  2007-10-23 14:18       ` Andreas Henriksson
  0 siblings, 2 replies; 10+ messages in thread
From: Patrick McHardy @ 2007-10-21 17:48 UTC (permalink / raw)
  To: Andreas Henriksson; +Cc: maximilian attems, shemminger, netdev

Andreas Henriksson wrote:
> -		for (;;) {
> +		while (round < MAX_ROUNDS) {
>  			if (rtnl_wilddump_request(&rth, filter.family, RTM_GETADDR) < 0) {
>  				perror("Cannot send dump request");
>  				exit(1);
> @@ -694,6 +696,8 @@ int ipaddr_list_or_flush(int argc, char **argv, int flush)
>  				fflush(stdout);
>  			}
>  		}
> +		fprintf(stderr, "*** Flush remains incomplete after %d rounds. ***\n", MAX_ROUNDS); fflush(stderr);


Again, please make this optional. People might want to make
sure the cache is flushed even if it takes a bit longer.

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

* Re: iproute2: git pull request from debian repo.
  2007-10-21 17:48     ` Patrick McHardy
@ 2007-10-23 13:56       ` Andreas Henriksson
  2007-10-23 14:18       ` Andreas Henriksson
  1 sibling, 0 replies; 10+ messages in thread
From: Andreas Henriksson @ 2007-10-23 13:56 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: maximilian attems, shemminger, netdev

On Sun, Oct 21, 2007 at 07:48:21PM +0200, Patrick McHardy wrote:
> Andreas Henriksson wrote:
> >-		for (;;) {
> >+		while (round < MAX_ROUNDS) {
> > 			if (rtnl_wilddump_request(&rth, filter.family, 
> > 			RTM_GETADDR) < 0) {
> > 				perror("Cannot send dump request");
> > 				exit(1);
> >@@ -694,6 +696,8 @@ int ipaddr_list_or_flush(int argc, char **argv, int 
> >flush)
> > 				fflush(stdout);
> > 			}
> > 		}
> >+		fprintf(stderr, "*** Flush remains incomplete after %d 
> >rounds. ***\n", MAX_ROUNDS); fflush(stderr);
> 
> 
> Again, please make this optional. People might want to make
> sure the cache is flushed even if it takes a bit longer.


Something like the patch below should hopefully please everybody. Making
MAX_ROUNDS configurable is possibly even better (and 0=infinite), I'll see if
I can whip up an alternativ patch so you can decide which one you like better.

I've not added it to the patches branch yet, since Stephen said next release
will probably be a bugfix only release and I consider this a new feature.
(The previous patch simply makes ip addr flush behave the same as ip neigh
flush, and avoids an endless loop which I consider a staight bugfix.
Plus I don't want to do anything more with it until it's confirmed
I've set up the git repo correctly.)
I guess it's up to Stephen if he wants this feature and if he wants in now or
later... Please tell me.


Only compile-tested for now (this is on top of the previous patch):


diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index ff9e318..356535b 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -65,7 +65,7 @@ static void usage(void)
 	fprintf(stderr, "Usage: ip addr {add|change|replace} IFADDR dev STRING [ LIFETIME ]\n");
 	fprintf(stderr, "                                                      [ CONFFLAG-LIST]\n");
 	fprintf(stderr, "       ip addr del IFADDR dev STRING\n");
-	fprintf(stderr, "       ip addr {show|flush} [ dev STRING ] [ scope SCOPE-ID ]\n");
+	fprintf(stderr, "       ip addr {show|flush|forceflush} [ dev STRING ] [ scope SCOPE-ID ]\n");
 	fprintf(stderr, "                            [ to PREFIX ] [ FLAG-LIST ] [ label PATTERN ]\n");
 	fprintf(stderr, "IFADDR := PREFIX | ADDR peer PREFIX\n");
 	fprintf(stderr, "          [ broadcast ADDR ] [ anycast ADDR ]\n");
@@ -559,7 +559,7 @@ static int store_nlmsg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 	return 0;
 }
 
-int ipaddr_list_or_flush(int argc, char **argv, int flush)
+int ipaddr_list_or_flush(int argc, char **argv, int flush, int force)
 {
 	struct nlmsg_list *linfo = NULL;
 	struct nlmsg_list *ainfo = NULL;
@@ -669,7 +669,7 @@ int ipaddr_list_or_flush(int argc, char **argv, int flush)
 		filter.flushp = 0;
 		filter.flushe = sizeof(flushb);
 
-		while (round < MAX_ROUNDS) {
+		while (force || (round < MAX_ROUNDS)) {
 			if (rtnl_wilddump_request(&rth, filter.family, RTM_GETADDR) < 0) {
 				perror("Cannot send dump request");
 				exit(1);
@@ -696,7 +696,8 @@ int ipaddr_list_or_flush(int argc, char **argv, int flush)
 				fflush(stdout);
 			}
 		}
-		fprintf(stderr, "*** Flush remains incomplete after %d rounds. ***\n", MAX_ROUNDS); fflush(stderr);
+		fprintf(stderr, "*** Flush remains incomplete, bailing out after %d rounds (use forceflush to never give up).\n", MAX_ROUNDS);
+		fflush(stderr);
 		return 1;
 	}
 
@@ -790,7 +791,7 @@ int ipaddr_list_link(int argc, char **argv)
 {
 	preferred_family = AF_PACKET;
 	do_link = 1;
-	return ipaddr_list_or_flush(argc, argv, 0);
+	return ipaddr_list_or_flush(argc, argv, 0, 0);
 }
 
 void ipaddr_reset_filter(int oneline)
@@ -1006,7 +1007,7 @@ int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 int do_ipaddr(int argc, char **argv)
 {
 	if (argc < 1)
-		return ipaddr_list_or_flush(0, NULL, 0);
+		return ipaddr_list_or_flush(0, NULL, 0, 0);
 	if (matches(*argv, "add") == 0)
 		return ipaddr_modify(RTM_NEWADDR, NLM_F_CREATE|NLM_F_EXCL, argc-1, argv+1);
 	if (matches(*argv, "change") == 0 ||
@@ -1018,9 +1019,11 @@ int do_ipaddr(int argc, char **argv)
 		return ipaddr_modify(RTM_DELADDR, 0, argc-1, argv+1);
 	if (matches(*argv, "list") == 0 || matches(*argv, "show") == 0
 	    || matches(*argv, "lst") == 0)
-		return ipaddr_list_or_flush(argc-1, argv+1, 0);
+		return ipaddr_list_or_flush(argc-1, argv+1, 0, 0);
 	if (matches(*argv, "flush") == 0)
-		return ipaddr_list_or_flush(argc-1, argv+1, 1);
+		return ipaddr_list_or_flush(argc-1, argv+1, 1, 0);
+	if (matches(*argv, "forceflush") == 0)
+		return ipaddr_list_or_flush(argc-1, argv+1, 1, 1);
 	if (matches(*argv, "help") == 0)
 		usage();
 	fprintf(stderr, "Command \"%s\" is unknown, try \"ip address help\".\n", *argv);
diff --git a/ip/ipneigh.c b/ip/ipneigh.c
index db684f5..6a99377 100644
--- a/ip/ipneigh.c
+++ b/ip/ipneigh.c
@@ -52,7 +52,7 @@ static void usage(void)
 	fprintf(stderr, "Usage: ip neigh { add | del | change | replace } { ADDR [ lladdr LLADDR ]\n"
 		        "          [ nud { permanent | noarp | stale | reachable } ]\n"
 		        "          | proxy ADDR } [ dev DEV ]\n");
-	fprintf(stderr, "       ip neigh {show|flush} [ to PREFIX ] [ dev DEV ] [ nud STATE ]\n");
+	fprintf(stderr, "       ip neigh {show|flush|forceflush} [ to PREFIX ] [ dev DEV ] [ nud STATE ]\n");
 	exit(-1);
 }
 
@@ -317,7 +317,7 @@ void ipneigh_reset_filter()
 	filter.state = ~0;
 }
 
-int do_show_or_flush(int argc, char **argv, int flush)
+int do_show_or_flush(int argc, char **argv, int flush, int force)
 {
 	char *filter_dev = NULL;
 	int state_given = 0;
@@ -392,7 +392,7 @@ int do_show_or_flush(int argc, char **argv, int flush)
 		filter.flushe = sizeof(flushb);
 		filter.state &= ~NUD_FAILED;
 
-		while (round < MAX_ROUNDS) {
+		while (force || (round < MAX_ROUNDS)) {
 			if (rtnl_wilddump_request(&rth, filter.family, RTM_GETNEIGH) < 0) {
 				perror("Cannot send dump request");
 				exit(1);
@@ -418,8 +418,9 @@ int do_show_or_flush(int argc, char **argv, int flush)
 				fflush(stdout);
 			}
 		}
-		printf("*** Flush not complete bailing out after %d rounds\n",
-			MAX_ROUNDS);
+		fprintf(stderr, "*** Flush remains incomplete, bailing out after %d rounds (use forceflush to never give up).\n", MAX_ROUNDS);
+		fflush(stderr);
+
 		return 1;
 	}
 
@@ -455,13 +456,15 @@ int do_ipneigh(int argc, char **argv)
 		if (matches(*argv, "show") == 0 ||
 		    matches(*argv, "lst") == 0 ||
 		    matches(*argv, "list") == 0)
-			return do_show_or_flush(argc-1, argv+1, 0);
+			return do_show_or_flush(argc-1, argv+1, 0, 0);
 		if (matches(*argv, "flush") == 0)
-			return do_show_or_flush(argc-1, argv+1, 1);
+			return do_show_or_flush(argc-1, argv+1, 1, 0);
+		if (matches(*argv, "forceflush") == 0)
+			return do_show_or_flush(argc-1, argv+1, 1, 1);
 		if (matches(*argv, "help") == 0)
 			usage();
 	} else
-		return do_show_or_flush(0, NULL, 0);
+		return do_show_or_flush(0, NULL, 0, 0);
 
 	fprintf(stderr, "Command \"%s\" is unknown, try \"ip neigh help\".\n", *argv);
 	exit(-1);


-- 
Regards,
Andreas Henriksson

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

* Re: iproute2: git pull request from debian repo.
  2007-10-21 17:48     ` Patrick McHardy
  2007-10-23 13:56       ` Andreas Henriksson
@ 2007-10-23 14:18       ` Andreas Henriksson
  2007-10-23 14:38         ` Patrick McHardy
  1 sibling, 1 reply; 10+ messages in thread
From: Andreas Henriksson @ 2007-10-23 14:18 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: maximilian attems, shemminger, netdev

On Sun, Oct 21, 2007 at 07:48:21PM +0200, Patrick McHardy wrote:
> Andreas Henriksson wrote:
> >-		for (;;) {
> >+		while (round < MAX_ROUNDS) {
> > 			if (rtnl_wilddump_request(&rth, filter.family, 
> > 			RTM_GETADDR) < 0) {
> > 				perror("Cannot send dump request");
> > 				exit(1);
> >@@ -694,6 +696,8 @@ int ipaddr_list_or_flush(int argc, char **argv, int 
> >flush)
> > 				fflush(stdout);
> > 			}
> > 		}
> >+		fprintf(stderr, "*** Flush remains incomplete after %d 
> >rounds. ***\n", MAX_ROUNDS); fflush(stderr);
> 
> 
> Again, please make this optional. People might want to make
> sure the cache is flushed even if it takes a bit longer.

And here's the alternativ with configurable max rounds (0=infinite)...


diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index ff9e318..e4ba340 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -67,6 +67,7 @@ static void usage(void)
 	fprintf(stderr, "       ip addr del IFADDR dev STRING\n");
 	fprintf(stderr, "       ip addr {show|flush} [ dev STRING ] [ scope SCOPE-ID ]\n");
 	fprintf(stderr, "                            [ to PREFIX ] [ FLAG-LIST ] [ label PATTERN ]\n");
+	fprintf(stderr, "                            [ maxrounds N ]\n");
 	fprintf(stderr, "IFADDR := PREFIX | ADDR peer PREFIX\n");
 	fprintf(stderr, "          [ broadcast ADDR ] [ anycast ADDR ]\n");
 	fprintf(stderr, "          [ label STRING ] [ scope SCOPE-ID ]\n");
@@ -566,6 +567,7 @@ int ipaddr_list_or_flush(int argc, char **argv, int flush)
 	struct nlmsg_list *l, *n;
 	char *filter_dev = NULL;
 	int no_link = 0;
+	int maxrounds = MAX_ROUNDS;
 
 	ipaddr_reset_filter(oneline);
 	filter.showqueue = 1;
@@ -630,6 +632,9 @@ int ipaddr_list_or_flush(int argc, char **argv, int flush)
 		} else if (strcmp(*argv, "label") == 0) {
 			NEXT_ARG();
 			filter.label = *argv;
+		} else if (strcmp(*argv, "maxrounds") == 0) {
+			NEXT_ARG();
+			maxrounds = atoi(*argv);
 		} else {
 			if (strcmp(*argv, "dev") == 0) {
 				NEXT_ARG();
@@ -669,7 +674,7 @@ int ipaddr_list_or_flush(int argc, char **argv, int flush)
 		filter.flushp = 0;
 		filter.flushe = sizeof(flushb);
 
-		while (round < MAX_ROUNDS) {
+		while ((maxrounds == 0) || (round < maxrounds)) {
 			if (rtnl_wilddump_request(&rth, filter.family, RTM_GETADDR) < 0) {
 				perror("Cannot send dump request");
 				exit(1);
@@ -696,7 +701,10 @@ int ipaddr_list_or_flush(int argc, char **argv, int flush)
 				fflush(stdout);
 			}
 		}
-		fprintf(stderr, "*** Flush remains incomplete after %d rounds. ***\n", MAX_ROUNDS); fflush(stderr);
+		fprintf(stderr,
+			"*** Flush remains incomplete after %d rounds. ***\n",
+			maxrounds);
+		fflush(stderr);
 		return 1;
 	}
 
diff --git a/ip/ipneigh.c b/ip/ipneigh.c
index db684f5..5a9ce40 100644
--- a/ip/ipneigh.c
+++ b/ip/ipneigh.c
@@ -53,6 +53,7 @@ static void usage(void)
 		        "          [ nud { permanent | noarp | stale | reachable } ]\n"
 		        "          | proxy ADDR } [ dev DEV ]\n");
 	fprintf(stderr, "       ip neigh {show|flush} [ to PREFIX ] [ dev DEV ] [ nud STATE ]\n");
+	fprintf(stderr, "                             [ maxrounds N ]\n");
 	exit(-1);
 }
 
@@ -321,6 +322,7 @@ int do_show_or_flush(int argc, char **argv, int flush)
 {
 	char *filter_dev = NULL;
 	int state_given = 0;
+	int maxrounds = MAX_ROUNDS;
 
 	ipneigh_reset_filter();
 
@@ -361,6 +363,9 @@ int do_show_or_flush(int argc, char **argv, int flush)
 			if (state == 0)
 				state = 0x100;
 			filter.state |= state;
+		} else if (strcmp(*argv, "maxrounds") == 0) {
+			NEXT_ARG();
+			maxrounds = atoi(*argv);
 		} else {
 			if (strcmp(*argv, "to") == 0) {
 				NEXT_ARG();
@@ -392,7 +397,7 @@ int do_show_or_flush(int argc, char **argv, int flush)
 		filter.flushe = sizeof(flushb);
 		filter.state &= ~NUD_FAILED;
 
-		while (round < MAX_ROUNDS) {
+		while ((maxrounds == 0) || (round < maxrounds)) {
 			if (rtnl_wilddump_request(&rth, filter.family, RTM_GETNEIGH) < 0) {
 				perror("Cannot send dump request");
 				exit(1);
@@ -418,8 +423,10 @@ int do_show_or_flush(int argc, char **argv, int flush)
 				fflush(stdout);
 			}
 		}
-		printf("*** Flush not complete bailing out after %d rounds\n",
-			MAX_ROUNDS);
+		fprintf(stderr,
+			"*** Flush remains incomplete after %d rounds. ***\n",
+			maxrounds);
+		fflush(stderr);
 		return 1;
 	}
 


-- 
Regards,
Andreas Henriksson

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

* Re: iproute2: git pull request from debian repo.
  2007-10-23 14:18       ` Andreas Henriksson
@ 2007-10-23 14:38         ` Patrick McHardy
  2007-10-23 15:01           ` Andreas Henriksson
  0 siblings, 1 reply; 10+ messages in thread
From: Patrick McHardy @ 2007-10-23 14:38 UTC (permalink / raw)
  To: Andreas Henriksson; +Cc: maximilian attems, shemminger, netdev

Andreas Henriksson wrote:
> On Sun, Oct 21, 2007 at 07:48:21PM +0200, Patrick McHardy wrote:
>> Andreas Henriksson wrote:
>>> -		for (;;) {
>>> +		while (round < MAX_ROUNDS) {
>>> 			if (rtnl_wilddump_request(&rth, filter.family, 
>>> 			RTM_GETADDR) < 0) {
>>> 				perror("Cannot send dump request");
>>> 				exit(1);
>>> @@ -694,6 +696,8 @@ int ipaddr_list_or_flush(int argc, char **argv, int 
>>> flush)
>>> 				fflush(stdout);
>>> 			}
>>> 		}
>>> +		fprintf(stderr, "*** Flush remains incomplete after %d 
>>> rounds. ***\n", MAX_ROUNDS); fflush(stderr);
>>
>> Again, please make this optional. People might want to make
>> sure the cache is flushed even if it takes a bit longer.
> 
> And here's the alternativ with configurable max rounds (0=infinite)...


Almost - the default behaviour shouldn't change IMO. Also please
send a complete patch, not one on top of your old one.

>  
> -		while (round < MAX_ROUNDS) {
> +		while ((maxrounds == 0) || (round < maxrounds)) {

And please remove the unnecessary parens.

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

* Re: iproute2: git pull request from debian repo.
  2007-10-23 14:38         ` Patrick McHardy
@ 2007-10-23 15:01           ` Andreas Henriksson
  2007-10-23 15:08             ` Patrick McHardy
  0 siblings, 1 reply; 10+ messages in thread
From: Andreas Henriksson @ 2007-10-23 15:01 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: maximilian attems, shemminger, netdev

On Tue, Oct 23, 2007 at 04:38:06PM +0200, Patrick McHardy wrote:
> Almost - the default behaviour shouldn't change IMO. Also please
> send a complete patch, not one on top of your old one.

Which "default behaviour"? ip addr flush or ip neigh flush behaviour?
Both of those should behave the same IMHO.

Defaulting to never bail out is causing problems in a bunch of firewalling
frontends which has been reported multiple times to both debian and ubuntu
(and possibly others I don't know about), and looping forever causes the
bootup to never finish.
Thats my reason for advocating not selecting the "loop forever" as the
default choice.

-- 
Regards,
Andreas Henriksson

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

* Re: iproute2: git pull request from debian repo.
  2007-10-23 15:01           ` Andreas Henriksson
@ 2007-10-23 15:08             ` Patrick McHardy
  2007-10-23 15:27               ` Andreas Henriksson
  0 siblings, 1 reply; 10+ messages in thread
From: Patrick McHardy @ 2007-10-23 15:08 UTC (permalink / raw)
  To: Andreas Henriksson; +Cc: maximilian attems, shemminger, netdev

Andreas Henriksson wrote:
> On Tue, Oct 23, 2007 at 04:38:06PM +0200, Patrick McHardy wrote:
>> Almost - the default behaviour shouldn't change IMO. Also please
>> send a complete patch, not one on top of your old one.
> 
> Which "default behaviour"? ip addr flush or ip neigh flush behaviour?
> Both of those should behave the same IMHO.

In both cases your changed behaviour should be optional and it should
default to the old behaviour. Not actually flushing completely when the
user specified "flush" is counter-intuitive.

> Defaulting to never bail out is causing problems in a bunch of firewalling
> frontends which has been reported multiple times to both debian and ubuntu
> (and possibly others I don't know about), and looping forever causes the
> bootup to never finish.
> Thats my reason for advocating not selecting the "loop forever" as the
> default choice.

Then those frontends should add "maxrounds N" if they want to abort
before the flush is complete.


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

* Re: iproute2: git pull request from debian repo.
  2007-10-23 15:08             ` Patrick McHardy
@ 2007-10-23 15:27               ` Andreas Henriksson
  0 siblings, 0 replies; 10+ messages in thread
From: Andreas Henriksson @ 2007-10-23 15:27 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: maximilian attems, shemminger, netdev

On Tue, Oct 23, 2007 at 05:08:10PM +0200, Patrick McHardy wrote:
> Andreas Henriksson wrote:
> >On Tue, Oct 23, 2007 at 04:38:06PM +0200, Patrick McHardy wrote:
> >>Almost - the default behaviour shouldn't change IMO. Also please
> >>send a complete patch, not one on top of your old one.
> >
> >Which "default behaviour"? ip addr flush or ip neigh flush behaviour?
> >Both of those should behave the same IMHO.
> 
> In both cases your changed behaviour should be optional and it should
> default to the old behaviour. Not actually flushing completely when the
> user specified "flush" is counter-intuitive.
> 

I did not change the behaviour in both cases! I'm only sending in a change
for ip addr flush to make it match how ip neigh flush already works!
Two different behaviours -> one change -> same behaviour for both.
I've mentioned this multiple times already... I don't know how to express
myself in another/better way about this.

> >Defaulting to never bail out is causing problems in a bunch of firewalling
> >frontends which has been reported multiple times to both debian and ubuntu
> >(and possibly others I don't know about), and looping forever causes the
> >bootup to never finish.
> >Thats my reason for advocating not selecting the "loop forever" as the
> >default choice.
> 
> Then those frontends should add "maxrounds N" if they want to abort
> before the flush is complete.
> 

So you prefer that I rather change the default for ip neigh flush, then touch
ip addr flush? Making the problematic version the default for both, and
changing every application out there who uses any of these two functions.
Sorry, I'll pass on that one and let someone else do it if it's the preferred
solution.

-- 
Regards,
Andreas Henriksson

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

end of thread, other threads:[~2007-10-23 15:31 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-19 12:27 iproute2: git pull request from debian repo Andreas Henriksson
2007-10-19 13:06 ` maximilian attems
2007-10-19 14:48   ` Andreas Henriksson
2007-10-21 17:48     ` Patrick McHardy
2007-10-23 13:56       ` Andreas Henriksson
2007-10-23 14:18       ` Andreas Henriksson
2007-10-23 14:38         ` Patrick McHardy
2007-10-23 15:01           ` Andreas Henriksson
2007-10-23 15:08             ` Patrick McHardy
2007-10-23 15:27               ` Andreas Henriksson

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