Netdev List
 help / color / mirror / Atom feed
* Re: Fix NULL dereference in x25_recvmsg
From: Eric Dumazet @ 2011-11-02  2:16 UTC (permalink / raw)
  To: Dave Jones; +Cc: netdev, Matthew Daley
In-Reply-To: <20111102015315.GA6569@redhat.com>

Le mardi 01 novembre 2011 à 21:53 -0400, Dave Jones a écrit :
> commit cb101ed2 in 3.0 introduced a bug in x25_recvmsg()
> When passed bogus junk from userspace, x25->neighbour can be NULL,
> as shown in this oops..
> 
> BUG: unable to handle kernel NULL pointer dereference at 000000000000001c
> IP: [<ffffffffa05482bd>] x25_recvmsg+0x4d/0x280 [x25]
> PGD 1015f3067 PUD 105072067 PMD 0 
> Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> CPU 0 
> Pid: 27928, comm: iknowthis Not tainted 3.1.0+ #2 Gigabyte Technology Co., Ltd. GA-MA78GM-S2H/GA-MA78GM-S2H
> RIP: 0010:[<ffffffffa05482bd>]  [<ffffffffa05482bd>] x25_recvmsg+0x4d/0x280 [x25]
> RSP: 0018:ffff88010c0b7cc8  EFLAGS: 00010282
> RAX: 0000000000000000 RBX: ffff88010c0b7d78 RCX: 0000000000000c02
> RDX: ffff88010c0b7d78 RSI: ffff88011c93dc00 RDI: ffff880103f667b0
> RBP: ffff88010c0b7d18 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: ffff880103f667b0
> R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> FS:  00007f479ce7f700(0000) GS:ffff88012a600000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: 000000000000001c CR3: 000000010529e000 CR4: 00000000000006f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process iknowthis (pid: 27928, threadinfo ffff88010c0b6000, task ffff880103faa4f0)
> Stack:
>  0000000000000c02 0000000000000c02 ffff88010c0b7d18 ffffff958153cb37
>  ffffffff8153cb60 0000000000000c02 ffff88011c93dc00 0000000000000000
>  0000000000000c02 ffff88010c0b7e10 ffff88010c0b7de8 ffffffff815372c2
> Call Trace:
>  [<ffffffff8153cb60>] ? sock_update_classid+0xb0/0x180
>  [<ffffffff815372c2>] sock_aio_read.part.10+0x142/0x150
>  [<ffffffff812d6752>] ? inode_has_perm+0x62/0xa0
>  [<ffffffff815372fd>] sock_aio_read+0x2d/0x40
>  [<ffffffff811b05e2>] do_sync_read+0xd2/0x110
>  [<ffffffff812d3796>] ? security_file_permission+0x96/0xb0
>  [<ffffffff811b0a91>] ? rw_verify_area+0x61/0x100
>  [<ffffffff811b103d>] vfs_read+0x16d/0x180
>  [<ffffffff811b109d>] sys_read+0x4d/0x90
>  [<ffffffff81657282>] system_call_fastpath+0x16/0x1b
> Code: 8b 66 20 4c 8b 32 48 89 d3 48 89 4d b8 45 89 c7 c7 45 cc 95 ff ff ff 4d 85 e4 0f 84 ed 01 00 00 49 8b 84 24 18 05 00 00 4c 89 e7 
>  78 1c 01 45 19 ed 31 f6 e8 d5 37 ff e0 41 0f b6 44 24 0e 41 
> 
> Signed-off-by: Dave Jones <davej@redhat.com>
> 
> diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
> index 5f03e4e..291b2e0 100644
> --- a/net/x25/af_x25.c
> +++ b/net/x25/af_x25.c
> @@ -1261,13 +1261,17 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	struct x25_sock *x25 = x25_sk(sk);
>  	struct sockaddr_x25 *sx25 = (struct sockaddr_x25 *)msg->msg_name;
>  	size_t copied;
> -	int qbit, header_len = x25->neighbour->extended ?
> -		X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
> -
> +	int qbit, header_len;
>  	struct sk_buff *skb;
>  	unsigned char *asmptr;
>  	int rc = -ENOTCONN;
>  
> +	if (x25->neighbour == NULL)
> +		return rc;
> +
> +	header_len = x25->neighbour->extended ?
> +		X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
> +
>  	lock_sock(sk);

BTW, you probably want to check x25->neighbour while sk is locked, not
before.

	lock_sock(sk);
	if (x25->neighbour == NULL)
		goto out;
	header_len = x25->neighbour->extended ?
		     X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
 

^ permalink raw reply

* Re: Fix NULL dereference in x25_recvmsg
From: Eric Dumazet @ 2011-11-02  2:19 UTC (permalink / raw)
  To: Dave Jones; +Cc: netdev, Matthew Daley
In-Reply-To: <20111102021525.GA8512@redhat.com>

Le mardi 01 novembre 2011 à 22:15 -0400, Dave Jones a écrit :
> On Wed, Nov 02, 2011 at 03:10:45AM +0100, Eric Dumazet wrote:
>  > Le mardi 01 novembre 2011 à 21:53 -0400, Dave Jones a écrit :
>  > > commit cb101ed2 in 3.0 introduced a bug in x25_recvmsg()
>  > > When passed bogus junk from userspace, x25->neighbour can be NULL,
>  > > as shown in this oops..
>  > > 
>  > 
>  > Your patch seems fine but :
>  > 
>  > Are you sure this bug is not present on previous kernels ?
>  > 
>  > It seems we had prior to this commit :
>  > 
>  > skb_pull(skb, x25->neighbour->extended ?
>  > 	X25_EXT_MIN_LEN : X25_STD_MIN_LEN);
> 
> It might have been possible with a specifically crafted set of arguments.
> 
> It never showed up in testing before now, probably because we were
> returning from the function before we got to that skb_pull
> via all the other tests that get performed.
> 

neighbour is not an x25_recvmsg() argument, but related to x25 socket
state.

Maybe your tests dont try to use x25_recvmsg() while socket has no
neighbour...

This bug was there before the cb101ed2 commit.

^ permalink raw reply

* Re: Fix NULL dereference in x25_recvmsg
From: Eric Dumazet @ 2011-11-02  2:21 UTC (permalink / raw)
  To: Dave Jones; +Cc: netdev, Matthew Daley
In-Reply-To: <1320200358.4728.28.camel@edumazet-laptop>

Le mercredi 02 novembre 2011 à 03:19 +0100, Eric Dumazet a écrit :
> Le mardi 01 novembre 2011 à 22:15 -0400, Dave Jones a écrit :
> > On Wed, Nov 02, 2011 at 03:10:45AM +0100, Eric Dumazet wrote:
> >  > Le mardi 01 novembre 2011 à 21:53 -0400, Dave Jones a écrit :
> >  > > commit cb101ed2 in 3.0 introduced a bug in x25_recvmsg()
> >  > > When passed bogus junk from userspace, x25->neighbour can be NULL,
> >  > > as shown in this oops..
> >  > > 
> >  > 
> >  > Your patch seems fine but :
> >  > 
> >  > Are you sure this bug is not present on previous kernels ?
> >  > 
> >  > It seems we had prior to this commit :
> >  > 
> >  > skb_pull(skb, x25->neighbour->extended ?
> >  > 	X25_EXT_MIN_LEN : X25_STD_MIN_LEN);
> > 
> > It might have been possible with a specifically crafted set of arguments.
> > 
> > It never showed up in testing before now, probably because we were
> > returning from the function before we got to that skb_pull
> > via all the other tests that get performed.
> > 
> 
> neighbour is not an x25_recvmsg() argument, but related to x25 socket
> state.
> 
> Maybe your tests dont try to use x25_recvmsg() while socket has no
> neighbour...
> 
> This bug was there before the cb101ed2 commit.
> 

Ah ok, I see now, we exited early because of 

if (sk->sk_state != TCP_ESTABLISHED)
	goto out;

So yes, commit cb101ed2 is the bug origin, sorry for the noise.

^ permalink raw reply

* [v2] Fix NULL dereference in x25_recvmsg
From: Dave Jones @ 2011-11-02  2:26 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, Matthew Daley
In-Reply-To: <1320200165.4728.25.camel@edumazet-laptop>

commit cb101ed2 in 3.0 introduced a bug in x25_recvmsg()
When passed bogus junk from userspace, x25->neighbour can be NULL,
as shown in this oops..

BUG: unable to handle kernel NULL pointer dereference at 000000000000001c
IP: [<ffffffffa05482bd>] x25_recvmsg+0x4d/0x280 [x25]
PGD 1015f3067 PUD 105072067 PMD 0 
Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
CPU 0 
Pid: 27928, comm: iknowthis Not tainted 3.1.0+ #2 Gigabyte Technology Co., Ltd. GA-MA78GM-S2H/GA-MA78GM-S2H
RIP: 0010:[<ffffffffa05482bd>]  [<ffffffffa05482bd>] x25_recvmsg+0x4d/0x280 [x25]
RSP: 0018:ffff88010c0b7cc8  EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffff88010c0b7d78 RCX: 0000000000000c02
RDX: ffff88010c0b7d78 RSI: ffff88011c93dc00 RDI: ffff880103f667b0
RBP: ffff88010c0b7d18 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffff880103f667b0
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
FS:  00007f479ce7f700(0000) GS:ffff88012a600000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 000000000000001c CR3: 000000010529e000 CR4: 00000000000006f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process iknowthis (pid: 27928, threadinfo ffff88010c0b6000, task ffff880103faa4f0)
Stack:
 0000000000000c02 0000000000000c02 ffff88010c0b7d18 ffffff958153cb37
 ffffffff8153cb60 0000000000000c02 ffff88011c93dc00 0000000000000000
 0000000000000c02 ffff88010c0b7e10 ffff88010c0b7de8 ffffffff815372c2
Call Trace:
 [<ffffffff8153cb60>] ? sock_update_classid+0xb0/0x180
 [<ffffffff815372c2>] sock_aio_read.part.10+0x142/0x150
 [<ffffffff812d6752>] ? inode_has_perm+0x62/0xa0
 [<ffffffff815372fd>] sock_aio_read+0x2d/0x40
 [<ffffffff811b05e2>] do_sync_read+0xd2/0x110
 [<ffffffff812d3796>] ? security_file_permission+0x96/0xb0
 [<ffffffff811b0a91>] ? rw_verify_area+0x61/0x100
 [<ffffffff811b103d>] vfs_read+0x16d/0x180
 [<ffffffff811b109d>] sys_read+0x4d/0x90
 [<ffffffff81657282>] system_call_fastpath+0x16/0x1b
Code: 8b 66 20 4c 8b 32 48 89 d3 48 89 4d b8 45 89 c7 c7 45 cc 95 ff ff ff 4d 85 e4 0f 84 ed 01 00 00 49 8b 84 24 18 05 00 00 4c 89 e7 
 78 1c 01 45 19 ed 31 f6 e8 d5 37 ff e0 41 0f b6 44 24 0e 41 

Signed-off-by: Dave Jones <davej@redhat.com>

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 5f03e4e..3e16c6a 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1261,14 +1261,19 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
 	struct x25_sock *x25 = x25_sk(sk);
 	struct sockaddr_x25 *sx25 = (struct sockaddr_x25 *)msg->msg_name;
 	size_t copied;
-	int qbit, header_len = x25->neighbour->extended ?
-		X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
-
+	int qbit, header_len;
 	struct sk_buff *skb;
 	unsigned char *asmptr;
 	int rc = -ENOTCONN;
 
 	lock_sock(sk);
+
+	if (x25->neighbour == NULL)
+		goto out;
+
+	header_len = x25->neighbour->extended ?
+		X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
+
 	/*
 	 * This works for seqpacket too. The receiver has ordered the queue for
 	 * us! We do one quick check first though

^ permalink raw reply related

* Re: [v2] Fix NULL dereference in x25_recvmsg
From: Eric Dumazet @ 2011-11-02  2:30 UTC (permalink / raw)
  To: Dave Jones; +Cc: netdev, Matthew Daley
In-Reply-To: <20111102022644.GB8512@redhat.com>

Le mardi 01 novembre 2011 à 22:26 -0400, Dave Jones a écrit :
> commit cb101ed2 in 3.0 introduced a bug in x25_recvmsg()
> When passed bogus junk from userspace, x25->neighbour can be NULL,
> as shown in this oops..
> 
...
> Signed-off-by: Dave Jones <davej@redhat.com>
> 
> diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
> index 5f03e4e..3e16c6a 100644
> --- a/net/x25/af_x25.c
> +++ b/net/x25/af_x25.c
> @@ -1261,14 +1261,19 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	struct x25_sock *x25 = x25_sk(sk);
>  	struct sockaddr_x25 *sx25 = (struct sockaddr_x25 *)msg->msg_name;
>  	size_t copied;
> -	int qbit, header_len = x25->neighbour->extended ?
> -		X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
> -
> +	int qbit, header_len;
>  	struct sk_buff *skb;
>  	unsigned char *asmptr;
>  	int rc = -ENOTCONN;
>  
>  	lock_sock(sk);
> +
> +	if (x25->neighbour == NULL)
> +		goto out;
> +
> +	header_len = x25->neighbour->extended ?
> +		X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
> +
>  	/*
>  	 * This works for seqpacket too. The receiver has ordered the queue for
>  	 * us! We do one quick check first though

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

^ permalink raw reply

* Contact UPS Via {ups_despatch.ng@dgoh.org} For Claims
From: Boerner, Hunter @ 2011-11-02  3:49 UTC (permalink / raw)



  
 
  
 
After much attempts to reach you on phone, I deemed it necessary and urgent to contact you via your e-mail and to notify you finally about your outstanding compensation payment.

During our last annual calculation of your banking and Internet activities we realized that you are eligible to receive a compensation payment of $2,811,041.00 USD.This compensation is being made to all of you who have suffered losses as a result of fraud, accident or illness.

For more information, contact the UPS agent for the delivery of your cashier check ($2,811,041.00 USD).

United Parcel Service (UPS)
Contact Name: Collins Abah
Tel:+2348168636983
E-mail: ups_despatch.ng@dgoh.org <mailto:ups_despatch.ng@dgoh.org> 

Please take note that you will pay a shipping/handling fee of $245.00 USD to UPS.

Thanks for your patience.

Boernar Hunter
Programme Manager
United Nations Human Settlements Programme

^ permalink raw reply

* ARP garbage collection issues in 3.1?
From: Anton Blanchard @ 2011-11-02  4:34 UTC (permalink / raw)
  To: netdev


Hi,

I've been getting complaints about machines not garbage collecting
ARP entries. The network in question has a lot of MAC addresses on it
(probably more than gc_thresh3 - 1024), but this machine is only
talking over the network to a few targets.

Over a period of hours or days the ARP table slowly grows until it hits
gc_thresh3 and at that stage we get "Neighbour table overflow" errors.
Increasing gc_thresh3 makes the issue go away as expected. The question
is, why isn't this slow growth of the ARP table being managed by the
gc_thresh1 and gc_thresh2 watermarks?

I tried this simple test on 3.1-git*:

machine A:
for i in `seq 2 254`; do ifconfig eth0:$i 192.168.111.$i; done

machine B:
echo 64 > /proc/sys/net/ipv4/neigh/default/gc_thresh1
echo 128 > /proc/sys/net/ipv4/neigh/default/gc_thresh2
echo 192 > /proc/sys/net/ipv4/neigh/default/gc_thresh3

ifconfig eth0:1 192.168.111.1
for i in `seq 2 254`; do ping -c 1 192.168.111.$i; done

I was expecting to get the "Neighbour table overflow" errors and then
after a period of time the route and ARP garbage collection would kick
in. I waited for over an hour and this had not happened, we had
gc_thresh3 (192) ARP entries and no new ARP entries could be added to
the table.

The problem seems to be that our neighbour garbage collection thresholds
are set low (1024) and our route threshold is set high (524288). When we
get into trouble we do try to garbage collect routes but
rt_garbage_collect never sees enough entries to invoke garbage
collection. In effect the ARP entries are pinned, and gc_thresh1
gc_thresh2 are useless.

My first thought was the patch below. It does help but it's Russian
Roulette since we shoot one entry down at random and it might not be
one in the neighbour table. As such, it often takes a number of goes to
clear a stale ARP entry.

Any ideas how we could make this behave a bit better? I know setting
gc_thresh3 higher is the ultimate solution, but if gc_thresh1 and
gc_thresh2 are always below the route threshold we should either fix
this issue or remove them completely.

Anton

---
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 155138d..8104d41 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -876,7 +876,7 @@ static void rt_emergency_hash_rebuild(struct net *net)
    and when load increases it reduces to limit cache size.
  */
 
-static int rt_garbage_collect(struct dst_ops *ops)
+static int __rt_garbage_collect(struct dst_ops *ops, int force)
 {
 	static unsigned long expire = RT_GC_TIMEOUT;
 	static unsigned long last_gc;
@@ -895,7 +895,7 @@ static int rt_garbage_collect(struct dst_ops *ops)
 
 	RT_CACHE_STAT_INC(gc_total);
 
-	if (now - last_gc < ip_rt_gc_min_interval &&
+	if (!force && now - last_gc < ip_rt_gc_min_interval &&
 	    entries < ip_rt_max_size) {
 		RT_CACHE_STAT_INC(gc_ignored);
 		goto out;
@@ -920,6 +920,9 @@ static int rt_garbage_collect(struct dst_ops *ops)
 		equilibrium = entries - goal;
 	}
 
+	if (force)
+		goal = 1;
+
 	if (now - last_gc >= ip_rt_gc_min_interval)
 		last_gc = now;
 
@@ -996,6 +999,11 @@ work_done:
 out:	return 0;
 }
 
+static int rt_garbage_collect(struct dst_ops *ops)
+{
+	return __rt_garbage_collect(ops, 0);
+}
+
 /*
  * Returns number of entries in a hash chain that have different hash_inputs
  */
@@ -1192,7 +1200,7 @@ restart:
 				int saved_int = ip_rt_gc_min_interval;
 				ip_rt_gc_elasticity	= 1;
 				ip_rt_gc_min_interval	= 0;
-				rt_garbage_collect(&ipv4_dst_ops);
+				__rt_garbage_collect(&ipv4_dst_ops, 1);
 				ip_rt_gc_min_interval	= saved_int;
 				ip_rt_gc_elasticity	= saved_elasticity;
 				goto restart;

^ permalink raw reply related

* Re: on bnx2x firmware...
From: Ben Hutchings @ 2011-11-02  4:36 UTC (permalink / raw)
  To: Maciej Żenczykowski; +Cc: Linux NetDev
In-Reply-To: <CAHo-OowF1LNmL0P_Rt=oKmhZMt=CVaVsAYkh_XqVJEOzBPXV7w@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]

On Tue, 2011-11-01 at 18:01 -0700, Maciej Żenczykowski wrote:
> ((v3.1))$ cat drivers/net/bnx2x/bnx2x_hsi.h | egrep BCM_5710_FW_
> 
> #define BCM_5710_FW_MAJOR_VERSION                       7
> #define BCM_5710_FW_MINOR_VERSION                       0
> #define BCM_5710_FW_REVISION_VERSION            23
> #define BCM_5710_FW_ENGINEERING_VERSION         0
> 
> So the desired firmware is version 7.0.23.0
> 
> ((v3.1))$ find | egrep bnx2x | egrep fw
> ./firmware/bnx2x/bnx2x-e1-6.2.9.0.fw.ihex
> ./firmware/bnx2x/bnx2x-e2-6.2.9.0.fw.ihex
> ./firmware/bnx2x/bnx2x-e1h-6.2.9.0.fw.ihex
> ./drivers/net/bnx2x/bnx2x_fw_defs.h
> ./drivers/net/bnx2x/bnx2x_fw_file_hdr.h
> 
> So it doesn't look like the in-tree fw matches the bnx2x driver in
> released v3.1.
> I can find this firmware in the firmware repository.
[...]

That's where it belongs.  But there is some confusion/disagreement on
this point.

Ben.

-- 
Ben Hutchings
Sturgeon's Law: Ninety percent of everything is crap.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: ARP garbage collection issues in 3.1?
From: David Miller @ 2011-11-02  4:48 UTC (permalink / raw)
  To: anton; +Cc: netdev
In-Reply-To: <20111102153443.38cc1e5c@kryten>

From: Anton Blanchard <anton@samba.org>
Date: Wed, 2 Nov 2011 15:34:43 +1100

> Any ideas how we could make this behave a bit better? I know setting
> gc_thresh3 higher is the ultimate solution, but if gc_thresh1 and
> gc_thresh2 are always below the route threshold we should either fix
> this issue or remove them completely.

The solution is to do refcount'less RCU lookups into the neigh
tables on every packet send, and long term that's what I intend
to implement.

That's what's behind making the recent change to make the ARP hash
cheaper etc.

See slides 5, 6, and 7 in:

http://vger.kernel.org/netconf2011_slides/davem_netconf2011.pdf

Once that's done you can trim whatever neigh entries you want,
whenever you want.

You are right that the current situation is silly, because if
we're willing to commit to N routing table entries we might as
well be willing to commit to N arp table entries as well.

^ permalink raw reply

* Re: [v2] Fix NULL dereference in x25_recvmsg
From: David Miller @ 2011-11-02  4:50 UTC (permalink / raw)
  To: eric.dumazet; +Cc: davej, netdev, mattjd
In-Reply-To: <1320201017.4728.31.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 02 Nov 2011 03:30:17 +0100

> Le mardi 01 novembre 2011 à 22:26 -0400, Dave Jones a écrit :
>> commit cb101ed2 in 3.0 introduced a bug in x25_recvmsg()
>> When passed bogus junk from userspace, x25->neighbour can be NULL,
>> as shown in this oops..
>> 
> ...
>> Signed-off-by: Dave Jones <davej@redhat.com>
 ...
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied and queued up for -stable, thanks everyone.

^ permalink raw reply

* Re: [PATCH] udp: fix a race in encap_rcv handling
From: David Miller @ 2011-11-02  4:51 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1320188219.4728.7.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 01 Nov 2011 23:56:59 +0100

> udp_queue_rcv_skb() has a possible race in encap_rcv handling, since
> this pointer can be changed anytime.
> 
> We should use ACCESS_ONCE() to close the race.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied and queued up for -stable, thanks!

^ permalink raw reply

* Re: [PATCH] net: ucc_geth, increase no. of HW RX descriptors
From: David Miller @ 2011-11-02  4:56 UTC (permalink / raw)
  To: Joakim.Tjernlund; +Cc: avorontsov, netdev
In-Reply-To: <1319548630-6561-1-git-send-email-Joakim.Tjernlund@transmode.se>

From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Date: Tue, 25 Oct 2011 15:17:10 +0200

> In a busy network we see ucc_geth is dropping RX pkgs every now
> and then. Increase the first RX queues HW descriptors from
> 16 to 32 to deal with this.
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>

1) This patch doesn't apply, because in the current tree this
   dirver lives under drivers/net/ethernet/freescale/ not
   drivers/net

2) Why not increase RX_BD_RING_LEN and thus all the ring sizes,
   is there a special reason only the first ring's size should
   be increased?

   Because the way you've made this change is confusing, someone
   will see RX_BD_RING_LEN's definition and believe that this is
   the size of the primary ring unless they happen to notice
   how .bdRingLenRx is initialized in this special way.

Thus, this patch needs changes before it can even be considered
for inclusion.

^ permalink raw reply

* Re: [PATCH net-next] bnx2x: stop using on-stack napi struct
From: David Miller @ 2011-11-02  5:00 UTC (permalink / raw)
  To: ariele; +Cc: davem, netdev, eilong
In-Reply-To: <1320163450.11011.19.camel@lb-tlvb-ariel.il.broadcom.com>

From: "Ariel Elior" <ariele@broadcom.com>
Date: Tue, 1 Nov 2011 18:04:10 +0200

> Napi structure was allocated on stack to hold temporary value of copied
> fastpath. This can be avoided by using the source fastpath as a
> scratchpad thus saving stack space and code.
> Signed-off-by: Ariel Elior <ariele@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>

Your patch has been severely corrupted by your email client.

I'm sure your colleagues at Broadcom can help you fix this
so that you can submit this patch properly. :-)

^ permalink raw reply

* Re: PROBLEM: pppol2tp over pppoe NULL pointer dereference
From: Misha Labjuk @ 2011-11-02  5:04 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1320186916.4728.1.camel@edumazet-laptop>

2011/11/2 Eric Dumazet <eric.dumazet@gmail.com>:
>
> On what kind of NIC this is happening ?
>

Realtek Semiconductor Co., Ltd. RTL8111/8168B PCI Express Gigabit
Ethernet controller (rev 02)
Kernel driver in use: r8169

^ permalink raw reply

* linux-next: build failure after merge of the final tree (net tree related)
From: Stephen Rothwell @ 2011-11-02  5:37 UTC (permalink / raw)
  To: David Miller, netdev; +Cc: linux-next, linux-kernel, Eric Dumazet

[-- Attachment #1: Type: text/plain, Size: 2019 bytes --]

Hi all,

[I thought I had reported this, but it was a similar problem in another
driver.]

After merging the final tree, today's linux-next build (powerpc
allyesconfig) failed like this:

drivers/net/ethernet/xilinx/ll_temac_main.c: In function 'temac_start_xmit':
drivers/net/ethernet/xilinx/ll_temac_main.c:717:3: error: implicit declaration of function 'frag_size' [-Werror=implicit-function-declaration]

Caused by commit 9e903e085262 ("net: add skb frag size accessors").

I have added this patch for today:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 2 Nov 2011 16:31:37 +1100
Subject: [PATCH] net: fix typo in drivers/net/ethernet/xilinx/ll_temac_main.c

Commit 9e903e085262 ("net: add skb frag size accessors") used frag_size
instead of skb_frag_size in this file.

Fixes this build error:

drivers/net/ethernet/xilinx/ll_temac_main.c: In function 'temac_start_xmit':
drivers/net/ethernet/xilinx/ll_temac_main.c:717:3: error: implicit declaration of function 'frag_size' [-Werror=implicit-function-declaration]

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/net/ethernet/xilinx/ll_temac_main.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c
index 4d1658e..caf3659 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_main.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
@@ -716,8 +716,8 @@ static int temac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 		cur_p = &lp->tx_bd_v[lp->tx_bd_tail];
 		cur_p->phys = dma_map_single(ndev->dev.parent,
 					     skb_frag_address(frag),
-					     frag_size(frag), DMA_TO_DEVICE);
-		cur_p->len = frag_size(frag);
+					     skb_frag_size(frag), DMA_TO_DEVICE);
+		cur_p->len = skb_frag_size(frag);
 		cur_p->app0 = 0;
 		frag++;
 	}
-- 
1.7.7

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply related

* Re: linux-next: build failure after merge of the final tree (net tree related)
From: David Miller @ 2011-11-02  5:50 UTC (permalink / raw)
  To: sfr; +Cc: netdev, linux-next, linux-kernel, eric.dumazet
In-Reply-To: <20111102163708.75d719e765dea20e0c4c5a04@canb.auug.org.au>

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 2 Nov 2011 16:37:08 +1100

> Subject: [PATCH] net: fix typo in drivers/net/ethernet/xilinx/ll_temac_main.c
> 
> Commit 9e903e085262 ("net: add skb frag size accessors") used frag_size
> instead of skb_frag_size in this file.
> 
> Fixes this build error:
> 
> drivers/net/ethernet/xilinx/ll_temac_main.c: In function 'temac_start_xmit':
> drivers/net/ethernet/xilinx/ll_temac_main.c:717:3: error: implicit declaration of function 'frag_size' [-Werror=implicit-function-declaration]
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>

Applied, thanks!

^ permalink raw reply

* Re: [PATCH] IPv6 - support for NLM_F_* flags at IPv6 routing requests
From: Maz The Northener @ 2011-11-02  6:28 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Matti Vaittinen, davem, netdev
In-Reply-To: <20111101224302.6d370068@s6510.linuxnetplumber.net>

On Tue, Nov 1, 2011 at 11:43 PM, Stephen Hemminger
<shemminger@vyatta.com> wrote:
> On Tue, 01 Nov 2011 16:22:27 +0200
> Matti Vaittinen <matti.vaittinen@nsn.com> wrote:
>
>>
>> +#define RT6_CANT_CREATE ((int)-1)
>>  #define RT6_DEBUG 2
>
> Rather than introduce a new error flag, why not convert the code
> to use the kernel standard PTR_ERR() macros?
> --

Thanks for comment Stephen, I admitt I felt there was something rotten
in adding a new define. And as to why not use PTR_ERR() - I did not
know about such a facility. I'll fix this to be

return ERR_PTR(-ENOENT);

and at calling function check for
if (PTR_ERR(fn) == -ENOENT)

I guess that is correct way?

And finally couple of questions about sending patches (I'm a new guy
in this "house" of yours):
Is it Ok to add patch as attachment? I'm having difficulties in using
a client which allows sending plain text.
And is it Ok to create patch against linux 3.1-rc4? I've never used
git, and thus obtaining 3.1-rc4 from kernel.org is the easiest way for
me.

--Matti Vaittinen

^ permalink raw reply

* Re: PROBLEM: pppol2tp over pppoe NULL pointer dereference
From: Eric Dumazet @ 2011-11-02  7:07 UTC (permalink / raw)
  To: Misha Labjuk; +Cc: netdev
In-Reply-To: <CAKpUy7ZTpXtUDXB+NCxa4QBr3qiwo7aBW9DmUY8ADqO8FgrUpw@mail.gmail.com>

Le mercredi 02 novembre 2011 à 09:04 +0400, Misha Labjuk a écrit :
> 2011/11/2 Eric Dumazet <eric.dumazet@gmail.com>:
> >
> > On what kind of NIC this is happening ?
> >
> 
> Realtek Semiconductor Co., Ltd. RTL8111/8168B PCI Express Gigabit
> Ethernet controller (rev 02)
> Kernel driver in use: r8169

OK thanks, could you try the following patch as well ?

If we release reorder_q.lock, we must not keep a dangling pointer (tmp)
and restart the whole loop.

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 34b2dde..bf8d50c 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -397,6 +397,7 @@ static void l2tp_recv_dequeue(struct l2tp_session *session)
 	 * expect to send up next, dequeue it and any other
 	 * in-sequence packets behind it.
 	 */
+start:
 	spin_lock_bh(&session->reorder_q.lock);
 	skb_queue_walk_safe(&session->reorder_q, skb, tmp) {
 		if (time_after(jiffies, L2TP_SKB_CB(skb)->expires)) {
@@ -433,7 +434,7 @@ static void l2tp_recv_dequeue(struct l2tp_session *session)
 		 */
 		spin_unlock_bh(&session->reorder_q.lock);
 		l2tp_recv_dequeue_skb(session, skb);
-		spin_lock_bh(&session->reorder_q.lock);
+		goto start;
 	}
 
 out:

^ permalink raw reply related

* Re: [PATCH] IPv6 - support for NLM_F_* flags at IPv6 routing requests
From: Matti Vaittinen @ 2011-11-02  7:09 UTC (permalink / raw)
  To: davem; +Cc: netdev, shemminger
In-Reply-To: <1320157347.11816.3.camel@lakki>

On Tue, 2011-11-01 at 16:27 +0200, Matti Vaittinen wrote:
> Hi dee Ho again.
> 
> Here's the support for NLM_F_* flags at IPv6 routing requests once again.
> 
> This time if no NLM_F_CREATE flag is not defined for RTM_NEWROUTE request,
> warning is printed, but no error is returned. Instead new route is added.
> 
> Exception is when NLM_F_REPLACE flag is given without NLM_F_CREATE, and
> no matching route is found. In this case it should be safe to assume
> that the request issuer is familiar with NLM_F_* flags, and does really
> not want route to be created.
> 
> Specifying NLM_F_REPLACE flag will now make the kernel to search for
> matching route, and replace it with new one. If no route is found and
> NLM_F_CREATE is specified as well, then new route is created.
> 
> Also, specifying NLM_F_EXCL will yield returning of error if matching route 
> is found.
> 
> Patch is created against linux-3.1-rc4
> 

New patch where the definition of new error is removed as Stephen suggested.

Signed-off-by: Matti Vaittinen <Mazziesaccount@gmail.com>
---
diff -uNr linux-3.1-rc4.orig/net/ipv6/ip6_fib.c linux-3.1-rc4.new/net/ipv6/ip6_fib.c
--- linux-3.1-rc4.orig/net/ipv6/ip6_fib.c	2011-11-01 14:01:55.000000000 +0200
+++ linux-3.1-rc4.new/net/ipv6/ip6_fib.c	2011-11-02 08:37:21.000000000 +0200
@@ -429,17 +429,34 @@
 
 static struct fib6_node * fib6_add_1(struct fib6_node *root, void *addr,
 				     int addrlen, int plen,
-				     int offset)
+				     int offset, struct nl_info *info)
 {
 	struct fib6_node *fn, *in, *ln;
 	struct fib6_node *pn = NULL;
 	struct rt6key *key;
 	int	bit;
+
+
+	int allow_create = 1;
+	int replace_required = 0;
+
+
 	__be32	dir = 0;
 	__u32	sernum = fib6_new_sernum();
 
 	RT6_TRACE("fib6_add_1\n");
 
+	if (NULL != info &&
+	    NULL != info->nlh &&
+	    (info->nlh->nlmsg_flags&NLM_F_REPLACE)) {
+		replace_required = 1;
+	}
+	if (NULL != info &&
+	    NULL != info->nlh &&
+	    !(info->nlh->nlmsg_flags&NLM_F_CREATE)) {
+			allow_create = 0;
+	}
+
 	/* insert node in tree */
 
 	fn = root;
@@ -451,8 +468,12 @@
 		 *	Prefix match
 		 */
 		if (plen < fn->fn_bit ||
-		    !ipv6_prefix_equal(&key->addr, addr, fn->fn_bit))
+		    !ipv6_prefix_equal(&key->addr, addr, fn->fn_bit)) {
+			if (!allow_create)
+				printk(KERN_WARNING
+				    "NLM_F_CREATE should be specified when creating new rt\n");
 			goto insert_above;
+		}
 
 		/*
 		 *	Exact match ?
@@ -481,10 +502,27 @@
 		fn = dir ? fn->right: fn->left;
 	} while (fn);
 
+
+	if (replace_required && !allow_create) {
+		/* We should not create new node because
+		 * NLM_F_REPLACE was specified without NLM_F_CREATE
+		 * I assume it is safe to require NLM_F_CREATE when
+		 * REPLACE flag is used! Later we may want to remove the
+		 * check for replace_required, because according
+		 * to netlink specification, NLM_F_CREATE
+		 * MUST be specified if new route is created.
+		 * That would keep IPv6 consistent with IPv4
+		 */
+		printk(KERN_WARNING
+		    "NLM_F_CREATE should be specified when creating new rt - ignoring request\n");
+		return ERR_PTR(-ENOENT);
+	}
 	/*
 	 *	We walked to the bottom of tree.
 	 *	Create new leaf node without children.
 	 */
+	if (!allow_create)
+		printk(KERN_WARNING "NLM_F_CREATE should be specified when creating new rt\n");
 
 	ln = node_alloc();
 
@@ -567,7 +605,6 @@
 		fn->parent = in;
 
 		ln->fn_sernum = sernum;
-
 		if (addr_bit_set(addr, bit)) {
 			in->right = ln;
 			in->left  = fn;
@@ -585,6 +622,7 @@
 
 		ln = node_alloc();
 
+
 		if (ln == NULL)
 			return NULL;
 
@@ -618,6 +656,12 @@
 {
 	struct rt6_info *iter = NULL;
 	struct rt6_info **ins;
+	int replace = (NULL != info &&
+	    NULL != info->nlh &&
+	    (info->nlh->nlmsg_flags&NLM_F_REPLACE));
+	int add = ((NULL == info || NULL == info->nlh) ||
+	    (info->nlh->nlmsg_flags&NLM_F_CREATE));
+	int found = 0;
 
 	ins = &fn->leaf;
 
@@ -630,6 +674,13 @@
 			/*
 			 *	Same priority level
 			 */
+			if (NULL != info->nlh &&
+			    (info->nlh->nlmsg_flags&NLM_F_EXCL))
+				return -EEXIST;
+			if (replace) {
+				found++;
+				break;
+			}
 
 			if (iter->rt6i_dev == rt->rt6i_dev &&
 			    iter->rt6i_idev == rt->rt6i_idev &&
@@ -659,19 +710,41 @@
 	/*
 	 *	insert node
 	 */
+	if (!replace) {
+		if (!add)
+			printk(KERN_WARNING "NLM_F_CREATE should be specified when creating new rt\n");
+
+add:
+		rt->dst.rt6_next = iter;
+		*ins = rt;
+		rt->rt6i_node = fn;
+		atomic_inc(&rt->rt6i_ref);
+		inet6_rt_notify(RTM_NEWROUTE, rt, info);
+		info->nl_net->ipv6.rt6_stats->fib_rt_entries++;
+
+		if ((fn->fn_flags & RTN_RTINFO) == 0) {
+			info->nl_net->ipv6.rt6_stats->fib_route_nodes++;
+			fn->fn_flags |= RTN_RTINFO;
+		}
 
-	rt->dst.rt6_next = iter;
-	*ins = rt;
-	rt->rt6i_node = fn;
-	atomic_inc(&rt->rt6i_ref);
-	inet6_rt_notify(RTM_NEWROUTE, rt, info);
-	info->nl_net->ipv6.rt6_stats->fib_rt_entries++;
-
-	if ((fn->fn_flags & RTN_RTINFO) == 0) {
-		info->nl_net->ipv6.rt6_stats->fib_route_nodes++;
-		fn->fn_flags |= RTN_RTINFO;
+	} else {
+		if (!found) {
+			if (add)
+				goto add;
+			printk(KERN_WARNING "add rtinfo to node - NLM_F_REPLACE specified, but no existing node found! bailing out\n");
+			return -ENOENT;
+		}
+		*ins = rt;
+		rt->rt6i_node = fn;
+		rt->dst.rt6_next = iter->dst.rt6_next;
+		atomic_inc(&rt->rt6i_ref);
+		inet6_rt_notify(RTM_NEWROUTE, rt, info);
+		rt6_release(iter);
+		if ((fn->fn_flags & RTN_RTINFO) == 0) {
+			info->nl_net->ipv6.rt6_stats->fib_route_nodes++;
+			fn->fn_flags |= RTN_RTINFO;
+		}
 	}
-
 	return 0;
 }
 
@@ -700,10 +773,29 @@
 {
 	struct fib6_node *fn, *pn = NULL;
 	int err = -ENOMEM;
+	int allow_create = 1;
+	int allow_replace = 1;
+	if (NULL != info &&
+	    NULL != info->nlh &&
+	    !(info->nlh->nlmsg_flags&NLM_F_REPLACE)) {
+		allow_replace = 0;
+	}
+	if (NULL != info &&
+	    NULL != info->nlh &&
+	    !(info->nlh->nlmsg_flags&NLM_F_CREATE)) {
+			allow_create = 0;
+	}
+	if (!allow_create && !allow_replace)
+		printk(KERN_WARNING "RTM_NEWROUTE with no NLM_F_CREATE or NLM_F_REPLACE\n");
 
 	fn = fib6_add_1(root, &rt->rt6i_dst.addr, sizeof(struct in6_addr),
-			rt->rt6i_dst.plen, offsetof(struct rt6_info, rt6i_dst));
+		    rt->rt6i_dst.plen, offsetof(struct rt6_info, rt6i_dst),
+		    info);
 
+	if (-ENOENT == PTR_ERR(fn)) {
+		err = -EINVAL;
+		fn = NULL;
+	}
 	if (fn == NULL)
 		goto out;
 
@@ -716,6 +808,8 @@
 		if (fn->subtree == NULL) {
 			struct fib6_node *sfn;
 
+			if (!allow_create)
+				printk(KERN_WARNING "NLM_F_CREATE should be specified when creating new rt\n");
 			/*
 			 * Create subtree.
 			 *
@@ -740,7 +834,8 @@
 
 			sn = fib6_add_1(sfn, &rt->rt6i_src.addr,
 					sizeof(struct in6_addr), rt->rt6i_src.plen,
-					offsetof(struct rt6_info, rt6i_src));
+					offsetof(struct rt6_info, rt6i_src),
+				    info);
 
 			if (sn == NULL) {
 				/* If it is failed, discard just allocated
@@ -757,8 +852,13 @@
 		} else {
 			sn = fib6_add_1(fn->subtree, &rt->rt6i_src.addr,
 					sizeof(struct in6_addr), rt->rt6i_src.plen,
-					offsetof(struct rt6_info, rt6i_src));
+					offsetof(struct rt6_info, rt6i_src),
+					info);
 
+			if (-ENOENT == PTR_ERR(sn)) {
+				err = -EINVAL;
+				sn = NULL;
+			}
 			if (sn == NULL)
 				goto st_failure;
 		}
diff -uNr linux-3.1-rc4.orig/net/ipv6/route.c linux-3.1-rc4.new/net/ipv6/route.c
--- linux-3.1-rc4.orig/net/ipv6/route.c	2011-11-01 14:01:55.000000000 +0200
+++ linux-3.1-rc4.new/net/ipv6/route.c	2011-10-27 10:45:05.000000000 +0300
@@ -1223,9 +1223,18 @@
 	if (cfg->fc_metric == 0)
 		cfg->fc_metric = IP6_RT_PRIO_USER;
 
-	table = fib6_new_table(net, cfg->fc_table);
+	err = -ENOBUFS;
+	if (NULL != cfg->fc_nlinfo.nlh &&
+	    !(cfg->fc_nlinfo.nlh->nlmsg_flags&NLM_F_CREATE)) {
+		table = fib6_get_table(net, cfg->fc_table);
+		if (table == NULL) {
+			printk(KERN_WARNING "NLM_F_CREATE should be specified when creating new rt\n");
+			table = fib6_new_table(net, cfg->fc_table);
+		}
+	} else {
+		table = fib6_new_table(net, cfg->fc_table);
+	}
 	if (table == NULL) {
-		err = -ENOBUFS;
 		goto out;
 	}
 

^ permalink raw reply

* Re: [PATCH] IPv6 - support for NLM_F_* flags at IPv6 routing requests
From: David Miller @ 2011-11-02  7:28 UTC (permalink / raw)
  To: matti.vaittinen; +Cc: netdev, shemminger
In-Reply-To: <1320217791.12052.12.camel@lakki>

From: Matti Vaittinen <matti.vaittinen@nsn.com>
Date: Wed, 02 Nov 2011 09:09:51 +0200

> On Tue, 2011-11-01 at 16:27 +0200, Matti Vaittinen wrote:
>> Hi dee Ho again.
>> 
>> Here's the support for NLM_F_* flags at IPv6 routing requests once again.
>> 
>> This time if no NLM_F_CREATE flag is not defined for RTM_NEWROUTE request,
>> warning is printed, but no error is returned. Instead new route is added.
>> 
>> Exception is when NLM_F_REPLACE flag is given without NLM_F_CREATE, and
>> no matching route is found. In this case it should be safe to assume
>> that the request issuer is familiar with NLM_F_* flags, and does really
>> not want route to be created.
>> 
>> Specifying NLM_F_REPLACE flag will now make the kernel to search for
>> matching route, and replace it with new one. If no route is found and
>> NLM_F_CREATE is specified as well, then new route is created.
>> 
>> Also, specifying NLM_F_EXCL will yield returning of error if matching route 
>> is found.
>> 
>> Patch is created against linux-3.1-rc4
>> 
> 
> New patch where the definition of new error is removed as Stephen suggested.
> 
> Signed-off-by: Matti Vaittinen <Mazziesaccount@gmail.com>

Please do not submit new versions of patches in this way by replying
and quoting your original commit log message.  That makes for lots
of work for me.

Instead, submit a fresh new full patch posting and prefix your subject
with something like "[PATCH v2]" to indicate it's a new version.

^ permalink raw reply

* Re: [PATCH] flexcan: Fix CAN_RAW_RECV_OWN_MSGS and CAN_RAW_LOOPBACK
From: Kurt Van Dijck @ 2011-11-02  8:04 UTC (permalink / raw)
  To: Reuben Dowle; +Cc: netdev, linux-can
In-Reply-To: <70F6AAAFDC054F41B9994A9BCD3DF64E16FAA1DD@exch01-aklnz.MARINE.NET.INT>

On Tue, Nov 01, 2011 at 11:18:03AM +1300, Reuben Dowle wrote:
> Currently the flexcan driver uses hardware local echo. This blindly echos all transmitted frames to all receiving sockets, regardless what CAN_RAW_RECV_OWN_MSGS and CAN_RAW_LOOPBACK are set to.
> 
> This patch now submits transmitted frames to be echoed in the transmit complete interrupt, preserving the reference to the sending socket. This allows the can protocol to correctly handle the local echo.
> 
> Signed-off-by: Reuben Dowle <reuben.dowle@navico.com>
> 
> ---
>  drivers/net/can/flexcan.c |   19 +++++++++++++++----
>  1 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index e023379..542ada8 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -302,7 +302,7 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	flexcan_write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
>  	flexcan_write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
>  
> -	kfree_skb(skb);
> +	can_put_echo_skb(skb, dev, 0);
>  
>  	/* tx_packets is incremented in flexcan_irq */
>  	stats->tx_bytes += cf->can_dlc;
Why not move the statistics to the place where the echo_skb is popped?
When no frame gets out (due to whatever reason), the statistics may have
incremented.

Or maybe this needs a seperate patch?

The rest looks good.

Kurt

^ permalink raw reply

* [PATCH v3] IPv6 - support for NLM_F_* flags at routing requests
From: Matti Vaittinen @ 2011-11-02  8:04 UTC (permalink / raw)
  To: davem; +Cc: netdev

Once more.. Changed returnvalue -EINVAL to -ENOENT for case
where NLM_F_REPLACE is specified without NLM_F_CREATE, and no
matching route is found.



The support for NLM_F_* flags at IPv6 routing requests.

If no NLM_F_CREATE flag is not defined for RTM_NEWROUTE request,
warning is printed, but no error is returned. Instead new route is added.

Exception is when NLM_F_REPLACE flag is given without NLM_F_CREATE, and
no matching route is found. In this case it should be safe to assume
that the request issuer is familiar with NLM_F_* flags, and does really
not want route to be created. 

Specifying NLM_F_REPLACE flag will now make the kernel to search for
matching route, and replace it with new one. If no route is found and
NLM_F_CREATE is specified as well, then new route is created.

Also, specifying NLM_F_EXCL will yield returning of error if matching route
is found.


Signed-off-by: Matti Vaittinen <Mazziesaccount@gmail.com>
---
diff -uNr linux-3.1-rc4.orig/net/ipv6/ip6_fib.c linux-3.1-rc4.new/net/ipv6/ip6_fib.c
--- linux-3.1-rc4.orig/net/ipv6/ip6_fib.c	2011-11-01 14:01:55.000000000 +0200
+++ linux-3.1-rc4.new/net/ipv6/ip6_fib.c	2011-11-02 08:37:21.000000000 +0200
@@ -429,17 +429,34 @@
 
 static struct fib6_node * fib6_add_1(struct fib6_node *root, void *addr,
 				     int addrlen, int plen,
-				     int offset)
+				     int offset, struct nl_info *info)
 {
 	struct fib6_node *fn, *in, *ln;
 	struct fib6_node *pn = NULL;
 	struct rt6key *key;
 	int	bit;
+
+
+	int allow_create = 1;
+	int replace_required = 0;
+
+
 	__be32	dir = 0;
 	__u32	sernum = fib6_new_sernum();
 
 	RT6_TRACE("fib6_add_1\n");
 
+	if (NULL != info &&
+	    NULL != info->nlh &&
+	    (info->nlh->nlmsg_flags&NLM_F_REPLACE)) {
+		replace_required = 1;
+	}
+	if (NULL != info &&
+	    NULL != info->nlh &&
+	    !(info->nlh->nlmsg_flags&NLM_F_CREATE)) {
+			allow_create = 0;
+	}
+
 	/* insert node in tree */
 
 	fn = root;
@@ -451,8 +468,12 @@
 		 *	Prefix match
 		 */
 		if (plen < fn->fn_bit ||
-		    !ipv6_prefix_equal(&key->addr, addr, fn->fn_bit))
+		    !ipv6_prefix_equal(&key->addr, addr, fn->fn_bit)) {
+			if (!allow_create)
+				printk(KERN_WARNING
+				    "NLM_F_CREATE should be specified when creating new rt\n");
 			goto insert_above;
+		}
 
 		/*
 		 *	Exact match ?
@@ -481,10 +502,27 @@
 		fn = dir ? fn->right: fn->left;
 	} while (fn);
 
+
+	if (replace_required && !allow_create) {
+		/* We should not create new node because
+		 * NLM_F_REPLACE was specified without NLM_F_CREATE
+		 * I assume it is safe to require NLM_F_CREATE when
+		 * REPLACE flag is used! Later we may want to remove the
+		 * check for replace_required, because according
+		 * to netlink specification, NLM_F_CREATE
+		 * MUST be specified if new route is created.
+		 * That would keep IPv6 consistent with IPv4
+		 */
+		printk(KERN_WARNING
+		    "NLM_F_CREATE should be specified when creating new rt - ignoring request\n");
+		return ERR_PTR(-ENOENT);
+	}
 	/*
 	 *	We walked to the bottom of tree.
 	 *	Create new leaf node without children.
 	 */
+	if (!allow_create)
+		printk(KERN_WARNING "NLM_F_CREATE should be specified when creating new rt\n");
 
 	ln = node_alloc();
 
@@ -567,7 +605,6 @@
 		fn->parent = in;
 
 		ln->fn_sernum = sernum;
-
 		if (addr_bit_set(addr, bit)) {
 			in->right = ln;
 			in->left  = fn;
@@ -585,6 +622,7 @@
 
 		ln = node_alloc();
 
+
 		if (ln == NULL)
 			return NULL;
 
@@ -618,6 +656,12 @@
 {
 	struct rt6_info *iter = NULL;
 	struct rt6_info **ins;
+	int replace = (NULL != info &&
+	    NULL != info->nlh &&
+	    (info->nlh->nlmsg_flags&NLM_F_REPLACE));
+	int add = ((NULL == info || NULL == info->nlh) ||
+	    (info->nlh->nlmsg_flags&NLM_F_CREATE));
+	int found = 0;
 
 	ins = &fn->leaf;
 
@@ -630,6 +674,13 @@
 			/*
 			 *	Same priority level
 			 */
+			if (NULL != info->nlh &&
+			    (info->nlh->nlmsg_flags&NLM_F_EXCL))
+				return -EEXIST;
+			if (replace) {
+				found++;
+				break;
+			}
 
 			if (iter->rt6i_dev == rt->rt6i_dev &&
 			    iter->rt6i_idev == rt->rt6i_idev &&
@@ -659,19 +710,41 @@
 	/*
 	 *	insert node
 	 */
+	if (!replace) {
+		if (!add)
+			printk(KERN_WARNING "NLM_F_CREATE should be specified when creating new rt\n");
+
+add:
+		rt->dst.rt6_next = iter;
+		*ins = rt;
+		rt->rt6i_node = fn;
+		atomic_inc(&rt->rt6i_ref);
+		inet6_rt_notify(RTM_NEWROUTE, rt, info);
+		info->nl_net->ipv6.rt6_stats->fib_rt_entries++;
+
+		if ((fn->fn_flags & RTN_RTINFO) == 0) {
+			info->nl_net->ipv6.rt6_stats->fib_route_nodes++;
+			fn->fn_flags |= RTN_RTINFO;
+		}
 
-	rt->dst.rt6_next = iter;
-	*ins = rt;
-	rt->rt6i_node = fn;
-	atomic_inc(&rt->rt6i_ref);
-	inet6_rt_notify(RTM_NEWROUTE, rt, info);
-	info->nl_net->ipv6.rt6_stats->fib_rt_entries++;
-
-	if ((fn->fn_flags & RTN_RTINFO) == 0) {
-		info->nl_net->ipv6.rt6_stats->fib_route_nodes++;
-		fn->fn_flags |= RTN_RTINFO;
+	} else {
+		if (!found) {
+			if (add)
+				goto add;
+			printk(KERN_WARNING "add rtinfo to node - NLM_F_REPLACE specified, but no existing node found! bailing out\n");
+			return -ENOENT;
+		}
+		*ins = rt;
+		rt->rt6i_node = fn;
+		rt->dst.rt6_next = iter->dst.rt6_next;
+		atomic_inc(&rt->rt6i_ref);
+		inet6_rt_notify(RTM_NEWROUTE, rt, info);
+		rt6_release(iter);
+		if ((fn->fn_flags & RTN_RTINFO) == 0) {
+			info->nl_net->ipv6.rt6_stats->fib_route_nodes++;
+			fn->fn_flags |= RTN_RTINFO;
+		}
 	}
-
 	return 0;
 }
 
@@ -700,10 +773,29 @@
 {
 	struct fib6_node *fn, *pn = NULL;
 	int err = -ENOMEM;
+	int allow_create = 1;
+	int allow_replace = 1;
+	if (NULL != info &&
+	    NULL != info->nlh &&
+	    !(info->nlh->nlmsg_flags&NLM_F_REPLACE)) {
+		allow_replace = 0;
+	}
+	if (NULL != info &&
+	    NULL != info->nlh &&
+	    !(info->nlh->nlmsg_flags&NLM_F_CREATE)) {
+			allow_create = 0;
+	}
+	if (!allow_create && !allow_replace)
+		printk(KERN_WARNING "RTM_NEWROUTE with no NLM_F_CREATE or NLM_F_REPLACE\n");
 
 	fn = fib6_add_1(root, &rt->rt6i_dst.addr, sizeof(struct in6_addr),
-			rt->rt6i_dst.plen, offsetof(struct rt6_info, rt6i_dst));
+		    rt->rt6i_dst.plen, offsetof(struct rt6_info, rt6i_dst),
+		    info);
 
+	if (-ENOENT == PTR_ERR(fn)) {
+		err = -ENOENT;
+		fn = NULL;
+	}
 	if (fn == NULL)
 		goto out;
 
@@ -716,6 +808,8 @@
 		if (fn->subtree == NULL) {
 			struct fib6_node *sfn;
 
+			if (!allow_create)
+				printk(KERN_WARNING "NLM_F_CREATE should be specified when creating new rt\n");
 			/*
 			 * Create subtree.
 			 *
@@ -740,7 +834,8 @@
 
 			sn = fib6_add_1(sfn, &rt->rt6i_src.addr,
 					sizeof(struct in6_addr), rt->rt6i_src.plen,
-					offsetof(struct rt6_info, rt6i_src));
+					offsetof(struct rt6_info, rt6i_src),
+				    info);
 
 			if (sn == NULL) {
 				/* If it is failed, discard just allocated
@@ -757,8 +852,13 @@
 		} else {
 			sn = fib6_add_1(fn->subtree, &rt->rt6i_src.addr,
 					sizeof(struct in6_addr), rt->rt6i_src.plen,
-					offsetof(struct rt6_info, rt6i_src));
+					offsetof(struct rt6_info, rt6i_src),
+					info);
 
+			if (-ENOENT == PTR_ERR(sn)) {
+				err = -ENOENT;
+				sn = NULL;
+			}
 			if (sn == NULL)
 				goto st_failure;
 		}
diff -uNr linux-3.1-rc4.orig/net/ipv6/route.c linux-3.1-rc4.new/net/ipv6/route.c
--- linux-3.1-rc4.orig/net/ipv6/route.c	2011-11-01 14:01:55.000000000 +0200
+++ linux-3.1-rc4.new/net/ipv6/route.c	2011-10-27 10:45:05.000000000 +0300
@@ -1223,9 +1223,18 @@
 	if (cfg->fc_metric == 0)
 		cfg->fc_metric = IP6_RT_PRIO_USER;
 
-	table = fib6_new_table(net, cfg->fc_table);
+	err = -ENOBUFS;
+	if (NULL != cfg->fc_nlinfo.nlh &&
+	    !(cfg->fc_nlinfo.nlh->nlmsg_flags&NLM_F_CREATE)) {
+		table = fib6_get_table(net, cfg->fc_table);
+		if (table == NULL) {
+			printk(KERN_WARNING "NLM_F_CREATE should be specified when creating new rt\n");
+			table = fib6_new_table(net, cfg->fc_table);
+		}
+	} else {
+		table = fib6_new_table(net, cfg->fc_table);
+	}
 	if (table == NULL) {
-		err = -ENOBUFS;
 		goto out;
 	}
 

^ permalink raw reply

* Re: Quick Fair Queue scheduler maturity and examples
From: Karel Rericha @ 2011-11-02  9:36 UTC (permalink / raw)
  To: Eric Dumazet, netdev
In-Reply-To: <1319732832.2601.43.camel@edumazet-laptop>

2011/10/27 Eric Dumazet <eric.dumazet@gmail.com>:
> Le jeudi 27 octobre 2011 à 18:08 +0200, Eric Dumazet a écrit :
>> Le jeudi 27 octobre 2011 à 14:46 +0200, Karel Rericha a écrit :
>>
>> > Actually I am doing some reseach to replace our main shaping machine
>> > with 60 000+ htb classes, which now saturates 12 core Xeon Westmere to
>> > 30% (there are five gigabit network ports on each interface affinited
>> > to cores). AFAIK QFQ should be O(1) complexity so it would bring
>> > saturation a requirements for number of cores down considerably (HTB
>> > has O(log(N)) complexity).
>> >
>> > I have test machine and about two months to decide if we will stay
>> > with HTB or we will try something else. So it would be VERY helpful,
>> > if you would search you memory instead your dead disk :-) and send me
>> > some example of QFQ usage, if I can ask for a little of your time. I
>> > promise to have results published here in return.
>> >
>> > Thanks, Karel
>> >
>>
>> That seems a good challenge to me ;)
>>
>> First upgrade to a recent kernel with QFQ included.
>> Also upgrade iproute2 to a recent enough version as well.
>>
>> Then you discover "tc  ... qfq help" is not that helpful :(
>>
>> # tc qdisc add dev eth3 root qfq help
>> Usage: ... qfq
>>
>> OK, its parameters are :
>>
>>       qfq weight num1 [maxpkt BYTES]
>>
>> You should not touch maxpkt, its default value being 2048
>>
>> Oh well, I just tried the obvious and my (remote) machine doesnt answer
>> to me anymore...
>>
>> Time for a bit of debugging I am afraid :(
>
> Never mind, it was an user error :)
>
> Here is what I used during my tests, I guess you can adapt your
> scripts...
>
> DEV=eth3
> RATE="rate 40Mbit"
> TNETS="10.2.2.0/25"
> ALLOT="allot 20000"
>
> tc qdisc del dev dummy0 root 2>/dev/null
>
> tc qdisc add dev $DEV root handle 1: cbq avpkt 1000 rate 1000Mbit \
>        bandwidth 1000Mbit
> tc class add dev $DEV parent 1: classid 1:1 \
>        est 1sec 8sec cbq allot 10000 mpu 64 \
>        rate 1000Mbit prio 1 avpkt 1500 bounded
>
> # output to test nets :  40 Mbit limit
> tc class add dev $DEV parent 1:1 classid 1:11 \
>        est 1sec 8sec cbq $ALLOT mpu 64      \
>        $RATE prio 2 avpkt 1400 bounded
>
> tc qdisc add dev $DEV parent 1:11 handle 11:  \
>        est 1sec 8sec qfq
>
> tc filter add dev $DEV protocol ip parent 11: handle 3 \
>        flow hash keys rxhash divisor 8
>
> for i in `seq 1 8`
> do
>  classid=11:$(printf %x $i)
>  tc class add dev $DEV classid $classid qfq
>  tc qdisc add dev $DEV parent $classid pfifo limit 30
> done
>
> for privnet in $TNETS
> do
>        tc filter add dev $DEV parent 1: protocol ip prio 100 u32 \
>                match ip dst $privnet flowid 1:11
> done
>
> tc filter add dev $DEV parent 1: protocol ip prio 100 u32 \
>        match ip protocol 0 0x00 flowid 1:1
>
> iperf -u -c 10.2.2.1 -P 32 -l 50
>
>
>

Thanks for example Eric.

But it only added more confusion to me now :-) I was under impression
(and read somewhere) that QFQ is non work conserving scheduler so I
can use it more or less like HTB or HFSC to set bandwidth constraints
to flows. But from this example (and from sources/patches/papers I try
not to pretend I fully understand) it looks to me like some multiqueue
scheduler with arbitrary number of queues and ability to arbitrary
assign flows to this queues. So some sort of fair division of
available bandwidth to flows without arbitrary bandwidth caps to these
flows. I really dont see what is non work conserving here :-S

Please save my soul and enlighten me because I am at dead end now :-)

^ permalink raw reply

* TCP_CORK questions
From: Mihai Maruseac @ 2011-11-02  9:43 UTC (permalink / raw)
  To: kernelnewbies; +Cc: netdev

Hi,

We are using TCP_CORK with TCP_NODELAY. We are setting TCP_NODELAY
first and then setting TCP_CORK before sending a bunch of data
buffers. After sending all the buffers we are clearing TCP_CORK. The
expectation -- as per manpage -- is that when TCP_CORK is set small
data packets will not be sent. When TCP_CORK is removed it will
immediately send out the remaining data even if the remaining amount
is very small.

However, this is not the observed behavior. Small packets are sent at
an approximatively constant rate. Digging further into the manpage for
tcp we saw that there is a 200ms ceiling on the time for which the
output is corked. Can we change this limit somehow? Ideally, in our
case we would like to have userspace mark a socket such that it will
only send full packets as long as it is marked, even if the completion
comes after a long time.

Digging into tcp.c when the socket options are set, we found that if
when setting TCP_NAGLE_CORK we also disable TCP_NAGLE_PUSH the code
works as intended. Is this the right way to solve it? Code comments
suggest that disabling this flag there has no effect.

Thanks,
Mihai

^ permalink raw reply

* Re: hiberante hangs TCP Re: [EXAMPLE CODE] Parasite thread injection and TCP connection hijacking
From: MyungJoo Ham @ 2011-11-02  9:44 UTC (permalink / raw)
  To: Tejun Heo; +Cc: netdev, linux-pm, David Fries, linux-kernel
In-Reply-To: <20111030201618.GA7696@google.com>

On Mon, Oct 31, 2011 at 5:16 AM, Tejun Heo <tj@kernel.org> wrote:
> (cc'ing Rafael and linux-pm)
>
> On Sat, Oct 29, 2011 at 11:48:21PM -0500, David Fries wrote:
>> I saw the write up on this on lwn.net, pretty creative by the way, and
>> it got me thinking about a different checkpoint/restart problem I've
>> been running into.  Specifically in hibernating to disk.  In the
>> hibernate case active TCP connections hang after resuming, while an
>> idle TCP connection will continue after the system is back up.  My
>> observation is the kernel checkpoints itself to memory, enables
>> devices, writes out that checkpoint image to storage, then powers off.
>> The problem is if TCP packets are received while writing to storage,
>> the kernel will continue to queue and ack those TCP packets, but the
>> running kernel and it's network state is shortly lost.  When the
>> computer resumes, those TCP byte sequences hang the TCP connection for
>> an extended period of time while the resumed computer refuses to
>> acknowledge the data that was received after checkpointing and the now
>> running kernel knew nothing about, and the other computer tries in
>> vain to resend any data that hadn't yet been acknowledged, which is
>> always after the data that was lost, until one of them eventually
>> gives up.
>>
>> I've been wondering if it was safe or possible to leave any network
>> interfaces down after the checkpoint, or what the right solution would
>> be.  I didn't think marking every TCP connection with a ZOMBIE_KERNEL
>> bit just after the kernel checkpoint (for the kernel is walking dead
>> and won't remember anything that happens), and then prevent any TCP
>> acks from being sent for those connections would be the right
>> solution.  I've taken to unplugging the physical lan cable,
>> hibernating to disk, and plugging it back in after the system is down,
>> to avoid the problem.  Any ideas?
>
> Hmmm... sounds like taking down network interfaces before starting
> hibernation sequence should be enough, which shouldn't be too
> difficult to implement from userland.  Rafael, what do you think?
>
> Thanks.

Um... it seems that the "thaw" callbacks of network interfaces or TCP
should do something on this.

Probably, the "thaw" callbacks should make sure that the TCP
connections are closed?



Cheers,
MyungJoo


>
> --
> tejun
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/linux-pm
>



-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics

^ 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