Netdev List
 help / color / mirror / Atom feed
* stmmac mitigation and ethtool coalesce parameters
From: Giuseppe CAVALLARO @ 2012-08-22  7:14 UTC (permalink / raw)
  To: ML netdev

Hello
I'm reworking the mitigation schema in the stmmac and removing the old
and dead STMMAC_TIMER code.

On new chips we have an HW RX-Watchdog that can be used for mitigating
the Rx-interrupts and first results look promising.

Before posting the patches I have a doubt about the ethtool coalesce
parameters to use in the driver to tune the following parameters:

- On Rx-side I have:
  a parameter that is the RI Watchdog Timer count. It indicates the
  number of system clock cycles.

- On Tx-side, the mitigation schema I'm using is based on a SW timer
  that calls the tx function (stmmac_tx) to reclaim the resource after
  transmitting the frames.
  Also there is another parameter (like a threshold) used to program
  the descriptors avoiding to set the interrupt on completion bit in
  when the frame is sent (xmit). This means that the stmmac_tx can be
  called by the ISR too.   This approach is showing really good figures
  as well.

so I wonder which are the appropriate ethtool coalesce parameters to
use? Or could I tune them by using sysfs entries?

Welcome advice.

Peppe

^ permalink raw reply

* Re[2]:  [PATCH 05/19] netfilter: nf_conntrack_ipv6: improve fragmentation handling
From: Julian Anastasov @ 2012-08-22  7:16 UTC (permalink / raw)
  To: Hans Schillstrom
  Cc: Jesper Dangaard Brouer, Patrick McHardy, netfilter-devel, netdev,
	Hans Schillstrom
In-Reply-To: <alpine.LFD.2.00.1208220954260.1698@ja.ssi.bg>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 736 bytes --]


	Hello,

On Wed, 22 Aug 2012, Julian Anastasov wrote:

> On Wed, 22 Aug 2012, Hans Schillstrom wrote:
> 
> > >Perhaps we could change/fix the MTU check in IPVS?
> > >(This would also solve issues I've seen with TSO/GSO frames, hitting
> > >this code path).
> > >
> > I ran into this as well, 
> > try this for the mtu check.
> > 
> >        if ((!skb->local_df && skb->len > mtu && !skb_is_gso(skb)) ||
> >            (IP6CB(skb)->frag_max_size && IP6CB(skb)->frag_max_size > mtu)) {
> 
> 	Better without local_df check, it is our job to
> set it.

	Ops, sorry. It seems now nf_ct_frag6_reasm (Patch 02/18)
will set head->local_df = 1, so we should check local_df
as Hans said.

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* [PATCH] iproute2: rtnl_wilddump_request: fix alignment for embedded platforms
From: Lutz Jaenicke @ 2012-08-22  7:38 UTC (permalink / raw)
  To: netdev; +Cc: Lutz Jaenicke

Platforms have different alignment requirements which need to be
fulfilled by the compiler. If the structure elements are already
4 byte (NLMGS_ALIGNTO) aligned by the compiler adding an explicit
padding element (align_rta) is not allowed.
Use __attribute__ ((aligned (NLMSG_ALIGNTO))) in order to achieve
the required alignment.
Experienced on ARM (xscale) with symptom
  netlink: 12 bytes leftover after parsing attributes

Signed-off-by: Lutz Jaenicke <ljaenicke@innominate.com>
---
 lib/libnetlink.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 8e8c8b9..05701ef 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -94,10 +94,9 @@ int rtnl_wilddump_request(struct rtnl_handle *rth, int family, int type)
 	struct {
 		struct nlmsghdr nlh;
 		struct rtgenmsg g;
-		__u16 align_rta;	/* attribute has to be 32bit aligned */
 		struct rtattr ext_req;
 		__u32 ext_filter_mask;
-	} req;
+	} req __attribute__ ((aligned (NLMSG_ALIGNTO)));
 
 	memset(&req, 0, sizeof(req));
 	req.nlh.nlmsg_len = sizeof(req);
-- 
1.7.2.5

^ permalink raw reply related

* Re: [PATCH 1/1] tcp: Wrong timeout for SYN segments
From: Eric Dumazet @ 2012-08-22  8:06 UTC (permalink / raw)
  To: Alex Bergmann
  Cc: davem, netdev, linux-kernel, Jerry Chu, Neal Cardwell,
	Nandita Dukkipati
In-Reply-To: <503419D3.1080700@linlab.net>

On Wed, 2012-08-22 at 01:29 +0200, Alex Bergmann wrote:
> Hi David,
> 
> I'm not 100% sure, but it looks like I found an RFC mismatch with the 
> current default values of the TCP implementation.
> 
> Alex
> 
> From 8b854a525eb45f64ad29dfab16f9d9f681e84495 Mon Sep 17 00:00:00 2001
> From: Alexander Bergmann <alex@linlab.net>
> Date: Wed, 22 Aug 2012 00:29:08 +0200
> Subject: [PATCH 1/1] tcp: Wrong timeout for SYN segments
> 
> Commit 9ad7c049 changed the initRTO from 3secs to 1sec in accordance to
> RFC6298 (former RFC2988bis). This introduced a gap with RFC1122 that
> defines a minimum retransmission window for SYN segments of at least
> 180secs.
> 
> Prior to 9ad7c049 the timeout was defined with 189secs. Now we have only
> a timeout of 63secs.
> 
>         ((2 << 5) - 1) * 3 secs = 189 secs
>         ((2 << 5) - 1) * 1 secs = 63 secs

Strange maths ... here I have :

(1+2+4+8+16) * 3 = 93 secs
vs
(1+2+4+8+16) * 1 = 31 secs

So even before said commit, we were not rfc1122 compliant.

Using 7 retries would give 127 seconds, still not rfc compliant.

> 
> To fulfill the MUST constraint in RFC1122 section 4.2.3.5 about R2 for
> SYN segments, the values of TCP_SYN_RETRIES and TCP_SYNACK_RETRIES must
> be changed to 7 reties.
> 
>         ((2 << 7) - 1) * 1 secs = 255 secs
> 
> This would result in an ETIMEDOUT of 4 minutes 15 seconds.
> 
> Signed-off-by: Alexander Bergmann <alex@linlab.net>
> ---
>  include/net/tcp.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 1f000ff..7eaae19 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -98,10 +98,10 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo);
>                                  * 15 is ~13-30min depending on RTO.
>                                  */
>  
> -#define TCP_SYN_RETRIES         5      /* number of times to retry active opening a
> +#define TCP_SYN_RETRIES         7      /* number of times to retry active opening a
>                                  * connection: ~180sec is RFC minimum   */
>  
> -#define TCP_SYNACK_RETRIES 5   /* number of times to retry passive opening a
> +#define TCP_SYNACK_RETRIES 7   /* number of times to retry passive opening a
>                                  * connection: ~180sec is RFC minimum   */
>  
>  #define TCP_TIMEWAIT_LEN (60*HZ) /* how long to wait to destroy TIME-WAIT

Nice catch !

I kind of disagree with the SYNACK part.

RFC 1122 says in 4.2.3.5 :

  However, the values of R1 and R2 may be different for SYN
  and data segments.  In particular, R2 for a SYN segment MUST
  be set large enough to provide retransmission of the segment
  for at least 3 minutes.  The application can close the
  connection (i.e., give up on the open attempt) sooner, of
  course.

I am not sure SYNACK segments were considered as a 'SYN segment' in this
section. (as the application cannot 'close' the connection on the passive
side, only kernel is aware of a SYN_RECV socket)

Increasing TCP_SYNACK_RETRIES from 5 to 7 or 8 amplifies the
SYN/synflood problem.

A valid client should retransmit its SYN packet for 180 seconds, I dont
believe we should make sure the SYNACK will be sent for 180 seconds as
well.

If we _really_ want to have a 3 minutes R2 for SYNACK, I suggest
changing things to that we dont send more than 5 SYNACKS, maybe using
RTO=6 after one retransmit

current situation :
1 sec
2 sec
4 sec
8 sec
16 sec
----
total of 31 seconds


1 sec
12 sec  // switch to RTO = 6
24 sec
48 sec
96 sec
-----
total of 181 seconds

^ permalink raw reply

* [PATCH] net: dev: fix the incorrect hold of net namespace's lo device
From: Gao feng @ 2012-08-22  8:31 UTC (permalink / raw)
  To: ebiederm; +Cc: davem, eric.dumazet, netdev, Gao feng

in dst_dev_event,we get the still referenced dst entries
from dst_garbage list,and call dst_ifdown to change these
dst entries' device to the net namesapce's lo device.

when we moving a net device(A) to another net namespace,
because free_fib_info_rcu is called after a grace period,
we may call dst_dev_event before free_fib_info_rcu putting
dst_entry into the dst_garbage list.

so in dst_dev_event, we can't see these dst entries through
dst_garbage list, and without changing their device to the
old net namespace's lo device. after a grace period, these
dst entries which dst->dev is device A will in the dst_garbage
list, and the device A will belong to the new net namespcae.

then we exit from this new net namespace, the dst_dev_event
is called again,it will get these dst entries from dst_garbage
list,and call dst_ifdown to hold the new net namespace's lo
device incorrectly and put the device A.

so it will tigger the emg message in netdev_wait_allrefs like
below.
unregister_netdevice: waiting for lo to become free. Usage count = 1

fix this problem by adding rcu_barrier() in dst_dev_event
when event is NETDEV_UNREGISTER.
with this,dst_ifdown will be called after the dst_garbage list
beeing updated.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 net/core/dst.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/core/dst.c b/net/core/dst.c
index 56d6361..38c2199 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -375,6 +375,7 @@ static int dst_dev_event(struct notifier_block *this, unsigned long event,
 
 	switch (event) {
 	case NETDEV_UNREGISTER:
+		rcu_barrier();
 	case NETDEV_DOWN:
 		mutex_lock(&dst_gc_mutex);
 		for (dst = dst_busy_list; dst; dst = dst->next) {
-- 
1.7.7.6

^ permalink raw reply related

* Re: [PATCH] net: dev: fix the incorrect hold of net namespace's lo device
From: Eric Dumazet @ 2012-08-22  8:39 UTC (permalink / raw)
  To: Gao feng; +Cc: ebiederm, davem, netdev
In-Reply-To: <1345624262-6554-1-git-send-email-gaofeng@cn.fujitsu.com>

On Wed, 2012-08-22 at 16:31 +0800, Gao feng wrote:
> in dst_dev_event,we get the still referenced dst entries
> from dst_garbage list,and call dst_ifdown to change these
> dst entries' device to the net namesapce's lo device.
> 
> when we moving a net device(A) to another net namespace,
> because free_fib_info_rcu is called after a grace period,
> we may call dst_dev_event before free_fib_info_rcu putting
> dst_entry into the dst_garbage list.
> 
> so in dst_dev_event, we can't see these dst entries through
> dst_garbage list, and without changing their device to the
> old net namespace's lo device. after a grace period, these
> dst entries which dst->dev is device A will in the dst_garbage
> list, and the device A will belong to the new net namespcae.
> 
> then we exit from this new net namespace, the dst_dev_event
> is called again,it will get these dst entries from dst_garbage
> list,and call dst_ifdown to hold the new net namespace's lo
> device incorrectly and put the device A.
> 
> so it will tigger the emg message in netdev_wait_allrefs like
> below.
> unregister_netdevice: waiting for lo to become free. Usage count = 1
> 
> fix this problem by adding rcu_barrier() in dst_dev_event
> when event is NETDEV_UNREGISTER.
> with this,dst_ifdown will be called after the dst_garbage list
> beeing updated.
> 
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> ---
>  net/core/dst.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/net/core/dst.c b/net/core/dst.c
> index 56d6361..38c2199 100644
> --- a/net/core/dst.c
> +++ b/net/core/dst.c
> @@ -375,6 +375,7 @@ static int dst_dev_event(struct notifier_block *this, unsigned long event,
>  
>  	switch (event) {
>  	case NETDEV_UNREGISTER:
> +		rcu_barrier();
>  	case NETDEV_DOWN:
>  		mutex_lock(&dst_gc_mutex);
>  		for (dst = dst_busy_list; dst; dst = dst->next) {


Did you miss http://patchwork.ozlabs.org/patch/176517/  or is this patch
an alternative ?

rcu_barrier() at this place will kill some workloads.

^ permalink raw reply

* Re: [PATCH 1/1] tcp: Wrong timeout for SYN segments
From: Alex Bergmann @ 2012-08-22  8:48 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: davem, netdev, linux-kernel, Jerry Chu, Neal Cardwell,
	Nandita Dukkipati
In-Reply-To: <1345622798.5158.717.camel@edumazet-glaptop>

On 08/22/2012 10:06 AM, Eric Dumazet wrote:
>> Prior to 9ad7c049 the timeout was defined with 189secs. Now we have only
>> a timeout of 63secs.
>>
>>          ((2 << 5) - 1) * 3 secs = 189 secs
>>          ((2 << 5) - 1) * 1 secs = 63 secs
> 
> Strange maths ... here I have :
> 
> (1+2+4+8+16) * 3 = 93 secs
> vs
> (1+2+4+8+16) * 1 = 31 secs
> 
> So even before said commit, we were not rfc1122 compliant.
> 
> Using 7 retries would give 127 seconds, still not rfc compliant.

You're missing the timeout after the 5th SYN packet was sent. This 
would result in another 32 seconds (96 seconds).

The timeout is calculated here:

net/ipv4/tcp_timer.c(146:150)

	if (boundary <= linear_backoff_thresh)
		timeout = ((2 << boundary) - 1) * rto_base;
	else
		timeout = ((2 << linear_backoff_thresh) - 1) * rto_base +
			(boundary - linear_backoff_thresh) * TCP_RTO_MAX;

> 
>>
>> To fulfill the MUST constraint in RFC1122 section 4.2.3.5 about R2 for
>> SYN segments, the values of TCP_SYN_RETRIES and TCP_SYNACK_RETRIES must
>> be changed to 7 reties.
>>
>>          ((2 << 7) - 1) * 1 secs = 255 secs
>>
>> This would result in an ETIMEDOUT of 4 minutes 15 seconds.
>>
>> Signed-off-by: Alexander Bergmann <alex@linlab.net>
>> ---
>>   include/net/tcp.h |    4 ++--
>>   1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/net/tcp.h b/include/net/tcp.h
>> index 1f000ff..7eaae19 100644
>> --- a/include/net/tcp.h
>> +++ b/include/net/tcp.h
>> @@ -98,10 +98,10 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo);
>>                                   * 15 is ~13-30min depending on RTO.
>>                                   */
>>   
>> -#define TCP_SYN_RETRIES         5      /* number of times to retry active opening a
>> +#define TCP_SYN_RETRIES         7      /* number of times to retry active opening a
>>                                   * connection: ~180sec is RFC minimum   */
>>   
>> -#define TCP_SYNACK_RETRIES 5   /* number of times to retry passive opening a
>> +#define TCP_SYNACK_RETRIES 7   /* number of times to retry passive opening a
>>                                   * connection: ~180sec is RFC minimum   */
>>   
>>   #define TCP_TIMEWAIT_LEN (60*HZ) /* how long to wait to destroy TIME-WAIT
> 
> Nice catch !
> 
> I kind of disagree with the SYNACK part.

I will look into this. 

> RFC 1122 says in 4.2.3.5 :
> 
>    However, the values of R1 and R2 may be different for SYN
>    and data segments.  In particular, R2 for a SYN segment MUST
>    be set large enough to provide retransmission of the segment
>    for at least 3 minutes.  The application can close the
>    connection (i.e., give up on the open attempt) sooner, of
>    course.
> 
> I am not sure SYNACK segments were considered as a 'SYN segment' in this
> section. (as the application cannot 'close' the connection on the passive
> side, only kernel is aware of a SYN_RECV socket)
> 
> Increasing TCP_SYNACK_RETRIES from 5 to 7 or 8 amplifies the
> SYN/synflood problem.
> 
> A valid client should retransmit its SYN packet for 180 seconds, I dont
> believe we should make sure the SYNACK will be sent for 180 seconds as
> well.
> 
> If we _really_ want to have a 3 minutes R2 for SYNACK, I suggest
> changing things to that we dont send more than 5 SYNACKS, maybe using
> RTO=6 after one retransmit
> 
> current situation :
> 1 sec
> 2 sec
> 4 sec
> 8 sec
> 16 sec
> ----
> total of 31 seconds
> 
> 
> 1 sec
> 12 sec  // switch to RTO = 6
> 24 sec
> 48 sec
> 96 sec
> -----
> total of 181 seconds
> 
> 
> 

^ permalink raw reply

* Re: [PATCH 1/1] tcp: Wrong timeout for SYN segments
From: Eric Dumazet @ 2012-08-22  8:58 UTC (permalink / raw)
  To: Alex Bergmann
  Cc: davem, netdev, linux-kernel, Jerry Chu, Neal Cardwell,
	Nandita Dukkipati
In-Reply-To: <50349CC5.3050601@linlab.net>

On Wed, 2012-08-22 at 10:48 +0200, Alex Bergmann wrote:
> On 08/22/2012 10:06 AM, Eric Dumazet wrote:
> >> Prior to 9ad7c049 the timeout was defined with 189secs. Now we have only
> >> a timeout of 63secs.
> >>
> >>          ((2 << 5) - 1) * 3 secs = 189 secs
> >>          ((2 << 5) - 1) * 1 secs = 63 secs
> > 
> > Strange maths ... here I have :
> > 
> > (1+2+4+8+16) * 3 = 93 secs
> > vs
> > (1+2+4+8+16) * 1 = 31 secs
> > 
> > So even before said commit, we were not rfc1122 compliant.
> > 
> > Using 7 retries would give 127 seconds, still not rfc compliant.
> 
> You're missing the timeout after the 5th SYN packet was sent. This 
> would result in another 32 seconds (96 seconds).
> 
> The timeout is calculated here:
> 
> net/ipv4/tcp_timer.c(146:150)
> 
> 	if (boundary <= linear_backoff_thresh)
> 		timeout = ((2 << boundary) - 1) * rto_base;
> 	else
> 		timeout = ((2 << linear_backoff_thresh) - 1) * rto_base +
> 			(boundary - linear_backoff_thresh) * TCP_RTO_MAX;

Thats the code yes but you miss the fact that last occurence of the
timer doesnt send a frame on the _network_

R2 is derived from the last frame sent.

Fact that the connect() is a bit long to return to user space is not
relevant. We could block the task for 2 hours and still be non RFC
compliant.

Actual 5 frames are sent, so the effective global timeout is the one I
quoted.

1 + 2 + 4 + 8 + 16   and its 31 

Just do a tcpdump and you can see it.

^ permalink raw reply

* Re: [PATCH] tun: don't zeroize sock->file on detach
From: Stanislav Kinsbursky @ 2012-08-22  9:14 UTC (permalink / raw)
  To: Neal Cardwell
  Cc: David Miller, dhowells@redhat.com, netdev@vger.kernel.org,
	rick.jones2@hp.com, ycheng@google.com,
	linux-kernel@vger.kernel.org, mikulas@artax.karlin.mff.cuni.cz
In-Reply-To: <CADVnQy=HGxFiczXGxb4iMumKUAXt77UzTWFqFwGzpLoQjvUR7Q@mail.gmail.com>

21.08.2012 21:18, Neal Cardwell пишет:
> On Tue, Aug 21, 2012 at 12:04 PM, Stanislav Kinsbursky
> <skinsbursky@parallels.com> wrote:
>> 10.08.2012 03:16, David Miller пишет:
>>
>>> From: Stanislav Kinsbursky <skinsbursky@parallels.com>
>>> Date: Thu, 09 Aug 2012 16:50:40 +0400
>>>
>>>> This is a fix for bug, introduced in 3.4 kernel by commit
>>>> 1ab5ecb90cb6a3df1476e052f76a6e8f6511cb3d, which, among other things,
>>>> replaced
>>>> simple sock_put() by sk_release_kernel(). Below is sequence, which leads
>>>> to
>>>> oops for non-persistent devices:
>>>>
>>>> tun_chr_close()
>>>> tun_detach()                            <== tun->socket.file = NULL
>>>> tun_free_netdev()
>>>> sk_release_sock()
>>>> sock_release(sock->file == NULL)
>>>> iput(SOCK_INODE(sock))                  <== dereference on NULL pointer
>>>>
>>>> This patch just removes zeroing of socket's file from __tun_detach().
>>>> sock_release() will do this.
>>>>
>>>> Cc: stable@vger.kernel.org
>>>> Reported-by: Ruan Zhijie <ruanzhijie@hotmail.com>
>>>> Tested-by: Ruan Zhijie <ruanzhijie@hotmail.com>
>>>> Acked-by: Al Viro <viro@ZenIV.linux.org.uk>
>>>> Acked-by: Eric Dumazet <edumazet@google.com>
>>>> Acked-by: Yuchung Cheng <ycheng@google.com>
>>>> Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
>>>
>>>
>>> Applied, thanks.
>>>
>>
>> Hi, David.
>> I found out, that this commit: b09e786bd1dd66418b69348cb110f3a64764626a
>> was previous attempt to fix the problem.
>> I believe this commit have to be dropped.
>
> Have you tried testing with that commit reverted? AFAICT from reading
> the code, if you revert b09e786bd1dd66418b69348cb110f3a64764626a then
> the sockets_in_use count becomes incorrect, because sock_release()
> will be calling this_cpu_sub() for each tun socket teardown when there
> was no corresponding this_cpu_add() for the tun socket (because the
> tun socket is not allocated with sock_alloc()).
>
> Can you sketch in more detail why that commit should be dropped?
>

Yep, I've noticed, that first commit patch fixes two problems simultaneously.
Here are they:
1) Dereference of invalid SOCK_INODE()
2) sockets_in_use incorrect value.

But I believe, that introducing new SOCK_EXTERNALLY_ALLOCATED socket flag and 
use it in generic code just to handle tun issues is overkill.
My patch solves first problem mush simpler, than mentioned commit.

About second problem...
What about this:

diff --git a/net/socket.c b/net/socket.c
index dfe5b66..dab462b 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -526,8 +526,8 @@ void sock_release(struct socket *sock)
         if (test_bit(SOCK_EXTERNALLY_ALLOCATED, &sock->flags))
                 return;

-       this_cpu_sub(sockets_in_use, 1);
         if (!sock->file) {
+               this_cpu_sub(sockets_in_use, 1);
                 iput(SOCK_INODE(sock));
                 return;
         }

?


> neal
>


-- 
Best regards,
Stanislav Kinsbursky

^ permalink raw reply related

* Re: [PATCH 00/18] netfilter: IPv6 NAT
From: David Miller @ 2012-08-22  9:28 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel, netdev
In-Reply-To: <1345434006-16549-1-git-send-email-kaber@trash.net>

From: Patrick McHardy <kaber@trash.net>
Date: Mon, 20 Aug 2012 05:39:48 +0200

> Following is the latest IPv6 NAT patchset, based on -rc2.

Feel free to push patch #1 via the nf tree when give the
final version of this to Pablo, and you can add my ACK to
it as well if you like:

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* Re: [PATCH 08/18] net: core: add function for incremental IPv6 pseudo header checksum updates
From: David Miller @ 2012-08-22  9:28 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel, netdev
In-Reply-To: <1345434006-16549-9-git-send-email-kaber@trash.net>

From: Patrick McHardy <kaber@trash.net>
Date: Mon, 20 Aug 2012 05:39:56 +0200

> Add inet_proto_csum_replace16 for incrementally updating IPv6 pseudo header
> checksums for IPv6 NAT.
> 
> Signed-off-by: Patrick McHardy <kaber@trash.net>

This is fine too:

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* Re: [PATCH 00/11] netlink: memory mapped I/O
From: David Miller @ 2012-08-22  9:29 UTC (permalink / raw)
  To: kaber; +Cc: Florian.Westphal, netdev, netfilter-devel
In-Reply-To: <1345443532-3707-1-git-send-email-kaber@trash.net>

From: Patrick McHardy <kaber@trash.net>
Date: Mon, 20 Aug 2012 08:18:41 +0200

> The following patches contain an implementation of memory mapped I/O for
> netlink, rebased onto the current net-next tree. The implementation is
> modelled after AF_PACKET memory mapped I/O with a few differences:

Thanks for taking this work so far.  Let me know when you have a version
with all the feedback integrated.

^ permalink raw reply

* Re: [PATCH 1/1] tcp: Wrong timeout for SYN segments
From: Alex Bergmann @ 2012-08-22  9:29 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: davem, netdev, linux-kernel, Jerry Chu, Neal Cardwell,
	Nandita Dukkipati
In-Reply-To: <1345625910.5158.793.camel@edumazet-glaptop>

On 08/22/2012 10:58 AM, Eric Dumazet wrote:
> On Wed, 2012-08-22 at 10:48 +0200, Alex Bergmann wrote:
>> On 08/22/2012 10:06 AM, Eric Dumazet wrote:
>>>> Prior to 9ad7c049 the timeout was defined with 189secs. Now we have only
>>>> a timeout of 63secs.
>>>>
>>>>           ((2 << 5) - 1) * 3 secs = 189 secs
>>>>           ((2 << 5) - 1) * 1 secs = 63 secs
>>>
>>> Strange maths ... here I have :
>>>
>>> (1+2+4+8+16) * 3 = 93 secs
>>> vs
>>> (1+2+4+8+16) * 1 = 31 secs
>>>
>>> So even before said commit, we were not rfc1122 compliant.
>>>
>>> Using 7 retries would give 127 seconds, still not rfc compliant.
>>
>> You're missing the timeout after the 5th SYN packet was sent. This
>> would result in another 32 seconds (96 seconds).
>>
>> The timeout is calculated here:
>>
>> net/ipv4/tcp_timer.c(146:150)
>>
>> 	if (boundary <= linear_backoff_thresh)
>> 		timeout = ((2 << boundary) - 1) * rto_base;
>> 	else
>> 		timeout = ((2 << linear_backoff_thresh) - 1) * rto_base +
>> 			(boundary - linear_backoff_thresh) * TCP_RTO_MAX;
>
> Thats the code yes but you miss the fact that last occurence of the
> timer doesnt send a frame on the _network_
>
> R2 is derived from the last frame sent.
>
> Fact that the connect() is a bit long to return to user space is not
> relevant. We could block the task for 2 hours and still be non RFC
> compliant.
>
> Actual 5 frames are sent, so the effective global timeout is the one I
> quoted.
>
> 1 + 2 + 4 + 8 + 16   and its 31
>
> Just do a tcpdump and you can see it.

Actual 6 SYN frames are sent. The initial one and 5 retries.

The kernel is waiting another 32 seconds for a SYN+ACK and then gives 
the ETIMEDOUT back to userspace.

Do you mean that we have to send another SYN packet after the 3 minutes?

^ permalink raw reply

* Re: [PATCH] libceph: Fix sparse warning
From: Daniel Baluta @ 2012-08-22  9:46 UTC (permalink / raw)
  To: sage; +Cc: davem, ceph-devel, netdev, Iulius Curt
In-Reply-To: <1344950857-32139-1-git-send-email-icurt@ixiacom.com>

On Tue, Aug 14, 2012 at 4:27 PM, Iulius Curt <iulius.curt@gmail.com> wrote:
> From: Iulius Curt <iulius.curt@gmail.com>
>
> Make ceph_monc_do_poolop() static to remove the following sparse warning:
>  * net/ceph/mon_client.c:616:5: warning: symbol 'ceph_monc_do_poolop' was not
>    declared. Should it be static?
>
> Signed-off-by: Iulius Curt <icurt@ixiacom.com>
> ---
>  net/ceph/mon_client.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ceph/mon_client.c b/net/ceph/mon_client.c
> index 105d533..3875c60 100644
> --- a/net/ceph/mon_client.c
> +++ b/net/ceph/mon_client.c
> @@ -613,7 +613,7 @@ bad:
>  /*
>   * Do a synchronous pool op.
>   */
> -int ceph_monc_do_poolop(struct ceph_mon_client *monc, u32 op,
> +static int ceph_monc_do_poolop(struct ceph_mon_client *monc, u32 op,
>                         u32 pool, u64 snapid,
>                         char *buf, int len)
>  {
> --
> 1.7.9.5
>
> --

Hi Sage,

Can you have a look on this? :)

thanks,
Daniel.

^ permalink raw reply

* Re: [PATCH 1/1] tcp: Wrong timeout for SYN segments
From: Eric Dumazet @ 2012-08-22  9:59 UTC (permalink / raw)
  To: Alex Bergmann
  Cc: davem, netdev, linux-kernel, Jerry Chu, Neal Cardwell,
	Nandita Dukkipati
In-Reply-To: <5034A678.3040207@linlab.net>

On Wed, 2012-08-22 at 11:29 +0200, Alex Bergmann wrote:

> Actual 6 SYN frames are sent. The initial one and 5 retries.
> 

first one had a t0 + 0 delay. How can it count ???

> The kernel is waiting another 32 seconds for a SYN+ACK and then gives 
> the ETIMEDOUT back to userspace.
> 
> Do you mean that we have to send another SYN packet after the 3 minutes?
> 

First SYN is not a retransmit

R2 = time_of_last_SYN - time_of_initial_SYN (t0) = 31

If you read RFC it states :

"In particular, R2 for a SYN segment MUST
 be set large enough to provide retransmission of the segment
 for at least 3 minutes."


That means that the last _retransmit_ MUST happen after 180 seconds.

And not :

Send all the restransmits at t0 + 1, then wait 180 seconds before giving
connect() a timeout indication.

^ permalink raw reply

* Re: [PATCH 1/1] tcp: Wrong timeout for SYN segments
From: Eric Dumazet @ 2012-08-22 10:03 UTC (permalink / raw)
  To: Alex Bergmann
  Cc: davem, netdev, linux-kernel, Jerry Chu, Neal Cardwell,
	Nandita Dukkipati
In-Reply-To: <1345629597.5158.924.camel@edumazet-glaptop>

On Wed, 2012-08-22 at 12:00 +0200, Eric Dumazet wrote:
> On Wed, 2012-08-22 at 11:29 +0200, Alex Bergmann wrote:
> 
> > Actual 6 SYN frames are sent. The initial one and 5 retries.
> > 
> 
> first one had a t0 + 0 delay. How can it count ???
> 
> > The kernel is waiting another 32 seconds for a SYN+ACK and then gives 
> > the ETIMEDOUT back to userspace.
> > 
> > Do you mean that we have to send another SYN packet after the 3 minutes?
> > 
> 
> First SYN is not a retransmit
> 
> R2 = time_of_last_SYN - time_of_initial_SYN (t0) = 31
> 
> If you read RFC it states :
> 
> "In particular, R2 for a SYN segment MUST
>  be set large enough to provide retransmission of the segment
>  for at least 3 minutes."
> 
> 
> That means that the last _retransmit_ MUST happen after 180 seconds.
> 
> And not :
> 
> Send all the restransmits at t0 + 1, then wait 180 seconds before giving
> connect() a timeout indication.
> 
> 

Therefore, the minimal connect() timeout should be : 180 + 100 seconds

(allowing 100 seconds for the SYNACKs sent in answer of the very last
retransmit to come back)

(100 seconds is the R2 for non SYN frames)

RFC quote : The value of R2 SHOULD
            correspond to at least 100 seconds. 

^ permalink raw reply

* Re: [PATCH] ipv4: properly update pmtu
From: Sylvain Munaut @ 2012-08-22 10:16 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, Julian Anastasov
In-Reply-To: <1345618109.5158.599.camel@edumazet-glaptop>

Hi,

> From: Eric Dumazet <edumazet@google.com>

> ip_rt_update_pmtu() calls dst_set_expires() to rearm a new expiration,
> but dst_set_expires() does nothing because dst.expires is already set.
>
> It seems we want to set the expires field to a new value, regardless
> of prior one.
>
> With help from Julian Anastasov.
>
> Reported-by: Sylvain Munaut <s.munaut@whatever-company.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> CC: Julian Anastasov <ja@ssi.bg>

Tested-by: Sylvain Munaut <s.munaut@whatever-company.com>

I confirm this corrects the issue for me. I tested this on 3 machines
with 2 different hw config that all previously exhibited the issue
short after boot and now they've been running with this version of the
patch for hours without problems.

@Eric: Thanks for looking into this.


Cheers,

    Sylvain

^ permalink raw reply

* Re: [PATCH] ipv4: properly update pmtu
From: Eric Dumazet @ 2012-08-22 10:21 UTC (permalink / raw)
  To: Sylvain Munaut; +Cc: David Miller, netdev, Julian Anastasov
In-Reply-To: <CAF6-1L77Z23dCfnee3nO3-V9U_tc1jz0ptXf9YcBcARGYnOvsw@mail.gmail.com>

On Wed, 2012-08-22 at 12:16 +0200, Sylvain Munaut wrote:
> Hi,
> 
> > From: Eric Dumazet <edumazet@google.com>
> 
> > ip_rt_update_pmtu() calls dst_set_expires() to rearm a new expiration,
> > but dst_set_expires() does nothing because dst.expires is already set.
> >
> > It seems we want to set the expires field to a new value, regardless
> > of prior one.
> >
> > With help from Julian Anastasov.
> >
> > Reported-by: Sylvain Munaut <s.munaut@whatever-company.com>
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > CC: Julian Anastasov <ja@ssi.bg>
> 
> Tested-by: Sylvain Munaut <s.munaut@whatever-company.com>
> 
> I confirm this corrects the issue for me. I tested this on 3 machines
> with 2 different hw config that all previously exhibited the issue
> short after boot and now they've been running with this version of the
> patch for hours without problems.
> 
> @Eric: Thanks for looking into this.

Thans Sylvain for being an early tester of bleeding edge kernel !

^ permalink raw reply

* Re: [PATCH] ipv4: properly update pmtu
From: Eric Dumazet @ 2012-08-22 10:23 UTC (permalink / raw)
  To: Sylvain Munaut; +Cc: David Miller, netdev, Julian Anastasov
In-Reply-To: <1345630873.5158.970.camel@edumazet-glaptop>

On Wed, 2012-08-22 at 12:21 +0200, Eric Dumazet wrote:

> Thans Sylvain for being an early tester of bleeding edge kernel !

Oh well, I meant to say Thanks ;)

^ permalink raw reply

* Hello...!!
From: Dr.Amadou Kebbi @ 2012-08-22 10:24 UTC (permalink / raw)


Good day

I work with a bank In West Africa,Burkina faso,I have a business
transaction for you.In my department we discovered an abandoned sum of
$10.5 Million US Dollars.In an account that belongs to one of our
foreign customer who died along with his entire family in plane crash.

Since his supposed next of kin died along side with him, there is
nobody to claim the left over balance in the account.It is therefore
upon this discovery that I and other officials in my department
decided to seek your assistance and present you to the bank as his
Next of kin or business associate.

If you accept i would give you the guide lines of how we can achieve
this transfer of the balance (10.5 Million Dollars) to your
account.and we will share the money 50-50%.Get back to me if you are
financially capable of handling this transaction.furnish me the below
information.for more details.

Your full name….
Your age and sex…
Your country......
Your Cell phone number..
Your Occupation....

My Regards to you and your family
Dr.Amadou Kebbi

N/B? Sorry if you received this letter in your spam, Due to recent
connection error here in the con.

^ permalink raw reply

* [PATCH (net.git) 1/2] stmmac: fix GMAC syn ID
From: Giuseppe CAVALLARO @ 2012-08-22 10:26 UTC (permalink / raw)
  To: netdev; +Cc: Giuseppe Cavallaro, Gianni Antoniazzi

Erroneously the DWMAC_CORE_3_40 was set to 34 instead of 0x34.
This can generate problems when run on old chips because
the driver assumes that there are the extra 16 regs available
for perfect filtering.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Gianni Antoniazzi <gianni.antoniazzi-ext@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac1000.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
index f90fcb5..b4c44a5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
@@ -229,6 +229,6 @@ enum rtc_control {
 #define GMAC_MMC_RX_CSUM_OFFLOAD   0x208
 
 /* Synopsys Core versions */
-#define	DWMAC_CORE_3_40	34
+#define	DWMAC_CORE_3_40	0x34
 
 extern const struct stmmac_dma_ops dwmac1000_dma_ops;
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH (net.git) 2/2] stmmac: fix a typo in the macro used to mask the mmc irq
From: Giuseppe CAVALLARO @ 2012-08-22 10:26 UTC (permalink / raw)
  To: netdev; +Cc: Giuseppe Cavallaro
In-Reply-To: <1345631176-29494-1-git-send-email-peppe.cavallaro@st.com>

This patch fixes the name of the macro used to mask the
mmc interrupt: erroneously it was used: MMC_DEFAUL_MASK.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/mmc_core.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/mmc_core.c b/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
index c07cfe9..0c74a70 100644
--- a/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
@@ -33,7 +33,7 @@
 #define MMC_TX_INTR		0x00000108	/* MMC TX Interrupt */
 #define MMC_RX_INTR_MASK	0x0000010c	/* MMC Interrupt Mask */
 #define MMC_TX_INTR_MASK	0x00000110	/* MMC Interrupt Mask */
-#define MMC_DEFAUL_MASK		0xffffffff
+#define MMC_DEFAULT_MASK		0xffffffff
 
 /* MMC TX counter registers */
 
@@ -147,8 +147,8 @@ void dwmac_mmc_ctrl(void __iomem *ioaddr, unsigned int mode)
 /* To mask all all interrupts.*/
 void dwmac_mmc_intr_all_mask(void __iomem *ioaddr)
 {
-	writel(MMC_DEFAUL_MASK, ioaddr + MMC_RX_INTR_MASK);
-	writel(MMC_DEFAUL_MASK, ioaddr + MMC_TX_INTR_MASK);
+	writel(MMC_DEFAULT_MASK, ioaddr + MMC_RX_INTR_MASK);
+	writel(MMC_DEFAULT_MASK, ioaddr + MMC_TX_INTR_MASK);
 }
 
 /* This reads the MAC core counters (if actaully supported).
-- 
1.7.4.4

^ permalink raw reply related

* Re: [PATCH] netvm: check for page == NULL when propogating the skb->pfmemalloc flag
From: Ian Campbell @ 2012-08-22 10:26 UTC (permalink / raw)
  To: David Miller
  Cc: mgorman@suse.de, linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, xen-devel@lists.xensource.com,
	konrad@darnok.org, akpm@linux-foundation.org
In-Reply-To: <20120808.155046.820543563969484712.davem@davemloft.net>

On Wed, 2012-08-08 at 23:50 +0100, David Miller wrote:
> Just use something like a call to __pskb_pull_tail(skb, len) and all
> that other crap around that area can simply be deleted.

I think you mean something like this, which works for me, although I've
only lightly tested it.

Ian.

8<----------------------------------------

>From 9e47e3e87a33b45974448649a97859a479183041 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Wed, 22 Aug 2012 10:15:29 +0100
Subject: [PATCH] xen-netfront: use __pskb_pull_tail to ensure linear area is big enough on RX

I'm slightly concerned by the "only in exceptional circumstances"
comment on __pskb_pull_tail but the structure of an skb just created
by netfront shouldn't hit any of the especially slow cases.

This approach still does slightly more work than the old way, since if
we pull up the entire first frag we now have to shuffle everything
down where before we just received into the right place in the first
place.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Mel Gorman <mgorman@suse.de>
Cc: xen-devel@lists.xensource.com
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/net/xen-netfront.c |   39 ++++++++++-----------------------------
 1 files changed, 10 insertions(+), 29 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 3089990..650f79a 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -57,8 +57,7 @@
 static const struct ethtool_ops xennet_ethtool_ops;
 
 struct netfront_cb {
-	struct page *page;
-	unsigned offset;
+	int pull_to;
 };
 
 #define NETFRONT_SKB_CB(skb)	((struct netfront_cb *)((skb)->cb))
@@ -867,15 +866,9 @@ static int handle_incoming_queue(struct net_device *dev,
 	struct sk_buff *skb;
 
 	while ((skb = __skb_dequeue(rxq)) != NULL) {
-		struct page *page = NETFRONT_SKB_CB(skb)->page;
-		void *vaddr = page_address(page);
-		unsigned offset = NETFRONT_SKB_CB(skb)->offset;
-
-		memcpy(skb->data, vaddr + offset,
-		       skb_headlen(skb));
+		int pull_to = NETFRONT_SKB_CB(skb)->pull_to;
 
-		if (page != skb_frag_page(&skb_shinfo(skb)->frags[0]))
-			__free_page(page);
+		__pskb_pull_tail(skb, pull_to - skb_headlen(skb));
 
 		/* Ethernet work: Delayed to here as it peeks the header. */
 		skb->protocol = eth_type_trans(skb, dev);
@@ -913,7 +906,6 @@ static int xennet_poll(struct napi_struct *napi, int budget)
 	struct sk_buff_head errq;
 	struct sk_buff_head tmpq;
 	unsigned long flags;
-	unsigned int len;
 	int err;
 
 	spin_lock(&np->rx_lock);
@@ -955,24 +947,13 @@ err:
 			}
 		}
 
-		NETFRONT_SKB_CB(skb)->page =
-			skb_frag_page(&skb_shinfo(skb)->frags[0]);
-		NETFRONT_SKB_CB(skb)->offset = rx->offset;
-
-		len = rx->status;
-		if (len > RX_COPY_THRESHOLD)
-			len = RX_COPY_THRESHOLD;
-		skb_put(skb, len);
+		NETFRONT_SKB_CB(skb)->pull_to = rx->status;
+		if (NETFRONT_SKB_CB(skb)->pull_to > RX_COPY_THRESHOLD)
+			NETFRONT_SKB_CB(skb)->pull_to = RX_COPY_THRESHOLD;
 
-		if (rx->status > len) {
-			skb_shinfo(skb)->frags[0].page_offset =
-				rx->offset + len;
-			skb_frag_size_set(&skb_shinfo(skb)->frags[0], rx->status - len);
-			skb->data_len = rx->status - len;
-		} else {
-			__skb_fill_page_desc(skb, 0, NULL, 0, 0);
-			skb_shinfo(skb)->nr_frags = 0;
-		}
+		skb_shinfo(skb)->frags[0].page_offset = rx->offset;
+		skb_frag_size_set(&skb_shinfo(skb)->frags[0], rx->status);
+		skb->data_len = rx->status;
 
 		i = xennet_fill_frags(np, skb, &tmpq);
 
@@ -999,7 +980,7 @@ err:
 		 * receive throughout using the standard receive
 		 * buffer size was cut by 25%(!!!).
 		 */
-		skb->truesize += skb->data_len - (RX_COPY_THRESHOLD - len);
+		skb->truesize += skb->data_len - RX_COPY_THRESHOLD;
 		skb->len += skb->data_len;
 
 		if (rx->flags & XEN_NETRXF_csum_blank)
-- 
1.7.2.5

^ permalink raw reply related

* NULL deref in bnx2 / crashes ? ( was: netconsole leads to stalled CPU task )
From: Sylvain Munaut @ 2012-08-22 10:53 UTC (permalink / raw)
  To: netdev

Hi again, a bit more detail:

> I'm trying to use the netconsole to feed kernel message to the outside
> but this lead to a stall ...
>
> This only happens in a fairly specific configuration where you have a
> bridge over vlan over bonding.
> I tested with only (bridge over vlan) and (vlan over bonding) and
> those work fine.
>
> [snip ... see original mail for all details]

I was previously testing under Xen.

For this round of test, I tried the kernel natively. And I also
included Dave Miller pending series ( e0e3cea4... ) since there was
patch related to netconsole and bridging / ...
So in the end, it's a 3.6-rc2 + Dave Miller tree (commit  e0e3cea4 ) +
pf malloc patch  + ip pmtu patch from Eric Dumazet.

I am now seeing more debug when I load netconsole in that config:

[   88.705138] netpoll: netconsole: local port 8888
[   88.705140] netpoll: netconsole: local IP 10.208.1.30
[   88.705141] netpoll: netconsole: interface 'mgmt'
[   88.705142] netpoll: netconsole: remote port 8000
[   88.705143] netpoll: netconsole: remote IP 10.208.1.3
[   88.705144] netpoll: netconsole: remote ethernet address 00:16:3e:1a:37:37
[   88.705469] BUG: unable to handle kernel NULL pointer dereference
at 0000000000000008
[   88.705475] IP: [<ffffffffa0006653>] bnx2_start_xmit+0x20b/0x539 [bnx2]
[   88.705476] PGD 0
[   88.705478] Oops: 0002 [#1] PREEMPT SMP
[   88.705509] Modules linked in: netconsole(+) configfs nfsd
auth_rpcgss nfs_acl nfs lockd fscache sunrpc bridge 8021q garp stp llc
bonding ext2 iTCO_wdt iTCO_vendor_support lpc_ich mfd_core coretemp
joydev kvm evdev crc32c_intel ghash_clmulni_intel aesni_intel
aes_x86_64 aes_generic acpi_power_meter psmouse serio_raw dcdbas
processor ablk_helper i7core_edac pcspkr cryptd edac_core microcode
button hid_generic ext4 crc16 jbd2 mbcache dm_mod raid10 raid456
async_raid6_recov async_memcpy async_pq async_xor xor async_tx
raid6_pq raid1 raid0 multipath linear md_mod sr_mod usbhid cdrom hid
ses sd_mod enclosure crc_t10dif usb_storage ata_generic pata_acpi uas
uhci_hcd megaraid_sas ata_piix ehci_hcd libata usbcore scsi_mod
usb_common bnx2
[   88.705511] CPU 2
[   88.705512] Pid: 3017, comm: modprobe Not tainted
3.6.0-rc2-00092-g9040592-dirty #6 Dell Inc. PowerEdge R610/0F0XJ6
[   88.705515] RIP: 0010:[<ffffffffa0006653>]  [<ffffffffa0006653>]
bnx2_start_xmit+0x20b/0x539 [bnx2]
[   88.705516] RSP: 0018:ffff88061e8fda28  EFLAGS: 00010002
[   88.705517] RAX: 0000000000000000 RBX: ffff8803200f2300 RCX: 0000000000000000
[   88.705519] RDX: 0000000320a95c02 RSI: 0000000000000003 RDI: ffff8800cb36f000
[   88.705519] RBP: ffff88031f814000 R08: 0000000000000054 R09: 0000000000000000
[   88.705520] R10: 000000000000ffff R11: 0000000000000000 R12: ffff8803215d52c0
[   88.705521] R13: ffff8803210e13c0 R14: 0000000000010008 R15: 0000000000000000
[   88.705522] FS:  00007fe9d0854700(0000) GS:ffff88062fc20000(0000)
knlGS:0000000000000000
[   88.705523] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[   88.705524] CR2: 0000000000000008 CR3: 0000000619ccb000 CR4: 00000000000007e0
[   88.705525] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   88.705526] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[   88.705528] Process modprobe (pid: 3017, threadinfo
ffff88061e8fc000, task ffff8806205e8000)
[   88.705528] Stack:
[   88.705530]  ffff88062ffecd80 0000000320a95c02 0000000000000054
ffffffff00000000
[   88.705532]  0000000000000041 ffff8803215d55f8 ffff88031f8167d8
ffffffff00000000
[   88.705534]  0000000000000000 0000000100000000 ffff88062ffedb08
ffff8803200f2300
[   88.705534] Call Trace:
[   88.705542]  [<ffffffff81280a76>] ? netpoll_send_skb_on_dev+0x201/0x31d
[   88.705546]  [<ffffffffa007fc4c>] ? bond_dev_queue_xmit+0x62/0x7f [bonding]
[   88.705549]  [<ffffffffa0084588>] ? bond_3ad_xmit_xor+0xe7/0x10c [bonding]
[   88.705552]  [<ffffffffa007fffd>] ? bond_start_xmit+0x394/0x3ff [bonding]
[   88.705554]  [<ffffffff81280a76>] ? netpoll_send_skb_on_dev+0x201/0x31d
[   88.705558]  [<ffffffffa004afd5>] ?
vlan_dev_hard_start_xmit+0xab/0xf6 [8021q]
[   88.705559]  [<ffffffff81280a76>] ? netpoll_send_skb_on_dev+0x201/0x31d
[   88.705564]  [<ffffffffa00938e8>] ? __br_deliver+0x93/0xbe [bridge]
[   88.705567]  [<ffffffffa009237d>] ? br_dev_xmit+0x14a/0x16b [bridge]
[   88.705569]  [<ffffffff81280a76>] ? netpoll_send_skb_on_dev+0x201/0x31d
[   88.705570]  [<ffffffff81280372>] ? find_skb.isra.23+0x31/0x78
[   88.705572]  [<ffffffff81280bbe>] ? netpoll_send_skb+0x2c/0x39
[   88.705574]  [<ffffffffa00a222a>] ? write_msg+0x98/0xf3 [netconsole]
[   88.705579]  [<ffffffff81037db2>] ?
call_console_drivers.constprop.17+0x6e/0x7d
[   88.705580]  [<ffffffff81038248>] ? console_unlock+0x2ab/0x351
[   88.705582]  [<ffffffff81039112>] ? register_console+0x273/0x303
[   88.705584]  [<ffffffffa00fa182>] ? init_netconsole+0x182/0x210 [netconsole]
[   88.705586]  [<ffffffffa00fa000>] ? 0xffffffffa00f9fff
[   88.705588]  [<ffffffff81002085>] ? do_one_initcall+0x75/0x12c
[   88.705590]  [<ffffffff81077b35>] ? sys_init_module+0x80/0x1c5
[   88.705593]  [<ffffffff813319b9>] ? system_call_fastpath+0x16/0x1b
[   88.705606] Code: 41 c1 e1 10 48 89 d6 48 6b c8 18 48 c1 e0 04 48
c1 ee 20 49 03 8c 24 50 03 00 00 45 09 c8 44 89 4c 24 38 c7 44 24 24
00 00 00 00 <48> 89 51 08 48 89 19 49 03 84 24 48 03 00 00 89 50 04 44
89 f2
[   88.705608] RIP  [<ffffffffa0006653>] bnx2_start_xmit+0x20b/0x539 [bnx2]
[   88.705609]  RSP <ffff88061e8fda28>
[   88.705609] CR2: 0000000000000008
[   88.705611] ---[ end trace 24b75fe520341c20 ]---
[   88.705985] note: modprobe[3017] exited with preempt_count 6
[   88.706135] Dead loop on virtual device mgmt, fix it urgently!
[   88.706201] Dead loop on virtual device mgmt, fix it urgently!
[  148.557967] INFO: rcu_preempt detected stalls on CPUs/tasks: {}
(detected by 0, t=60002 jiffies)
[  148.557967] INFO: Stall ended before state dump start
[  328.112761] INFO: rcu_preempt detected stalls on CPUs/tasks: {}
(detected by 2, t=240007 jiffies)
[  328.112761] INFO: Stall ended before state dump start


And when trying on another machine that has Intel network cards, it
just completely freezes the machine ... nothing even gets printed on
the screen or anywhere I can see.

Also note that this also doesn't work in 3.5.1 so it's not a new
behavior. 3.2.x don't support netconsole over vlan at all so can't
test on it.

Cheers,

    Sylvain Munaut

^ permalink raw reply

* Re: [PATCH] net: dev: fix the incorrect hold of net namespace's lo device
From: Gao feng @ 2012-08-22 11:00 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: ebiederm, davem, netdev
In-Reply-To: <1345624786.5158.759.camel@edumazet-glaptop>

On Wed, 2012-08-22 at 16:39 +0800, Eric Dumazet wrote:
> On Wed, 2012-08-22 at 16:31 +0800, Gao feng wrote:
>> in dst_dev_event,we get the still referenced dst entries
>> from dst_garbage list,and call dst_ifdown to change these
>> dst entries' device to the net namesapce's lo device.
>>
>> when we moving a net device(A) to another net namespace,
>> because free_fib_info_rcu is called after a grace period,
>> we may call dst_dev_event before free_fib_info_rcu putting
>> dst_entry into the dst_garbage list.
>>
>> so in dst_dev_event, we can't see these dst entries through
>> dst_garbage list, and without changing their device to the
>> old net namespace's lo device. after a grace period, these
>> dst entries which dst->dev is device A will in the dst_garbage
>> list, and the device A will belong to the new net namespcae.
>>
>> then we exit from this new net namespace, the dst_dev_event
>> is called again,it will get these dst entries from dst_garbage
>> list,and call dst_ifdown to hold the new net namespace's lo
>> device incorrectly and put the device A.
>>
>> so it will tigger the emg message in netdev_wait_allrefs like
>> below.
>> unregister_netdevice: waiting for lo to become free. Usage count = 1
>>
>> fix this problem by adding rcu_barrier() in dst_dev_event
>> when event is NETDEV_UNREGISTER.
>> with this,dst_ifdown will be called after the dst_garbage list
>> beeing updated.
>>
>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>> ---
>>  net/core/dst.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/core/dst.c b/net/core/dst.c
>> index 56d6361..38c2199 100644
>> --- a/net/core/dst.c
>> +++ b/net/core/dst.c
>> @@ -375,6 +375,7 @@ static int dst_dev_event(struct notifier_block *this, unsigned long event,
>>  
>>  	switch (event) {
>>  	case NETDEV_UNREGISTER:
>> +		rcu_barrier();
>>  	case NETDEV_DOWN:
>>  		mutex_lock(&dst_gc_mutex);
>>  		for (dst = dst_busy_list; dst; dst = dst->next) {
> 
> 
> Did you miss http://patchwork.ozlabs.org/patch/176517/  or is this patch
> an alternative ?
> 

Hi Eric

I saw your patch and think this patch is clear and doesn't change too much logic.

I test your patch, it not fix this problem.

In my test case,when moving a net device to another net namespace,
Because you patch delete NETDEV_UNREGISTER event from dst_dev_event,
we will just put dst entries into the dst garbage list in event
NETDEV_DOWN,without call dst_ifdown to change these dst entries' device
to the lo device,and now this net device belongs to the new net namespace.

After the net device beeing moved to another net namespace, I rmmod this
net device's driver,this will trigger the new added event NETDEV_UNREGISTER_FINISH,
so in dst_dev_event,we will change these dst entries's device to the new net
namespace's lo device,and this will make the referenct count of the new net namespace's
lo device incorrect. when we exit the new net namespace,this emg message is still exist.

Message from syslogd@Donkey at Aug 22 18:50:13 ...
 kernel:[ 1161.979036] unregister_netdevice: waiting for lo to become free. Usage count = 1

And because net_mutex is locked here,so we can't create new net namespace.

> rcu_barrier() at this place will kill some workloads.
> 

I think this will only add some workloads when unregister a net device.
Do I miss something?

^ 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