Netdev List
 help / color / mirror / Atom feed
* Re: iproute2 (addr flush) infinite loop when unprivileged users
From: Alon Bar-Lev @ 2008-01-26  0:22 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20080125161259.0356c990@deepthought>

On 1/26/08, Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> The issue is that iproute is just blindly sending the deletes and
> not asking for acknowledgment status. Here is a trivial patch to iproute
> to fix that, but the problem is that it means it will slow down bulk removal.
>
> Maybe it should just check the first, or last delete to see if there are
> errors?
>
> diff --git a/ip/iproute.c b/ip/iproute.c
> index 7a885b0..b2ae879 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c

This should also be applied into ip/ipaddress.c, ip/ipneigh.c
Or even make one common function?

I don't quite understand how "fast" is good if not complete... But
anyway... I will be happy to see this fix in next version... Maybe add
--fast argument? :)

Alon.

^ permalink raw reply

* [PATCH] hamradio: fix dmascc section mismatch
From: Randy Dunlap @ 2008-01-26  0:35 UTC (permalink / raw)
  To: netdev; +Cc: davem, klaus.kudielka, akpm, samr

From: Randy Dunlap <randy.dunlap@oracle.com>

cc: Klaus Kudielka <klaus.kudielka@gmx.net>

hw[] is used in both init and exit functions so it cannot be initdata
(section mismatch is when CONFIG_MODULES=n and CONFIG_DMASCC=y).

WARNING: vmlinux.o(.exit.text+0xba7): Section mismatch: reference to .init.data: (between 'dmascc_exit' and 'sixpack_exit_driver')

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 drivers/net/hamradio/dmascc.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- linux-2624-bld.orig/drivers/net/hamradio/dmascc.c
+++ linux-2624-bld/drivers/net/hamradio/dmascc.c
@@ -262,8 +262,8 @@ static void tm_isr(struct scc_priv *priv
 
 static int io[MAX_NUM_DEVS] __initdata = { 0, };
 
-/* Beware! hw[] is also used in cleanup_module(). */
-static struct scc_hardware hw[NUM_TYPES] __initdata_or_module = HARDWARE;
+/* Beware! hw[] is also used in dmascc_exit(). */
+static struct scc_hardware hw[NUM_TYPES] = HARDWARE;
 
 
 /* Global variables */

^ permalink raw reply

* [PATCH] [TIPC]: Supress minor sparse warnings.
From: Florian Westphal @ 2008-01-26  2:07 UTC (permalink / raw)
  To: netdev; +Cc: Florian Westphal, Allan Stephens, Jon Paul Maloy

cluster.c:145:2: warning: Using plain integer as NULL pointer
link.c:3254:36: warning: Using plain integer as NULL pointer
ref.c:151:15: warning: Using plain integer as NULL pointer
socket.c:91:13: warning: context imbalance in 'sock_lock' - wrong count at exit
socket.c:99:13: warning: context imbalance in 'sock_unlock' - unexpected unlock

CC: Allan Stephens <allan.stephens@windriver.com>
CC: Jon Paul Maloy <jon.maloy@ericsson.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/tipc/cluster.c |    2 +-
 net/tipc/link.c    |    2 +-
 net/tipc/ref.c     |    2 +-
 net/tipc/socket.c  |    2 ++
 net/tipc/zone.c    |    2 +-
 5 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/tipc/cluster.c b/net/tipc/cluster.c
index 95b3739..4bb3404 100644
--- a/net/tipc/cluster.c
+++ b/net/tipc/cluster.c
@@ -142,7 +142,7 @@ void tipc_cltr_attach_node(struct cluster *c_ptr, struct node *n_ptr)
 		max_n_num = tipc_highest_allowed_slave;
 	assert(n_num > 0);
 	assert(n_num <= max_n_num);
-	assert(c_ptr->nodes[n_num] == 0);
+	assert(c_ptr->nodes[n_num] == NULL);
 	c_ptr->nodes[n_num] = n_ptr;
 	if (n_num > c_ptr->highest_node)
 		c_ptr->highest_node = n_num;
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 1b17fec..cefa998 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -3251,7 +3251,7 @@ static void link_print(struct link *l_ptr, struct print_buf *buf,
 		if ((mod(msg_seqno(buf_msg(l_ptr->last_out)) -
 			 msg_seqno(buf_msg(l_ptr->first_out)))
 		     != (l_ptr->out_queue_size - 1))
-		    || (l_ptr->last_out->next != 0)) {
+		    || (l_ptr->last_out->next != NULL)) {
 			tipc_printf(buf, "\nSend queue inconsistency\n");
 			tipc_printf(buf, "first_out= %x ", l_ptr->first_out);
 			tipc_printf(buf, "next_out= %x ", l_ptr->next_out);
diff --git a/net/tipc/ref.c b/net/tipc/ref.c
index 6704a58..c38744c 100644
--- a/net/tipc/ref.c
+++ b/net/tipc/ref.c
@@ -148,7 +148,7 @@ u32 tipc_ref_acquire(void *object, spinlock_t **lock)
 		reference = (next_plus_upper & ~index_mask) + index;
 		entry->data.reference = reference;
 		entry->object = object;
-		if (lock != 0)
+		if (lock != NULL)
 			*lock = &entry->lock;
 		spin_unlock_bh(&entry->lock);
 	}
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 2290903..26ea0a6 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -89,6 +89,7 @@ static atomic_t tipc_queue_size = ATOMIC_INIT(0);
  * See net.c for description of locking policy.
  */
 static void sock_lock(struct tipc_sock* tsock)
+__acquires(tsock->p->lock)
 {
 	spin_lock_bh(tsock->p->lock);
 }
@@ -97,6 +98,7 @@ static void sock_lock(struct tipc_sock* tsock)
  * sock_unlock(): Unlock a port/socket pair
  */
 static void sock_unlock(struct tipc_sock* tsock)
+__releases(tsock->p->lock)
 {
 	spin_unlock_bh(tsock->p->lock);
 }
diff --git a/net/tipc/zone.c b/net/tipc/zone.c
index 114e173..3506f85 100644
--- a/net/tipc/zone.c
+++ b/net/tipc/zone.c
@@ -82,7 +82,7 @@ void tipc_zone_attach_cluster(struct _zone *z_ptr, struct cluster *c_ptr)
 
 	assert(c_ptr->addr);
 	assert(c_num <= tipc_max_clusters);
-	assert(z_ptr->clusters[c_num] == 0);
+	assert(z_ptr->clusters[c_num] == NULL);
 	z_ptr->clusters[c_num] = c_ptr;
 }
 
-- 
1.5.3.7


^ permalink raw reply related

* [PATCH 2.6.24] fib_trie: apply fixes from fib_hash
From: Julian Anastasov @ 2008-01-26  2:40 UTC (permalink / raw)
  To: David S. Miller; +Cc: Robert Olsson, netdev, Joonwoo Park


	Update fib_trie with some fib_hash fixes:
- check for duplicate alternative routes for prefix+tos+priority when
replacing route
- properly insert by matching tos together with priority
- fix alias walking to use list_for_each_entry_continue for insertion
and deletion when fa_head is not NULL
- copy state from fa to new_fa on replace (not a problem for now)

Signed-off-by: Julian Anastasov <ja@ssi.bg>
---

	Not tested, someone please check the findings

--- linux-2.6.24/net/ipv4/fib_trie.c_orig	2008-01-25 10:45:06.000000000 +0200
+++ linux-2.6.24/net/ipv4/fib_trie.c	2008-01-26 03:54:33.000000000 +0200
@@ -1203,20 +1203,42 @@ static int fn_trie_insert(struct fib_tab
 	 * and we need to allocate a new one of those as well.
 	 */
 
-	if (fa && fa->fa_info->fib_priority == fi->fib_priority) {
-		struct fib_alias *fa_orig;
+	if (fa && fa->fa_tos == tos &&
+	    fa->fa_info->fib_priority == fi->fib_priority) {
+		struct fib_alias *fa_first, *fa_match;
 
 		err = -EEXIST;
 		if (cfg->fc_nlflags & NLM_F_EXCL)
 			goto out;
 
+		/* We have 2 goals:
+		 * 1. Find exact match for type, scope, fib_info to avoid
+		 * duplicate routes
+		 * 2. Find next 'fa' (or head), NLM_F_APPEND inserts before it
+		 */
+		fa_match = NULL;
+		fa_first = fa;
+		fa = list_entry(fa->fa_list.prev, struct fib_alias, fa_list);
+		list_for_each_entry_continue(fa, fa_head, fa_list) {
+			if (fa->fa_tos != tos)
+				break;
+			if (fa->fa_info->fib_priority != fi->fib_priority)
+				break;
+			if (fa->fa_type == cfg->fc_type &&
+			    fa->fa_scope == cfg->fc_scope &&
+			    fa->fa_info == fi) {
+				fa_match = fa;
+				break;
+			}
+		}
+
 		if (cfg->fc_nlflags & NLM_F_REPLACE) {
 			struct fib_info *fi_drop;
 			u8 state;
 
-			if (fi->fib_treeref > 1)
+			fa = fa_first;
+			if (fa_match && fa != fa_match)
 				goto out;
-
 			err = -ENOBUFS;
 			new_fa = kmem_cache_alloc(fn_alias_kmem, GFP_KERNEL);
 			if (new_fa == NULL)
@@ -1228,7 +1250,7 @@ static int fn_trie_insert(struct fib_tab
 			new_fa->fa_type = cfg->fc_type;
 			new_fa->fa_scope = cfg->fc_scope;
 			state = fa->fa_state;
-			new_fa->fa_state &= ~FA_S_ACCESSED;
+			new_fa->fa_state = state & ~FA_S_ACCESSED;
 
 			list_replace_rcu(&fa->fa_list, &new_fa->fa_list);
 			alias_free_mem_rcu(fa);
@@ -1245,20 +1267,11 @@ static int fn_trie_insert(struct fib_tab
 		 * uses the same scope, type, and nexthop
 		 * information.
 		 */
-		fa_orig = fa;
-		list_for_each_entry(fa, fa_orig->fa_list.prev, fa_list) {
-			if (fa->fa_tos != tos)
-				break;
-			if (fa->fa_info->fib_priority != fi->fib_priority)
-				break;
-			if (fa->fa_type == cfg->fc_type &&
-			    fa->fa_scope == cfg->fc_scope &&
-			    fa->fa_info == fi) {
-				goto out;
-			}
-		}
+		if (fa_match)
+			goto out;
+
 		if (!(cfg->fc_nlflags & NLM_F_APPEND))
-			fa = fa_orig;
+			fa = fa_first;
 	}
 	err = -ENOENT;
 	if (!(cfg->fc_nlflags & NLM_F_CREATE))
@@ -1614,9 +1627,8 @@ static int fn_trie_delete(struct fib_tab
 	pr_debug("Deleting %08x/%d tos=%d t=%p\n", key, plen, tos, t);
 
 	fa_to_delete = NULL;
-	fa_head = fa->fa_list.prev;
-
-	list_for_each_entry(fa, fa_head, fa_list) {
+	fa = list_entry(fa->fa_list.prev, struct fib_alias, fa_list);
+	list_for_each_entry_continue(fa, fa_head, fa_list) {
 		struct fib_info *fi = fa->fa_info;
 
 		if (fa->fa_tos != tos)

^ permalink raw reply

* Re: [Bugme-new] [Bug 9816] New: cannot replace route
From: Andrew Morton @ 2008-01-26  3:20 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: netdev, joonwpark81, bugme-daemon, schwab
In-Reply-To: <479A6CBD.4010908@gmail.com>

> On Sat, 26 Jan 2008 00:11:57 +0100 Jarek Poplawski <jarkao2@gmail.com> wrote:
> Andrew Morton wrote, On 01/25/2008 11:26 PM:
> 
> >> On Fri, 25 Jan 2008 13:23:49 -0800 (PST) bugme-daemon@bugzilla.kernel.org wrote:
> >> http://bugzilla.kernel.org/show_bug.cgi?id=9816
> 
> ...
> 
> > I'd agree with Andrea: replacing a route with itself a) used to work and b)
> > should still work (surely)?
> 
> ...on the other hand:
> 
> $ touch file1
> $ cp file1 file1
> cp: `file1' and `file1' are the same file
> $ mv file1 file1
> mv: `file1' and `file1' are the same file
> 
> and: 'everything' in 'linux' is file...
> 
> ergo: route cannot replace with itself!
> 

That's not a very good analogy - the source is a kernel object.  A better
example would be:

linux-2.6.24-rc8:

echo foo > /tmp/1
echo bar > /tmp/2
echo foo > /tmp/1

linux-2.6.24:

echo foo > /tmp/1
echo bar > /tmp/2
echo foo > /tmp/1
sh: cannot write /tmp/1: Inalid argument

But whatever.   It used to work.  People's scripts will break.  Regression.

^ permalink raw reply

* Re: [Bugme-new] [Bug 9816] New: cannot replace route
From: Joonwoo Park @ 2008-01-26  5:16 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Jarek Poplawski, netdev, bugme-daemon, schwab
In-Reply-To: <20080125192026.e667f396.akpm@linux-foundation.org>

2008/1/26, Andrew Morton <akpm@linux-foundation.org>:
>
> But whatever.   It used to work.  People's scripts will break.  Regression.
>

Also I thought that 'replace with itself' should be error as like Jarek.
But it used to work and patch made a regression, it's my bad :(

I think Julian's recent patches on the list would work for this
replacement issue and regression.

Thanks,
Joonwoo

^ permalink raw reply

* Re: [PATCH 2.6.24] fib_trie: apply fixes from fib_hash
From: Joonwoo Park @ 2008-01-26  5:18 UTC (permalink / raw)
  To: Julian Anastasov
  Cc: David S. Miller, Robert Olsson, netdev, Joonwoo Park,
	Andrew Morton, Jarek Poplawski, schwab
In-Reply-To: <Pine.LNX.4.58.0801260427400.12635@u.domain.uli>

On Sat, Jan 26, 2008 at 04:40:30AM +0200, Julian Anastasov wrote:
> 
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
> ---
> 
> 	Not tested, someone please check the findings
> 
> --- linux-2.6.24/net/ipv4/fib_trie.c_orig	2008-01-25 10:45:06.000000000 +0200
> +
>  		if (cfg->fc_nlflags & NLM_F_REPLACE) {
>  			struct fib_info *fi_drop;
>  			u8 state;
>  
> -			if (fi->fib_treeref > 1)
> +			fa = fa_first;
> +			if (fa_match && fa != fa_match)
>  				goto out;
> -

Isn't it possible to do this (fib_hash too)?
			if (fa_match) {
				if (fa != fa_match)
					return 0;
				goto out;
			}

Thanks,
Joonwoo

^ permalink raw reply

* Re: [PATCH 2.6.24] fib_trie: apply fixes from fib_hash
From: Joonwoo Park @ 2008-01-26  5:20 UTC (permalink / raw)
  To: Julian Anastasov
  Cc: David S. Miller, Robert Olsson, netdev, Joonwoo Park,
	Andrew Morton, Jarek Poplawski, schwab
In-Reply-To: <20080126051815.GA17278@ehus.geninetworks.com>

2008/1/26, Joonwoo Park <joonwpark81@gmail.com>:
> On Sat, Jan 26, 2008 at 04:40:30AM +0200, Julian Anastasov wrote:
> >
> > Signed-off-by: Julian Anastasov <ja@ssi.bg>
> > ---
> >
> >       Not tested, someone please check the findings
> >
> > --- linux-2.6.24/net/ipv4/fib_trie.c_orig     2008-01-25 10:45:06.000000000 +0200
> > +
> >               if (cfg->fc_nlflags & NLM_F_REPLACE) {
> >                       struct fib_info *fi_drop;
> >                       u8 state;
> >
> > -                     if (fi->fib_treeref > 1)
> > +                     fa = fa_first;
> > +                     if (fa_match && fa != fa_match)
> >                               goto out;
> > -
>
> Isn't it possible to do this (fib_hash too)?
>                        if (fa_match) {
>                                if (fa != fa_match)
Sorry!
                                  if (fa == fa_match)
>                                        return 0;
>                                goto out;
>                        }
>
> Thanks,
> Joonwoo
>

^ permalink raw reply

* Re: iproute2 (addr flush) infinite loop when unprivileged users
From: Alon Bar-Lev @ 2008-01-26  8:58 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <9e0cf0bf0801251622w203c515fn80cba5f233bd599d@mail.gmail.com>

On 1/26/08, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
> On 1/26/08, Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> > The issue is that iproute is just blindly sending the deletes and
> > not asking for acknowledgment status. Here is a trivial patch to iproute
> > to fix that, but the problem is that it means it will slow down bulk removal.
> >
> > Maybe it should just check the first, or last delete to see if there are
> > errors?
> >
> > diff --git a/ip/iproute.c b/ip/iproute.c
> > index 7a885b0..b2ae879 100644
> > --- a/ip/iproute.c
> > +++ b/ip/iproute.c
>
> This should also be applied into ip/ipaddress.c, ip/ipneigh.c
> Or even make one common function?
>
> I don't quite understand how "fast" is good if not complete... But
> anyway... I will be happy to see this fix in next version... Maybe add
> --fast argument? :)
>
> Alon.
>

Pulled your latest git head. See some updates but not this one...
Just thought to report the following [new] build error:


gcc -Wl,-export-dynamic  ip.o ipaddress.o iproute.o iprule.o rtm_map.o
iptunnel.o ip6tunnel.o tunnel.o ipneigh.o ipntable.o iplink.o
ipmaddr.o ipmonitor.o ipmroute.o ipprefix.o ipxfrm.o xfrm_state.o
xfrm_policy.o xfrm_monitor.o iplink_vlan.o link_veth.o
../lib/libnetlink.a ../lib/libutil.a  -lresolv -L../lib -lnetlink
-lutil -ldl -o ip
iptunnel.o: In function `parse_args':
iptunnel.c:(.text+0x2f6): undefined reference to `__constant_htons'
iptunnel.c:(.text+0x434): undefined reference to `__constant_htons'
iptunnel.c:(.text+0x4c1): undefined reference to `__constant_htons'
iptunnel.c:(.text+0x4da): undefined reference to `__constant_htons'
iptunnel.c:(.text+0x51c): undefined reference to `__constant_htons'
iptunnel.o:iptunnel.c:(.text+0x535): more undefined references to
`__constant_htons' follow
collect2: ld returned 1 exit status
make[1]: *** [ip] Error 1

^ permalink raw reply

* Re: [PATCH 2.6.24] fib_trie: apply fixes from fib_hash
From: Julian Anastasov @ 2008-01-26  9:46 UTC (permalink / raw)
  To: Joonwoo Park
  Cc: David S. Miller, Robert Olsson, netdev, Andrew Morton,
	Jarek Poplawski, schwab
In-Reply-To: <b25c3fa70801252120v1b928b15pd5a4e7bd2934161b@mail.gmail.com>


	Hello,

On Sat, 26 Jan 2008, Joonwoo Park wrote:

> > >       Not tested, someone please check the findings

	news: simple testing of patched fib_trie seems to work

> > > -                     if (fi->fib_treeref > 1)
> > > +                     fa = fa_first;
> > > +                     if (fa_match && fa != fa_match)
> > >                               goto out;
> > > -
> >
> > Isn't it possible to do this (fib_hash too)?
> >                        if (fa_match) {
> >                                if (fa != fa_match)
> Sorry!
>                                   if (fa == fa_match)
> >                                        return 0;
> >                                goto out;
> >                        }

	I see, your idea is to optimize the case when matched
parameters are same. Considering the used fi is same I don't see
any problems. I'll prepare 2nd version of both patches in
next hours.

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* Re: [Bugme-new] [Bug 9816] New: cannot replace route
From: Jarek Poplawski @ 2008-01-26 11:40 UTC (permalink / raw)
  To: Joonwoo Park; +Cc: Andrew Morton, netdev, bugme-daemon, schwab
In-Reply-To: <b25c3fa70801252116h2b9f12c9j65a8a75dbd0d5662@mail.gmail.com>

On Sat, Jan 26, 2008 at 02:16:01PM +0900, Joonwoo Park wrote:
> 2008/1/26, Andrew Morton <akpm@linux-foundation.org>:
> >
> > But whatever.   It used to work.  People's scripts will break.  Regression.
> >
> 
> Also I thought that 'replace with itself' should be error as like Jarek.
> But it used to work and patch made a regression, it's my bad :(

Actually, I don't think 'replace with itself' should be an error. I've
only meant that lack of this possibility shouldn't be necessarily seen
as error - there could be arguments for both sides.

IMHO, there should be simply analyzed pros and cons of doing it in
this particular place, so: if there is any gain in doing this, and if
possible complications or problems with performance, security etc.
don't prevail such a gain.

And I don't think a regression argument should be valid at all if
there are removed any unlogical, error-prone or otherwise problematic
options (I don't know if this is such case), even if they are not
obvious bugs - especially if it's still possible to do the same
'proper' way.

Regards,
Jarek P.

^ permalink raw reply

* Re: [Bugme-new] [Bug 9816] New: cannot replace route
From: Jarek Poplawski @ 2008-01-26 12:10 UTC (permalink / raw)
  To: Andrew Morton; +Cc: netdev, joonwpark81, bugme-daemon, schwab
In-Reply-To: <20080125192026.e667f396.akpm@linux-foundation.org>

On Fri, Jan 25, 2008 at 07:20:26PM -0800, Andrew Morton wrote:
...
> That's not a very good analogy - the source is a kernel object.  A better
> example would be:
> 
> linux-2.6.24-rc8:
> 
> echo foo > /tmp/1
> echo bar > /tmp/2
> echo foo > /tmp/1
> 
> linux-2.6.24:
> 
> echo foo > /tmp/1
> echo bar > /tmp/2
> echo foo > /tmp/1
> sh: cannot write /tmp/1: Inalid argument

Probably you are right. When I have some time to build and boot 2.6.24
at last, I hope I'll find what this example is about...

> But whatever.   It used to work.  People's scripts will break.  Regression.

Do you mean people didn't have to change their scripts before, e.g.
from linux 2.0.? Scripts are kind of programs, and both APIs and some
'undocumented' features are achanging...

Regards,
Jarek P.

Regards,
Jarek P.

^ permalink raw reply

* Re: [PATCH net-2.6.25 4/7][ATM]: [br2864] routed support
From: chas williams - CONTRACTOR @ 2008-01-26 12:20 UTC (permalink / raw)
  To: Chung-Chi Lo; +Cc: netdev, davem
In-Reply-To: <3f696b20801152016r52048016g7c293dbfdc6c9f29@mail.gmail.com>

In message <3f696b20801152016r52048016g7c293dbfdc6c9f29@mail.gmail.com>,"Chung-
Chi Lo" writes:
>Question to decode the encaps=VCMUX and bridge mode:
>
>This line
>skb_pull((skb, BR2684_PAD_LEN + ETH_HLEN);
>should be replaced by
>skb_pull((skb, BR2684_PAD_LEN);

you are correct.

>By the way, this routed mode patch doesn't include encaps=VCMUX and
>RFC2684 routed
>protocol decapsulation?

yep.  eric believes the following should fix both problems:

commit 43e4b025ffe130cd6a292fa9ff909e39a88f849c
Author: Chas Williams - CONTRACTOR <chas@relax.cmf.nrl.navy.mil>
Date:   Sat Jan 26 07:18:26 2008 -0500

    [ATM]: [br2864] fix vcmux support
    
    From: Eric Kinzie <ekinzie@cmf.nrl.navy.mil>
    Signed-off-by: Chas Williams <chas@cmf.nrl.navy.mil>

diff --git a/net/atm/br2684.c b/net/atm/br2684.c
index 31347cb..97b422c 100644
--- a/net/atm/br2684.c
+++ b/net/atm/br2684.c
@@ -400,15 +400,20 @@ static void br2684_push(struct atm_vcc *atmvcc, struct sk_buff *skb)
 			return;
 		}
 
-	} else {
-		/* first 2 chars should be 0 */
-		if (*((u16 *) (skb->data)) != 0) {
-			brdev->stats.rx_errors++;
-			dev_kfree_skb(skb);
-			return;
+	} else { /* vc-mux */
+		if (brdev->payload == p_routed) {
+			skb_reset_network_header(skb);
+			skb->pkt_type = PACKET_HOST;
+		} else { /* p_bridged */
+			/* first 2 chars should be 0 */
+			if (*((u16 *) (skb->data)) != 0) {
+				brdev->stats.rx_errors++;
+				dev_kfree_skb(skb);
+				return;
+			}
+			skb_pull(skb, BR2684_PAD_LEN);
+			skb->protocol = eth_type_trans(skb, net_dev);
 		}
-		skb_pull(skb, BR2684_PAD_LEN + ETH_HLEN);	/* pad, dstmac, srcmac, ethtype */
-		skb->protocol = eth_type_trans(skb, net_dev);
 	}
 
 #ifdef CONFIG_ATM_BR2684_IPFILTER

^ permalink raw reply related

* Re: [PATCH 00/14][v3]: Driver for Wireless RNDIS USB devices.
From: Jussi Kivilinna @ 2008-01-26 12:21 UTC (permalink / raw)
  To: David Brownell
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, bjd-a1rhEgazXTw
In-Reply-To: <200801251509.24377.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>

On Fri, 2008-01-25 at 15:09 -0800, David Brownell wrote:
> If they're the same, my ack (as maintainer for that infrastructure
> and, for now, rndis_host) still stands.  I won't look at them again.
> 

Ah, ok.

^ permalink raw reply

* [PATCHv2 2.6.24] fib: fix route replacement, fib_info is shared
From: Julian Anastasov @ 2008-01-26 12:41 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Joonwoo Park


	fib_info can be shared by many route prefixes but we don't
want duplicate alternative routes for a prefix+tos+priority. Last
change was not correct to check fib_treeref because it accounts usage
from other prefixes. Additionally, avoid replacement without error
if new route is same, as Joonwoo Park suggests.

Signed-off-by: Julian Anastasov <ja@ssi.bg>
---

--- linux-2.6.24/net/ipv4/fib_hash.c_orig	2008-01-25 10:45:06.000000000 +0200
+++ linux-2.6.24/net/ipv4/fib_hash.c	2008-01-26 14:11:34.000000000 +0200
@@ -434,19 +434,43 @@ static int fn_hash_insert(struct fib_tab
 
 	if (fa && fa->fa_tos == tos &&
 	    fa->fa_info->fib_priority == fi->fib_priority) {
-		struct fib_alias *fa_orig;
+		struct fib_alias *fa_first, *fa_match;
 
 		err = -EEXIST;
 		if (cfg->fc_nlflags & NLM_F_EXCL)
 			goto out;
 
+		/* We have 2 goals:
+		 * 1. Find exact match for type, scope, fib_info to avoid
+		 * duplicate routes
+		 * 2. Find next 'fa' (or head), NLM_F_APPEND inserts before it
+		 */
+		fa_match = NULL;
+		fa_first = fa;
+		fa = list_entry(fa->fa_list.prev, struct fib_alias, fa_list);
+		list_for_each_entry_continue(fa, &f->fn_alias, fa_list) {
+			if (fa->fa_tos != tos)
+				break;
+			if (fa->fa_info->fib_priority != fi->fib_priority)
+				break;
+			if (fa->fa_type == cfg->fc_type &&
+			    fa->fa_scope == cfg->fc_scope &&
+			    fa->fa_info == fi) {
+				fa_match = fa;
+				break;
+			}
+		}
+
 		if (cfg->fc_nlflags & NLM_F_REPLACE) {
 			struct fib_info *fi_drop;
 			u8 state;
 
-			if (fi->fib_treeref > 1)
+			fa = fa_first;
+			if (fa_match) {
+				if (fa == fa_match)
+					err = 0;
 				goto out;
-
+			}
 			write_lock_bh(&fib_hash_lock);
 			fi_drop = fa->fa_info;
 			fa->fa_info = fi;
@@ -469,20 +493,11 @@ static int fn_hash_insert(struct fib_tab
 		 * uses the same scope, type, and nexthop
 		 * information.
 		 */
-		fa_orig = fa;
-		fa = list_entry(fa->fa_list.prev, struct fib_alias, fa_list);
-		list_for_each_entry_continue(fa, &f->fn_alias, fa_list) {
-			if (fa->fa_tos != tos)
-				break;
-			if (fa->fa_info->fib_priority != fi->fib_priority)
-				break;
-			if (fa->fa_type == cfg->fc_type &&
-			    fa->fa_scope == cfg->fc_scope &&
-			    fa->fa_info == fi)
-				goto out;
-		}
+		if (fa_match)
+			goto out;
+
 		if (!(cfg->fc_nlflags & NLM_F_APPEND))
-			fa = fa_orig;
+			fa = fa_first;
 	}
 
 	err = -ENOENT;

^ permalink raw reply

* [PATCHv2 2.6.24] fib_trie: apply fixes from fib_hash
From: Julian Anastasov @ 2008-01-26 12:44 UTC (permalink / raw)
  To: David S. Miller; +Cc: Robert Olsson, netdev, Joonwoo Park


	Update fib_trie with some fib_hash fixes:
- check for duplicate alternative routes for prefix+tos+priority when
replacing route
- properly insert by matching tos together with priority
- fix alias walking to use list_for_each_entry_continue for insertion
and deletion when fa_head is not NULL
- copy state from fa to new_fa on replace (not a problem for now)
- additionally, avoid replacement without error if new route is same,
as Joonwoo Park suggests.

Signed-off-by: Julian Anastasov <ja@ssi.bg>
---

--- linux-2.6.24/net/ipv4/fib_trie.c_orig	2008-01-25 10:45:06.000000000 +0200
+++ linux-2.6.24/net/ipv4/fib_trie.c	2008-01-26 14:11:27.000000000 +0200
@@ -1203,20 +1203,45 @@ static int fn_trie_insert(struct fib_tab
 	 * and we need to allocate a new one of those as well.
 	 */
 
-	if (fa && fa->fa_info->fib_priority == fi->fib_priority) {
-		struct fib_alias *fa_orig;
+	if (fa && fa->fa_tos == tos &&
+	    fa->fa_info->fib_priority == fi->fib_priority) {
+		struct fib_alias *fa_first, *fa_match;
 
 		err = -EEXIST;
 		if (cfg->fc_nlflags & NLM_F_EXCL)
 			goto out;
 
+		/* We have 2 goals:
+		 * 1. Find exact match for type, scope, fib_info to avoid
+		 * duplicate routes
+		 * 2. Find next 'fa' (or head), NLM_F_APPEND inserts before it
+		 */
+		fa_match = NULL;
+		fa_first = fa;
+		fa = list_entry(fa->fa_list.prev, struct fib_alias, fa_list);
+		list_for_each_entry_continue(fa, fa_head, fa_list) {
+			if (fa->fa_tos != tos)
+				break;
+			if (fa->fa_info->fib_priority != fi->fib_priority)
+				break;
+			if (fa->fa_type == cfg->fc_type &&
+			    fa->fa_scope == cfg->fc_scope &&
+			    fa->fa_info == fi) {
+				fa_match = fa;
+				break;
+			}
+		}
+
 		if (cfg->fc_nlflags & NLM_F_REPLACE) {
 			struct fib_info *fi_drop;
 			u8 state;
 
-			if (fi->fib_treeref > 1)
+			fa = fa_first;
+			if (fa_match) {
+				if (fa == fa_match)
+					err = 0;
 				goto out;
-
+			}
 			err = -ENOBUFS;
 			new_fa = kmem_cache_alloc(fn_alias_kmem, GFP_KERNEL);
 			if (new_fa == NULL)
@@ -1228,7 +1253,7 @@ static int fn_trie_insert(struct fib_tab
 			new_fa->fa_type = cfg->fc_type;
 			new_fa->fa_scope = cfg->fc_scope;
 			state = fa->fa_state;
-			new_fa->fa_state &= ~FA_S_ACCESSED;
+			new_fa->fa_state = state & ~FA_S_ACCESSED;
 
 			list_replace_rcu(&fa->fa_list, &new_fa->fa_list);
 			alias_free_mem_rcu(fa);
@@ -1245,20 +1270,11 @@ static int fn_trie_insert(struct fib_tab
 		 * uses the same scope, type, and nexthop
 		 * information.
 		 */
-		fa_orig = fa;
-		list_for_each_entry(fa, fa_orig->fa_list.prev, fa_list) {
-			if (fa->fa_tos != tos)
-				break;
-			if (fa->fa_info->fib_priority != fi->fib_priority)
-				break;
-			if (fa->fa_type == cfg->fc_type &&
-			    fa->fa_scope == cfg->fc_scope &&
-			    fa->fa_info == fi) {
-				goto out;
-			}
-		}
+		if (fa_match)
+			goto out;
+
 		if (!(cfg->fc_nlflags & NLM_F_APPEND))
-			fa = fa_orig;
+			fa = fa_first;
 	}
 	err = -ENOENT;
 	if (!(cfg->fc_nlflags & NLM_F_CREATE))
@@ -1614,9 +1630,8 @@ static int fn_trie_delete(struct fib_tab
 	pr_debug("Deleting %08x/%d tos=%d t=%p\n", key, plen, tos, t);
 
 	fa_to_delete = NULL;
-	fa_head = fa->fa_list.prev;
-
-	list_for_each_entry(fa, fa_head, fa_list) {
+	fa = list_entry(fa->fa_list.prev, struct fib_alias, fa_list);
+	list_for_each_entry_continue(fa, fa_head, fa_list) {
 		struct fib_info *fi = fa->fa_info;
 
 		if (fa->fa_tos != tos)

^ permalink raw reply

* Re: [Bugme-new] [Bug 9816] New: cannot replace route
From: Jarek Poplawski @ 2008-01-26 14:10 UTC (permalink / raw)
  To: Joonwoo Park; +Cc: Andrew Morton, netdev, bugme-daemon, schwab
In-Reply-To: <20080126114036.GA2624@ami.dom.local>

On Sat, Jan 26, 2008 at 12:40:36PM +0100, Jarek Poplawski wrote:
> On Sat, Jan 26, 2008 at 02:16:01PM +0900, Joonwoo Park wrote:
> > 2008/1/26, Andrew Morton <akpm@linux-foundation.org>:
> > >
> > > But whatever.   It used to work.  People's scripts will break.  Regression.
> > >
> > 
> > Also I thought that 'replace with itself' should be error as like Jarek.
> > But it used to work and patch made a regression, it's my bad :(
> 
> Actually, I don't think 'replace with itself' should be an error. I've
> only meant that lack of this possibility shouldn't be necessarily seen
> as error - there could be arguments for both sides.

...On the other hand, after some re-thinking, I actually think 'replace
with itself' should be considered a bug. I wondered about the possible
reason of this behaviour in a file system, and it seems replace just
means things like overwrite, so old thing is always supposed to be
destroyed (of course it's a matter of implementation or conditions in
which moment this destruction takes place).

So, 'replace with itself' is simply ambiguous: we can always delete the
object first, to prepare the place for replacement, and find there is
nothing to do after this - and it's probably not what somebody wanted.
And, after re-reading this bugzilla report, I'm pretty sure the thing
should be done with 'ip route change' (but I didn't check if 2.6.24
knows about this...).

Jarek P.

^ permalink raw reply

* Re: [Bugme-new] [Bug 9816] New: cannot replace route
From: Andreas Schwab @ 2008-01-26 14:27 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: Joonwoo Park, Andrew Morton, netdev, bugme-daemon
In-Reply-To: <20080126141010.GC2624@ami.dom.local>

Jarek Poplawski <jarkao2@gmail.com> writes:

> And, after re-reading this bugzilla report, I'm pretty sure the thing
> should be done with 'ip route change' (but I didn't check if 2.6.24
> knows about this...).

$ man ip
[...]
   ip route add - add new route
   ip route change - change route
   ip route replace - change or add new one
[...]

According to this "replace" should be a superset of "change".

Also, please check out comment#3, it also fails for replacing a route
with something different (it's a route to an ipsec tunnel).

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: [Bugme-new] [Bug 9816] New: cannot replace route
From: Jarek Poplawski @ 2008-01-26 14:32 UTC (permalink / raw)
  To: Joonwoo Park; +Cc: Andrew Morton, netdev, bugme-daemon, schwab
In-Reply-To: <20080126141010.GC2624@ami.dom.local>

On Sat, Jan 26, 2008 at 03:10:10PM +0100, Jarek Poplawski wrote:
...
> [...] so old thing is always supposed to be
> destroyed (of course it's a matter of implementation or conditions in
> which moment this destruction takes place).
> 
> So, 'replace with itself' is simply ambiguous: we can always delete the
> object first, to prepare the place for replacement, and find there is
> nothing to do after this - and it's probably not what somebody wanted.

As a matter of fact, the moment of destruction doesn't even matter:
assuming the replaced thing is destroyed in all 'common' cases, doing
this at the end isn't probably wanted as well - and skipping this
action makes an exception - what IMHO proves the concept is at least
inconsistent.

Jarek P.

^ permalink raw reply

* Re: [Bugme-new] [Bug 9816] New: cannot replace route
From: Jarek Poplawski @ 2008-01-26 15:19 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Joonwoo Park, Andrew Morton, netdev, bugme-daemon
In-Reply-To: <jeve5gd5mz.fsf@sykes.suse.de>

On Sat, Jan 26, 2008 at 03:27:00PM +0100, Andreas Schwab wrote:
> Jarek Poplawski <jarkao2@gmail.com> writes:
> 
> > And, after re-reading this bugzilla report, I'm pretty sure the thing
> > should be done with 'ip route change' (but I didn't check if 2.6.24
> > knows about this...).
> 
> $ man ip
> [...]
>    ip route add - add new route
>    ip route change - change route
>    ip route replace - change or add new one
> [...]
> 
> According to this "replace" should be a superset of "change".

According to this "replace" should be ...ambiguous. I could read this
"my/proper(?) way":

    ip route replace - change with new one or add new one

And ...man could be wrong too after all! (...but not me!)

> Also, please check out comment#3, it also fails for replacing a route
> with something different (it's a route to an ipsec tunnel).

It all depends on which routes should be considered different (and it
should be specified somewhere BTW...).

But, I should've added my all reasoning was more about logic, and since
in real life change and replace are often equivalent, and iproute is
famous from using such equivalents in many places, your claim WRT this
man page could be completely right!

There should be only considered, if realization of this doesn't imply
bugs in some other places, like the one which fix caused this
"regression".

Regards,
Jarek P.

^ permalink raw reply

* Re: [PATCH]  FIXED_MII_100_FDX
From: Kumar Gala @ 2008-01-26 15:40 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev list, netdev, Jeff Garzik
In-Reply-To: <479AD93D.30202@pikatech.com>


On Jan 26, 2008, at 12:54 AM, Sean MacLennan wrote:

> On the last git pull,  FIXED_MII_100_FDX was removed from the phy
> Kconfig, but the Kconfig for CPMAC still tried to select it.
>
> Cheers,
>   Sean
>
> Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
> ---
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 9af05a2..297a4b5 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -1710,7 +1710,6 @@ config CPMAC
> 	depends on NET_ETHERNET && EXPERIMENTAL && AR7
> 	select PHYLIB
> 	select FIXED_PHY
> -	select FIXED_MII_100_FDX
> 	help
> 	  TI AR7 CPMAC Ethernet support

This patch isn't sufficient.  AntonV has posted a proper fix that we  
need Jeff to pick up.

http://ozlabs.org/pipermail/linuxppc-dev/2008-January/050330.html

- k

^ permalink raw reply

* [PATCH] SCTP: Fix kernel panic while received retransmitted ASCONF chunk 3 times
From: Wei Yongjun @ 2008-01-26 16:09 UTC (permalink / raw)
  To: lksctp-developers, netdev; +Cc: Vlad Yasevich

While endpoint recived the ASCONF chunk with the same serial number for 
3 times, the endpoint will panic.

Test as following:

  Endpoint A                   Endpoint B
(ESTABLISHED)                 (ESTABLISHED)
     ASCONF      ---------->
     (serial = 1) 
                 <----------   ASCONF-ACK
     ASCONF      ---------->
     (serial = 1) 
                 <----------   ASCONF-ACK
     ASCONF      ---------->
     (serial = 1) 
                               kernel panic

This is besause if endpoint received the first ASCONF chunk, ASCONF-ACK 
chunk will be send to reponse this ASCONF chunk, and the ASCONF-ACK will 
be cached as asoc->addip_last_asconf_ack. After this, if ASCONF chunk 
with the same serial number received will be treat as retransmitted 
ASCONF chunk and just responsed with the cached 
asoc->addip_last_asconf_ack. But before we use this cached chunk, we not 
increase the user count of this chunk with sctp_chunk_hold() ,  after 
send ASCONF-ACK, sctp_chunk_free() will be used to free 
asoc->addip_last_asconf_ack. So when the third ASCONF chunk with the 
same serial number is received, it responsed with the cached 
asoc->addip_last_asconf_ack too, but asoc->addip_last_asconf_ack has 
been freed, So kernel panic is occurred.

Folowing is the kernel panic message. And this patch fix this problem.

kernel BUG at net/sctp/outqueue.c:789!
invalid opcode: 0000 [#1] SMP
Modules linked in: md5 sctp ipv6 dm_mirror dm_mod sbs sbshc battery lp snd_ens1371

Pid: 0, comm: swapper Not tainted (2.6.24 #1)
EIP: 0060:[<c8a8f9ba>] EFLAGS: 00010216 CPU: 0
EIP is at sctp_outq_flush+0x16b/0x5d3 [sctp]
EAX: c735f43c EBX: c8a9dc64 ECX: 00000046 EDX: 00000000
ESI: c7aa3b00 EDI: c794ea00 EBP: c7a953d0 ESP: c0757cf4
 DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
Process swapper (pid: 0, ti=c0757000 task=c06d63a0 task.ti=c070f000)
Stack: c1107d80 c0441e6f c7aa3b00 00000064 00000000 c7a953bc c794eae0 c7a94000
       034d0be8 00000001 00000000 00000000 00000001 00000000 c043bc66 c7aa3b00
       c7a953bc c7a94000 c8a9c760 c042bfc6 c8a9f18d c7aa3b00 c7a953bc c8a90259
Call Trace:
 [<c0441e6f>] tick_handle_periodic+0x17/0x5c
 [<c043bc66>] autoremove_wake_function+0x15/0x35
 [<c042bfc6>] printk+0x1b/0x1f
 [<c8a90259>] sctp_outq_tail+0x128/0x172 [sctp]
 [<c8a87631>] sctp_do_sm+0xeb0/0x103f [sctp]
 [<c8a8a639>] sctp_assoc_bh_rcv+0xc1/0xf4 [sctp]
 [<c8a8eb77>] sctp_inq_push+0x2a/0x2d [sctp]
 [<c8a9924b>] sctp_rcv+0x5c3/0x6a4 [sctp]
 [<c0425247>] try_to_wake_up+0x3bb/0x3c5
 [<c0421f8a>] __update_rq_clock+0x19/0x156
 [<c04409e1>] clocksource_get_next+0x39/0x3f
 [<c0421f8a>] __update_rq_clock+0x19/0x156
 [<c05dd9ba>] ip_local_deliver_finish+0xda/0x17d
 [<c05dd8c1>] ip_rcv_finish+0x2c5/0x2e4
 [<c0441e6f>] tick_handle_periodic+0x17/0x5c
 [<c041ae2a>] smp_apic_timer_interrupt+0x74/0x80
 [<c05ddb19>] ip_rcv+0x0/0x237
 [<c05c15ed>] netif_receive_skb+0x328/0x392
 [<c05c39c0>] process_backlog+0x5c/0x9a
 [<c05c34ce>] net_rx_action+0x8d/0x163
 [<c0432dbb>] run_timer_softirq+0x2f/0x156
 [<c042fdd7>] __do_softirq+0x5d/0xc1
 [<c0406f38>] do_softirq+0x59/0xa8
 [<c0441e6f>] tick_handle_periodic+0x17/0x5c
 [<c041ae2a>] smp_apic_timer_interrupt+0x74/0x80
 [<c0403c87>] default_idle+0x0/0x3e
 [<c0403c87>] default_idle+0x0/0x3e
 [<c04058c0>] apic_timer_interrupt+0x28/0x30
 [<c0403c87>] default_idle+0x0/0x3e
 [<c0403cb3>] default_idle+0x2c/0x3e
 [<c0403571>] cpu_idle+0x92/0xab
 [<c07148ea>] start_kernel+0x2f7/0x2ff
 [<c07140e0>] unknown_bootoption+0x0/0x195
 =======================
Code: 00 89 f2 89 d8 e8 b3 88 00 00 89 d8 e8 1e 83 00 00 85 c0 89 44 24 2c 
EIP: [<c8a8f9ba>] sctp_outq_flush+0x16b/0x5d3 [sctp] SS:ESP 0068:c0757cf4
Kernel panic - not syncing: Fatal exception in interrupt

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>

--- a/net/sctp/sm_statefuns.c	2008-01-25 00:50:27.000000000 -0500
+++ b/net/sctp/sm_statefuns.c	2008-01-25 03:03:15.000000000 -0500
@@ -3420,9 +3420,10 @@ sctp_disposition_t sctp_sf_do_asconf(con
 		 * time and instead of re-processing the ASCONF (with the same
 		 * serial number) it may just re-transmit the ASCONF-ACK.
 		 */
-		if (asoc->addip_last_asconf_ack)
+		if (asoc->addip_last_asconf_ack) {
 			asconf_ack = asoc->addip_last_asconf_ack;
-		else
+			sctp_chunk_hold(asconf_ack);
+		} else
 			return SCTP_DISPOSITION_DISCARD;
 	} else {
 		/* ADDIP 4.2 C4) Otherwise, the ASCONF Chunk is discarded since




^ permalink raw reply

* Re: iproute2 (addr flush) infinite loop when unprivileged users
From: Stephen Hemminger @ 2008-01-26 19:11 UTC (permalink / raw)
  To: Alon Bar-Lev; +Cc: netdev
In-Reply-To: <9e0cf0bf0801260058s797d2ea3r7af013d0957554d8@mail.gmail.com>

On Sat, 26 Jan 2008 10:58:58 +0200
"Alon Bar-Lev" <alon.barlev@gmail.com> wrote:

> On 1/26/08, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
> > On 1/26/08, Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> > > The issue is that iproute is just blindly sending the deletes and
> > > not asking for acknowledgment status. Here is a trivial patch to iproute
> > > to fix that, but the problem is that it means it will slow down bulk removal.
> > >
> > > Maybe it should just check the first, or last delete to see if there are
> > > errors?
> > >
> > > diff --git a/ip/iproute.c b/ip/iproute.c
> > > index 7a885b0..b2ae879 100644
> > > --- a/ip/iproute.c
> > > +++ b/ip/iproute.c
> >
> > This should also be applied into ip/ipaddress.c, ip/ipneigh.c
> > Or even make one common function?
> >
> > I don't quite understand how "fast" is good if not complete... But
> > anyway... I will be happy to see this fix in next version... Maybe add
> > --fast argument? :)
> >
> > Alon.
> >


I found the correct solution. The kernel will send back an error after a bad
netlink message, even without ACK. so the code now checks.  Should be checked in
and pushed for next version.


-- 
Stephen Hemminger <stephen.hemminger@vyatta.com>

^ permalink raw reply

* Re: iproute2 (addr flush) infinite loop when unprivileged users
From: Stephen Hemminger @ 2008-01-26 19:14 UTC (permalink / raw)
  To: Alon Bar-Lev; +Cc: netdev
In-Reply-To: <9e0cf0bf0801260058s797d2ea3r7af013d0957554d8@mail.gmail.com>

On Sat, 26 Jan 2008 10:58:58 +0200
"Alon Bar-Lev" <alon.barlev@gmail.com> wrote:

> On 1/26/08, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
> > On 1/26/08, Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> > > The issue is that iproute is just blindly sending the deletes and
> > > not asking for acknowledgment status. Here is a trivial patch to iproute
> > > to fix that, but the problem is that it means it will slow down bulk removal.
> > >
> > > Maybe it should just check the first, or last delete to see if there are
> > > errors?
> > >
> > > diff --git a/ip/iproute.c b/ip/iproute.c
> > > index 7a885b0..b2ae879 100644
> > > --- a/ip/iproute.c
> > > +++ b/ip/iproute.c
> >
> > This should also be applied into ip/ipaddress.c, ip/ipneigh.c
> > Or even make one common function?
> >
> > I don't quite understand how "fast" is good if not complete... But
> > anyway... I will be happy to see this fix in next version... Maybe add
> > --fast argument? :)
> >
> > Alon.
> >
> 
> Pulled your latest git head. See some updates but not this one...
> Just thought to report the following [new] build error:
> 
> 
> gcc -Wl,-export-dynamic  ip.o ipaddress.o iproute.o iprule.o rtm_map.o
> iptunnel.o ip6tunnel.o tunnel.o ipneigh.o ipntable.o iplink.o
> ipmaddr.o ipmonitor.o ipmroute.o ipprefix.o ipxfrm.o xfrm_state.o
> xfrm_policy.o xfrm_monitor.o iplink_vlan.o link_veth.o
> ../lib/libnetlink.a ../lib/libutil.a  -lresolv -L../lib -lnetlink
> -lutil -ldl -o ip
> iptunnel.o: In function `parse_args':
> iptunnel.c:(.text+0x2f6): undefined reference to `__constant_htons'
> iptunnel.c:(.text+0x434): undefined reference to `__constant_htons'
> iptunnel.c:(.text+0x4c1): undefined reference to `__constant_htons'
> iptunnel.c:(.text+0x4da): undefined reference to `__constant_htons'
> iptunnel.c:(.text+0x51c): undefined reference to `__constant_htons'
> iptunnel.o:iptunnel.c:(.text+0x535): more undefined references to
> `__constant_htons' follow
> collect2: ld returned 1 exit status
> make[1]: *** [ip] Error 1

What system are you building on.
This is defined in linux/byteorder.h

-- 
Stephen Hemminger <stephen.hemminger@vyatta.com>

^ permalink raw reply

* Re: iproute2 (addr flush) infinite loop when unprivileged users
From: Stephen Hemminger @ 2008-01-26 19:19 UTC (permalink / raw)
  To: Alon Bar-Lev; +Cc: netdev
In-Reply-To: <9e0cf0bf0801260058s797d2ea3r7af013d0957554d8@mail.gmail.com>

On Sat, 26 Jan 2008 10:58:58 +0200
"Alon Bar-Lev" <alon.barlev@gmail.com> wrote:

> On 1/26/08, Alon Bar-Lev <alon.barlev@gmail.com> wrote:
> > On 1/26/08, Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> > > The issue is that iproute is just blindly sending the deletes and
> > > not asking for acknowledgment status. Here is a trivial patch to iproute
> > > to fix that, but the problem is that it means it will slow down bulk removal.
> > >
> > > Maybe it should just check the first, or last delete to see if there are
> > > errors?
> > >
> > > diff --git a/ip/iproute.c b/ip/iproute.c
> > > index 7a885b0..b2ae879 100644
> > > --- a/ip/iproute.c
> > > +++ b/ip/iproute.c
> >
> > This should also be applied into ip/ipaddress.c, ip/ipneigh.c
> > Or even make one common function?
> >
> > I don't quite understand how "fast" is good if not complete... But
> > anyway... I will be happy to see this fix in next version... Maybe add
> > --fast argument? :)
> >
> > Alon.
> >
> 
> Pulled your latest git head. See some updates but not this one...
> Just thought to report the following [new] build error:
> 
> 
> gcc -Wl,-export-dynamic  ip.o ipaddress.o iproute.o iprule.o rtm_map.o
> iptunnel.o ip6tunnel.o tunnel.o ipneigh.o ipntable.o iplink.o
> ipmaddr.o ipmonitor.o ipmroute.o ipprefix.o ipxfrm.o xfrm_state.o
> xfrm_policy.o xfrm_monitor.o iplink_vlan.o link_veth.o
> ../lib/libnetlink.a ../lib/libutil.a  -lresolv -L../lib -lnetlink
> -lutil -ldl -o ip
> iptunnel.o: In function `parse_args':
> iptunnel.c:(.text+0x2f6): undefined reference to `__constant_htons'
> iptunnel.c:(.text+0x434): undefined reference to `__constant_htons'
> iptunnel.c:(.text+0x4c1): undefined reference to `__constant_htons'
> iptunnel.c:(.text+0x4da): undefined reference to `__constant_htons'
> iptunnel.c:(.text+0x51c): undefined reference to `__constant_htons'
> iptunnel.o:iptunnel.c:(.text+0x535): more undefined references to
> `__constant_htons' follow
> collect2: ld returned 1 exit status
> make[1]: *** [ip] Error 1

This is defined via:
  /usr/include/linux/ip.h
  #include <asm/byteorder.h>

-- 
Stephen Hemminger <stephen.hemminger@vyatta.com>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox