headers
Nicolas de Pesloüan | 1 Jan 2012 01:09
Picon

Re: [PATCH] bonding: fix error handling if slave is busy (v2)

Le 01/01/2012 00:26, Stephen Hemminger a écrit :
> If slave device already has a receive handler registered, then the
> error unwind of bonding device enslave function is broken.
>
> The following will leave a pointer to freed memory in the slave
> device list, causing a later kernel panic.
> # modprobe dummy
> # ip li add dummy0-1 link dummy0 type macvlan
> # modprobe bonding
> # echo +dummy0>/sys/class/net/bond0/bonding/slaves
>
> The fix is to detach the slave (which removes it from the list)
> in the unwind path.
>
> Signed-off-by: Stephen Hemminger<shemminger <at> vyatta.com>

Thanks Stephen.

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian <at> free.fr>

> ---
> v2 - need to keep original err_close for other unwind
>
> --- a/drivers/net/bonding/bond_main.c	2011-12-30 14:20:03.171823181 -0800
> +++ b/drivers/net/bonding/bond_main.c	2011-12-31 15:20:16.493379415 -0800
>  <at>  <at>  -1822,7 +1822,7  <at>  <at>  int bond_enslave(struct net_device *bond
>   				 "but new slave device does not support netpoll.\n",
>   				 bond_dev->name);
>   			res = -EBUSY;
> -			goto err_close;
(Continue reading)

Permalink | Reply | 3 comments |
headers
Stephen Hemminger | 1 Jan 2012 01:13
Favicon

Re: [PATCH] bonding: fix error handling if slave is busy (v2)

On Sun, 01 Jan 2012 01:09:50 +0100
Nicolas de Pesloüan <nicolas.2p.debian <at> gmail.com> wrote:

> Le 01/01/2012 00:26, Stephen Hemminger a écrit :
> > If slave device already has a receive handler registered, then the
> > error unwind of bonding device enslave function is broken.
> >
> > The following will leave a pointer to freed memory in the slave
> > device list, causing a later kernel panic.
> > # modprobe dummy
> > # ip li add dummy0-1 link dummy0 type macvlan
> > # modprobe bonding
> > # echo +dummy0>/sys/class/net/bond0/bonding/slaves
> >
> > The fix is to detach the slave (which removes it from the list)
> > in the unwind path.
> >
> > Signed-off-by: Stephen Hemminger<shemminger <at> vyatta.com>
> 
> Thanks Stephen.
> 
> Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian <at> free.fr>

The locking in bond driver is a tangled web.

Would be cleaner to get rid of bond->lock altogether.
Slave add/delete should be protected by RTNL, and the lookup should
be converted to RCU.  The problem is that bonding driver implements
own form of circular list to handle round-robin etc.
(Continue reading)

Permalink | Reply | 1 comment |
headers
Nicolas de Pesloüan | 1 Jan 2012 01:28
Picon

Re: [PATCH] bonding: fix error handling if slave is busy (v2)

Le 01/01/2012 01:13, Stephen Hemminger a écrit :
> On Sun, 01 Jan 2012 01:09:50 +0100
> Nicolas de Pesloüan<nicolas.2p.debian <at> gmail.com>  wrote:
>
>> Le 01/01/2012 00:26, Stephen Hemminger a écrit :
>>> If slave device already has a receive handler registered, then the
>>> error unwind of bonding device enslave function is broken.
>>>
>>> The following will leave a pointer to freed memory in the slave
>>> device list, causing a later kernel panic.
>>> # modprobe dummy
>>> # ip li add dummy0-1 link dummy0 type macvlan
>>> # modprobe bonding
>>> # echo +dummy0>/sys/class/net/bond0/bonding/slaves
>>>
>>> The fix is to detach the slave (which removes it from the list)
>>> in the unwind path.
>>>
>>> Signed-off-by: Stephen Hemminger<shemminger <at> vyatta.com>
>>
>> Thanks Stephen.
>>
>> Reviewed-by: Nicolas de Pesloüan<nicolas.2p.debian <at> free.fr>
>
> The locking in bond driver is a tangled web.
>
> Would be cleaner to get rid of bond->lock altogether.
> Slave add/delete should be protected by RTNL, and the lookup should
> be converted to RCU.  The problem is that bonding driver implements
> own form of circular list to handle round-robin etc.
(Continue reading)

Permalink | Reply | 0 comments |
headers
John A. Sullivan III | 1 Jan 2012 03:30

tc filter mask for ACK packets off?

Hello, all.  I've been noticing that virtually all the documentation
says we should prioritize ACK only packets and that they can be
identified with match u8 0x10 0xff.  However, isn't the actual flag
field only 6 bits longs and the first two belong to a previous 6 bit
reserved field?
If that is true, if ever those bits are set, our filters will
unnecessarily break.  Shouldn't it be match u8 0x10 0x3f? Then again,
I'm very new at this.  Thanks - John
Permalink | Reply | 0 comments |
headers
John A. Sullivan III | 1 Jan 2012 03:30

tc filter mask for ACK packets off?

Hello, all.  I've been noticing that virtually all the documentation
says we should prioritize ACK only packets and that they can be
identified with match u8 0x10 0xff.  However, isn't the actual flag
field only 6 bits longs and the first two belong to a previous 6 bit
reserved field?
If that is true, if ever those bits are set, our filters will
unnecessarily break.  Shouldn't it be match u8 0x10 0x3f? Then again,
I'm very new at this.  Thanks - John
Permalink | Reply | 15 comments |
headers
Pablo Neira Ayuso | 1 Jan 2012 17:23
Favicon

Re: [PATCH] fix for nfacct infrastructure in net-next

On Sun, Jan 01, 2012 at 05:22:24PM +0100, pablo <at> netfilter.org wrote:
> From: Pablo Neira Ayuso <pablo <at> netfilter.org>
> 
> Hi Dave,
> 
> Please, pull the following fix for the nfacct infrastructure already
> in your net-next tree.
> 
> You can pull it from:
> 
> git://1984.lsi.us.es/net-net nf-next
                       ^^^^^^^
I meant to say net-next, of course.

Sorry.
Permalink | Reply | 3 comments |
headers
Pablo Neira Ayuso | 2 Jan 2012 02:04
Favicon

Re: [PATCH] fix for nfacct infrastructure in net-next

Hi Dave,

On Sun, Jan 01, 2012 at 06:02:29PM -0500, David Miller wrote:
> I'm not pulling this.
> 
> If I pull from your "corrected" URL, "git://1984.lsi.us.es/net-next nf-next"
> it starts to download 17937 objects.  Which means it's going to bring in all
> of your -next work as well as my net-next tree upon which it is based, not
> just this one bug fix.
>
> These mistakes should never happen in the first place.
> 
> If you actually use "git request-pull" to generate the pull request
> email body, you can NEVER get the URL wrong, GIT enforces it to be
> correct and it enforces that the changes in your local tree are
> present at the remote URL as well.

You caught me here, I wasn't using git request-pull. I'll do next time,
sorry.

> You can't make these kinds of errors and oversights when you want to
> ask me to pull in bug fixes at the very last minute when Linus is
> about to make a final release.

I think there's a misunderstanding.

This is not intended for current 3.2-rc7. This is a fix for something
that is in your net-next tree (upcoming 3.3).

I can wait and resubmit this later, we have the entire 3.3-rc cycle to
(Continue reading)

Permalink | Reply | 1 comment |
headers
Matt Ginzton | 2 Jan 2012 03:03

Re: reproducible kernel freeze using r8169 in Linux 3.0

On Dec 18, 2011, at 12:32 AM, Francois Romieu wrote:

> Matt Ginzton <matt <at> ginzton.net> :
> [...]
>> Please let me know if I'm reporting this to the right place, or if there's
>> any other helpful info I can provide.
> 
> Please grep for a XID line in dmesg and send it so we can figure the
> specific revision of your chipset (lspci can not figure it).
> 
> If it says something like "blah blah XID 1c4...", you should give current
> -rc a try (who shouldn't ?).

Just to follow up on this old thread:

- my XID line says 1c4000c0
- I tried 3.2-rc7 kernel, and couldn't reproduce any problems at all -- not the complete system freeze from
3.0 kernels, nor the slow_path kernel warning and backtrace from 3.1 kernels. A big improvement indeed --
thank you.

I do note that TX performance as measured by pktgen is lower than r8168 -- with r8169 in 3.2-rc7 I see
something like 455MB/sec and with r8168, IIRC, the number was closer to 800MB/sec. This may be a very
artificial statistic, and isn't something I happen to care about and I'm certainly not going to use r8168
because of it, but it may be worth looking into?

cheers,

Matt
(Continue reading)

Permalink | Reply | 2 comments |
headers
David Ward | 2 Jan 2012 04:32
Picon

[PATCH net-next] xfrm: Call IP receive handler directly for inbound tunnel-mode packets

For IPsec tunnel mode (or BEET mode), after inbound packets are xfrm'ed,
call the IPv4/IPv6 receive handler directly instead of calling netif_rx.
In addition to avoiding unneeded re-processing of the MAC layer, packets
will not be received a second time on network taps. (Note that outbound
packets are only received on network taps post-xfrm, but inbound packets
were being received both pre- and post-xfrm. So now network taps will
receive packets in either direction only once, in the form that they go
"over the wire".)

Signed-off-by: David Ward <david.ward <at> ll.mit.edu>
Cc: Herbert Xu <herbert <at> gondor.apana.org.au>
---
 include/net/xfrm.h     |    3 +++
 net/ipv4/xfrm4_input.c |    5 +++++
 net/ipv4/xfrm4_state.c |    1 +
 net/ipv6/xfrm6_input.c |    5 +++++
 net/ipv6/xfrm6_state.c |    1 +
 net/xfrm/xfrm_input.c  |    4 +++-
 6 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index b203e14..423a779 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
 <at>  <at>  -329,6 +329,7  <at>  <at>  struct xfrm_state_afinfo {
 						 struct sk_buff *skb);
 	int			(*extract_output)(struct xfrm_state *x,
 						  struct sk_buff *skb);
+	int			(*tunnel_finish)(struct sk_buff *skb);
 	int			(*transport_finish)(struct sk_buff *skb,
(Continue reading)

Permalink | Reply | 5 comments |
headers
Eric Dumazet | 2 Jan 2012 05:33
Picon

Re: BQL + Basic Latency under load results - 100Mbit, GSO/TSO off, pfifo_fast vs SFQ vs QFQ

Le lundi 02 janvier 2012 à 00:17 +0100, Dave Taht a écrit :
> QFQ wins even bigger vs SFQ at 50 iperfs
> 
> http://www.teklibre.com/~d/bloat/pfifo_sfq_vs_qfq_linear50.png
> 
> And I think it's going to win even bigger at 10 Mbit.
> 

Happy new year !

This makes no sense to me for such a low amount of flows, SFQ should
perform the same than QFQ :)

You dont find out why it is so.

Please try following patch :

[PATCH net-next] sch_sfq: dont put new flow at the end of flows

SFQ enqueue algo puts a new flow _behind_ all pre-existing flows in the
circular list. In fact this is probably an old SFQ implementation bug.

100 Mbits = ~8333 full frames per second, or ~8 frames per ms.

With 50 flows, it means your "new flow" will have to wait 50 packets
being sent before its own packet. Thats the ~6ms.

We certainly can change SFQ to give a priority advantage to new flows,
so that next dequeued packet is taken from a new flow, not an old one.
(Continue reading)

Permalink | Reply | 4 comments |

Gmane