* 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 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 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/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] 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 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 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] 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
* [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 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] 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[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
* 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:07 UTC (permalink / raw)
To: Hans Schillstrom
Cc: Jesper Dangaard Brouer, Patrick McHardy, netfilter-devel, netdev,
Hans Schillstrom
In-Reply-To: <mdaatbj.b2f3561b17fe4f00a6e947611a0959a1@obelix.schillstrom.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 532 bytes --]
Hello,
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.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH net-next] packet: Protect packet sk list with mutex (v2)
From: Eric Dumazet @ 2012-08-22 6:59 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: David Miller, Linux Netdev List
In-Reply-To: <50336BC7.7030209@parallels.com>
On Tue, 2012-08-21 at 15:06 +0400, Pavel Emelyanov wrote:
> Change since v1:
>
> * Fixed inuse counters access spotted by Eric
>
> In patch eea68e2f (packet: Report socket mclist info via diag module) I've
> introduced a "scheduling in atomic" problem in packet diag module -- the
> socket list is traversed under rcu_read_lock() while performed under it sk
> mclist access requires rtnl lock (i.e. -- mutex) to be taken.
>
> [152363.820563] BUG: scheduling while atomic: crtools/12517/0x10000002
> [152363.820573] 4 locks held by crtools/12517:
> [152363.820581] #0: (sock_diag_mutex){+.+.+.}, at: [<ffffffff81a2dcb5>] sock_diag_rcv+0x1f/0x3e
> [152363.820613] #1: (sock_diag_table_mutex){+.+.+.}, at: [<ffffffff81a2de70>] sock_diag_rcv_msg+0xdb/0x11a
> [152363.820644] #2: (nlk->cb_mutex){+.+.+.}, at: [<ffffffff81a67d01>] netlink_dump+0x23/0x1ab
> [152363.820693] #3: (rcu_read_lock){.+.+..}, at: [<ffffffff81b6a049>] packet_diag_dump+0x0/0x1af
>
> Similar thing was then re-introduced by further packet diag patches (fanount
> mutex and pgvec mutex for rings) :(
>
> Apart from being terribly sorry for the above, I propose to change the packet
> sk list protection from spinlock to mutex. This lock currently protects two
> modifications:
>
> * sklist
> * prot inuse counters
>
> The sklist modifications can be just reprotected with mutex since they already
> occur in a sleeping context. The inuse counters modifications are trickier -- the
> __this_cpu_-s are used inside, thus requiring the caller to handle the potential
> issues with contexts himself. Since packet sockets' counters are modified in two
> places only (packet_create and packet_release) we only need to protect the context
> from being preempted. BH disabling is not required in this case.
>
> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
>
> ---
>
> diff --git a/include/net/netns/packet.h b/include/net/netns/packet.h
> index cb4e894..4780b08 100644
> --- a/include/net/netns/packet.h
> +++ b/include/net/netns/packet.h
> @@ -8,7 +8,7 @@
> #include <linux/spinlock.h>
>
> struct netns_packet {
> - spinlock_t sklist_lock;
> + struct mutex sklist_lock;
> struct hlist_head sklist;
> };
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 226b2cd..79bc69c 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -2308,10 +2308,13 @@ static int packet_release(struct socket *sock)
> net = sock_net(sk);
> po = pkt_sk(sk);
>
> - spin_lock_bh(&net->packet.sklist_lock);
> + mutex_lock(&net->packet.sklist_lock);
> sk_del_node_init_rcu(sk);
> + mutex_unlock(&net->packet.sklist_lock);
I am still a bit uncomfortable : are we allowed to sleep in a release()
handler ?
It seems yes, so :
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [PATCH] ipv4: properly update pmtu
From: Eric Dumazet @ 2012-08-22 6:48 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Julian Anastasov, Sylvain Munaut
From: Eric Dumazet <edumazet@google.com>
Sylvain Munault reported following info :
- TCP connection get "stuck" with data in send queue when doing
"large" transfers ( like typing 'ps ax' on a ssh connection )
- Only happens on path where the PMTU is lower than the MTU of
the interface
- Is not present right after boot, it only appears 10-20min after
boot or so. (and that's inside the _same_ TCP connection, it works
fine at first and then in the same ssh session, it'll get stuck)
- Definitely seems related to fragments somehow since I see a router
sending ICMP message saying fragmentation is needed.
- Exact same setup works fine with kernel 3.5.1
Problem happens when the 10 minutes (ip_rt_mtu_expires) expiration
period is over.
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>
---
net/ipv4/route.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index e4ba974..8d6d320 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -956,7 +956,7 @@ static void ip_rt_update_pmtu(struct dst_entry *dst, struct sock *sk,
dst->obsolete = DST_OBSOLETE_KILL;
} else {
rt->rt_pmtu = mtu;
- dst_set_expires(&rt->dst, ip_rt_mtu_expires);
+ rt->dst.expires = max(1UL, jiffies + ip_rt_mtu_expires);
}
}
^ permalink raw reply related
* [PATCH v8] bonding: support for IPv6 transmit hashing
From: John Eaglesham @ 2012-08-22 6:43 UTC (permalink / raw)
To: netdev; +Cc: John Eaglesham
In-Reply-To: <2e01d8f94c42c61af9886683a4c35caf6816bc3d.1345610688.git.linux@8192.net>
From: John Eaglesham <linux@8192.net>
Currently the "bonding" driver does not support load balancing outgoing
traffic in LACP mode for IPv6 traffic. IPv4 (and TCP or UDP over IPv4)
are currently supported; this patch adds transmit hashing for IPv6 (and
TCP or UDP over IPv6), bringing IPv6 up to par with IPv4 support in the
bonding driver. In addition, bounds checking has been added to all
transmit hashing functions.
The algorithm chosen (xor'ing the bottom three quads of the source and
destination addresses together, then xor'ing each byte of that result into
the bottom byte, finally xor'ing with the last bytes of the MAC addresses)
was selected after testing almost 400,000 unique IPv6 addresses harvested
from server logs. This algorithm had the most even distribution for both
big- and little-endian architectures while still using few instructions. Its
behavior also attempts to closely match that of the IPv4 algorithm.
The IPv6 flow label was intentionally not included in the hash as it appears
to be unset in the vast majority of IPv6 traffic sampled, and the current
algorithm not using the flow label already offers a very even distribution.
Fragmented IPv6 packets are handled the same way as fragmented IPv4 packets,
ie, they are not balanced based on layer 4 information. Additionally,
IPv6 packets with intermediate headers are not balanced based on layer
4 information. In practice these intermediate headers are not common and
this should not cause any problems, and the alternative (a packet-parsing
loop and look-up table) seemed slow and complicated for little gain.
Tested-by: John Eaglesham <linux@8192.net>
Signed-off-by: John Eaglesham <linux@8192.net>
---
Changes:
v2)
* Clarify description
* Add bounds checking to more functions
* All functions call bond_xmit_hash_policy_l2 rather than re-
implement the same logic.
v3)
* Patch against net-next.
* Style corrections.
v4)
* Correct indenting.
v5)
* Squash documentation and code patches into one.
v6)
* Modify IPv6 hash to behave more like the IPv4 hash, update
documentation with modified algorithm.
* Clean up formatting.
* Move all variable declaration to the top of the function.
* Minor change to IPv6 layer 4 hash to match IPv4 hash behavior
(mix all hashed address bits together rather than just the
bottom 24 bits).
v7)
* Improve bounds checking code (handle truncated IPv6 header,
removed goto, fewer if statements).
* Re-write pseudocode in documentation to match actual code more
closely.
* Correct indenting, align parentheses, wrap code at <= 80 columns
(based on Jay's changes).
v8)
* Correct patch submission format.
Documentation/networking/bonding.txt | 30 ++++++++++--
drivers/net/bonding/bond_main.c | 89 +++++++++++++++++++++++++-----------
2 files changed, 88 insertions(+), 31 deletions(-)
diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 6b1c711..10a015c 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -752,12 +752,22 @@ xmit_hash_policy
protocol information to generate the hash.
Uses XOR of hardware MAC addresses and IP addresses to
- generate the hash. The formula is
+ generate the hash. The IPv4 formula is
(((source IP XOR dest IP) AND 0xffff) XOR
( source MAC XOR destination MAC ))
modulo slave count
+ The IPv6 formula is
+
+ hash = (source ip quad 2 XOR dest IP quad 2) XOR
+ (source ip quad 3 XOR dest IP quad 3) XOR
+ (source ip quad 4 XOR dest IP quad 4)
+
+ (((hash >> 24) XOR (hash >> 16) XOR (hash >> 8) XOR hash)
+ XOR (source MAC XOR destination MAC))
+ modulo slave count
+
This algorithm will place all traffic to a particular
network peer on the same slave. For non-IP traffic,
the formula is the same as for the layer2 transmit
@@ -778,19 +788,29 @@ xmit_hash_policy
slaves, although a single connection will not span
multiple slaves.
- The formula for unfragmented TCP and UDP packets is
+ The formula for unfragmented IPv4 TCP and UDP packets is
((source port XOR dest port) XOR
((source IP XOR dest IP) AND 0xffff)
modulo slave count
- For fragmented TCP or UDP packets and all other IP
- protocol traffic, the source and destination port
+ The formula for unfragmented IPv6 TCP and UDP packets is
+
+ hash = (source port XOR dest port) XOR
+ ((source ip quad 2 XOR dest IP quad 2) XOR
+ (source ip quad 3 XOR dest IP quad 3) XOR
+ (source ip quad 4 XOR dest IP quad 4))
+
+ ((hash >> 24) XOR (hash >> 16) XOR (hash >> 8) XOR hash)
+ modulo slave count
+
+ For fragmented TCP or UDP packets and all other IPv4 and
+ IPv6 protocol traffic, the source and destination port
information is omitted. For non-IP traffic, the
formula is the same as for the layer2 transmit hash
policy.
- This policy is intended to mimic the behavior of
+ The IPv4 policy is intended to mimic the behavior of
certain switches, notably Cisco switches with PFC2 as
well as some Foundry and IBM products.
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index d95fbc3..4221e57 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3354,56 +3354,93 @@ static struct notifier_block bond_netdev_notifier = {
/*---------------------------- Hashing Policies -----------------------------*/
/*
+ * Hash for the output device based upon layer 2 data
+ */
+static int bond_xmit_hash_policy_l2(struct sk_buff *skb, int count)
+{
+ struct ethhdr *data = (struct ethhdr *)skb->data;
+
+ if (skb_headlen(skb) >= offsetof(struct ethhdr, h_proto))
+ return (data->h_dest[5] ^ data->h_source[5]) % count;
+
+ return 0;
+}
+
+/*
* Hash for the output device based upon layer 2 and layer 3 data. If
- * the packet is not IP mimic bond_xmit_hash_policy_l2()
+ * the packet is not IP, fall back on bond_xmit_hash_policy_l2()
*/
static int bond_xmit_hash_policy_l23(struct sk_buff *skb, int count)
{
struct ethhdr *data = (struct ethhdr *)skb->data;
- struct iphdr *iph = ip_hdr(skb);
-
- if (skb->protocol == htons(ETH_P_IP)) {
+ struct iphdr *iph;
+ struct ipv6hdr *ipv6h;
+ u32 v6hash;
+ __be32 *s, *d;
+
+ if (skb->protocol == htons(ETH_P_IP) &&
+ skb_network_header_len(skb) >= sizeof(*iph)) {
+ iph = ip_hdr(skb);
return ((ntohl(iph->saddr ^ iph->daddr) & 0xffff) ^
(data->h_dest[5] ^ data->h_source[5])) % count;
+ } else if (skb->protocol == htons(ETH_P_IPV6) &&
+ skb_network_header_len(skb) >= sizeof(*ipv6h)) {
+ ipv6h = ipv6_hdr(skb);
+ s = &ipv6h->saddr.s6_addr32[0];
+ d = &ipv6h->daddr.s6_addr32[0];
+ v6hash = (s[1] ^ d[1]) ^ (s[2] ^ d[2]) ^ (s[3] ^ d[3]);
+ v6hash ^= (v6hash >> 24) ^ (v6hash >> 16) ^ (v6hash >> 8);
+ return (v6hash ^ data->h_dest[5] ^ data->h_source[5]) % count;
}
- return (data->h_dest[5] ^ data->h_source[5]) % count;
+ return bond_xmit_hash_policy_l2(skb, count);
}
/*
* Hash for the output device based upon layer 3 and layer 4 data. If
* the packet is a frag or not TCP or UDP, just use layer 3 data. If it is
- * altogether not IP, mimic bond_xmit_hash_policy_l2()
+ * altogether not IP, fall back on bond_xmit_hash_policy_l2()
*/
static int bond_xmit_hash_policy_l34(struct sk_buff *skb, int count)
{
- struct ethhdr *data = (struct ethhdr *)skb->data;
- struct iphdr *iph = ip_hdr(skb);
- __be16 *layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
- int layer4_xor = 0;
-
- if (skb->protocol == htons(ETH_P_IP)) {
+ u32 layer4_xor = 0;
+ struct iphdr *iph;
+ struct ipv6hdr *ipv6h;
+ __be32 *s, *d;
+ __be16 *layer4hdr;
+
+ if (skb->protocol == htons(ETH_P_IP) &&
+ skb_network_header_len(skb) >= sizeof(*iph)) {
+ iph = ip_hdr(skb);
if (!ip_is_fragment(iph) &&
(iph->protocol == IPPROTO_TCP ||
- iph->protocol == IPPROTO_UDP)) {
- layer4_xor = ntohs((*layer4hdr ^ *(layer4hdr + 1)));
+ iph->protocol == IPPROTO_UDP) &&
+ (skb_headlen(skb) - skb_network_offset(skb) >=
+ iph->ihl * sizeof(u32) + sizeof(*layer4hdr) * 2)) {
+ layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
+ layer4_xor = ntohs(*layer4hdr ^ *(layer4hdr + 1));
}
return (layer4_xor ^
((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
-
+ } else if (skb->protocol == htons(ETH_P_IPV6) &&
+ skb_network_header_len(skb) >= sizeof(*ipv6h)) {
+ ipv6h = ipv6_hdr(skb);
+ if ((ipv6h->nexthdr == IPPROTO_TCP ||
+ ipv6h->nexthdr == IPPROTO_UDP) &&
+ (skb_headlen(skb) - skb_network_offset(skb) >=
+ sizeof(*ipv6h) + sizeof(*layer4hdr) * 2)) {
+ layer4hdr = (__be16 *)(ipv6h + 1);
+ layer4_xor = ntohs(*layer4hdr ^ *(layer4hdr + 1));
+ }
+ s = &ipv6h->saddr.s6_addr32[0];
+ d = &ipv6h->daddr.s6_addr32[0];
+ layer4_xor ^= (s[1] ^ d[1]) ^ (s[2] ^ d[2]) ^ (s[3] ^ d[3]);
+ layer4_xor ^= (layer4_xor >> 24) ^ (layer4_xor >> 16) ^
+ (layer4_xor >> 8);
+ return layer4_xor % count;
}
- return (data->h_dest[5] ^ data->h_source[5]) % count;
-}
-
-/*
- * Hash for the output device based upon layer 2 data
- */
-static int bond_xmit_hash_policy_l2(struct sk_buff *skb, int count)
-{
- struct ethhdr *data = (struct ethhdr *)skb->data;
-
- return (data->h_dest[5] ^ data->h_source[5]) % count;
+ return bond_xmit_hash_policy_l2(skb, count);
}
/*-------------------------- Device entry points ----------------------------*/
--
1.7.11
^ permalink raw reply related
* Re: [PATCH 0/3] ipvs: IPv6 fragment handling for IPVS
From: Simon Horman @ 2012-08-22 6:42 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Julian Anastasov, netdev, Patrick McHardy, Hans Schillstrom,
LVS devel, Wensong Zhang, netfilter-devel, Pablo Neira Ayuso
In-Reply-To: <1345535487.3069.281.camel@localhost>
On Tue, Aug 21, 2012 at 09:51:27AM +0200, Jesper Dangaard Brouer wrote:
> On Tue, 2012-08-21 at 14:24 +0900, Simon Horman wrote:
> > I have no objection to these changes, but I would be more comfortable
> > applying them after a review from Hans, Julian or Pablo.
> >
> I would appreciate a review, especially from Julian.
>
> I'm going to do some more extensive testing on the patchset, in this
> week. So, no hurry on applying these, we have time for a good review
> process.
Thanks Jesper, and Julian and Hans who also responded.
Let me know when you feel the series is ready to merge.
I am also in no hurry.
^ permalink raw reply
* Re: smsc75xx & smsc95xx, setting skb->truesize correctly?
From: Eric Dumazet @ 2012-08-22 6:34 UTC (permalink / raw)
To: Jussi Kivilinna; +Cc: netdev, Steve Glendinning
In-Reply-To: <20120822083549.57260n3jdqfgxtj4@www.81.fi>
On Wed, 2012-08-22 at 08:35 +0300, Jussi Kivilinna wrote:
> Quoting Eric Dumazet <eric.dumazet@gmail.com>:
>
> > On Mon, 2012-08-20 at 17:57 +0300, Jussi Kivilinna wrote:
> >> Hello,
> >>
> >> Is setting skb->truesize in smsc75xx and smsc95xx correct?
> >> In smsc75xx/smsc95xx_rx_fixup(), input skb containing multiple packets
> >> is cloned and truesize for each clone is set to packet-size +
> >> sizeof(struct sk_buff), but input skb has minimum allocation size of
> >> 9000 bytes (MAX_SINGLE_PACKET_SIZE) and maximum of 18944 bytes
> >> (DEFAULT_HS_BURST_CAP_SIZE) (+ NET_IP_ALIGN). Doesn't this cause
> >> truesize to be underestimated?
> >
> > This has been discussed in a "TCP transmit performance regression"
> > thread some weeks ago.
> >
> > More generally, skb_clone() is not a good idea in rx path.
>
> So all skb_clone use in drivers/net/usb/ should be removed/replaced
> with following?
Doing a copy might be expensive on some low end hardware, so I can
understand why this skb_clone() idea was deployed years ago.
Gigabit r8169 has to perform the copy because of security issue, and so
far nobody complained of performance impact.
Best thing would be to not use large buffers from the beginning,
and switch to a frag idea.
(A large frame would needs 2 or 3 medium buffers, as done in ath9k)
Check https://gerrit.chromium.org/gerrit/#/c/18412/
and commit 0d95521ea74735826cb2e28bebf6a07392c75bfa (ath9k: use split rx
buffers to get rid of order-1 skb allocations)
^ permalink raw reply
* Re: smsc75xx & smsc95xx, setting skb->truesize correctly?
From: Jussi Kivilinna @ 2012-08-22 5:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Steve Glendinning
In-Reply-To: <1345542398.5158.446.camel@edumazet-glaptop>
Quoting Eric Dumazet <eric.dumazet@gmail.com>:
> On Mon, 2012-08-20 at 17:57 +0300, Jussi Kivilinna wrote:
>> Hello,
>>
>> Is setting skb->truesize in smsc75xx and smsc95xx correct?
>> In smsc75xx/smsc95xx_rx_fixup(), input skb containing multiple packets
>> is cloned and truesize for each clone is set to packet-size +
>> sizeof(struct sk_buff), but input skb has minimum allocation size of
>> 9000 bytes (MAX_SINGLE_PACKET_SIZE) and maximum of 18944 bytes
>> (DEFAULT_HS_BURST_CAP_SIZE) (+ NET_IP_ALIGN). Doesn't this cause
>> truesize to be underestimated?
>
> This has been discussed in a "TCP transmit performance regression"
> thread some weeks ago.
>
> More generally, skb_clone() is not a good idea in rx path.
So all skb_clone use in drivers/net/usb/ should be removed/replaced
with following?
>
> I dont have the hardware so cannot send a formal patch.
Neither do I, was looking for gigabit-usb dongle to buy and ended up
checking various drivers.
I guess I could do and test this for rndis_host/rndis_wlan and add
rx-skb recycling to usbnet core for drivers that do copy-break in
rx_fixup.
-Jussi
>
> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index b1112e7..3d9566f 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -1080,30 +1080,17 @@ static int smsc95xx_rx_fixup(struct usbnet
> *dev, struct sk_buff *skb)
> return 0;
> }
>
> - /* last frame in this batch */
> - if (skb->len == size) {
> - if (dev->net->features & NETIF_F_RXCSUM)
> - smsc95xx_rx_csum_offload(skb);
> - skb_trim(skb, skb->len - 4); /* remove fcs */
> - skb->truesize = size + sizeof(struct sk_buff);
> -
> - return 1;
> - }
> -
> - ax_skb = skb_clone(skb, GFP_ATOMIC);
> + ax_skb = netdev_alloc_skb_ip_align(dev->net, size);
> if (unlikely(!ax_skb)) {
> netdev_warn(dev->net, "Error allocating skb\n");
> return 0;
> }
>
> - ax_skb->len = size;
> - ax_skb->data = packet;
> - skb_set_tail_pointer(ax_skb, size);
> + memcpy(skb_put(ax_skb, size), packet, size);
>
> if (dev->net->features & NETIF_F_RXCSUM)
> smsc95xx_rx_csum_offload(ax_skb);
> - skb_trim(ax_skb, ax_skb->len - 4); /* remove fcs */
> - ax_skb->truesize = size + sizeof(struct sk_buff);
> + __skb_trim(ax_skb, ax_skb->len - 4); /* remove fcs */
>
> usbnet_skb_return(dev, ax_skb);
> }
>
>
>
>
^ permalink raw reply
* Re: [PATCH v7] bonding: support for IPv6 transmit hashing
From: David Miller @ 2012-08-22 5:29 UTC (permalink / raw)
To: linux; +Cc: netdev
In-Reply-To: <2e01d8f94c42c61af9886683a4c35caf6816bc3d.1345610688.git.linux@8192.net>
From: John Eaglesham <linux@8192.net>
Date: Tue, 21 Aug 2012 22:12:33 -0700
> Signed-off-by: John Eaglesham
This is not a proper signoff, you must put an email address on this line.
Also, the changelog between the versions doesn't belong in the commit
log message itself. It gets placed in commentary after the "---" line.
^ permalink raw reply
* Re: copy_skb_header() and sk_buff flags (question)
From: Eric Dumazet @ 2012-08-22 5:21 UTC (permalink / raw)
To: Kevin Wilson; +Cc: netdev
In-Reply-To: <CAGXs5wVp2bLnwFiuYEg6ZOv0t3DX4PCCgY-EURbs5bySgEqtdw@mail.gmail.com>
On Wed, 2012-08-22 at 07:55 +0300, Kevin Wilson wrote:
> Hello,
>
> I have a question if I may, as I cannot understand this point:
>
> I try to understand why in __copy_skb_header() we copy various flags
> (like ip_summed,
> local_df, pkt_type, priority, ipvs_property, and more. But there are
> flags which we do
> not copy, like nohdr, or fclone, or peeked.
>
> Aren't these flags part of the sk_buff header?
Caller should take care of said bits.
peeked is probably always 0 at this point...
I tried to use a single memcpy() to speedup things, but found out
it was not a trivial thing...
^ permalink raw reply
* [PATCH v7] bonding: support for IPv6 transmit hashing
From: John Eaglesham @ 2012-08-22 5:12 UTC (permalink / raw)
To: netdev; +Cc: John Eaglesham
In-Reply-To: <eb20e8f67e6aad94c233219e058c3e793ed755cb.1341167171.git.linux@8192.net>
From: John Eaglesham <linux@8192.net>
Currently the "bonding" driver does not support load balancing outgoing
traffic in LACP mode for IPv6 traffic. IPv4 (and TCP or UDP over IPv4)
are currently supported; this patch adds transmit hashing for IPv6 (and
TCP or UDP over IPv6), bringing IPv6 up to par with IPv4 support in the
bonding driver. In addition, bounds checking has been added to all
transmit hashing functions.
The algorithm chosen (xor'ing the bottom three quads of the source and
destination addresses together, then xor'ing each byte of that result into
the bottom byte, finally xor'ing with the last bytes of the MAC addresses)
was selected after testing almost 400,000 unique IPv6 addresses harvested
from server logs. This algorithm had the most even distribution for both
big- and little-endian architectures while still using few instructions. Its
behavior also attempts to closely match that of the IPv4 algorithm.
The IPv6 flow label was intentionally not included in the hash as it appears
to be unset in the vast majority of IPv6 traffic sampled, and the current
algorithm not using the flow label already offers a very even distribution.
Fragmented IPv6 packets are handled the same way as fragmented IPv4 packets,
ie, they are not balanced based on layer 4 information. Additionally,
IPv6 packets with intermediate headers are not balanced based on layer
4 information. In practice these intermediate headers are not common and
this should not cause any problems, and the alternative (a packet-parsing
loop and look-up table) seemed slow and complicated for little gain.
Changes:
v2)
* Clarify description
* Add bounds checking to more functions
* All functions call bond_xmit_hash_policy_l2 rather than re-
implement the same logic.
v3)
* Patch against net-next.
* Style corrections.
v4)
* Correct indenting.
v5)
* Squash documentation and code patches into one.
v6)
* Modify IPv6 hash to behave more like the IPv4 hash, update
documentation with modified algorithm.
* Clean up formatting.
* Move all variable declaration to the top of the function.
* Minor change to IPv6 layer 4 hash to match IPv4 hash behavior
(mix all hashed address bits together rather than just the
bottom 24 bits).
v7)
* Improve bounds checking code (handle truncated IPv6 header,
removed goto, fewer if statements).
* Re-write pseudocode in documentation to match actual code more
closely.
* Correct indenting, align parentheses, wrap code at <= 80 columns
(based on Jay's changes).
Patch has been tested and performs as expected.
Signed-off-by: John Eaglesham
---
Documentation/networking/bonding.txt | 30 ++++++++++--
drivers/net/bonding/bond_main.c | 89 +++++++++++++++++++++++++-----------
2 files changed, 88 insertions(+), 31 deletions(-)
diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 6b1c711..10a015c 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -752,12 +752,22 @@ xmit_hash_policy
protocol information to generate the hash.
Uses XOR of hardware MAC addresses and IP addresses to
- generate the hash. The formula is
+ generate the hash. The IPv4 formula is
(((source IP XOR dest IP) AND 0xffff) XOR
( source MAC XOR destination MAC ))
modulo slave count
+ The IPv6 formula is
+
+ hash = (source ip quad 2 XOR dest IP quad 2) XOR
+ (source ip quad 3 XOR dest IP quad 3) XOR
+ (source ip quad 4 XOR dest IP quad 4)
+
+ (((hash >> 24) XOR (hash >> 16) XOR (hash >> 8) XOR hash)
+ XOR (source MAC XOR destination MAC))
+ modulo slave count
+
This algorithm will place all traffic to a particular
network peer on the same slave. For non-IP traffic,
the formula is the same as for the layer2 transmit
@@ -778,19 +788,29 @@ xmit_hash_policy
slaves, although a single connection will not span
multiple slaves.
- The formula for unfragmented TCP and UDP packets is
+ The formula for unfragmented IPv4 TCP and UDP packets is
((source port XOR dest port) XOR
((source IP XOR dest IP) AND 0xffff)
modulo slave count
- For fragmented TCP or UDP packets and all other IP
- protocol traffic, the source and destination port
+ The formula for unfragmented IPv6 TCP and UDP packets is
+
+ hash = (source port XOR dest port) XOR
+ ((source ip quad 2 XOR dest IP quad 2) XOR
+ (source ip quad 3 XOR dest IP quad 3) XOR
+ (source ip quad 4 XOR dest IP quad 4))
+
+ ((hash >> 24) XOR (hash >> 16) XOR (hash >> 8) XOR hash)
+ modulo slave count
+
+ For fragmented TCP or UDP packets and all other IPv4 and
+ IPv6 protocol traffic, the source and destination port
information is omitted. For non-IP traffic, the
formula is the same as for the layer2 transmit hash
policy.
- This policy is intended to mimic the behavior of
+ The IPv4 policy is intended to mimic the behavior of
certain switches, notably Cisco switches with PFC2 as
well as some Foundry and IBM products.
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index d95fbc3..4221e57 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3354,56 +3354,93 @@ static struct notifier_block bond_netdev_notifier = {
/*---------------------------- Hashing Policies -----------------------------*/
/*
+ * Hash for the output device based upon layer 2 data
+ */
+static int bond_xmit_hash_policy_l2(struct sk_buff *skb, int count)
+{
+ struct ethhdr *data = (struct ethhdr *)skb->data;
+
+ if (skb_headlen(skb) >= offsetof(struct ethhdr, h_proto))
+ return (data->h_dest[5] ^ data->h_source[5]) % count;
+
+ return 0;
+}
+
+/*
* Hash for the output device based upon layer 2 and layer 3 data. If
- * the packet is not IP mimic bond_xmit_hash_policy_l2()
+ * the packet is not IP, fall back on bond_xmit_hash_policy_l2()
*/
static int bond_xmit_hash_policy_l23(struct sk_buff *skb, int count)
{
struct ethhdr *data = (struct ethhdr *)skb->data;
- struct iphdr *iph = ip_hdr(skb);
-
- if (skb->protocol == htons(ETH_P_IP)) {
+ struct iphdr *iph;
+ struct ipv6hdr *ipv6h;
+ u32 v6hash;
+ __be32 *s, *d;
+
+ if (skb->protocol == htons(ETH_P_IP) &&
+ skb_network_header_len(skb) >= sizeof(*iph)) {
+ iph = ip_hdr(skb);
return ((ntohl(iph->saddr ^ iph->daddr) & 0xffff) ^
(data->h_dest[5] ^ data->h_source[5])) % count;
+ } else if (skb->protocol == htons(ETH_P_IPV6) &&
+ skb_network_header_len(skb) >= sizeof(*ipv6h)) {
+ ipv6h = ipv6_hdr(skb);
+ s = &ipv6h->saddr.s6_addr32[0];
+ d = &ipv6h->daddr.s6_addr32[0];
+ v6hash = (s[1] ^ d[1]) ^ (s[2] ^ d[2]) ^ (s[3] ^ d[3]);
+ v6hash ^= (v6hash >> 24) ^ (v6hash >> 16) ^ (v6hash >> 8);
+ return (v6hash ^ data->h_dest[5] ^ data->h_source[5]) % count;
}
- return (data->h_dest[5] ^ data->h_source[5]) % count;
+ return bond_xmit_hash_policy_l2(skb, count);
}
/*
* Hash for the output device based upon layer 3 and layer 4 data. If
* the packet is a frag or not TCP or UDP, just use layer 3 data. If it is
- * altogether not IP, mimic bond_xmit_hash_policy_l2()
+ * altogether not IP, fall back on bond_xmit_hash_policy_l2()
*/
static int bond_xmit_hash_policy_l34(struct sk_buff *skb, int count)
{
- struct ethhdr *data = (struct ethhdr *)skb->data;
- struct iphdr *iph = ip_hdr(skb);
- __be16 *layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
- int layer4_xor = 0;
-
- if (skb->protocol == htons(ETH_P_IP)) {
+ u32 layer4_xor = 0;
+ struct iphdr *iph;
+ struct ipv6hdr *ipv6h;
+ __be32 *s, *d;
+ __be16 *layer4hdr;
+
+ if (skb->protocol == htons(ETH_P_IP) &&
+ skb_network_header_len(skb) >= sizeof(*iph)) {
+ iph = ip_hdr(skb);
if (!ip_is_fragment(iph) &&
(iph->protocol == IPPROTO_TCP ||
- iph->protocol == IPPROTO_UDP)) {
- layer4_xor = ntohs((*layer4hdr ^ *(layer4hdr + 1)));
+ iph->protocol == IPPROTO_UDP) &&
+ (skb_headlen(skb) - skb_network_offset(skb) >=
+ iph->ihl * sizeof(u32) + sizeof(*layer4hdr) * 2)) {
+ layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
+ layer4_xor = ntohs(*layer4hdr ^ *(layer4hdr + 1));
}
return (layer4_xor ^
((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
-
+ } else if (skb->protocol == htons(ETH_P_IPV6) &&
+ skb_network_header_len(skb) >= sizeof(*ipv6h)) {
+ ipv6h = ipv6_hdr(skb);
+ if ((ipv6h->nexthdr == IPPROTO_TCP ||
+ ipv6h->nexthdr == IPPROTO_UDP) &&
+ (skb_headlen(skb) - skb_network_offset(skb) >=
+ sizeof(*ipv6h) + sizeof(*layer4hdr) * 2)) {
+ layer4hdr = (__be16 *)(ipv6h + 1);
+ layer4_xor = ntohs(*layer4hdr ^ *(layer4hdr + 1));
+ }
+ s = &ipv6h->saddr.s6_addr32[0];
+ d = &ipv6h->daddr.s6_addr32[0];
+ layer4_xor ^= (s[1] ^ d[1]) ^ (s[2] ^ d[2]) ^ (s[3] ^ d[3]);
+ layer4_xor ^= (layer4_xor >> 24) ^ (layer4_xor >> 16) ^
+ (layer4_xor >> 8);
+ return layer4_xor % count;
}
- return (data->h_dest[5] ^ data->h_source[5]) % count;
-}
-
-/*
- * Hash for the output device based upon layer 2 data
- */
-static int bond_xmit_hash_policy_l2(struct sk_buff *skb, int count)
-{
- struct ethhdr *data = (struct ethhdr *)skb->data;
-
- return (data->h_dest[5] ^ data->h_source[5]) % count;
+ return bond_xmit_hash_policy_l2(skb, count);
}
/*-------------------------- Device entry points ----------------------------*/
--
1.7.11
^ permalink raw reply related
* Re[2]: [PATCH 05/19] netfilter: nf_conntrack_ipv6: improve fragmentation handling
From: Hans Schillstrom @ 2012-08-22 5:01 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Patrick McHardy, netfilter-devel, netdev, Julian Anastasov,
Hans Schillstrom
Hi
>
>On Sun, 2012-08-19 at 21:44 +0200, Patrick McHardy wrote:
>> On Sun, 19 Aug 2012, Jesper Dangaard Brouer wrote:
>> > On Sat, 2012-08-18 at 14:26 +0200, Patrick McHardy wrote:
>[...]
>
>> > Don't I need to load some of the helper modules, or just the
>> > nf_conntrack_ipv6 module, or perhaps only nf_defrag_ipv6 ?
>>
>> Not with the entire patchset, just IPv6 conntrack is enough. Aith IPv6 NAT
>> the first packet of a connection must always be defragemented, independant
>> of an assigned helper.
>
>When loading "nf_conntrack_ipv6" I run into issues.
>
>When sending a fragmented UDP packet. With these patches, the IPVS
>stack will no longer see the fragmented packets, but instead see one
>large SKB. This will trigger a MTU path check in e.g.
>ip_vs_dr_xmit_v6() and an ICMPv6 too big packet is send back.
>
> IPVS: ip_vs_dr_xmit_v6(): frag needed
>
>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)) {
/Hans
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: David Miller @ 2012-08-22 5:00 UTC (permalink / raw)
To: torvalds; +Cc: hpa, bhutchings, tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <CA+55aFz39tCSO7sf7+czNEBcEx7q4=KbnAcKEoS-KBwr6D3Psg@mail.gmail.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Tue, 21 Aug 2012 21:35:22 -0700
> My biggest reason to question this all is that I don't think it's
> worth it. Why would we ever care to do all this in the first place?
> There's no really sane use for it.
All the x86 crypto code hits this case all the time, easiest example
is doing a dm-crypt on a block device when an IPSEC packet arrives.
The crypto code has all of this special code and layering that is
there purely so it can fall back to the slow non-optimized version
of the crypto operation when it hits this can't-nest-fpu-saving
situation.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox