* Re: E1000E/82567LM-3: link reported up too soon
From: David Miller @ 2010-09-20 18:22 UTC (permalink / raw)
To: arno; +Cc: brian.haley, netdev
In-Reply-To: <87bp7vnnpj.fsf@small.ssi.corp>
From: arno@natisbad.org (Arnaud Ebalard)
Date: Sat, 18 Sep 2010 16:14:16 +0200
> Below, UMIP is configured w/o any additional delay before RS emission on
> that interface. No DHCP running either:
>
> 15:40:22.984311 ethertype IPv6 (0x86dd), length 62: :: > ff02::2: ICMP6, router solicitation, length 8
> 15:40:26.984845 ethertype IPv6 (0x86dd), length 62: :: > ff02::2: ICMP6, router solicitation, length 8
> 15:40:26.989264 ethertype IPv6 (0x86dd), length 158: fe80::224:d5ff:fed4:476c > ff02::1: ICMP6, router advertisement
>
> David, any idea on where this may come from and how to track the cause?
I have no context, what's the exact problem?
^ permalink raw reply
* Re: Regression, bisected: reference leak with IPSec since ~2.6.31
From: Eric Dumazet @ 2010-09-20 18:20 UTC (permalink / raw)
To: Nick Bowler; +Cc: linux-kernel, netdev, David S. Miller
In-Reply-To: <20100920174443.GA5515@elliptictech.com>
Le lundi 20 septembre 2010 à 13:44 -0400, Nick Bowler a écrit :
> Since 2.6.31, one of our UDP test programs has resulted in an SA leak: after
> running the test and flushing the SAD/SPD, the esp module is left with a
> non-zero reference count. This reference is never released: closer
> inspection reveals that esp_destroy is never called on the SA.
>
> I've attached a simplified version of the test program which reproduces the
> issue. To reproduce:
>
> (1) Create a Tx SA -- I used the following setkey script for my tests:
>
> add 192.168.42.2 192.168.42.1 esp 0x327B23C6 -f seq-pad
> -E rijndael-cbc 0x3D1B58BA507ED7AB2EB141F241B71EFB
> -A null;
>
> spdadd 192.168.42.2 192.168.42.1 any -P out ipsec
> esp/transport//require;
>
> (2) lsmod | grep esp4
> (note that the reference count is 1)
>
> (3) run the test program, e.g. udp_burst 192.168.42.1 for the above SA.
>
> (4) setkey -F; setkey -P -F
>
> (5) wait as long as you want.
>
> (6) lsmod | grep esp4
> (note that the reference count is still 1, not 0 as it should be)
>
> You can repeat the test until the zombie SAs consume all available
> memory. Cursory tests show similar problems with AH, IPv6 and Rx SAs,
> but I only really tested ESP Tx.
>
> Bisection implicates the following:
>
> 2b85a34e911bf483c27cfdd124aeb1605145dc80 is the first bad commit
> commit 2b85a34e911bf483c27cfdd124aeb1605145dc80
> Author: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu Jun 11 02:55:43 2009 -0700
>
> net: No more expensive sock_hold()/sock_put() on each tx
>
> One of the problem with sock memory accounting is it uses
> a pair of sock_hold()/sock_put() for each transmitted packet.
>
> This slows down bidirectional flows because the receive path
> also needs to take a refcount on socket and might use a different
> cpu than transmit path or transmit completion path. So these
> two atomic operations also trigger cache line bounces.
>
> We can see this in tx or tx/rx workloads (media gateways for example),
> where sock_wfree() can be in top five functions in profiles.
>
> We use this sock_hold()/sock_put() so that sock freeing
> is delayed until all tx packets are completed.
>
> As we also update sk_wmem_alloc, we could offset sk_wmem_alloc
> by one unit at init time, until sk_free() is called.
> Once sk_free() is called, we atomic_dec_and_test(sk_wmem_alloc)
> to decrement initial offset and atomicaly check if any packets
> are in flight.
>
> skb_set_owner_w() doesnt call sock_hold() anymore
>
> sock_wfree() doesnt call sock_put() anymore, but check if sk_wmem_alloc
> reached 0 to perform the final freeing.
>
> Drawback is that a skb->truesize error could lead to unfreeable sockets, or
> even worse, prematurely calling __sk_free() on a live socket.
>
> Nice speedups on SMP. tbench for example, going from 2691 MB/s to 2711 MB/s
> on my 8 cpu dev machine, even if tbench was not really hitting sk_refcnt
> contention point. 5 % speedup on a UDP transmit workload (depends
> on number of flows), lowering TX completion cpu usage.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> :040000 040000 13fc48f3903764863486c4e557a50281e8d790e6 4801aa898e906ad61729a84e33f1afb114abdf47 M include
> :040000 040000 042d86ad3f4d34cb96f137acb356c8251c2f8efc bd0698ec5bf8507c8ed1c5cf1e7791b4a5ed5596 M net
>
> git bisect start
> # good: [4a6908a3a050aacc9c3a2f36b276b46c0629ad91] Linux 2.6.28
> git bisect good 4a6908a3a050aacc9c3a2f36b276b46c0629ad91
> # bad: [22763c5cf3690a681551162c15d34d935308c8d7] Linux 2.6.32
> git bisect bad 22763c5cf3690a681551162c15d34d935308c8d7
> # good: [37ecfd807b82bf547429fe1376e1fe7000ba7cff] Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/drzeus/mmc
> git bisect good 37ecfd807b82bf547429fe1376e1fe7000ba7cff
> # bad: [2187550525d7bcb8c87689e4eca41b1955bf9ac3] xfs: rationalize xfs_inobt_lookup*
> git bisect bad 2187550525d7bcb8c87689e4eca41b1955bf9ac3
> # bad: [9cbc1cb8cd46ce1f7645b9de249b2ce8460129bb] Merge branch 'master' of master.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux-2.6
> git bisect bad 9cbc1cb8cd46ce1f7645b9de249b2ce8460129bb
> # good: [8a1ca8cedd108c8e76a6ab34079d0bbb4f244799] Merge branch 'perfcounters-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip
> git bisect good 8a1ca8cedd108c8e76a6ab34079d0bbb4f244799
> # good: [2cf4d4514d5b43c1f3b64bd0ec8b9853bde8f1dc] Merge branch 'for-linus' of master.kernel.org:/home/rmk/linux-2.6-arm
> git bisect good 2cf4d4514d5b43c1f3b64bd0ec8b9853bde8f1dc
> # good: [267a90127472be70b02ab13cbd355b5013e2aa51] ath9k: Optimize TBTT/DTIM calculation for timers
> git bisect good 267a90127472be70b02ab13cbd355b5013e2aa51
> # good: [5b1a002ade68173f21b2126a778278df72202ba6] datagram: Use frag list abstraction interfaces.
> git bisect good 5b1a002ade68173f21b2126a778278df72202ba6
> # bad: [5b2c4b972c0226406361f83b747eb5cdab51e68e] net: fix network drivers ndo_start_xmit() return values (part 8)
> git bisect bad 5b2c4b972c0226406361f83b747eb5cdab51e68e
> # bad: [2b85a34e911bf483c27cfdd124aeb1605145dc80] net: No more expensive sock_hold()/sock_put() on each tx
> git bisect bad 2b85a34e911bf483c27cfdd124aeb1605145dc80
> # good: [558f6d3229ddb9f11ca4ffee0439046c283882ff] cfg80211: fix for duplicate response for driver reg request
> git bisect good 558f6d3229ddb9f11ca4ffee0439046c283882ff
> # good: [84503ddd65e804ccdeedee3f307b40d80ff793e6] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-next-2.6
> git bisect good 84503ddd65e804ccdeedee3f307b40d80ff793e6
> # good: [87433bfc75f34599c38137e172b6bf8fd41971ba] r8169: use dev_kfree_skb() instead of dev_kfree_skb_irq()
> git bisect good 87433bfc75f34599c38137e172b6bf8fd41971ba
> # good: [6811086899f2740c08d0ade26f8b9d705708e0cc] be2net: fix netdev stats rx_errors and rx_dropped
> git bisect good 6811086899f2740c08d0ade26f8b9d705708e0cc
> # good: [a7a0ef31def6b6badd94fc96c8f17c2e18d91513] be2net: Fix early reset of rx-completion
> git bisect good a7a0ef31def6b6badd94fc96c8f17c2e18d91513
> # good: [f2333a014c1e13ac8e1b73a6fd77731c524eff78] netxen: No need to check vfree() pointer.
> git bisect good f2333a014c1e13ac8e1b73a6fd77731c524eff78
>
Hi Nick
If you change your program to send small frames (so they are not
fragmented), is the problem still present ?
The commit you mention was buggy and a fix was added later in
sock_wfree()
(commit d99927f4d93f36553699573b279e0ff98ad7dea6
net: Fix sock_wfree() race)
^ permalink raw reply
* Re: [GIT PULL net-2.6] vhost-net: fix range checking
From: David Miller @ 2010-09-20 18:15 UTC (permalink / raw)
To: mst; +Cc: kvm, virtualization, netdev, linux-kernel
In-Reply-To: <20100920174222.GA30510@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Mon, 20 Sep 2010 19:42:22 +0200
> git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-net
Pulled, thanks!
^ permalink raw reply
* Re: [PATCH] xfrm: Allow different selector family in temporary state
From: David Miller @ 2010-09-20 18:11 UTC (permalink / raw)
To: steffen.klassert; +Cc: herbert, thomas.egerer, netdev
In-Reply-To: <20100920115253.GG4134@gauss.secunet.de>
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Mon, 20 Sep 2010 13:52:53 +0200
> From: Thomas Egerer <thomas.egerer@secunet.com>
>
> The family parameter xfrm_state_find is used to find a state matching a
> certain policy. This value is set to the template's family
> (encap_family) right before xfrm_state_find is called.
> The family parameter is however also used to construct a temporary state
> in xfrm_state_find itself which is wrong for inter-family scenarios
> because it produces a selector for the wrong family. Since this selector
> is included in the xfrm_user_acquire structure, user space programs
> misinterpret IPv6 addresses as IPv4 and vice versa.
> This patch splits up the original init_tempsel function into a part that
> initializes the selector respectively the props and id of the temporary
> state, to allow for differing ip address families whithin the state.
>
> Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Looks good, applied, thanks a lot.
^ permalink raw reply
* Re: [PATCH] xmit_compl_seq: information to reclaim vmsplice buffers
From: Tom Herbert @ 2010-09-20 17:50 UTC (permalink / raw)
To: Changli Gao; +Cc: netdev, davem, sridharr
In-Reply-To: <AANLkTinyZxF4ep7tv_uSpcPD8gfNoddX6G7p+ZuA2ze9@mail.gmail.com>
>
> > Using recvmsg data in this manner is sort of a cheap way to get a
> > "callback" for when a vmspliced buffer is consumed. It will work
> > well for a client where the response causes recvmsg to return.
>
> In this case, we don't need it. The response is sufficient to indicate
> the request is sent out, and the corresponding pages are available for
> the other use.
The response might seem sufficient, but there is no guarantee this
works generally for every application protocol. The specific
requirement for TCP is that the page cannot be reused before its data
has been acked; if this does not hold then data corruption becomes an
issue. Given that danger, I would rather the mechanism be unambiguous
about when the data has been consumed, rather than each application
protocol trying to figure this out using its heuristics.
>
> > On the server side it works well if there are a sufficient
> > number of requests coming on the connection (resorting to the
> > timeout if necessary as described above).
>
> If no response is sent out by the client, the timeout mechanism may
> slow down the data transfer.
>
Why would that slow down a transfer?
>
> I think we should not mix RX and TX. Maybe we can add a new epoll
> event to indicate that the snd bufer is advanced.
>
This is possible, but seems more heavyweight that needed. I don't see
that it's going to be critical to reclaim buffers as soon as they have
been completed, some laziness in do this seems like a reasonable
tradeoff and we should still have a reasonable working set.
Tom
> --
> Regards,
> Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] net: core: dev.c: use spin_lock_irqsave() rather than
From: Michał Nazarewicz @ 2010-09-20 17:48 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, linux-kernel, linux-usb, netdev
In-Reply-To: <1284998621.3420.539.camel@edumazet-laptop>
On Mon, 20 Sep 2010 18:03:41 +0200, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> dev_get_stats(dev, &temp) is called from drivers/usb/gadget/rndis.c,
> while I suspect underlying stats are already provided in dev->stats
So, you are saying that it is likely that if I change rndis.c and replace
"dev_get_stats(net, &temp)" with plain "&net->stats" things should work
correctly (rndis.c does not use the "temp" so it's not a problem)?
I actually did that and nothing seems to crash but dunno if it didn't
make g_ether report some not fully up-to-date values or something.
--
Best regards, _ _
| Humble Liege of Serenely Enlightened Majesty of o' \,=./ `o
| Computer Science, Michał "mina86" Nazarewicz (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--
^ permalink raw reply
* Regression, bisected: reference leak with IPSec since ~2.6.31
From: Nick Bowler @ 2010-09-20 17:44 UTC (permalink / raw)
To: linux-kernel, netdev; +Cc: Eric Dumazet, David S. Miller
[-- Attachment #1: Type: text/plain, Size: 6415 bytes --]
Since 2.6.31, one of our UDP test programs has resulted in an SA leak: after
running the test and flushing the SAD/SPD, the esp module is left with a
non-zero reference count. This reference is never released: closer
inspection reveals that esp_destroy is never called on the SA.
I've attached a simplified version of the test program which reproduces the
issue. To reproduce:
(1) Create a Tx SA -- I used the following setkey script for my tests:
add 192.168.42.2 192.168.42.1 esp 0x327B23C6 -f seq-pad
-E rijndael-cbc 0x3D1B58BA507ED7AB2EB141F241B71EFB
-A null;
spdadd 192.168.42.2 192.168.42.1 any -P out ipsec
esp/transport//require;
(2) lsmod | grep esp4
(note that the reference count is 1)
(3) run the test program, e.g. udp_burst 192.168.42.1 for the above SA.
(4) setkey -F; setkey -P -F
(5) wait as long as you want.
(6) lsmod | grep esp4
(note that the reference count is still 1, not 0 as it should be)
You can repeat the test until the zombie SAs consume all available
memory. Cursory tests show similar problems with AH, IPv6 and Rx SAs,
but I only really tested ESP Tx.
Bisection implicates the following:
2b85a34e911bf483c27cfdd124aeb1605145dc80 is the first bad commit
commit 2b85a34e911bf483c27cfdd124aeb1605145dc80
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu Jun 11 02:55:43 2009 -0700
net: No more expensive sock_hold()/sock_put() on each tx
One of the problem with sock memory accounting is it uses
a pair of sock_hold()/sock_put() for each transmitted packet.
This slows down bidirectional flows because the receive path
also needs to take a refcount on socket and might use a different
cpu than transmit path or transmit completion path. So these
two atomic operations also trigger cache line bounces.
We can see this in tx or tx/rx workloads (media gateways for example),
where sock_wfree() can be in top five functions in profiles.
We use this sock_hold()/sock_put() so that sock freeing
is delayed until all tx packets are completed.
As we also update sk_wmem_alloc, we could offset sk_wmem_alloc
by one unit at init time, until sk_free() is called.
Once sk_free() is called, we atomic_dec_and_test(sk_wmem_alloc)
to decrement initial offset and atomicaly check if any packets
are in flight.
skb_set_owner_w() doesnt call sock_hold() anymore
sock_wfree() doesnt call sock_put() anymore, but check if sk_wmem_alloc
reached 0 to perform the final freeing.
Drawback is that a skb->truesize error could lead to unfreeable sockets, or
even worse, prematurely calling __sk_free() on a live socket.
Nice speedups on SMP. tbench for example, going from 2691 MB/s to 2711 MB/s
on my 8 cpu dev machine, even if tbench was not really hitting sk_refcnt
contention point. 5 % speedup on a UDP transmit workload (depends
on number of flows), lowering TX completion cpu usage.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
:040000 040000 13fc48f3903764863486c4e557a50281e8d790e6 4801aa898e906ad61729a84e33f1afb114abdf47 M include
:040000 040000 042d86ad3f4d34cb96f137acb356c8251c2f8efc bd0698ec5bf8507c8ed1c5cf1e7791b4a5ed5596 M net
git bisect start
# good: [4a6908a3a050aacc9c3a2f36b276b46c0629ad91] Linux 2.6.28
git bisect good 4a6908a3a050aacc9c3a2f36b276b46c0629ad91
# bad: [22763c5cf3690a681551162c15d34d935308c8d7] Linux 2.6.32
git bisect bad 22763c5cf3690a681551162c15d34d935308c8d7
# good: [37ecfd807b82bf547429fe1376e1fe7000ba7cff] Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/drzeus/mmc
git bisect good 37ecfd807b82bf547429fe1376e1fe7000ba7cff
# bad: [2187550525d7bcb8c87689e4eca41b1955bf9ac3] xfs: rationalize xfs_inobt_lookup*
git bisect bad 2187550525d7bcb8c87689e4eca41b1955bf9ac3
# bad: [9cbc1cb8cd46ce1f7645b9de249b2ce8460129bb] Merge branch 'master' of master.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux-2.6
git bisect bad 9cbc1cb8cd46ce1f7645b9de249b2ce8460129bb
# good: [8a1ca8cedd108c8e76a6ab34079d0bbb4f244799] Merge branch 'perfcounters-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip
git bisect good 8a1ca8cedd108c8e76a6ab34079d0bbb4f244799
# good: [2cf4d4514d5b43c1f3b64bd0ec8b9853bde8f1dc] Merge branch 'for-linus' of master.kernel.org:/home/rmk/linux-2.6-arm
git bisect good 2cf4d4514d5b43c1f3b64bd0ec8b9853bde8f1dc
# good: [267a90127472be70b02ab13cbd355b5013e2aa51] ath9k: Optimize TBTT/DTIM calculation for timers
git bisect good 267a90127472be70b02ab13cbd355b5013e2aa51
# good: [5b1a002ade68173f21b2126a778278df72202ba6] datagram: Use frag list abstraction interfaces.
git bisect good 5b1a002ade68173f21b2126a778278df72202ba6
# bad: [5b2c4b972c0226406361f83b747eb5cdab51e68e] net: fix network drivers ndo_start_xmit() return values (part 8)
git bisect bad 5b2c4b972c0226406361f83b747eb5cdab51e68e
# bad: [2b85a34e911bf483c27cfdd124aeb1605145dc80] net: No more expensive sock_hold()/sock_put() on each tx
git bisect bad 2b85a34e911bf483c27cfdd124aeb1605145dc80
# good: [558f6d3229ddb9f11ca4ffee0439046c283882ff] cfg80211: fix for duplicate response for driver reg request
git bisect good 558f6d3229ddb9f11ca4ffee0439046c283882ff
# good: [84503ddd65e804ccdeedee3f307b40d80ff793e6] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-next-2.6
git bisect good 84503ddd65e804ccdeedee3f307b40d80ff793e6
# good: [87433bfc75f34599c38137e172b6bf8fd41971ba] r8169: use dev_kfree_skb() instead of dev_kfree_skb_irq()
git bisect good 87433bfc75f34599c38137e172b6bf8fd41971ba
# good: [6811086899f2740c08d0ade26f8b9d705708e0cc] be2net: fix netdev stats rx_errors and rx_dropped
git bisect good 6811086899f2740c08d0ade26f8b9d705708e0cc
# good: [a7a0ef31def6b6badd94fc96c8f17c2e18d91513] be2net: Fix early reset of rx-completion
git bisect good a7a0ef31def6b6badd94fc96c8f17c2e18d91513
# good: [f2333a014c1e13ac8e1b73a6fd77731c524eff78] netxen: No need to check vfree() pointer.
git bisect good f2333a014c1e13ac8e1b73a6fd77731c524eff78
--
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)
[-- Attachment #2: udp_burst.c --]
[-- Type: text/x-c, Size: 1182 bytes --]
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netdb.h>
#define MAX_DGRAM_SIZE 10000
static char buf[MAX_DGRAM_SIZE];
int main(int argc, char **argv)
{
char *addr = NULL, *port = "9000";
struct addrinfo *info, hints = {
.ai_family = AF_UNSPEC,
.ai_socktype = SOCK_DGRAM,
.ai_flags = AI_PASSIVE,
};
int i, rc, sock;
if (argc > 1)
addr = argv[1];
if (argc > 2)
addr = argv[2];
if (!addr) {
fprintf(stderr, "usage: %s addr [port]\n", argv[0]);
return EXIT_FAILURE;
}
rc = getaddrinfo(addr, port, &hints, &info);
if (rc != 0) {
fprintf(stderr, "getaddrinfo: %s\n", gai_strerror(rc));
return EXIT_FAILURE;
}
sock = socket(info->ai_family, info->ai_socktype, info->ai_protocol);
if (sock == -1) {
perror("socket");
return EXIT_FAILURE;
}
if (connect(sock, info->ai_addr, info->ai_addrlen) == -1) {
perror("connect");
return EXIT_FAILURE;
}
for (i = 0; i < MAX_DGRAM_SIZE; i++) {
if (send(sock, buf, i+1, MSG_DONTWAIT) == -1) {
if (errno != EAGAIN && errno != EINTR) {
perror("send");
return EXIT_FAILURE;
}
}
}
return 0;
}
^ permalink raw reply
* [GIT PULL net-2.6] vhost-net: fix range checking
From: Michael S. Tsirkin @ 2010-09-20 17:42 UTC (permalink / raw)
To: David Miller; +Cc: kvm, virtualization, netdev, linux-kernel
David,
The following tree includes a bugfix for vhost-net.
It is on top of net-2.6. Please merge it for 2.6.36.
Thanks!
The following changes since commit 053d8f6622701f849fda2ca2c9ae596c13599ba9:
Merge branch 'vhost-net' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost (2010-09-09 21:59:51 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-net
Michael S. Tsirkin (1):
vhost-net: fix range checking in mrg bufs case
drivers/vhost/net.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
^ permalink raw reply
* Re: [PATCH] nf_nat_snmp: fix checksum calculation (v3)
From: Patrick McHardy @ 2010-09-20 17:35 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Íõ躣šŒÆËã»ú¿ÆѧѧԺ£©,
akpm, bugzilla-daemon, bugme-daemon, netdev
In-Reply-To: <20100920094448.67210244@nehalam>
[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]
On 20.09.2010 18:44, Stephen Hemminger wrote:
> Revised version of the original patch in the bug
> https://bugzilla.kernel.org/show_bug.cgi?id=17622
> from clark wang <wtweeker@163.com>
>
> I took the opportunity to do some cleanup here.
> * reorder the assignment to make the byte order clear
> * get rid of unnecessary ref/deref and just pass the bytes
> * use sizeof() instead of hard coding size
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Thanks Stephen, but this patch didn't compile:
net/ipv4/netfilter/nf_nat_snmp_basic.c: In function 'fast_csum':
net/ipv4/netfilter/nf_nat_snmp_basic.c:893: error: 's' undeclared (first
use in this function)
net/ipv4/netfilter/nf_nat_snmp_basic.c:893: error: (Each undeclared
identifier is reported only once
net/ipv4/netfilter/nf_nat_snmp_basic.c:893: error: for each function it
appears in.)
net/ipv4/netfilter/nf_nat_snmp_basic.c:890: warning: unused variable 'diff'
Since I prefer to keep this fix to the minimal size at this point,
I've committed this patch based on Clark's and your patches:
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1031 bytes --]
commit 8d70d82cdcc6da0a77440c6d860957a1f50b8089
Author: Patrick McHardy <kaber@trash.net>
Date: Mon Sep 20 19:29:40 2010 +0200
netfilter: nf_nat_snmp: fix checksum calculation (v4)
Fix checksum calculation in nf_nat_snmp_basic.
Based on patches by Clark Wang <wtweeker@163.com> and
Stephen Hemminger <shemminger@vyatta.com>.
https://bugzilla.kernel.org/show_bug.cgi?id=17622
Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/net/ipv4/netfilter/nf_nat_snmp_basic.c b/net/ipv4/netfilter/nf_nat_snmp_basic.c
index 1679e2c..ee5f419 100644
--- a/net/ipv4/netfilter/nf_nat_snmp_basic.c
+++ b/net/ipv4/netfilter/nf_nat_snmp_basic.c
@@ -893,13 +893,15 @@ static void fast_csum(__sum16 *csum,
unsigned char s[4];
if (offset & 1) {
- s[0] = s[2] = 0;
+ s[0] = ~0;
s[1] = ~*optr;
+ s[2] = 0;
s[3] = *nptr;
} else {
- s[1] = s[3] = 0;
s[0] = ~*optr;
+ s[1] = ~0;
s[2] = *nptr;
+ s[3] = 0;
}
*csum = csum_fold(csum_partial(s, 4, ~csum_unfold(*csum)));
^ permalink raw reply related
* Re: [PATCH net-next-2.6] net: rx_dropped accounting
From: David Miller @ 2010-09-20 17:09 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1284977531.3420.137.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 20 Sep 2010 12:12:11 +0200
> Under load, netif_rx() can drop incoming packets but administrators dont
> have a chance to spot which device needs some tuning (RPS activation for
> example)
>
> This patch adds rx_dropped accounting in vlans and tunnels.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH net-next-2.6] qlnic: dont set skb->truesize
From: David Miller @ 2010-09-20 17:09 UTC (permalink / raw)
To: eric.dumazet; +Cc: amit.salecha, netdev, ameen.rahman, anirban.chakraborty
In-Reply-To: <1284985739.3420.284.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 20 Sep 2010 14:28:59 +0200
> [PATCH net-next-2.6] qlnic: dont set skb->truesize
>
> skb->truesize is set in core network.
>
> Dont change it unless dealing with fragments.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Looks good to me too, applied thanks!
^ permalink raw reply
* Re: [PATCH v5 2/2] netfilter: save the hash of the tuple in the original direction for latter use
From: Patrick McHardy @ 2010-09-20 17:08 UTC (permalink / raw)
To: Changli Gao
Cc: David S. Miller, Eric Dumazet, Mathieu Desnoyers, akpm,
netfilter-devel, netdev, linux-kernel
In-Reply-To: <AANLkTikZeFrS9pj6jxmOxqY6dsaAf1U+FAXe5jNwQ-nV@mail.gmail.com>
On 20.09.2010 17:04, Changli Gao wrote:
> On Thu, Sep 16, 2010 at 2:18 PM, Patrick McHardy <kaber@trash.net> wrote:
>> On 21.08.2010 00:49, Changli Gao wrote:
>>> Since we don't change the tuple in the original direction, we can save it
>>> in ct->tuplehash[IP_CT_DIR_REPLY].hnode.pprev for __nf_conntrack_confirm()
>>> use.
>>
>> I like this idea. We could actually do the same for the reply tuple
>> and invalidate the saved hash in case the reply tuple is changed
>> (nf_conntrack_alter_reply()), which only happens when NAT is used.
>>
>
> We can't do that, as the unconfirmed ct owned maybe dropped, and
> pre-computing will wast CPU cycles in this case.
Sure we can, dropping unconfirmed conntracks is a rare exception,
not a common case. Even under DoS we usually drop *unassured*
conntracks, which have already enterered the hash. If we're unable
to do that, we won't even allocate a new conntrack.
^ permalink raw reply
* Re: [PATCH] net: core: dev.c: use spin_lock_irqsave() rather than
From: Michał Nazarewicz @ 2010-09-20 17:07 UTC (permalink / raw)
To: eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w, David Miller
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20100920.091407.241426689.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Mon, 20 Sep 2010 18:14:07 +0200, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> From: Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Date: Mon, 20 Sep 2010 18:03:41 +0200
>
>> Hmm, while your patch is technically correct, it seems strange all this
>> stuff runs in hard irq context.
>
> I would much rather this RNDIS query response handler get
> scheduled to run into softirq context or similar instead.
>
> I am not applying this patch.
Eh... I'll try to look at RNDIS than.
--
Best regards, _ _
| Humble Liege of Serenely Enlightened Majesty of o' \,=./ `o
| Computer Science, Michał "mina86" Nazarewicz (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] ip_gre: CONFIG_IPV6_MODULE support
From: David Miller @ 2010-09-20 17:06 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1284987700.3420.333.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 20 Sep 2010 15:01:40 +0200
> ipv6 can be a module, we should test CONFIG_IPV6 and CONFIG_IPV6_MODULE
> to enable ipv6 bits in ip_gre.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied to net-2.6, thanks Eric.
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: pskb_expand_head() optimization
From: David Miller @ 2010-09-20 16:59 UTC (permalink / raw)
To: jarkao2; +Cc: eric.dumazet, netdev
In-Reply-To: <20100920072149.GA6353@ff.dom.local>
From: Jarek Poplawski <jarkao2@gmail.com>
Date: Mon, 20 Sep 2010 07:21:49 +0000
> On Sun, Sep 19, 2010 at 05:17:25PM -0700, David Miller wrote:
>> We definitely don't use frag lists for performance, if you look
>> in the GRO history the GRO code specifically has been changed
>> to prefer accumulating into the page vector whenever possible
>> because using frag lists is a lot slower.
>
> Yes, and GRO made frag lists more popular btw.
Yes. However, realize that this is only really a legacy from
inet_lro.
These drivers could restructure themselves to operate on pages.
I am pretty sure the hardware would be amicable to this and it
would likely improve performance just as favoring page vector
accumulation did for GRO.
> Btw, I wonder what is the exact reason we can't use only
> NET_SKBUFF_DATA_USES_OFFSET?
As Eric explained, and then demonstrated, it generates a non-trivial
increased amount of code.
This is almost certainly why Arnaldo didn't make it unconditional
back when he implemented the SKB data offset changes for 64-bit.
In my opinion, this duality of SKB pointer handling never causes
real problems because any change mistakenly accessing the pointers
directly usually gets caught by the first person who builds it on
64-bit :-)
^ permalink raw reply
* [PATCH] nf_nat_snmp: fix checksum calculation (v3)
From: Stephen Hemminger @ 2010-09-20 16:44 UTC (permalink / raw)
To: 王韬（计算机科学学院）
Cc: Patrick McHardy, akpm, bugzilla-daemon, bugme-daemon, netdev
In-Reply-To: <2a014b7c.92ce.12b293c61ba.Coremail.wtweeker@163.com>
Revised version of the original patch in the bug
https://bugzilla.kernel.org/show_bug.cgi?id=17622
from clark wang <wtweeker@163.com>
I took the opportunity to do some cleanup here.
* reorder the assignment to make the byte order clear
* get rid of unnecessary ref/deref and just pass the bytes
* use sizeof() instead of hard coding size
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
--- a/net/ipv4/netfilter/nf_nat_snmp_basic.c 2010-09-20 09:05:11.752067965 -0700
+++ b/net/ipv4/netfilter/nf_nat_snmp_basic.c 2010-09-20 09:38:48.518661840 -0700
@@ -885,24 +885,23 @@ static unsigned char snmp_request_decode
* Fast checksum update for possibly oddly-aligned UDP byte, from the
* code example in the draft.
*/
-static void fast_csum(__sum16 *csum,
- const unsigned char *optr,
- const unsigned char *nptr,
- int offset)
+static void fast_csum(__sum16 *csum, __u8 from, __u8 to, int offset)
{
- unsigned char s[4];
+ __be16 diff[2];
if (offset & 1) {
- s[0] = s[2] = 0;
- s[1] = ~*optr;
- s[3] = *nptr;
+ s[0] = ~0;
+ s[1] = ~from;
+ s[2] = 0;
+ s[3] = to;
} else {
- s[1] = s[3] = 0;
- s[0] = ~*optr;
- s[2] = *nptr;
+ s[0] = ~from;
+ s[1] = ~0;
+ s[2] = to;
+ s[3] = 0;
}
- *csum = csum_fold(csum_partial(s, 4, ~csum_unfold(*csum)));
+ *csum = csum_fold(csum_partial(s, sizeof(s), ~csum_unfold(*csum)));
}
/*
@@ -924,11 +923,8 @@ static inline void mangle_address(unsign
*addr = map->to;
/* Update UDP checksum if being used */
- if (*check) {
- fast_csum(check,
- &map->from, &map->to, addr - begin);
-
- }
+ if (*check)
+ fast_csum(check, map->from, map->to, addr - begin);
if (debug)
printk(KERN_DEBUG "bsalg: mapped %pI4 to %pI4\n",
^ permalink raw reply
* RE PRIVATE MATTER DEAR FRIEND.
From: WILLIAMS @ 2010-09-20 14:32 UTC (permalink / raw)
Please find attach. Original message.....
^ permalink raw reply
* Re: [PATCH] net: core: dev.c: use spin_lock_irqsave() rather than
From: David Miller @ 2010-09-20 16:14 UTC (permalink / raw)
To: eric.dumazet; +Cc: m.nazarewicz, linux-kernel, linux-usb, netdev
In-Reply-To: <1284998621.3420.539.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 20 Sep 2010 18:03:41 +0200
> Hmm, while your patch is technically correct, it seems strange all this
> stuff runs in hard irq context.
I would much rather this RNDIS query response handler get
scheduled to run into softirq context or similar instead.
I am not applying this patch.
^ permalink raw reply
* Fw: [Bug 18822] New: TCP Communications gets blocked, then resetted
From: Stephen Hemminger @ 2010-09-20 16:04 UTC (permalink / raw)
To: netdev
Begin forwarded message:
Date: Mon, 20 Sep 2010 10:39:47 GMT
From: bugzilla-daemon@bugzilla.kernel.org
To: shemminger@linux-foundation.org
Subject: [Bug 18822] New: TCP Communications gets blocked, then resetted
https://bugzilla.kernel.org/show_bug.cgi?id=18822
Summary: TCP Communications gets blocked, then resetted
Product: Networking
Version: 2.5
Kernel Version: 2.6.32-24-generic #43-Ubuntu
Platform: All
OS/Version: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: IPV4
AssignedTo: shemminger@linux-foundation.org
ReportedBy: dc6iq@gmx.de
Regression: No
Created an attachment (id=30682)
--> (https://bugzilla.kernel.org/attachment.cgi?id=30682)
both machine dumps
having a freshly installed bacula server i could never get a backup from my
working machine done to the bacula server.
The Connection transmits approximately 3 GiB, then locks up and resets the
connection.
Transmission gets stuck at a certain point, then the bacula server does not
reply to retransmitted packets on IPV4 stack. Retry Count on disks machine (my
working machine) raises up to 13, then the Connection is gone. bacula server
tries to send a Push packet (after KeepAlive timer runs out), and get the final
RST packet from disks, because the connection is gone.
In the Attachment you will find the tcpdumps from both machines, actually the
sending machine dropped some packets in the dump.
It might be a possible help: disks is running an 64 bit kernel whereas bacula
is running 32 bit. I haven't looked into the option bits very well but it looks
like there is a problem hidden:
last ack being ok:
09:32:56.142876 IP bacula.elkenet.bacula-sd > disks.elkenet.50766: Flags [.],
ack 2005754588, win 9582, options [nop,nop,TS val 21825875 ecr 4530083], length
0
next ack packet:
09:32:56.144763 IP bacula.elkenet.bacula-sd > disks.elkenet.50766: Flags [.],
ack 2005773412, win 9308, options [nop,nop,TS val 21825876 ecr
4530083,nop,nop,sack 1 {2005774860:2005776308}], length 0
root@disks:~# uname -a
Linux disks 2.6.32-24-generic #43-Ubuntu SMP Thu Sep 16 14:58:24 UTC 2010
x86_64 GNU/Linux
root@bacula:~# uname -a
Linux bacula 2.6.32-24-generic-pae #43-Ubuntu SMP Thu Sep 16 15:30:27 UTC 2010
i686 GNU/Linux
Doing a 20GB backup on a debian server works fine
server:~# uname -a
Linux server 2.6.32-5-486 #1 Sat Sep 18 01:43:00 UTC 2010 i686 GNU/Linux
Doing a 26 GB backup from a 32 bit Ubuntu works fine as well. Maybe its a 64
bit issue...
root@elke:~# uname -a
Linux elke 2.6.32-24-generic #43-Ubuntu SMP Thu Sep 16 14:17:33 UTC 2010 i686
GNU/Linux
If any further input is required, just let me know...
--
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
--
^ permalink raw reply
* Re: [PATCH] net: core: dev.c: use spin_lock_irqsave() rather than
From: Eric Dumazet @ 2010-09-20 16:03 UTC (permalink / raw)
To: Michal Nazarewicz; +Cc: David S. Miller, linux-kernel, linux-usb, netdev
In-Reply-To: <ee14e5e38e7a5df520d902ef8a236bbe9ed206fa.1284991149.git.m.nazarewicz@samsung.com>
Le lundi 20 septembre 2010 à 17:08 +0200, Michal Nazarewicz a écrit :
> This commit fixes a warning that was issued when g_ether gadget
> was connected to Windows host. In g_ether, the dev_txq_stats_fold()
> can be called from context other then soft-irq so _bh version of
> spin_lock is not adequate.
>
> Changing from spin_lock_bh() to spin_lock_irqsave() is always safe (as
> irqsave is superset of all other spin lock operations) is always safe
> so this commit should not break anything.
>
> As Eric Dumazet said, dev_txq_stats_fold() is a slow patch so there
> is no need to optimise that much.
>
> Signed-off-by: Michal Nazarewicz <m.nazarewicz@samsung.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> net/core/dev.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> Hello David,
>
> could you pull this patch. I think it's best to get it in 2.6.36.
>
>
> Without this patch, I got the following warning when RNDIS
> configuration is chosen:
>
> > ------------[ cut here ]------------
> > WARNING: at kernel/softirq.c:143 local_bh_enable_ip+0x44/0xc0()
> > Modules linked in:
> > [<c002d73c>] (unwind_backtrace+0x0/0xf0) from [<c0043b60>] (warn_slowpath_common+0x4c/0x64)
> > [<c0043b60>] (warn_slowpath_common+0x4c/0x64) from [<c0043b90>] (warn_slowpath_null+0x18/0x1c)
> > [<c0043b90>] (warn_slowpath_null+0x18/0x1c) from [<c0049024>] (local_bh_enable_ip+0x44/0xc0)
> > [<c0049024>] (local_bh_enable_ip+0x44/0xc0) from [<c025ef18>] (dev_txq_stats_fold+0xac/0x108)
> > [<c025ef18>] (dev_txq_stats_fold+0xac/0x108) from [<c025f018>] (dev_get_stats+0xa4/0xac)
> > [<c025f018>] (dev_get_stats+0xa4/0xac) from [<c01f72bc>] (gen_ndis_query_resp+0x4c/0x43c)
> > [<c01f72bc>] (gen_ndis_query_resp+0x4c/0x43c) from [<c01f784c>] (rndis_msg_parser+0x1a0/0x32c)
> > [<c01f784c>] (rndis_msg_parser+0x1a0/0x32c) from [<c01f79f8>] (rndis_command_complete+0x20/0x4c)
> > [<c01f79f8>] (rndis_command_complete+0x20/0x4c) from [<c01f3278>] (done+0x5c/0x70)
> > [<c01f3278>] (done+0x5c/0x70) from [<c01f3c60>] (complete_tx+0xf0/0x1a8)
> > [<c01f3c60>] (complete_tx+0xf0/0x1a8) from [<c01f3d8c>] (process_ep_in_intr+0x74/0x14c)
> > [<c01f3d8c>] (process_ep_in_intr+0x74/0x14c) from [<c01f470c>] (s3c_udc_irq+0x2c8/0x3f4)
> > [<c01f470c>] (s3c_udc_irq+0x2c8/0x3f4) from [<c006dfd0>] (handle_IRQ_event+0x24/0xe4)
> > [<c006dfd0>] (handle_IRQ_event+0x24/0xe4) from [<c006fe40>] (handle_level_irq+0xb0/0x12c)
> > [<c006fe40>] (handle_level_irq+0xb0/0x12c) from [<c0027074>] (asm_do_IRQ+0x74/0x98)
> > [<c0027074>] (asm_do_IRQ+0x74/0x98) from [<c0027ca4>] (__irq_usr+0x44/0xc0)
> > Exception stack(0xe735ffb0 to 0xe735fff8)
> > ffa0: 000cc328 00000000 00000000 bec76520
> > ffc0: 000ee008 000bd210 00000000 00000000 000ee068 000ee008 bec76524 bec76520
> > ffe0: 000ec108 bec76508 000471f4 000458e8 80000010 ffffffff
> > ---[ end trace 92e33c96fb76fb3d ]---
>
> After some investigation I found out that commit
> bd27290a593f80cb99e95287cb29c72c0d57608b is the culprit:
>
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index fdc3f29..b626289 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -501,9 +501,9 @@ struct netdev_queue {
> > * please use this field instead of dev->trans_start
> > */
> > unsigned long trans_start;
> > - unsigned long tx_bytes;
> > - unsigned long tx_packets;
> > - unsigned long tx_dropped;
> > + u64 tx_bytes;
> > + u64 tx_packets;
> > + u64 tx_dropped;
> > } ____cacheline_aligned_in_smp;
> >
> > #ifdef CONFIG_RPS
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 1c002c7..9de75cd 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -5282,15 +5282,17 @@ void netdev_run_todo(void)
> > void dev_txq_stats_fold(const struct net_device *dev,
> > struct rtnl_link_stats64 *stats)
> > {
> > - unsigned long tx_bytes = 0, tx_packets = 0, tx_dropped = 0;
> > + u64 tx_bytes = 0, tx_packets = 0, tx_dropped = 0;
> > unsigned int i;
> > struct netdev_queue *txq;
> >
> > for (i = 0; i < dev->num_tx_queues; i++) {
> > txq = netdev_get_tx_queue(dev, i);
> > + spin_lock_bh(&txq->_xmit_lock);
> > tx_bytes += txq->tx_bytes;
> > tx_packets += txq->tx_packets;
> > tx_dropped += txq->tx_dropped;
> > + spin_unlock_bh(&txq->_xmit_lock);
> > }
> > if (tx_bytes || tx_packets || tx_dropped) {
> > stats->tx_bytes = tx_bytes;
>
> Changing spin_lock_bh() to spin_lock_irqsave().
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 1ae6543..278bd08 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5289,12 +5289,13 @@ void dev_txq_stats_fold(const struct net_device *dev,
> struct netdev_queue *txq;
>
> for (i = 0; i < dev->num_tx_queues; i++) {
> + unsigned long flags;
> txq = netdev_get_tx_queue(dev, i);
> - spin_lock_bh(&txq->_xmit_lock);
> + spin_lock_irqsave(&txq->_xmit_lock, flags);
> tx_bytes += txq->tx_bytes;
> tx_packets += txq->tx_packets;
> tx_dropped += txq->tx_dropped;
> - spin_unlock_bh(&txq->_xmit_lock);
> + spin_unlock_irqrestore(&txq->_xmit_lock, flags);
> }
> if (tx_bytes || tx_packets || tx_dropped) {
> stats->tx_bytes = tx_bytes;
Hmm, while your patch is technically correct, it seems strange all this
stuff runs in hard irq context.
If we accept dev_get_stats() being called from hard irq context, we must
audit all ndo_get_stats() & ndo_get_stats64() to make sure they use
spin_lock_irqsave() variants. I am pretty sure we have a lot of work...
For example:
drivers/net/macvlan.c uses a _bh() variant
drivers/net/cxgb4vf/cxgb4vf_main.c uses a spin_lock()
drivers/net/bonding/bond_main.c uses a read_lock_bh()
drivers/net/sunhme.c uses a spin_lock_irq()/spin_unlock_irq()
drivers/net/ehea/ehea_main.c uses a get_zeroed_page(GFP_KERNEL);
drivers/net/sfc/efx.c uses a spin_lock_bh()
...
dev_get_stats(dev, &temp) is called from drivers/usb/gadget/rndis.c,
while I suspect underlying stats are already provided in dev->stats
^ permalink raw reply
* Re: [PATCH] qlcnic: dont assume NET_IP_ALIGN is 2
From: David Miller @ 2010-09-20 15:58 UTC (permalink / raw)
To: amit.salecha; +Cc: eric.dumazet, netdev, ameen.rahman, anirban.chakraborty
In-Reply-To: <99737F4847ED0A48AECC9F4A1974A4B80F86F8018D@MNEXMB2.qlogic.org>
From: Amit Salecha <amit.salecha@qlogic.com>
Date: Mon, 20 Sep 2010 06:16:31 -0500
> Though I have one doubt. We are allocating larger packets than the actual data used.
> Doesn't it will break accounting ?
No, it will "fix" accounting.
You must charge to the SKB all of the non-shared memory that was
allocated to the SKB.
This means even if the packet only uses 128 bytes of the SKB
data area, you must still account for the full blob of linear
memory that was allocated for the SKB data area in skb->truesize.
Otherwise remote attackers could consume enormous amounts of memory by
tricking our socket accounting via carefully sized packets.
^ permalink raw reply
* neighbour struct
From: Nicola Padovano @ 2010-09-20 15:09 UTC (permalink / raw)
To: netfilter-devel, netdev
Hello there.
In the neighbour struct i see the field: struct neighbour* next.
what does this field mean? What is its usage?
Thanks!
--
Nicola Padovano
e-mail: nicola.padovano@gmail.com
web: http://npadovano.altervista.org
^ permalink raw reply
* [PATCH] net: core: dev.c: use spin_lock_irqsave() rather than
From: Michal Nazarewicz @ 2010-09-20 15:08 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, David S. Miller, Eric Dumazet
Cc: linux-kernel, linux-usb, netdev, Eric Dumazet
This commit fixes a warning that was issued when g_ether gadget
was connected to Windows host. In g_ether, the dev_txq_stats_fold()
can be called from context other then soft-irq so _bh version of
spin_lock is not adequate.
Changing from spin_lock_bh() to spin_lock_irqsave() is always safe (as
irqsave is superset of all other spin lock operations) is always safe
so this commit should not break anything.
As Eric Dumazet said, dev_txq_stats_fold() is a slow patch so there
is no need to optimise that much.
Signed-off-by: Michal Nazarewicz <m.nazarewicz@samsung.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
---
net/core/dev.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
Hello David,
could you pull this patch. I think it's best to get it in 2.6.36.
Without this patch, I got the following warning when RNDIS
configuration is chosen:
> ------------[ cut here ]------------
> WARNING: at kernel/softirq.c:143 local_bh_enable_ip+0x44/0xc0()
> Modules linked in:
> [<c002d73c>] (unwind_backtrace+0x0/0xf0) from [<c0043b60>] (warn_slowpath_common+0x4c/0x64)
> [<c0043b60>] (warn_slowpath_common+0x4c/0x64) from [<c0043b90>] (warn_slowpath_null+0x18/0x1c)
> [<c0043b90>] (warn_slowpath_null+0x18/0x1c) from [<c0049024>] (local_bh_enable_ip+0x44/0xc0)
> [<c0049024>] (local_bh_enable_ip+0x44/0xc0) from [<c025ef18>] (dev_txq_stats_fold+0xac/0x108)
> [<c025ef18>] (dev_txq_stats_fold+0xac/0x108) from [<c025f018>] (dev_get_stats+0xa4/0xac)
> [<c025f018>] (dev_get_stats+0xa4/0xac) from [<c01f72bc>] (gen_ndis_query_resp+0x4c/0x43c)
> [<c01f72bc>] (gen_ndis_query_resp+0x4c/0x43c) from [<c01f784c>] (rndis_msg_parser+0x1a0/0x32c)
> [<c01f784c>] (rndis_msg_parser+0x1a0/0x32c) from [<c01f79f8>] (rndis_command_complete+0x20/0x4c)
> [<c01f79f8>] (rndis_command_complete+0x20/0x4c) from [<c01f3278>] (done+0x5c/0x70)
> [<c01f3278>] (done+0x5c/0x70) from [<c01f3c60>] (complete_tx+0xf0/0x1a8)
> [<c01f3c60>] (complete_tx+0xf0/0x1a8) from [<c01f3d8c>] (process_ep_in_intr+0x74/0x14c)
> [<c01f3d8c>] (process_ep_in_intr+0x74/0x14c) from [<c01f470c>] (s3c_udc_irq+0x2c8/0x3f4)
> [<c01f470c>] (s3c_udc_irq+0x2c8/0x3f4) from [<c006dfd0>] (handle_IRQ_event+0x24/0xe4)
> [<c006dfd0>] (handle_IRQ_event+0x24/0xe4) from [<c006fe40>] (handle_level_irq+0xb0/0x12c)
> [<c006fe40>] (handle_level_irq+0xb0/0x12c) from [<c0027074>] (asm_do_IRQ+0x74/0x98)
> [<c0027074>] (asm_do_IRQ+0x74/0x98) from [<c0027ca4>] (__irq_usr+0x44/0xc0)
> Exception stack(0xe735ffb0 to 0xe735fff8)
> ffa0: 000cc328 00000000 00000000 bec76520
> ffc0: 000ee008 000bd210 00000000 00000000 000ee068 000ee008 bec76524 bec76520
> ffe0: 000ec108 bec76508 000471f4 000458e8 80000010 ffffffff
> ---[ end trace 92e33c96fb76fb3d ]---
After some investigation I found out that commit
bd27290a593f80cb99e95287cb29c72c0d57608b is the culprit:
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index fdc3f29..b626289 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -501,9 +501,9 @@ struct netdev_queue {
> * please use this field instead of dev->trans_start
> */
> unsigned long trans_start;
> - unsigned long tx_bytes;
> - unsigned long tx_packets;
> - unsigned long tx_dropped;
> + u64 tx_bytes;
> + u64 tx_packets;
> + u64 tx_dropped;
> } ____cacheline_aligned_in_smp;
>
> #ifdef CONFIG_RPS
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 1c002c7..9de75cd 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5282,15 +5282,17 @@ void netdev_run_todo(void)
> void dev_txq_stats_fold(const struct net_device *dev,
> struct rtnl_link_stats64 *stats)
> {
> - unsigned long tx_bytes = 0, tx_packets = 0, tx_dropped = 0;
> + u64 tx_bytes = 0, tx_packets = 0, tx_dropped = 0;
> unsigned int i;
> struct netdev_queue *txq;
>
> for (i = 0; i < dev->num_tx_queues; i++) {
> txq = netdev_get_tx_queue(dev, i);
> + spin_lock_bh(&txq->_xmit_lock);
> tx_bytes += txq->tx_bytes;
> tx_packets += txq->tx_packets;
> tx_dropped += txq->tx_dropped;
> + spin_unlock_bh(&txq->_xmit_lock);
> }
> if (tx_bytes || tx_packets || tx_dropped) {
> stats->tx_bytes = tx_bytes;
Changing spin_lock_bh() to spin_lock_irqsave().
diff --git a/net/core/dev.c b/net/core/dev.c
index 1ae6543..278bd08 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5289,12 +5289,13 @@ void dev_txq_stats_fold(const struct net_device *dev,
struct netdev_queue *txq;
for (i = 0; i < dev->num_tx_queues; i++) {
+ unsigned long flags;
txq = netdev_get_tx_queue(dev, i);
- spin_lock_bh(&txq->_xmit_lock);
+ spin_lock_irqsave(&txq->_xmit_lock, flags);
tx_bytes += txq->tx_bytes;
tx_packets += txq->tx_packets;
tx_dropped += txq->tx_dropped;
- spin_unlock_bh(&txq->_xmit_lock);
+ spin_unlock_irqrestore(&txq->_xmit_lock, flags);
}
if (tx_bytes || tx_packets || tx_dropped) {
stats->tx_bytes = tx_bytes;
--
1.7.1
^ permalink raw reply related
* [PATCH v6] netfilter: save the hash of the tuple in the original direction for latter use
From: Changli Gao @ 2010-09-20 15:06 UTC (permalink / raw)
To: Patrick McHardy
Cc: David S. Miller, Eric Dumazet, Mathieu Desnoyers, akpm,
netfilter-devel, netdev, linux-kernel, Changli Gao
Since we don't change the tuple in the original direction, we can save it
in ct->tuplehash[IP_CT_DIR_REPLY].hnode.pprev for __nf_conntrack_confirm()
use.
__hash_conntrack() is split into two steps: hash_conntrack_raw() is used
to get the raw hash, and __hash_bucket() is used to get the bucket id.
In SYN-flood case, early_drop() doesn't need to recompute the hash again.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
v6: some advices from Patrick.
v5: respin it due to the change of 1/2.
v4: init rnd when allocating conntrack.
v3: define static variable rnd out of the function ____hash_conntrack(),
and call get_random_bytes() until we get a non-zero random int.
v2: use cmpxchg() to save 2 variables.
net/netfilter/nf_conntrack_core.c | 112 ++++++++++++++++++++++++++------------
1 file changed, 78 insertions(+), 34 deletions(-)
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 4c0ad9b..1eacf8d 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -67,29 +67,40 @@ EXPORT_PER_CPU_SYMBOL(nf_conntrack_untracked);
static unsigned int nf_conntrack_hash_rnd __read_mostly;
-static u_int32_t __hash_conntrack(const struct nf_conntrack_tuple *tuple,
- u16 zone, unsigned int size, unsigned int rnd)
+static u32 hash_conntrack_raw(const struct nf_conntrack_tuple *tuple, u16 zone)
{
unsigned int n;
- u_int32_t h;
/* The direction must be ignored, so we hash everything up to the
* destination ports (which is a multiple of 4) and treat the last
* three bytes manually.
*/
n = (sizeof(tuple->src) + sizeof(tuple->dst.u3)) / sizeof(u32);
- h = jhash2((u32 *)tuple, n,
- zone ^ rnd ^ (((__force __u16)tuple->dst.u.all << 16) |
- tuple->dst.protonum));
+ return jhash2((u32 *)tuple, n, zone ^ nf_conntrack_hash_rnd ^
+ (((__force __u16)tuple->dst.u.all << 16) |
+ tuple->dst.protonum));
+}
+
+static u32 __hash_bucket(u32 hash, unsigned int size)
+{
+ return ((u64)hash * size) >> 32;
+}
+
+static u32 hash_bucket(u32 hash, const struct net *net)
+{
+ return __hash_bucket(hash, net->ct.htable_size);
+}
- return ((u64)h * size) >> 32;
+static u_int32_t __hash_conntrack(const struct nf_conntrack_tuple *tuple,
+ u16 zone, unsigned int size)
+{
+ return __hash_bucket(hash_conntrack_raw(tuple, zone), size);
}
static inline u_int32_t hash_conntrack(const struct net *net, u16 zone,
const struct nf_conntrack_tuple *tuple)
{
- return __hash_conntrack(tuple, zone, net->ct.htable_size,
- nf_conntrack_hash_rnd);
+ return __hash_conntrack(tuple, zone, net->ct.htable_size);
}
bool
@@ -291,20 +302,20 @@ static void death_by_timeout(unsigned long ul_conntrack)
* OR
* - Caller must lock nf_conntrack_lock before calling this function
*/
-struct nf_conntrack_tuple_hash *
-__nf_conntrack_find(struct net *net, u16 zone,
- const struct nf_conntrack_tuple *tuple)
+static struct nf_conntrack_tuple_hash *
+____nf_conntrack_find(struct net *net, u16 zone,
+ const struct nf_conntrack_tuple *tuple, u32 hash)
{
struct nf_conntrack_tuple_hash *h;
struct hlist_nulls_node *n;
- unsigned int hash = hash_conntrack(net, zone, tuple);
+ unsigned int bucket = hash_bucket(hash, net);
/* Disable BHs the entire time since we normally need to disable them
* at least once for the stats anyway.
*/
local_bh_disable();
begin:
- hlist_nulls_for_each_entry_rcu(h, n, &net->ct.hash[hash], hnnode) {
+ hlist_nulls_for_each_entry_rcu(h, n, &net->ct.hash[bucket], hnnode) {
if (nf_ct_tuple_equal(tuple, &h->tuple) &&
nf_ct_zone(nf_ct_tuplehash_to_ctrack(h)) == zone) {
NF_CT_STAT_INC(net, found);
@@ -318,7 +329,7 @@ begin:
* not the expected one, we must restart lookup.
* We probably met an item that was moved to another chain.
*/
- if (get_nulls_value(n) != hash) {
+ if (get_nulls_value(n) != bucket) {
NF_CT_STAT_INC(net, search_restart);
goto begin;
}
@@ -326,19 +337,27 @@ begin:
return NULL;
}
+
+struct nf_conntrack_tuple_hash *
+__nf_conntrack_find(struct net *net, u16 zone,
+ const struct nf_conntrack_tuple *tuple)
+{
+ return ____nf_conntrack_find(net, zone, tuple,
+ hash_conntrack_raw(tuple, zone));
+}
EXPORT_SYMBOL_GPL(__nf_conntrack_find);
/* Find a connection corresponding to a tuple. */
-struct nf_conntrack_tuple_hash *
-nf_conntrack_find_get(struct net *net, u16 zone,
- const struct nf_conntrack_tuple *tuple)
+static struct nf_conntrack_tuple_hash *
+__nf_conntrack_find_get(struct net *net, u16 zone,
+ const struct nf_conntrack_tuple *tuple, u32 hash)
{
struct nf_conntrack_tuple_hash *h;
struct nf_conn *ct;
rcu_read_lock();
begin:
- h = __nf_conntrack_find(net, zone, tuple);
+ h = ____nf_conntrack_find(net, zone, tuple, hash);
if (h) {
ct = nf_ct_tuplehash_to_ctrack(h);
if (unlikely(nf_ct_is_dying(ct) ||
@@ -356,6 +375,14 @@ begin:
return h;
}
+
+struct nf_conntrack_tuple_hash *
+nf_conntrack_find_get(struct net *net, u16 zone,
+ const struct nf_conntrack_tuple *tuple)
+{
+ return __nf_conntrack_find_get(net, zone, tuple,
+ hash_conntrack_raw(tuple, zone));
+}
EXPORT_SYMBOL_GPL(nf_conntrack_find_get);
static void __nf_conntrack_hash_insert(struct nf_conn *ct,
@@ -408,8 +435,11 @@ __nf_conntrack_confirm(struct sk_buff *skb)
return NF_ACCEPT;
zone = nf_ct_zone(ct);
- hash = hash_conntrack(net, zone, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
- repl_hash = hash_conntrack(net, zone, &ct->tuplehash[IP_CT_DIR_REPLY].tuple);
+ /* reuse the hash saved before */
+ hash = *(unsigned long *)&ct->tuplehash[IP_CT_DIR_REPLY].hnnode.pprev;
+ hash = hash_bucket(hash, net);
+ repl_hash = hash_conntrack(net, zone,
+ &ct->tuplehash[IP_CT_DIR_REPLY].tuple);
/* We're not in hash table, and we refuse to set up related
connections for unconfirmed conns. But packet copies and
@@ -566,10 +596,11 @@ static noinline int early_drop(struct net *net, unsigned int hash)
return dropped;
}
-struct nf_conn *nf_conntrack_alloc(struct net *net, u16 zone,
- const struct nf_conntrack_tuple *orig,
- const struct nf_conntrack_tuple *repl,
- gfp_t gfp)
+static struct nf_conn *
+__nf_conntrack_alloc(struct net *net, u16 zone,
+ const struct nf_conntrack_tuple *orig,
+ const struct nf_conntrack_tuple *repl,
+ gfp_t gfp, u32 hash)
{
struct nf_conn *ct;
@@ -585,6 +616,9 @@ struct nf_conn *nf_conntrack_alloc(struct net *net, u16 zone,
get_random_bytes(&rand, sizeof(rand));
} while (!rand);
cmpxchg(&nf_conntrack_hash_rnd, 0, rand);
+
+ /* recompute the hash as nf_conntrack_hash_rnd is initialized */
+ hash = hash_conntrack_raw(orig, zone);
}
/* We don't want any race condition at early drop stage */
@@ -592,8 +626,7 @@ struct nf_conn *nf_conntrack_alloc(struct net *net, u16 zone,
if (nf_conntrack_max &&
unlikely(atomic_read(&net->ct.count) > nf_conntrack_max)) {
- unsigned int hash = hash_conntrack(net, zone, orig);
- if (!early_drop(net, hash)) {
+ if (!early_drop(net, hash_bucket(hash, net))) {
atomic_dec(&net->ct.count);
if (net_ratelimit())
printk(KERN_WARNING
@@ -623,7 +656,8 @@ struct nf_conn *nf_conntrack_alloc(struct net *net, u16 zone,
ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple = *orig;
ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode.pprev = NULL;
ct->tuplehash[IP_CT_DIR_REPLY].tuple = *repl;
- ct->tuplehash[IP_CT_DIR_REPLY].hnnode.pprev = NULL;
+ /* save hash for reusing when confirming */
+ *(unsigned long *)(&ct->tuplehash[IP_CT_DIR_REPLY].hnnode.pprev) = hash;
/* Don't set timer yet: wait for confirmation */
setup_timer(&ct->timeout, death_by_timeout, (unsigned long)ct);
write_pnet(&ct->ct_net, net);
@@ -650,6 +684,14 @@ out_free:
return ERR_PTR(-ENOMEM);
#endif
}
+
+struct nf_conn *nf_conntrack_alloc(struct net *net, u16 zone,
+ const struct nf_conntrack_tuple *orig,
+ const struct nf_conntrack_tuple *repl,
+ gfp_t gfp)
+{
+ return __nf_conntrack_alloc(net, zone, orig, repl, gfp, 0);
+}
EXPORT_SYMBOL_GPL(nf_conntrack_alloc);
void nf_conntrack_free(struct nf_conn *ct)
@@ -671,7 +713,7 @@ init_conntrack(struct net *net, struct nf_conn *tmpl,
struct nf_conntrack_l3proto *l3proto,
struct nf_conntrack_l4proto *l4proto,
struct sk_buff *skb,
- unsigned int dataoff)
+ unsigned int dataoff, u32 hash)
{
struct nf_conn *ct;
struct nf_conn_help *help;
@@ -685,7 +727,8 @@ init_conntrack(struct net *net, struct nf_conn *tmpl,
return NULL;
}
- ct = nf_conntrack_alloc(net, zone, tuple, &repl_tuple, GFP_ATOMIC);
+ ct = __nf_conntrack_alloc(net, zone, tuple, &repl_tuple, GFP_ATOMIC,
+ hash);
if (IS_ERR(ct)) {
pr_debug("Can't allocate conntrack.\n");
return (struct nf_conntrack_tuple_hash *)ct;
@@ -762,6 +805,7 @@ resolve_normal_ct(struct net *net, struct nf_conn *tmpl,
struct nf_conntrack_tuple_hash *h;
struct nf_conn *ct;
u16 zone = tmpl ? nf_ct_zone(tmpl) : NF_CT_DEFAULT_ZONE;
+ u32 hash;
if (!nf_ct_get_tuple(skb, skb_network_offset(skb),
dataoff, l3num, protonum, &tuple, l3proto,
@@ -771,10 +815,11 @@ resolve_normal_ct(struct net *net, struct nf_conn *tmpl,
}
/* look for tuple match */
- h = nf_conntrack_find_get(net, zone, &tuple);
+ hash = hash_conntrack_raw(&tuple, zone);
+ h = __nf_conntrack_find_get(net, zone, &tuple, hash);
if (!h) {
h = init_conntrack(net, tmpl, &tuple, l3proto, l4proto,
- skb, dataoff);
+ skb, dataoff, hash);
if (!h)
return NULL;
if (IS_ERR(h))
@@ -1314,8 +1359,7 @@ int nf_conntrack_set_hashsize(const char *val, struct kernel_param *kp)
ct = nf_ct_tuplehash_to_ctrack(h);
hlist_nulls_del_rcu(&h->hnnode);
bucket = __hash_conntrack(&h->tuple, nf_ct_zone(ct),
- hashsize,
- nf_conntrack_hash_rnd);
+ hashsize);
hlist_nulls_add_head_rcu(&h->hnnode, &hash[bucket]);
}
}
^ permalink raw reply related
* Re: [PATCH v5 2/2] netfilter: save the hash of the tuple in the original direction for latter use
From: Changli Gao @ 2010-09-20 15:04 UTC (permalink / raw)
To: Patrick McHardy
Cc: David S. Miller, Eric Dumazet, Mathieu Desnoyers, akpm,
netfilter-devel, netdev, linux-kernel
In-Reply-To: <4C91B6BE.8020300@trash.net>
On Thu, Sep 16, 2010 at 2:18 PM, Patrick McHardy <kaber@trash.net> wrote:
> On 21.08.2010 00:49, Changli Gao wrote:
>> Since we don't change the tuple in the original direction, we can save it
>> in ct->tuplehash[IP_CT_DIR_REPLY].hnode.pprev for __nf_conntrack_confirm()
>> use.
>
> I like this idea. We could actually do the same for the reply tuple
> and invalidate the saved hash in case the reply tuple is changed
> (nf_conntrack_alter_reply()), which only happens when NAT is used.
>
We can't do that, as the unconfirmed ct owned maybe dropped, and
pre-computing will wast CPU cycles in this case.
>> __hash_conntrack() is split into two steps: ____hash_conntrack() is used
>> to get the raw hash, and __hash_bucket() is used to get the bucket id.
>
> This patch uses underscores a bit excessively, how about renaming:
>
> - ____hash_conntrack() => hash_conntrack_raw()
> - __hash variables => hash
> - hash variables => bucket
OK. Thanks.
>
>> @@ -408,7 +438,8 @@ __nf_conntrack_confirm(struct sk_buff *skb)
>> return NF_ACCEPT;
>>
>> zone = nf_ct_zone(ct);
>> - hash = hash_conntrack(net, zone, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
>> + /* reuse the __hash saved before */
>> + hash = hash_bucket(*(unsigned long *)&ct->tuplehash[IP_CT_DIR_REPLY].hnnode.pprev, net);
>
> Please try to stay at least close to the 80 characters limit.
>
OK. Thanks.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ 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