* Re: Linux sends IPv6 NS packets with the link-local address
From: Hannes Frederic Sowa @ 2013-11-07 0:46 UTC (permalink / raw)
To: Steinar H. Gunderson; +Cc: netdev, ayourtch
In-Reply-To: <20131107002946.GA16324@sesse.net>
Hi Steinar,
On Thu, Nov 07, 2013 at 01:29:46AM +0100, Steinar H. Gunderson wrote:
> Since this is a SHOULD, there's nothing inherently _illegal_ about sending the
> packets with the link-layer address as source, but the RFC does have a point.
> Most of the time, Linux indeed seems to be sending with the global source
> address (heeding the SHOULD), and again, for some reason it suddenly switches
> to it in this case.
>
> Would anyone happen to know why it uses the link-local in the first place,
> and why it suddenly changes its mind after ten tries or so? I'm not sure if I
> can _force_ this behavior, but judging from a tcpdump, it happens many times
> a day from that specific machine.
I am currently a bit busy to look into this closely but maybe there is
a patch which does help; it is currently in net-next:
<https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/net/core/neighbour.c?id=4ed377e36ec2f385484d12e516faf88516fad31c>
Problem was, if we enqueue a packet to the resolving queue we use those
packets source address as the address we use as the source address. This
patch chooses the last enqueued skb's source address to do the resolving
thus switching over to a global address much more early (hopefully). I
have seen this problem with routers doing uRPF on the source address of
arp packets, too.
Does this happen when you configure addresses? Do your addresses have
a short lifetime advertised by radvd etc?
Greetings,
Hannes
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Herbert Xu @ 2013-11-07 0:47 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <1383783321.2878.6.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, Nov 06, 2013 at 04:15:21PM -0800, Eric Dumazet wrote:
>
> Here is the patch I came into (I tested it and it works very fine)
Thanks. However, unless I'm missing something aren't you now
copying every linear skb frag_list? With the current code we
just do a clone and add the headers.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH net-next v4 3/9] static_key: WARN on usage before jump_label_init was called
From: Hannes Frederic Sowa @ 2013-11-07 0:50 UTC (permalink / raw)
To: Steven Rostedt
Cc: netdev, linux-kernel, Peter Zijlstra, Andi Kleen, Ingo Molnar,
Jason Baron
In-Reply-To: <20131106161649.2bb1dbb0@gandalf.local.home>
On Wed, Nov 06, 2013 at 04:16:49PM -0500, Steven Rostedt wrote:
> Sorry for the late reply, but this was sent while I was getting ready
> for my two week conference trip.
>
> Note, this should not go through the net tree, but instead should go
> through tip, as it deals with jump labels and not networking.
>
> Otherwise, this patch looks good.
>
> Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
Thanks for the review!
The patch already got queued up in net-next. Is this ok or what do you
suggest to resolve this?
Greetings,
Hannes
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Eric Dumazet @ 2013-11-07 0:56 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131107004704.GB28156@gondor.apana.org.au>
On Thu, 2013-11-07 at 08:47 +0800, Herbert Xu wrote:
> On Wed, Nov 06, 2013 at 04:15:21PM -0800, Eric Dumazet wrote:
> >
> > Here is the patch I came into (I tested it and it works very fine)
>
> Thanks. However, unless I'm missing something aren't you now
> copying every linear skb frag_list? With the current code we
> just do a clone and add the headers.
No copy at all.
As explained, all RX skb are now backed to a page frag. (see
skb->head_frag)
For safety I probably need to check this is safe.
^ permalink raw reply
* Re: Linux sends IPv6 NS packets with the link-local address
From: Steinar H. Gunderson @ 2013-11-07 0:56 UTC (permalink / raw)
To: netdev, ayourtch
In-Reply-To: <20131107004610.GB8144@order.stressinduktion.org>
On Thu, Nov 07, 2013 at 01:46:11AM +0100, Hannes Frederic Sowa wrote:
> Problem was, if we enqueue a packet to the resolving queue we use those
> packets source address as the address we use as the source address. This
> patch chooses the last enqueued skb's source address to do the resolving
> thus switching over to a global address much more early (hopefully). I
> have seen this problem with routers doing uRPF on the source address of
> arp packets, too.
>
> Does this happen when you configure addresses? Do your addresses have
> a short lifetime advertised by radvd etc?
There are no autoconfigured addresses on the machine; statically set plus
link-layer only. It does _serve_ RA for another VLAN, though (it does
802.1q, some tunnels, and some BGP).
/* Steinar */
--
Homepage: http://www.sesse.net/
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Herbert Xu @ 2013-11-07 1:00 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <1383785763.2878.8.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, Nov 06, 2013 at 04:56:03PM -0800, Eric Dumazet wrote:
> On Thu, 2013-11-07 at 08:47 +0800, Herbert Xu wrote:
> > On Wed, Nov 06, 2013 at 04:15:21PM -0800, Eric Dumazet wrote:
> > >
> > > Here is the patch I came into (I tested it and it works very fine)
> >
> > Thanks. However, unless I'm missing something aren't you now
> > copying every linear skb frag_list? With the current code we
> > just do a clone and add the headers.
>
> No copy at all.
>
> As explained, all RX skb are now backed to a page frag. (see
> skb->head_frag)
Even if we have called pskb_expand_head on it?
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH net-next v4 3/9] static_key: WARN on usage before jump_label_init was called
From: Steven Rostedt @ 2013-11-07 1:02 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: netdev, linux-kernel, Peter Zijlstra, Andi Kleen, Ingo Molnar,
Jason Baron
In-Reply-To: <20131107005027.GC8144@order.stressinduktion.org>
On Thu, 7 Nov 2013 01:50:27 +0100
Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> On Wed, Nov 06, 2013 at 04:16:49PM -0500, Steven Rostedt wrote:
> > Sorry for the late reply, but this was sent while I was getting ready
> > for my two week conference trip.
> >
> > Note, this should not go through the net tree, but instead should go
> > through tip, as it deals with jump labels and not networking.
> >
> > Otherwise, this patch looks good.
> >
> > Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
>
> Thanks for the review!
>
> The patch already got queued up in net-next. Is this ok or what do you
> suggest to resolve this?
>
I'm fine, but really. Changes need to go through the trees they are
maintained by. Would Dave Miller like it if I pushed patches that
touched the net directory without a single Ack?
-- Steve
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Eric Dumazet @ 2013-11-07 1:03 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton, mst, Jason Wang
In-Reply-To: <20131107003658.GA27976@gondor.apana.org.au>
On Thu, 2013-11-07 at 08:36 +0800, Herbert Xu wrote:
> On Wed, Nov 06, 2013 at 07:01:10AM -0800, Eric Dumazet wrote:
> > Have you thought about arches having PAGE_SIZE=65536, and how bad it is
> > to use a full page per network frame ? It is lazy and x86 centered.
>
> So instead if we were sending a full 64K packet on such an arch to
> another guest, we'd now chop it up into 1.5K chunks and reassemble them.
>
Yep, and speed is now better than before the patches.
I understand you do not believe it. But this is the truth.
And now your guest can receive a bunch of small UDP frames, without
having to drop them because sk->rcvbuf limit is hit.
> > So after our patches, we now have an optimal situation, even on these
> > arches.
>
> Optimal only for physical incoming packets with no jumbo frames.
Have you actually tested this ?
>
> What's worse, I now realise that the coalesce thing isn't even
> guaranteed to work. It probably works in your benchmarks because
> you're working with freshly allocated pages.
>
Oh well.
> But once the system has been running for a while, I see nothing
> in the virtio_net code that tries to prevent fragmentation. Once
> fragmentation sets in, you'll be back in the terrible situation
> that we were in prior to the coalesce patch.
>
There is no fragmentation, since we allocate 32Kb pages.
Michael Dalton worked on a patch to add EWMA for auto sizing and a
private page_frag per virtio queue, instead of using the per cpu one.
On x86 :
- All offloads enabled (average packet size should be >> MTU-size)
net-next trunk w/ virtio_net prior to 2613af0ed (PAGE_SIZE bufs): 14179.17Gb/s
net-next trunk (MTU-size bufs): 13390.69Gb/s
net-next trunk + auto-tune - 14358.41Gb/s
- guest_tso4/guest_csum disabled (forces MTU-sized packets on receiver)
net-next trunk w/ virtio_net prior to 2613af0ed: 4059.49Gb/s
net-next trunk (MTU 1500- packet takes two bufs due to sizing bug): 4174.30Gb/s
net-next trunk (MTU 1480- packet fits in one buf): 6672.16Gb/s
net-next trunk + auto-tune (MTU 1500- fixed, packet uses one buf) - 6791.28Gb/s
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Eric Dumazet @ 2013-11-07 1:08 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131107010037.GA28421@gondor.apana.org.au>
On Thu, 2013-11-07 at 09:00 +0800, Herbert Xu wrote:
> On Wed, Nov 06, 2013 at 04:56:03PM -0800, Eric Dumazet wrote:
> > On Thu, 2013-11-07 at 08:47 +0800, Herbert Xu wrote:
> > > On Wed, Nov 06, 2013 at 04:15:21PM -0800, Eric Dumazet wrote:
> > > >
> > > > Here is the patch I came into (I tested it and it works very fine)
> > >
> > > Thanks. However, unless I'm missing something aren't you now
> > > copying every linear skb frag_list? With the current code we
> > > just do a clone and add the headers.
> >
> > No copy at all.
> >
> > As explained, all RX skb are now backed to a page frag. (see
> > skb->head_frag)
>
> Even if we have called pskb_expand_head on it?
Well, in this case we pulled only the headers in skb->head.
Only few buggy drivers do a pull of say 64 bytes in skb->head before
calling eth_type_trans() so could possibly have a few bytes of TCP
payload in skb->head
And this case is handled without copy.
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Hannes Frederic Sowa @ 2013-11-07 1:13 UTC (permalink / raw)
To: Eric Dumazet
Cc: Herbert Xu, Ben Hutchings, David Miller, christoph.paasch, netdev,
hkchu, mwdalton
In-Reply-To: <1383783321.2878.6.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, Nov 06, 2013 at 04:15:21PM -0800, Eric Dumazet wrote:
> On Wed, 2013-11-06 at 11:47 -0800, Eric Dumazet wrote:
> > I'll try a different way.
> >
> > The frag_list would contain a bunch of frags, that we logically add to the bunch
> > of frags found in the first skb shared_info structure.
>
> Here is the patch I came into (I tested it and it works very fine)
>
> The theory is that in GRO stack, all skbs use the head_frag trick,
> so even if one NIC pulled some payload into skb->head, we do not have to
> copy anything. Outside of GRO stack, we are not supposed to provide data
> in skb->head (I am speaking of the skb found on the frag_list extension,
> not the skb_head)
>
> I put a fallback code, just in case, with a WARN_ON_ONCE() so that we
> can catch the offenders (if any) to fix them.
>
> I renamed @skb to @skb_head to more clearly document this code.
> Same for @i renamed to @cur_frag
I wanted to understand this code more closely and tried it with a test case I
used for the UDP_CORK bugs and also for the tbf panic.
The packet is allocated as an UFO one and gets segmented by tbf.
# tc qdisc replace dev eth0 root tbf rate 200kbit latency 20ms burst 5kb
# ./udptest
(Just doing two writes of 200 bytes, then a write of 4096 bytes on a udp
socket. I can send you the source (or a stripped down version, because it got
realy noisy.))
[ 370.372237] ------------[ cut here ]------------
[ 370.374110] WARNING: CPU: 1 PID: 9359 at net/core/skbuff.c:2875 skb_segment+0x725/0x7a0()
[ 370.382857] Modules linked in: sch_tbf(F) joydev nfsd auth_rpcgss nfs_acl lockd i2c_piix4 i2c_core virtio_balloon sunrpc serio_raw virtio_blk virtio_net virtio_pci virtio_ring ata_generic virtio pata_acpi
[ 370.393546] CPU: 1 PID: 9359 Comm: udptest Tainted: GF 3.12.0+ #1
[ 370.395541] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 370.397182] 0000000000000009 ffff88011804f8a8 ffffffff816423eb 0000000000000000
[ 370.401220] ffff88011804f8e0 ffffffff81067dcd ffff8800d2d52900 ffffea00045fac00
[ 370.403912] 00000000000005c8 0000000000000000 ffff8800c45b7d10 ffff88011804f8f0
[ 370.406681] Call Trace:
[ 370.407514] [<ffffffff816423eb>] dump_stack+0x45/0x56
[ 370.409094] [<ffffffff81067dcd>] warn_slowpath_common+0x7d/0xa0
[ 370.421411] [<ffffffff81067eaa>] warn_slowpath_null+0x1a/0x20
[ 370.423776] [<ffffffff8153f6f5>] skb_segment+0x725/0x7a0
[ 370.425961] [<ffffffff815b0322>] udp4_ufo_fragment+0xc2/0x120
[ 370.430097] [<ffffffff815b8add>] inet_gso_segment+0x11d/0x330
[ 370.432763] [<ffffffff812a0469>] ? selinux_ip_postroute+0x99/0x2c0
[ 370.436632] [<ffffffff8154c29c>] skb_mac_gso_segment+0x9c/0x180
[ 370.439060] [<ffffffff8154c3e0>] __skb_gso_segment+0x60/0xc0
[ 370.446392] [<ffffffffa00c9a5f>] tbf_enqueue+0x5f/0x1f0 [sch_tbf]
[ 370.451242] [<ffffffff8154cd2b>] dev_queue_xmit+0x24b/0x480
[ 370.454330] [<ffffffff81585289>] ip_finish_output+0x2c9/0x3b0
[ 370.456631] [<ffffffff815866b8>] ip_output+0x58/0x90
[ 370.458439] [<ffffffff81585e25>] ip_local_out+0x25/0x30
[ 370.461258] [<ffffffff815871c5>] ip_send_skb+0x15/0x50
[ 370.463113] [<ffffffff815abaaf>] udp_send_skb+0x20f/0x2a0
[ 370.465031] [<ffffffff815842b0>] ? ip_copy_metadata+0xc0/0xc0
[ 370.467024] [<ffffffff815ac9fa>] udp_sendmsg+0x2fa/0xa10
[ 370.468884] [<ffffffff8129a560>] ? sock_has_perm+0x70/0x90
[ 370.470922] [<ffffffff8153799c>] ? release_sock+0x10c/0x160
[ 370.473651] [<ffffffff815b8724>] inet_sendmsg+0x64/0xb0
[ 370.475621] [<ffffffff8129a693>] ? selinux_socket_sendmsg+0x23/0x30
[ 370.478025] [<ffffffff815335db>] sock_sendmsg+0x8b/0xc0
[ 370.483445] [<ffffffff812b193e>] ? security_netlbl_sid_to_secattr+0x6e/0xb0
[ 370.487287] [<ffffffff8162fae2>] ? netlbl_domhsh_hash+0x12/0x50
[ 370.489441] [<ffffffff8162fb3f>] ? netlbl_domhsh_search+0x1f/0x90
[ 370.492770] [<ffffffff81533781>] SYSC_sendto+0x121/0x1c0
[ 370.495053] [<ffffffff811a81ce>] ? alloc_file+0x1e/0xc0
[ 370.497309] [<ffffffff815305fc>] ? sock_alloc_file+0x9c/0x130
[ 370.500255] [<ffffffff8153429e>] SyS_sendto+0xe/0x10
[ 370.503751] [<ffffffff81651329>] system_call_fastpath+0x16/0x1b
[ 370.506104] ---[ end trace 117f3806fa493b38 ]---
Maybe it does help.
Greetings,
Hannes
^ permalink raw reply
* Re: Linux sends IPv6 NS packets with the link-local address
From: Hannes Frederic Sowa @ 2013-11-07 1:16 UTC (permalink / raw)
To: Steinar H. Gunderson; +Cc: netdev, ayourtch
In-Reply-To: <20131107005615.GA21230@sesse.net>
On Thu, Nov 07, 2013 at 01:56:15AM +0100, Steinar H. Gunderson wrote:
> On Thu, Nov 07, 2013 at 01:46:11AM +0100, Hannes Frederic Sowa wrote:
> > Problem was, if we enqueue a packet to the resolving queue we use those
> > packets source address as the address we use as the source address. This
> > patch chooses the last enqueued skb's source address to do the resolving
> > thus switching over to a global address much more early (hopefully). I
> > have seen this problem with routers doing uRPF on the source address of
> > arp packets, too.
> >
> > Does this happen when you configure addresses? Do your addresses have
> > a short lifetime advertised by radvd etc?
>
> There are no autoconfigured addresses on the machine; statically set plus
> link-layer only. It does _serve_ RA for another VLAN, though (it does
> 802.1q, some tunnels, and some BGP).
Ok, still the patch should improve the situation if the first packet queued up
used the ll-address as source.
Greetings,
Hannes
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Eric Dumazet @ 2013-11-07 1:21 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Herbert Xu, Ben Hutchings, David Miller, christoph.paasch, netdev,
hkchu, mwdalton
In-Reply-To: <20131107011337.GD8144@order.stressinduktion.org>
On Thu, 2013-11-07 at 02:13 +0100, Hannes Frederic Sowa wrote:
> On Wed, Nov 06, 2013 at 04:15:21PM -0800, Eric Dumazet wrote:
> > On Wed, 2013-11-06 at 11:47 -0800, Eric Dumazet wrote:
> > > I'll try a different way.
> > >
> > > The frag_list would contain a bunch of frags, that we logically add to the bunch
> > > of frags found in the first skb shared_info structure.
> >
> > Here is the patch I came into (I tested it and it works very fine)
> >
> > The theory is that in GRO stack, all skbs use the head_frag trick,
> > so even if one NIC pulled some payload into skb->head, we do not have to
> > copy anything. Outside of GRO stack, we are not supposed to provide data
> > in skb->head (I am speaking of the skb found on the frag_list extension,
> > not the skb_head)
> >
> > I put a fallback code, just in case, with a WARN_ON_ONCE() so that we
> > can catch the offenders (if any) to fix them.
> >
> > I renamed @skb to @skb_head to more clearly document this code.
> > Same for @i renamed to @cur_frag
>
> I wanted to understand this code more closely and tried it with a test case I
> used for the UDP_CORK bugs and also for the tbf panic.
>
> The packet is allocated as an UFO one and gets segmented by tbf.
>
> # tc qdisc replace dev eth0 root tbf rate 200kbit latency 20ms burst 5kb
> # ./udptest
> (Just doing two writes of 200 bytes, then a write of 4096 bytes on a udp
> socket. I can send you the source (or a stripped down version, because it got
> realy noisy.))
Interesting :
if (cskb == head_skb)
cskb = skb_shinfo(head_skb)->frag_list;
else
cskb = cskb->next;
if (!cskb) {
WARN_ON_ONCE(1);
goto err;
}
So here either head_skb->frag_list is NULL, or the frag_list chain finishes too early.
More probably I have a bug in the code ;)
I'll take a look, thanks !
^ permalink raw reply
* Re: [PATCH net-next 2/3] net_sched: fix some checkpatch errors
From: Yang Yingliang @ 2013-11-07 1:33 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20131106073741.154f8590@samsung-9>
On 2013/11/6 23:37, Stephen Hemminger wrote:
> On Wed, 6 Nov 2013 16:05:08 +0800
> Yang Yingliang <yangyingliang@huawei.com> wrote:
>
>> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
>> index fd70728..c8aadfa 100644
>> --- a/net/sched/act_api.c
>> +++ b/net/sched/act_api.c
>> @@ -191,7 +191,7 @@ u32 tcf_hash_new_index(u32 *idx_gen, struct tcf_hashinfo *hinfo)
>> val = 1;
>> } while (tcf_hash_lookup(val, hinfo));
>>
>> - return (*idx_gen = val);
>> + return *idx_gen = val;
>
> That is not an improvement. I would rather see the return on a assingment and return
> on separate lines.
>
>
ok, i will fix it in v2.
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Eric Dumazet @ 2013-11-07 1:34 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Herbert Xu, Ben Hutchings, David Miller, christoph.paasch, netdev,
hkchu, mwdalton
In-Reply-To: <1383787279.2878.22.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, 2013-11-06 at 17:21 -0800, Eric Dumazet wrote:
> On Thu, 2013-11-07 at 02:13 +0100, Hannes Frederic Sowa wrote:
> > On Wed, Nov 06, 2013 at 04:15:21PM -0800, Eric Dumazet wrote:
> > > On Wed, 2013-11-06 at 11:47 -0800, Eric Dumazet wrote:
> > > > I'll try a different way.
> > > >
> > > > The frag_list would contain a bunch of frags, that we logically add to the bunch
> > > > of frags found in the first skb shared_info structure.
> > >
> > > Here is the patch I came into (I tested it and it works very fine)
> > >
> > > The theory is that in GRO stack, all skbs use the head_frag trick,
> > > so even if one NIC pulled some payload into skb->head, we do not have to
> > > copy anything. Outside of GRO stack, we are not supposed to provide data
> > > in skb->head (I am speaking of the skb found on the frag_list extension,
> > > not the skb_head)
> > >
> > > I put a fallback code, just in case, with a WARN_ON_ONCE() so that we
> > > can catch the offenders (if any) to fix them.
> > >
> > > I renamed @skb to @skb_head to more clearly document this code.
> > > Same for @i renamed to @cur_frag
> >
> > I wanted to understand this code more closely and tried it with a test case I
> > used for the UDP_CORK bugs and also for the tbf panic.
> >
> > The packet is allocated as an UFO one and gets segmented by tbf.
> >
> > # tc qdisc replace dev eth0 root tbf rate 200kbit latency 20ms burst 5kb
> > # ./udptest
> > (Just doing two writes of 200 bytes, then a write of 4096 bytes on a udp
> > socket. I can send you the source (or a stripped down version, because it got
> > realy noisy.))
>
> Interesting :
>
> if (cskb == head_skb)
> cskb = skb_shinfo(head_skb)->frag_list;
> else
> cskb = cskb->next;
> if (!cskb) {
> WARN_ON_ONCE(1);
> goto err;
> }
>
> So here either head_skb->frag_list is NULL, or the frag_list chain finishes too early.
>
> More probably I have a bug in the code ;)
Oh yes, I missed a : data_len += remain;
at line 2906 :
offset = remain;
+ data_len += remain;
continue;
^ permalink raw reply
* Re: [PATCH net-next 2/9] bonding: remove bond read lock for bond_mii_monitor()
From: Ding Tianhong @ 2013-11-07 1:38 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Andy Gospodarek, David S. Miller, Nikolay Aleksandrov,
Veaceslav Falico, Netdev
In-Reply-To: <29223.1383779906@death.nxdomain>
On 2013/11/7 7:18, Jay Vosburgh wrote:
> Ding Tianhong <dingtianhong@huawei.com> wrote:
>
>> The bond_mii_monitor() still use bond lock read to protect
>> bond_has_slaves() and bond_miimon_inspect(), it is no effect,
>> so I move the RTNL to the top of the function to protect the
>> whole monitor, of course, if the bond state did not changed,
>> the monitor still calling RTNL, it may bring more performance
>> loss, but as a slow path, it is negligible.
>
> I'm not sure this last part is true (about it being ok to
> acquire RTNL every pass). The reason the bond_miimon_* functions are
> arranged in the way they are is specifically to avoid taking RTNL
> unnecessarily. A common setting is miimon=100, which will acquire and
> release RTNL ten times per second.
>
> The inspect function can be make RCU safe, and then the function
> will operate pretty much as it does now (with the multiple phases). If
> a slave disappears between the phases, that's ok; one extra cycle on
> RTNL isn't a big deal, but 10 per second arguably is.
>
> My comment also applies to the later patches in the series that
> make similar "always acquire RTNL" changes to the ARP monitor, ALB
> monitoring function, and the 802.3ad state machine. That would be
> patches:
>
> Subject: [PATCH net-next 3/9] bonding: rebuild the lock use for
> bond_alb_monitor()
> Subject: [PATCH net-next 4/9] bonding: rebuild the lock use for
> bond_loadbalance_arp_mon()
> Subject: [PATCH net-next 5/9] bonding: rebuild the lock use for
> bond_activebackup_arp_mon()
> Subject: [PATCH net-next 6/9] bonding: use RTNL instead of bond lock for
> bond_3ad_state_machine_handler()
>
> The 802.3ad state machine patch or the balance-alb patch,
> combined with the miimon patch, will acquire and release the RTNL lock
> 20 times per second (10 for the 3ad state machine or alb monitor, and 10
> more for a typical miimon configuration). I don't believe this is a
> reasonable implementation.
>
> -J
>
Thansk for your reply. your opinion is reasonable and clear, I will
optimization the lock for these patch, the RTNL should be replace by
RCU in some place.
for better performance :)
Ding
>> also in bond_miimon_commit(), I remove the unwanted curr_slave_lock
>> when calling the bond_select_active_slave(), the RTNL is enough.
>>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bond_main.c | 44 ++++++++++++-----------------------------
>> 1 file changed, 13 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 9c9803c..98171eb 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -2074,9 +2074,7 @@ static void bond_miimon_commit(struct bonding *bond)
>> do_failover:
>> ASSERT_RTNL();
>> block_netpoll_tx();
>> - write_lock_bh(&bond->curr_slave_lock);
>> bond_select_active_slave(bond);
>> - write_unlock_bh(&bond->curr_slave_lock);
>> unblock_netpoll_tx();
>> }
>>
>> @@ -2098,47 +2096,31 @@ void bond_mii_monitor(struct work_struct *work)
>> bool should_notify_peers = false;
>> unsigned long delay;
>>
>> - read_lock(&bond->lock);
>> -
>> delay = msecs_to_jiffies(bond->params.miimon);
>>
>> - if (!bond_has_slaves(bond))
>> + if (!rtnl_trylock()) {
>> + delay = 1;
>> goto re_arm;
>> + }
>>
>> - should_notify_peers = bond_should_notify_peers(bond);
>> -
>> - if (bond_miimon_inspect(bond)) {
>> - read_unlock(&bond->lock);
>> -
>> - /* Race avoidance with bond_close cancel of workqueue */
>> - if (!rtnl_trylock()) {
>> - read_lock(&bond->lock);
>> - delay = 1;
>> - should_notify_peers = false;
>> - goto re_arm;
>> - }
>> + if (!bond_has_slaves(bond)) {
>> + rtnl_unlock();
>> + goto re_arm;
>> + }
>>
>> - read_lock(&bond->lock);
>> + should_notify_peers = bond_should_notify_peers(bond);
>>
>> + if (bond_miimon_inspect(bond))
>> bond_miimon_commit(bond);
>>
>> - read_unlock(&bond->lock);
>> - rtnl_unlock(); /* might sleep, hold no other locks */
>> - read_lock(&bond->lock);
>> - }
>> + if (should_notify_peers)
>> + call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev);
>> +
>> + rtnl_unlock();
>>
>> re_arm:
>> if (bond->params.miimon)
>> queue_delayed_work(bond->wq, &bond->mii_work, delay);
>> -
>> - read_unlock(&bond->lock);
>> -
>> - if (should_notify_peers) {
>> - if (!rtnl_trylock())
>> - return;
>> - call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev);
>> - rtnl_unlock();
>> - }
>> }
>>
>> static bool bond_has_this_ip(struct bonding *bond, __be32 ip)
>> --
>> 1.8.2.1
>
> ---
> -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
>
>
> .
>
^ permalink raw reply
* Re: [PATCH net-next 6/9] bonding: use RTNL instead of bond lock for bond_3ad_state_machine_handler()
From: Ding Tianhong @ 2013-11-07 1:40 UTC (permalink / raw)
To: Sergei Shtylyov, Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
In-Reply-To: <527AD5BC.7040907@cogentembedded.com>
On 2013/11/7 7:50, Sergei Shtylyov wrote:
> Hello.
>
> On 11/06/2013 09:53 AM, Ding Tianhong wrote:
>
>> The bond_3ad_state_machine_handler() is a slow path, it use the bond
>> lock to protect the bond slave list and slave port, the slave list
>> could not be protect by bond lock anymore, so I need to use RTNL or
>> RCU instead of bond lock, but if I remove the bond lock, the
>> bond_3ad_state_machine_handler() may use the slave port when bond
>> releasing the slave, it will occur problems.
>
>> As the bond_3ad_unbind_slave() only protected by bond lock or RTNL,
>> so add RCU is not a good solution as it could not protect the slave
>> port, so RTNL is fit here.
>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bond_3ad.c | 15 +++++++--------
>> 1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>> index 187b1b7..a4d190e 100644
>> --- a/drivers/net/bonding/bond_3ad.c
>> +++ b/drivers/net/bonding/bond_3ad.c
>> @@ -2068,18 +2068,19 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
> [...]
>> - //check if there are any slaves
>> if (!bond_has_slaves(bond))
>> goto re_arm;
>>
>> - // check if agg_select_timer timer after initialize is timed out
>> + /* check if agg_select_timer timer after initialize is timed out */
>> if (BOND_AD_INFO(bond).agg_select_timer && !(--BOND_AD_INFO(bond).agg_select_timer)) {
>> slave = bond_first_slave(bond);
>> port = slave ? &(SLAVE_AD_INFO(slave).port) : NULL;
>>
>> - // select the active aggregator for the bond
>> if (port) {
>> if (!port->slave) {
>> pr_warning("%s: Warning: bond's first port is uninitialized\n",
>> @@ -2093,7 +2094,6 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
>> bond_3ad_set_carrier(bond);
>> }
>>
>> - // for each port run the state machines
>> bond_for_each_slave(bond, slave, iter) {
>> port = &(SLAVE_AD_INFO(slave).port);
>> if (!port->slave) {
>> @@ -2114,7 +2114,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
>> ad_mux_machine(port);
>> ad_tx_machine(port);
>>
>> - // turn off the BEGIN bit, since we already handled it
>> + /* turn off the BEGIN bit, since we already handled it */
>> if (port->sm_vars & AD_PORT_BEGIN)
>> port->sm_vars &= ~AD_PORT_BEGIN;
>>
>
> These comment changes are not documented in the changelog and should most probably be in a separate patch.
>
> WBR, Sergei
>
thanks for your reply, I miss it in the changelog, it is already a big patch set, so I really do not want to add patch any more, I will add it in changelog.
Tnanks
Ding
>
> .
>
^ permalink raw reply
* [PATCH v2 04/11] net ipset: use rbtree postorder iteration instead of opencoding
From: Cody P Schafer @ 2013-11-07 1:42 UTC (permalink / raw)
To: Andrew Morton, EXT4, Jan Kara, rostedt, David S. Miller,
Florian Westphal, Jozsef Kadlecsik, Pablo Neira Ayuso,
YOSHIFUJI Hideaki
Cc: LKML, Cody P Schafer, coreteam, netdev, netfilter-devel,
netfilter, Patrick McHardy
In-Reply-To: <1383788572-25938-1-git-send-email-cody@linux.vnet.ibm.com>
Use rbtree_postorder_for_each_entry_safe() to destroy the rbtree instead
of opencoding an alternate postorder iteration that modifies the tree
Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
net/netfilter/ipset/ip_set_hash_netiface.c | 27 ++++-----------------------
1 file changed, 4 insertions(+), 23 deletions(-)
diff --git a/net/netfilter/ipset/ip_set_hash_netiface.c b/net/netfilter/ipset/ip_set_hash_netiface.c
index 7d798d5..99dba4c 100644
--- a/net/netfilter/ipset/ip_set_hash_netiface.c
+++ b/net/netfilter/ipset/ip_set_hash_netiface.c
@@ -45,31 +45,12 @@ struct iface_node {
static void
rbtree_destroy(struct rb_root *root)
{
- struct rb_node *p, *n = root->rb_node;
- struct iface_node *node;
-
- /* Non-recursive destroy, like in ext3 */
- while (n) {
- if (n->rb_left) {
- n = n->rb_left;
- continue;
- }
- if (n->rb_right) {
- n = n->rb_right;
- continue;
- }
- p = rb_parent(n);
- node = rb_entry(n, struct iface_node, node);
- if (!p)
- *root = RB_ROOT;
- else if (p->rb_left == n)
- p->rb_left = NULL;
- else if (p->rb_right == n)
- p->rb_right = NULL;
+ struct iface_node *node, *next;
+ rbtree_postorder_for_each_entry_safe(node, next, root, node)
kfree(node);
- n = p;
- }
+
+ *root = RB_ROOT;
}
static int
--
1.8.4.2
^ permalink raw reply related
* Re: gso: Attempt to handle mega-GRO packets
From: Herbert Xu @ 2013-11-07 1:47 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton, mst, Jason Wang
In-Reply-To: <1383786208.2878.15.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, Nov 06, 2013 at 05:03:28PM -0800, Eric Dumazet wrote:
>
> > But once the system has been running for a while, I see nothing
> > in the virtio_net code that tries to prevent fragmentation. Once
> > fragmentation sets in, you'll be back in the terrible situation
> > that we were in prior to the coalesce patch.
>
> There is no fragmentation, since we allocate 32Kb pages.
Say the system is fragmented sufficiently that you'll end up with
0-order pages. In that case you'll only ever be able to coalesce
two packets.
Real systems that run for more than a day do end up with seriously
fragmented memory.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] ipv6: replace RTF_ROUTEINFO with RTF_ADDRCONF in rt6_get_route_info()
From: Hannes Frederic Sowa @ 2013-11-07 1:51 UTC (permalink / raw)
To: Duan Jiong; +Cc: David Miller, netdev
In-Reply-To: <5279EF31.4040705@cn.fujitsu.com>
On Wed, Nov 06, 2013 at 03:26:41PM +0800, Duan Jiong wrote:
>
> As the rfc 4191 said, the Router Preference and Lifetime values in a
> ::/0 Route Information Option should override the preference and lifetime
> values in the Router Advertisement header. But when the kernel deals with
> a ::/0 Route Information Option, the rt6_get_route_info() always return
> NULL, that means that overriding will not happen, because those default
> routers were added without flag RTF_ROUTEINFO in rt6_add_dflt_router().
>
> In order to match those default routers, we can replace RTF_ROUTEINFO
> with RTF_ADDRCONF in rt6_get_route_info().
>
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Hmm, that looks like a bug. Nice catch!
Couldn't we just replace the rt6_get_route_info in rt6_route_rcv with a call
to rt6_get_dflt_router? Seems easier, already handles the ::/0 case and also
does preserve the check for the RTF_ROUTEINFO flag in rt6_add_route_info.
Greetings,
Hannes
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Eric Dumazet @ 2013-11-07 2:02 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton, mst, Jason Wang
In-Reply-To: <20131107014724.GA28946@gondor.apana.org.au>
On Thu, 2013-11-07 at 09:47 +0800, Herbert Xu wrote:
> Say the system is fragmented sufficiently that you'll end up with
> 0-order pages. In that case you'll only ever be able to coalesce
> two packets.
4K page will contain 2 frags and they will coalesce.
Performance will still be quite good.
We probably add a tweak, to not have any hole in this case.
>
> Real systems that run for more than a day do end up with seriously
> fragmented memory.
Sure, but having shallow skbs in the first place help quite a bit.
There is no perfect solution, unless of course you change virtio_net to
provide different queues, with different frag sizes.
Sort of what NIU driver uses.
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Hannes Frederic Sowa @ 2013-11-07 2:03 UTC (permalink / raw)
To: Eric Dumazet
Cc: Herbert Xu, Ben Hutchings, David Miller, christoph.paasch, netdev,
hkchu, mwdalton
In-Reply-To: <1383788068.2878.26.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, Nov 06, 2013 at 05:34:28PM -0800, Eric Dumazet wrote:
> On Wed, 2013-11-06 at 17:21 -0800, Eric Dumazet wrote:
> > On Thu, 2013-11-07 at 02:13 +0100, Hannes Frederic Sowa wrote:
> > > On Wed, Nov 06, 2013 at 04:15:21PM -0800, Eric Dumazet wrote:
> > > > On Wed, 2013-11-06 at 11:47 -0800, Eric Dumazet wrote:
> > > > > I'll try a different way.
> > > > >
> > > > > The frag_list would contain a bunch of frags, that we logically add to the bunch
> > > > > of frags found in the first skb shared_info structure.
> > > >
> > > > Here is the patch I came into (I tested it and it works very fine)
> > > >
> > > > The theory is that in GRO stack, all skbs use the head_frag trick,
> > > > so even if one NIC pulled some payload into skb->head, we do not have to
> > > > copy anything. Outside of GRO stack, we are not supposed to provide data
> > > > in skb->head (I am speaking of the skb found on the frag_list extension,
> > > > not the skb_head)
> > > >
> > > > I put a fallback code, just in case, with a WARN_ON_ONCE() so that we
> > > > can catch the offenders (if any) to fix them.
> > > >
> > > > I renamed @skb to @skb_head to more clearly document this code.
> > > > Same for @i renamed to @cur_frag
> > >
> > > I wanted to understand this code more closely and tried it with a test case I
> > > used for the UDP_CORK bugs and also for the tbf panic.
> > >
> > > The packet is allocated as an UFO one and gets segmented by tbf.
> > >
> > > # tc qdisc replace dev eth0 root tbf rate 200kbit latency 20ms burst 5kb
> > > # ./udptest
> > > (Just doing two writes of 200 bytes, then a write of 4096 bytes on a udp
> > > socket. I can send you the source (or a stripped down version, because it got
> > > realy noisy.))
> >
> > Interesting :
> >
> > if (cskb == head_skb)
> > cskb = skb_shinfo(head_skb)->frag_list;
> > else
> > cskb = cskb->next;
> > if (!cskb) {
> > WARN_ON_ONCE(1);
> > goto err;
> > }
> >
> > So here either head_skb->frag_list is NULL, or the frag_list chain finishes too early.
> >
> > More probably I have a bug in the code ;)
>
> Oh yes, I missed a : data_len += remain;
>
> at line 2906 :
>
> offset = remain;
> + data_len += remain;
> continue;
Hm, I still hit the WARN_ON_ONCE (same test as above) with this fixed up:
[ 27.962782] ------------[ cut here ]------------
[ 27.964728] WARNING: CPU: 1 PID: 485 at net/core/skbuff.c:2875 skb_segment+0x72d/0x7b0()
[ 27.967179] Modules linked in: sch_tbf joydev i2c_piix4 virtio_balloon i2c_core serio_raw nfsd auth_rpcgss nfs_acl lockd sunrpc virtio_blk virtio_net virtio_pci virtio_ring virtio ata_generic pata_acpi
[ 27.979403] CPU: 1 PID: 485 Comm: udptest Not tainted 3.12.0+ #5
[ 27.982402] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 27.984125] 0000000000000009 ffff8800d581b8a8 ffffffff816423fb 0000000000000000
[ 27.986873] ffff8800d581b8e0 ffffffff81067dcd ffff8800372c2c00 ffffea0000de5800
[ 27.990107] 00000000000005c8 0000000000000000 ffff8800372b4710 ffff8800d581b8f0
[ 27.993473] Call Trace:
[ 27.994506] [<ffffffff816423fb>] dump_stack+0x45/0x56
[ 27.996172] [<ffffffff81067dcd>] warn_slowpath_common+0x7d/0xa0
[ 27.998295] [<ffffffff81067eaa>] warn_slowpath_null+0x1a/0x20
[ 28.000127] [<ffffffff8153f6fd>] skb_segment+0x72d/0x7b0
[ 28.002090] [<ffffffff815b0332>] udp4_ufo_fragment+0xc2/0x120
[ 28.006193] [<ffffffff815b8aed>] inet_gso_segment+0x11d/0x330
[ 28.008292] [<ffffffff812a0469>] ? selinux_ip_postroute+0x99/0x2c0
[ 28.010469] [<ffffffff8154c2ac>] skb_mac_gso_segment+0x9c/0x180
[ 28.013065] [<ffffffff8154c3f0>] __skb_gso_segment+0x60/0xc0
[ 28.015326] [<ffffffffa00e1a5f>] tbf_enqueue+0x5f/0x1f0 [sch_tbf]
[ 28.017752] [<ffffffff8154cd3b>] dev_queue_xmit+0x24b/0x480
[ 28.019906] [<ffffffff81585299>] ip_finish_output+0x2c9/0x3b0
[ 28.022189] [<ffffffff815866c8>] ip_output+0x58/0x90
[ 28.024167] [<ffffffff81585e35>] ip_local_out+0x25/0x30
[ 28.026409] [<ffffffff815871d5>] ip_send_skb+0x15/0x50
[ 28.028517] [<ffffffff815ababf>] udp_send_skb+0x20f/0x2a0
[ 28.030700] [<ffffffff815842c0>] ? ip_copy_metadata+0xc0/0xc0
[ 28.033053] [<ffffffff815aca0a>] udp_sendmsg+0x2fa/0xa10
[ 28.035024] [<ffffffff8129a560>] ? sock_has_perm+0x70/0x90
[ 28.042218] [<ffffffff8153799c>] ? release_sock+0x10c/0x160
[ 28.044222] [<ffffffff815b8734>] inet_sendmsg+0x64/0xb0
[ 28.047297] [<ffffffff8129a693>] ? selinux_socket_sendmsg+0x23/0x30
[ 28.049649] [<ffffffff815335db>] sock_sendmsg+0x8b/0xc0
[ 28.052800] [<ffffffff812b193e>] ? security_netlbl_sid_to_secattr+0x6e/0xb0
[ 28.055594] [<ffffffff811bdc45>] ? __d_alloc+0x25/0x180
[ 28.057651] [<ffffffff8162fb4f>] ? netlbl_domhsh_search+0x1f/0x90
[ 28.060103] [<ffffffff81533781>] SYSC_sendto+0x121/0x1c0
[ 28.062332] [<ffffffff811a81ce>] ? alloc_file+0x1e/0xc0
[ 28.064561] [<ffffffff815305fc>] ? sock_alloc_file+0x9c/0x130
[ 28.066758] [<ffffffff8153429e>] SyS_sendto+0xe/0x10
[ 28.072865] [<ffffffff81651329>] system_call_fastpath+0x16/0x1b
[ 28.074981] ---[ end trace 351089f5102f0c6a ]---
^ permalink raw reply
* Re: [PATCH] ipv6: replace RTF_ROUTEINFO with RTF_ADDRCONF in rt6_get_route_info()
From: Duan Jiong @ 2013-11-07 2:01 UTC (permalink / raw)
To: hannes; +Cc: David Miller, netdev
In-Reply-To: <20131107015100.GF8144@order.stressinduktion.org>
于 2013年11月07日 09:51, Hannes Frederic Sowa 写道:
> On Wed, Nov 06, 2013 at 03:26:41PM +0800, Duan Jiong wrote:
>>
>> As the rfc 4191 said, the Router Preference and Lifetime values in a
>> ::/0 Route Information Option should override the preference and lifetime
>> values in the Router Advertisement header. But when the kernel deals with
>> a ::/0 Route Information Option, the rt6_get_route_info() always return
>> NULL, that means that overriding will not happen, because those default
>> routers were added without flag RTF_ROUTEINFO in rt6_add_dflt_router().
>>
>> In order to match those default routers, we can replace RTF_ROUTEINFO
>> with RTF_ADDRCONF in rt6_get_route_info().
>>
>> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
>
> Hmm, that looks like a bug. Nice catch!
>
> Couldn't we just replace the rt6_get_route_info in rt6_route_rcv with a call
> to rt6_get_dflt_router? Seems easier, already handles the ::/0 case and also
> does preserve the check for the RTF_ROUTEINFO flag in rt6_add_route_info.
Yeah, your idea is better. I will modify my patch.
Thanks,
Duan
>
> Greetings,
>
> Hannes
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" 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: gso: Attempt to handle mega-GRO packets
From: Eric Dumazet @ 2013-11-07 2:08 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton, mst, Jason Wang
In-Reply-To: <1383789758.2878.32.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, 2013-11-06 at 18:02 -0800, Eric Dumazet wrote:
> On Thu, 2013-11-07 at 09:47 +0800, Herbert Xu wrote:
>
> > Say the system is fragmented sufficiently that you'll end up with
> > 0-order pages. In that case you'll only ever be able to coalesce
> > two packets.
>
> 4K page will contain 2 frags and they will coalesce.
>
> Performance will still be quite good.
>
> We probably add a tweak, to not have any hole in this case.
After skb_page_frag_refill() call, its trivial to check if the page is
order-0.
If yes, use the whole remaining space, instead of MAX_PACKET_LEN
-> If memory is fragmented, we switch back to old behavior.
Really, its that simple.
^ permalink raw reply
* [PATCH net-next v2 1/3] net_sched: tbf: support of 64bit rates
From: Yang Yingliang @ 2013-11-07 2:13 UTC (permalink / raw)
To: davem, netdev; +Cc: eric.dumazet, jhs, stephen
In-Reply-To: <1383790412-41944-1-git-send-email-yangyingliang@huawei.com>
With psched_ratecfg_precompute(), tbf can deal with 64bit rates.
Add two new attributes so that tc can use them to break the 32bit
limit.
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
include/uapi/linux/pkt_sched.h | 2 ++
net/sched/sch_tbf.c | 22 ++++++++++++++++++----
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index f2624b5..307f293 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -171,6 +171,8 @@ enum {
TCA_TBF_PARMS,
TCA_TBF_RTAB,
TCA_TBF_PTAB,
+ TCA_TBF_RATE64,
+ TCA_TBF_PRATE64,
__TCA_TBF_MAX,
};
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index b057122..b736517 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -266,20 +266,23 @@ static const struct nla_policy tbf_policy[TCA_TBF_MAX + 1] = {
[TCA_TBF_PARMS] = { .len = sizeof(struct tc_tbf_qopt) },
[TCA_TBF_RTAB] = { .type = NLA_BINARY, .len = TC_RTAB_SIZE },
[TCA_TBF_PTAB] = { .type = NLA_BINARY, .len = TC_RTAB_SIZE },
+ [TCA_TBF_RATE64] = { .type = NLA_U64 },
+ [TCA_TBF_PRATE64] = { .type = NLA_U64 },
};
static int tbf_change(struct Qdisc *sch, struct nlattr *opt)
{
int err;
struct tbf_sched_data *q = qdisc_priv(sch);
- struct nlattr *tb[TCA_TBF_PTAB + 1];
+ struct nlattr *tb[TCA_TBF_MAX + 1];
struct tc_tbf_qopt *qopt;
struct qdisc_rate_table *rtab = NULL;
struct qdisc_rate_table *ptab = NULL;
struct Qdisc *child = NULL;
int max_size, n;
+ u64 rate64 = 0, prate64 = 0;
- err = nla_parse_nested(tb, TCA_TBF_PTAB, opt, tbf_policy);
+ err = nla_parse_nested(tb, TCA_TBF_MAX, opt, tbf_policy);
if (err < 0)
return err;
@@ -341,9 +344,13 @@ static int tbf_change(struct Qdisc *sch, struct nlattr *opt)
q->tokens = q->buffer;
q->ptokens = q->mtu;
- psched_ratecfg_precompute(&q->rate, &rtab->rate, 0);
+ if (tb[TCA_TBF_RATE64])
+ rate64 = nla_get_u64(tb[TCA_TBF_RATE64]);
+ psched_ratecfg_precompute(&q->rate, &rtab->rate, rate64);
if (ptab) {
- psched_ratecfg_precompute(&q->peak, &ptab->rate, 0);
+ if (tb[TCA_TBF_PRATE64])
+ prate64 = nla_get_u64(tb[TCA_TBF_PRATE64]);
+ psched_ratecfg_precompute(&q->peak, &ptab->rate, prate64);
q->peak_present = true;
} else {
q->peak_present = false;
@@ -402,6 +409,13 @@ static int tbf_dump(struct Qdisc *sch, struct sk_buff *skb)
opt.buffer = PSCHED_NS2TICKS(q->buffer);
if (nla_put(skb, TCA_TBF_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
+ if ((q->rate.rate_bytes_ps >= (1ULL << 32)) &&
+ nla_put_u64(skb, TCA_TBF_RATE64, q->rate.rate_bytes_ps))
+ goto nla_put_failure;
+ if (q->peak_present &&
+ (q->peak.rate_bytes_ps >= (1ULL << 32)) &&
+ nla_put_u64(skb, TCA_TBF_PRATE64, q->peak.rate_bytes_ps))
+ goto nla_put_failure;
nla_nest_end(skb, nest);
return skb->len;
--
1.7.12
^ permalink raw reply related
* [PATCH net-next v2 2/3] net_sched: fix some checkpatch errors
From: Yang Yingliang @ 2013-11-07 2:13 UTC (permalink / raw)
To: davem, netdev; +Cc: eric.dumazet, jhs, stephen
In-Reply-To: <1383790412-41944-1-git-send-email-yangyingliang@huawei.com>
There are some checkpatch errors, fix them.
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
---
net/sched/act_api.c | 5 +++--
net/sched/cls_bpf.c | 2 +-
net/sched/cls_u32.c | 2 +-
net/sched/sch_cbq.c | 3 ++-
net/sched/sch_generic.c | 4 ++--
net/sched/sch_htb.c | 13 +++++++------
net/sched/sch_netem.c | 2 +-
net/sched/sch_sfq.c | 10 ++++++----
8 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index fd70728..d92a90e9 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -191,7 +191,8 @@ u32 tcf_hash_new_index(u32 *idx_gen, struct tcf_hashinfo *hinfo)
val = 1;
} while (tcf_hash_lookup(val, hinfo));
- return (*idx_gen = val);
+ *idx_gen = val;
+ return *idx_gen;
}
EXPORT_SYMBOL(tcf_hash_new_index);
@@ -263,7 +264,7 @@ void tcf_hash_insert(struct tcf_common *p, struct tcf_hashinfo *hinfo)
}
EXPORT_SYMBOL(tcf_hash_insert);
-static struct tc_action_ops *act_base = NULL;
+static struct tc_action_ops *act_base;
static DEFINE_RWLOCK(act_mod_lock);
int tcf_register_action(struct tc_action_ops *act)
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 1002a82..d7c72be 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -323,7 +323,7 @@ static int cls_bpf_dump(struct tcf_proto *tp, unsigned long fh,
if (nla == NULL)
goto nla_put_failure;
- memcpy(nla_data(nla), prog->bpf_ops, nla_len(nla));
+ memcpy(nla_data(nla), prog->bpf_ops, nla_len(nla));
if (tcf_exts_dump(skb, &prog->exts, &bpf_ext_map) < 0)
goto nla_put_failure;
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index eb07a1e..59e546c 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -352,7 +352,7 @@ static int u32_destroy_key(struct tcf_proto *tp, struct tc_u_knode *n)
return 0;
}
-static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode* key)
+static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode *key)
{
struct tc_u_knode **kp;
struct tc_u_hnode *ht = key->ht_up;
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 7a42c81..a8f40f5 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1058,7 +1058,8 @@ static void cbq_normalize_quanta(struct cbq_sched_data *q, int prio)
cl->quantum = (cl->weight*cl->allot*q->nclasses[prio])/
q->quanta[prio];
}
- if (cl->quantum <= 0 || cl->quantum>32*qdisc_dev(cl->qdisc)->mtu) {
+ if (cl->quantum <= 0 ||
+ cl->quantum > 32*qdisc_dev(cl->qdisc)->mtu) {
pr_warning("CBQ: class %08x has bad quantum==%ld, repaired.\n",
cl->common.classid, cl->quantum);
cl->quantum = qdisc_dev(cl->qdisc)->mtu/2 + 1;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 7fc899a..9421350 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -338,13 +338,13 @@ EXPORT_SYMBOL(netif_carrier_off);
cheaper.
*/
-static int noop_enqueue(struct sk_buff *skb, struct Qdisc * qdisc)
+static int noop_enqueue(struct sk_buff *skb, struct Qdisc *qdisc)
{
kfree_skb(skb);
return NET_XMIT_CN;
}
-static struct sk_buff *noop_dequeue(struct Qdisc * qdisc)
+static struct sk_buff *noop_dequeue(struct Qdisc *qdisc)
{
return NULL;
}
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 0e1e38b..f6e8a74 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -54,7 +54,7 @@
one less than their parent.
*/
-static int htb_hysteresis __read_mostly = 0; /* whether to use mode hysteresis for speedup */
+static int htb_hysteresis __read_mostly; /* whether to use mode hysteresis for speedup */
#define HTB_VER 0x30011 /* major must be matched with number suplied by TC as version */
#if HTB_VER >> 16 != TC_HTB_PROTOVER
@@ -65,7 +65,7 @@ static int htb_hysteresis __read_mostly = 0; /* whether to use mode hysteresis f
module_param (htb_hysteresis, int, 0640);
MODULE_PARM_DESC(htb_hysteresis, "Hysteresis mode, less CPU load, less accurate");
-static int htb_rate_est = 0; /* htb classes have a default rate estimator */
+static int htb_rate_est; /* htb classes have a default rate estimator */
module_param(htb_rate_est, int, 0640);
MODULE_PARM_DESC(htb_rate_est, "setup a default rate estimator (4sec 16sec) for htb classes");
@@ -846,7 +846,7 @@ next:
break;
qdisc_warn_nonwc("htb", cl->un.leaf.q);
- htb_next_rb_node(level ? &cl->parent->un.inner.clprio[prio].ptr:
+ htb_next_rb_node(level ? &cl->parent->un.inner.clprio[prio].ptr :
&q->hlevel[0].hprio[prio].ptr);
cl = htb_lookup_leaf(hprio, prio);
@@ -1276,9 +1276,10 @@ static int htb_delete(struct Qdisc *sch, unsigned long arg)
struct Qdisc *new_q = NULL;
int last_child = 0;
- // TODO: why don't allow to delete subtree ? references ? does
- // tc subsys quarantee us that in htb_destroy it holds no class
- // refs so that we can remove children safely there ?
+ /* TODO: why don't allow to delete subtree ? references ? does
+ * tc subsys quarantee us that in htb_destroy it holds no class
+ * refs so that we can remove children safely there ?
+ */
if (cl->children || cl->filter_cnt)
return -EBUSY;
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 75c94e5..9765c3f 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -728,7 +728,7 @@ static int get_loss_clg(struct Qdisc *sch, const struct nlattr *attr)
nla_for_each_nested(la, attr, rem) {
u16 type = nla_type(la);
- switch(type) {
+ switch (type) {
case NETEM_LOSS_GI: {
const struct tc_netem_gimodel *gi = nla_data(la);
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index d3a1bc2..76f01e0 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -237,10 +237,12 @@ static inline void sfq_link(struct sfq_sched_data *q, sfq_index x)
}
#define sfq_unlink(q, x, n, p) \
- n = q->slots[x].dep.next; \
- p = q->slots[x].dep.prev; \
- sfq_dep_head(q, p)->next = n; \
- sfq_dep_head(q, n)->prev = p
+ do { \
+ n = q->slots[x].dep.next; \
+ p = q->slots[x].dep.prev; \
+ sfq_dep_head(q, p)->next = n; \
+ sfq_dep_head(q, n)->prev = p; \
+ } while (0)
static inline void sfq_dec(struct sfq_sched_data *q, sfq_index x)
--
1.7.12
^ permalink raw reply related
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