* [PATCH] staging: r8188eu: Fix AP mode
From: Larry Finger @ 2013-11-17 19:28 UTC (permalink / raw)
To: gregkh; +Cc: devel, netdev, Stable, Larry Finger
Two code lines were accidentally deleted in the code. Restore them.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Stable <stable@vger.kernel.org>
---
drivers/staging/rtl8188eu/core/rtw_ap.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
index 2c73823..2a13a41 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -1115,6 +1115,9 @@ int rtw_check_beacon_data(struct adapter *padapter, u8 *pbuf, int len)
return _FAIL;
}
+ /* fix bug of flush_cam_entry at STOP AP mode */
+ psta->state |= WIFI_AP_STATE;
+ rtw_indicate_connect(padapter);
pmlmepriv->cur_network.join_res = true;/* for check if already set beacon */
return ret;
}
--
1.8.4
^ permalink raw reply related
* Re: [PATCH v3] net: don't return uninitialized addresses on concurrent socket shutdown
From: Hannes Frederic Sowa @ 2013-11-17 19:22 UTC (permalink / raw)
To: Eric Dumazet; +Cc: mpb, netdev
In-Reply-To: <1384648584.8604.39.camel@edumazet-glaptop2.roam.corp.google.com>
On Sat, Nov 16, 2013 at 04:36:24PM -0800, Eric Dumazet wrote:
> On Sat, 2013-11-16 at 23:43 +0100, Hannes Frederic Sowa wrote:
>
> > I do think it is common to call recvfrom, process the packet and sendto
> > back a packet with the updated values from recvfrom. We accept AF_UNSPEC
> > on an IPv4 UDP socket and use the addresses as it would be a AF_INET
> > sockaddr. We only bail out if the port is 0.
> >
> > It was my intend to at least clear the addressing portions of the regular
> > sockaddr_* structure for the user as it could be reused as explained
> > earlier and be allocated uninitialized on the stack (or reused, so
> > sending packet to a previous destination). I think it is very uncommon to
> > expect a non-error value on a recvfrom/recvmsg and have AF_UNSPEC in the
> > sockaddr.
> >
> > (I erroneously stated that we could return the full 128 zero bytes, we only
> > clear 128 bytes and return only max(128, msg.msg_namelen). msg_namelen gets
> > updated by the recvmsg handler and that only iff we have this concurrent
> > shutdown and blocking read issue.)
> >
> > If the socket structure is cleared a following sendto would produce a -EINVAL.
> >
> > Maybe I am too sensible regarding such problems and will think about that a
> > bit more (and check for AF_INVALID/AF_UNSPEC).
> >
>
> I think the _default_ should be to clear it.
>
> - msg.msg_namelen = sizeof(address);
> + msg.msg_namelen = 0;
>
> And subsystems filling a real address would set it back to the length
> they took care of.
>
> in recvfrom() paths, the kernel _knows_ it uses an array of 128 bytes.
> (struct sockaddr_storage)
I agree, that's the only sane approach we can take. Both the AF_UNSPEC and
also the AF_INVALID approach does conflict how packet sockets structure their
sockaddr_pkt (dev->type is put in sa_family and both 0 and 0xFFFF are valid
interface types).
I'll check it out.
Thanks,
Hannes
^ permalink raw reply
* Re: oops in pskb_expand_head - 3.11.6
From: Hannes Frederic Sowa @ 2013-11-17 19:18 UTC (permalink / raw)
To: Michele Baldessari; +Cc: netdev, jiri
In-Reply-To: <20131117111150.GC24327@marquez.int.rhx>
On Sun, Nov 17, 2013 at 11:11:50AM +0000, Michele Baldessari wrote:
> Hi Hannes,
>
> On Sun, Nov 17, 2013 at 12:42:23AM +0100, Hannes Frederic Sowa wrote:
> > On Sat, Nov 16, 2013 at 11:16:15PM +0000, Michele Baldessari wrote:
> > > Two oops like the following were reported in Fedora 19 - kernel 3.11.6:
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1015905
> >
> > I have not followed that issue that closely, but could you try linus tree
> > or net-next?
> >
> > Maybe those two patches improve the situation:
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=9037c3579a277f3a23ba476664629fda8c35f7c4
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=6aafeef03b9d9ecf255f3a80ed85ee070260e1ae
> >
> indeed a current kernel seems to fix the crash (according to the
> reporters). I'll see if I manage to bisect exactly.
Great, if you confirm this we can ask David if he adds the commits to the
-stable queue.
Greetings,
Hannes
^ permalink raw reply
* kernel BUG at net/core/skbuff.c:2839 RIP [<ffffffff819109a2>] skb_segment+0x6b2/0x6d0
From: Sander Eikelenboom @ 2013-11-17 19:17 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
Hi Eric,
With the linux-net changes from this merge window i get the kernel panic below (not with 3.12.0).
It's on a machine running Xen, 2x rtl8169 nic, and using a bridge for guest networking.
This panic in the host kernel only seems to occur when generating a lot of network traffic to and from a guest.
I tried reverting "tcp: gso: fix truesize tracking" 0d08c42cf9a71530fef5ebcfe368f38f2dd0476f, but that didn't help.
--
Sander
[ 1164.511712] ------------[ cut here ]------------
[ 1164.518446] kernel BUG at net/core/skbuff.c:2839!
[ 1164.525226] invalid opcode: 0000 [#2] PREEMPT SMP
[ 1164.532024] Modules linked in:
[ 1164.538713] CPU: 0 PID: 3 Comm: ksoftirqd/0 Tainted: G D 3.12.0-mw-20131117+ #1
[ 1164.545649] Hardware name: MSI MS-7640/890FXA-GD70 (MS-7640) , BIOS V1.8B1 09/13/2010
[ 1164.552659] task: ffff880059b8a180 ti: ffff880059b96000 task.ti: ffff880059b96000
[ 1164.559649] RIP: e030:[<ffffffff819109a2>] [<ffffffff819109a2>] skb_segment+0x6b2/0x6d0
[ 1164.566860] RSP: e02b:ffff880059b97448 EFLAGS: 00010216
[ 1164.574023] RAX: 0000000000000011 RBX: 0000000000006612 RCX: 0000000000006612
[ 1164.581169] RDX: 00000000000005a8 RSI: 0000000000006612 RDI: ffff8800478ff682
[ 1164.588115] RBP: ffff880059b97518 R08: ffff88004ca06a00 R09: 0000000000000011
[ 1164.595169] R10: 000000000000606a R11: 0000000000000011 R12: 0000000000000000
[ 1164.602214] R13: ffff88004ca06900 R14: ffff88004ca06a00 R15: ffff88004bb57f00
[ 1164.609274] FS: 00007eff7dc67700(0000) GS:ffff88005f600000(0000) knlGS:0000000000000000
[ 1164.616394] CS: e033 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 1164.623562] CR2: 00007faa868fff30 CR3: 000000005877e000 CR4: 0000000000000660
[ 1164.630807] Stack:
[ 1164.637959] ffff880059b97458 ffffffff810d90dd ffff880059b97478 ffffffff8109f69a
[ 1164.645296] ffff88004b824628 ffff88004b824400 ffff880059b974e8 ffff88004ca06a00
[ 1164.652626] 0000001100000001 0000000000000040 0000000000000042 ffffffffffffffbe
[ 1164.659816] Call Trace:
[ 1164.667034] [<ffffffff810d90dd>] ? trace_hardirqs_on+0xd/0x10
[ 1164.674313] [<ffffffff8109f69a>] ? local_bh_enable+0xaa/0x110
[ 1164.681543] [<ffffffff819dafc2>] tcp_gso_segment+0x102/0x3e0
[ 1164.688691] [<ffffffff819b8224>] ? ip_queue_xmit+0x194/0x480
[ 1164.695741] [<ffffffff819e9fe4>] inet_gso_segment+0x124/0x350
[ 1164.702836] [<ffffffff8191cb25>] skb_mac_gso_segment+0xd5/0x1d0
[ 1164.709735] [<ffffffff8191ca92>] ? skb_mac_gso_segment+0x42/0x1d0
[ 1164.716739] [<ffffffff8191cc7b>] __skb_gso_segment+0x5b/0xc0
[ 1164.723802] [<ffffffff8191ce80>] dev_hard_start_xmit+0x1a0/0x500
[ 1164.730744] [<ffffffff81939780>] sch_direct_xmit+0x100/0x280
[ 1164.737531] [<ffffffff8191d404>] dev_queue_xmit+0x224/0x600
[ 1164.744403] [<ffffffff8191d1e0>] ? dev_hard_start_xmit+0x500/0x500
[ 1164.751317] [<ffffffff8194765e>] ? nf_hook_slow+0x11e/0x160
[ 1164.758332] [<ffffffff81a14040>] ? deliver_clone+0x60/0x60
[ 1164.765264] [<ffffffff81a140d7>] br_dev_queue_push_xmit+0x97/0x140
[ 1164.772082] [<ffffffff81a1419d>] br_forward_finish+0x1d/0x60
[ 1164.778925] [<ffffffff81a12490>] ? br_dev_free+0x30/0x30
[ 1164.785714] [<ffffffff81a142f2>] __br_deliver+0x52/0x180
[ 1164.792355] [<ffffffff81a146cd>] br_deliver+0x3d/0x50
[ 1164.798950] [<ffffffff81a126be>] br_dev_xmit+0x22e/0x290
[ 1164.805576] [<ffffffff81a12490>] ? br_dev_free+0x30/0x30
[ 1164.812106] [<ffffffff8191d18d>] dev_hard_start_xmit+0x4ad/0x500
[ 1164.818729] [<ffffffff8191d55e>] dev_queue_xmit+0x37e/0x600
[ 1164.825314] [<ffffffff8191d1e0>] ? dev_hard_start_xmit+0x500/0x500
[ 1164.831898] [<ffffffff819b7043>] ip_finish_output+0x293/0x610
[ 1164.838483] [<ffffffff819b8a44>] ? ip_output+0x54/0xf0
[ 1164.845055] [<ffffffff819b8a44>] ip_output+0x54/0xf0
[ 1164.851400] [<ffffffff819b3e71>] ip_forward_finish+0x71/0x1a0
[ 1164.857725] [<ffffffff819b42d8>] ip_forward+0x338/0x420
[ 1164.864167] [<ffffffff819b1ca0>] ip_rcv_finish+0x150/0x660
[ 1164.870477] [<ffffffff819b275b>] ip_rcv+0x22b/0x370
[ 1164.876707] [<ffffffff81a0de22>] ? packet_rcv_spkt+0x42/0x190
[ 1164.883040] [<ffffffff8191a3a2>] __netif_receive_skb_core+0x6e2/0x8b0
[ 1164.889193] [<ffffffff81919dd4>] ? __netif_receive_skb_core+0x114/0x8b0
[ 1164.895039] [<ffffffff810f28b9>] ? getnstimeofday+0x9/0x30
[ 1164.900704] [<ffffffff8191a58c>] __netif_receive_skb+0x1c/0x70
[ 1164.906327] [<ffffffff8191a7af>] netif_receive_skb+0x3f/0x50
[ 1164.911892] [<ffffffff8191a8d4>] napi_gro_complete+0x114/0x140
[ 1164.917459] [<ffffffff8191a7e0>] ? napi_gro_complete+0x20/0x140
[ 1164.923048] [<ffffffff810dcf3a>] ? lock_release+0x12a/0x240
[ 1164.928595] [<ffffffff819e9cb7>] ? inet_gro_receive+0x57/0x260
[ 1164.934042] [<ffffffff8191b552>] dev_gro_receive+0x2b2/0x3f0
[ 1164.939384] [<ffffffff8191b48b>] ? dev_gro_receive+0x1eb/0x3f0
[ 1164.944704] [<ffffffff8191b849>] napi_gro_receive+0x29/0xc0
[ 1164.949906] [<ffffffff816d9253>] rtl8169_poll+0x2d3/0x680
[ 1164.955036] [<ffffffff8191aba1>] net_rx_action+0x171/0x270
[ 1164.960180] [<ffffffff8109f27d>] __do_softirq+0xed/0x210
[ 1164.965285] [<ffffffff8109f3f5>] run_ksoftirqd+0x55/0x90
[ 1164.970334] [<ffffffff810c1e29>] smpboot_thread_fn+0x199/0x2a0
[ 1164.975402] [<ffffffff810c1c90>] ? SyS_setgroups+0x150/0x150
[ 1164.980438] [<ffffffff810bb00f>] kthread+0xdf/0x100
[ 1164.985309] [<ffffffff810baf30>] ? __init_kthread_worker+0x70/0x70
[ 1164.990221] [<ffffffff81a8a9cc>] ret_from_fork+0x7c/0xb0
[ 1164.995085] [<ffffffff810baf30>] ? __init_kthread_worker+0x70/0x70
[ 1164.999925] Code: ff 4c 8b 85 68 ff ff ff 44 8b 8d 50 ff ff ff 44 8b 95 48 ff ff ff 44 8b 9d 40 ff ff ff 0f 85 2c fe ff ff e9 23 fe ff ff 90 0f 0b <0f> 0b 48 c7 45 b0 ea ff ff ff e9 cf fc ff ff 0f 0b 0f 0b 66 66
[ 1165.010399] RIP [<ffffffff819109a2>] skb_segment+0x6b2/0x6d0
[ 1165.015512] RSP <ffff880059b97448>
[ 1165.020980] ---[ end trace 88f75f0c791ac25c ]---
[ 1165.026033] Kernel panic - not syncing: Fatal exception in interrupt
^ permalink raw reply
* Re: net/usb/ax88179_178a driver broken in linux-3.12
From: Mark Lord @ 2013-11-17 19:04 UTC (permalink / raw)
To: Eric Dumazet, Ming Lei, davem, netdev
In-Reply-To: <52891178.2080509@pobox.com>
On 13-11-17 01:56 PM, Mark Lord wrote:
> On 13-11-17 01:35 PM, Mark Lord wrote:
>> The USB3 network adapter locks up consistently for me here in 3.12,
>> but was working without issues in 3.11.x
>>
>> Source of the problem is this patch:
>> http://patchwork.ozlabs.org/patch/264021/
>>
>> Reverting the patch fixes the adapter.
>
> Okay, upon closer inspection, the bug appears to be a math error.
> Here's a simpler (non-revert) patch to fix the bug that was introduced in 3.12.
>
> (un-mangled copy attached; reproduced below for ease of viewing).
>
> Don't exceed more than the 8 bytes of reserved ("needed_headroom") space
> when calling pskb_expand_head. This fixes a bug introduced in linux-3.12.0.
>
> Signed-off-by: Mark Lord <mlord@pobox.com>
>
>
> --- ax88179_178a.c.orig 2013-11-03 18:41:51.000000000 -0500
> +++ linux/drivers/net/usb/ax88179_178a.c 2013-11-17 13:47:26.127971404 -0500
> @@ -1183,10 +1183,10 @@
> if (((skb->len + 8) % frame_size) == 0)
> tx_hdr2 |= 0x80008000; /* Enable padding */
>
> - headroom = skb_headroom(skb) - 8;
> + headroom = skb_headroom(skb);
>
> - if ((skb_header_cloned(skb) || headroom < 0) &&
> - pskb_expand_head(skb, headroom < 0 ? 8 : 0, 0, GFP_ATOMIC)) {
> + if ((skb_header_cloned(skb) || headroom < 8) &&
> + pskb_expand_head(skb, headroom < 8 ? 8 - headroom : 0, 0, GFP_ATOMIC)) {
> dev_kfree_skb_any(skb);
> return NULL;
> }
>
Dagnabbit.. that still locked up just after I sent the email,
despite working fine in testing prior to that.
So.. sticking with the full revert here for now.
Perhaps one of you chaps can spot the bug.
--
Mark Lord
Real-Time Remedies Inc.
mlord@pobox.com
^ permalink raw reply
* Re: net/usb/ax88179_178a driver broken in linux-3.12
From: Mark Lord @ 2013-11-17 18:56 UTC (permalink / raw)
To: Eric Dumazet, Ming Lei, davem, netdev
In-Reply-To: <52890C7E.6000607@pobox.com>
[-- Attachment #1: Type: text/plain, Size: 1460 bytes --]
On 13-11-17 01:35 PM, Mark Lord wrote:
> The USB3 network adapter locks up consistently for me here in 3.12,
> but was working without issues in 3.11.x
>
> Source of the problem is this patch:
> http://patchwork.ozlabs.org/patch/264021/
>
> Reverting the patch fixes the adapter.
Okay, upon closer inspection, the bug appears to be a math error.
Here's a simpler (non-revert) patch to fix the bug that was introduced in 3.12.
(un-mangled copy attached; reproduced below for ease of viewing).
Don't exceed more than the 8 bytes of reserved ("needed_headroom") space
when calling pskb_expand_head. This fixes a bug introduced in linux-3.12.0.
Signed-off-by: Mark Lord <mlord@pobox.com>
--- ax88179_178a.c.orig 2013-11-03 18:41:51.000000000 -0500
+++ linux/drivers/net/usb/ax88179_178a.c 2013-11-17 13:47:26.127971404 -0500
@@ -1183,10 +1183,10 @@
if (((skb->len + 8) % frame_size) == 0)
tx_hdr2 |= 0x80008000; /* Enable padding */
- headroom = skb_headroom(skb) - 8;
+ headroom = skb_headroom(skb);
- if ((skb_header_cloned(skb) || headroom < 0) &&
- pskb_expand_head(skb, headroom < 0 ? 8 : 0, 0, GFP_ATOMIC)) {
+ if ((skb_header_cloned(skb) || headroom < 8) &&
+ pskb_expand_head(skb, headroom < 8 ? 8 - headroom : 0, 0, GFP_ATOMIC)) {
dev_kfree_skb_any(skb);
return NULL;
}
--
Mark Lord
Real-Time Remedies Inc.
mlord@pobox.com
[-- Attachment #2: 51_ax88179_178a_fix_3.12_lockups.patch --]
[-- Type: text/x-patch, Size: 811 bytes --]
Don't exceed more than the 8 bytes of reserved ("needed_headroom") space
when calling pskb_expand_head. This fixes a bug introduced in linux-3.12.0.
Signed-off-by: Mark Lord <mlord@pobox.com>
--- ax88179_178a.c.orig 2013-11-03 18:41:51.000000000 -0500
+++ linux/drivers/net/usb/ax88179_178a.c 2013-11-17 13:47:26.127971404 -0500
@@ -1183,10 +1183,10 @@
if (((skb->len + 8) % frame_size) == 0)
tx_hdr2 |= 0x80008000; /* Enable padding */
- headroom = skb_headroom(skb) - 8;
+ headroom = skb_headroom(skb);
- if ((skb_header_cloned(skb) || headroom < 0) &&
- pskb_expand_head(skb, headroom < 0 ? 8 : 0, 0, GFP_ATOMIC)) {
+ if ((skb_header_cloned(skb) || headroom < 8) &&
+ pskb_expand_head(skb, headroom < 8 ? 8 - headroom : 0, 0, GFP_ATOMIC)) {
dev_kfree_skb_any(skb);
return NULL;
}
^ permalink raw reply
* Re: [PATCH net-next] veth: extend features to support tunneling
From: Alexei Starovoitov @ 2013-11-17 19:00 UTC (permalink / raw)
To: Eric Dumazet
Cc: Or Gerlitz, David Miller, Eric Dumazet, Stephen Hemminger,
netdev@vger.kernel.org, Michael S. Tsirkin, John Fastabend
In-Reply-To: <1384708847.8604.50.camel@edumazet-glaptop2.roam.corp.google.com>
On Sun, Nov 17, 2013 at 9:20 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sat, 2013-11-16 at 23:31 -0800, Alexei Starovoitov wrote:
>
>> In case of VMs sending gso packets over tap and tunnel in the host,
>> ip_forward is not in the picture.
>>
>
> I was specifically answering to #2 which should use ip forwarding, of
> course. Note that my patch was a POC : We have many other places where
> the typical MTU check is simply disabled as soon as skb is GSO.
I don't think #2 will do ip_forward either. veth goes into a bridge
and vxlan just adds encap.
>> when host mtu doesn't account for overhead of tunnel, the neat trick
>> we can do is to decrease gso_size while adding tunnel header.
>
> That would be very broken to change gso_size, this breaks DF flag
> semantic. You need to send an ICMP, and the sender will take appropriate
> action.
>
> GRO + GSO request that forwarded segments are the same than incoming
> ones. It's not like a proxy that can chose to aggregate as it wants.
In case of ip_forwarding - yes, but in case of tunnel we may look at
gso differently.
I'm not saying that 'gso_size -= tunnel_header' trick should be unconditional.
Obviously DF flag needs to be respected for pmtu to work.
ip_tunnel_xmit() already suppose to send icmp_frag_needed back, I'm
not sure it works for vxlan though.
What I'm proposing if tunnel receives normal gso packet with df bit
not set, it can safely
decrease that skb's gso_size. This way guest vm can have 1500 mtu just
like host mtu 1500,
until guest tries to pmtu, then vxlan sends icmp into guest and guest
adjusts itself if it can.
Since host cannot guarantee that guest will do the right thing with
icmp_frag_needed, it may continue doing 'gso_size -= tunnel_header'
trick without breaking anything.
^ permalink raw reply
* Re: [PATCH v3] net: Do not include padding in TCP GRO checksum
From: Alexander Duyck @ 2013-11-17 18:24 UTC (permalink / raw)
To: Herbert Xu; +Cc: Alexander Duyck, davem, netdev, edumazet
In-Reply-To: <20131117031710.GA18254@gondor.apana.org.au>
On 11/16/2013 07:17 PM, Herbert Xu wrote:
> On Sat, Nov 16, 2013 at 09:02:34AM -0800, Alexander Duyck wrote:
>> That being the case though, why don't we set the flush flag on detecting
>> a bad checksum and hand it off to tcp_gro_receive instead of returning
>> NULL? It seems like it would be in our interest to flush the flow and
>> then report the bad checksum instead of keeping the flow and handing off
>> the bad checksum to the stack.
> Because if the TCP checksum is wrong then it may belong to a
> different flow.
>
> Cheers,
It seems like it would be much more likely that a checksum error
occuring with a padded frame would corrupt the flow identifying data
then one that isn't padded.
I suppose it doesn't really matter though since checksum errors are
probably not going to be all that common of a case to deal with anyway.
Thanks,
Alex
^ permalink raw reply
* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Eric Dumazet @ 2013-11-17 17:41 UTC (permalink / raw)
To: Willy Tarreau
Cc: Arnaud Ebalard, Cong Wang, edumazet, linux-arm-kernel, netdev,
Thomas Petazzoni
In-Reply-To: <20131117141940.GA18569@1wt.eu>
On Sun, 2013-11-17 at 15:19 +0100, Willy Tarreau wrote:
>
> So it is fairly possible that in your case you can't fill the link if you
> consume too many descriptors. For example, if your server uses TCP_NODELAY
> and sends incomplete segments (which is quite common), it's very easy to
> run out of descriptors before the link is full.
BTW I have a very simple patch for TCP stack that could help this exact
situation...
Idea is to use TCP Small Queue so that we dont fill qdisc/TX ring with
very small frames, and let tcp_sendmsg() have more chance to fill
complete packets.
Again, for this to work very well, you need that NIC performs TX
completion in reasonable amount of time...
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 3dc0c6c..10456cf 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -624,13 +624,19 @@ static inline void tcp_push(struct sock *sk, int flags, int mss_now,
{
if (tcp_send_head(sk)) {
struct tcp_sock *tp = tcp_sk(sk);
+ struct sk_buff *skb = tcp_write_queue_tail(sk);
if (!(flags & MSG_MORE) || forced_push(tp))
- tcp_mark_push(tp, tcp_write_queue_tail(sk));
+ tcp_mark_push(tp, skb);
tcp_mark_urg(tp, flags);
- __tcp_push_pending_frames(sk, mss_now,
- (flags & MSG_MORE) ? TCP_NAGLE_CORK : nonagle);
+ if (flags & MSG_MORE)
+ nonagle = TCP_NAGLE_CORK;
+ if (atomic_read(&sk->sk_wmem_alloc) > 2048) {
+ set_bit(TSQ_THROTTLED, &tp->tsq_flags);
+ nonagle = TCP_NAGLE_CORK;
+ }
+ __tcp_push_pending_frames(sk, mss_now, nonagle);
}
}
^ permalink raw reply related
* Re: [PATCH net-next] veth: extend features to support tunneling
From: Eric Dumazet @ 2013-11-17 17:20 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Or Gerlitz, David Miller, Eric Dumazet, Stephen Hemminger,
netdev@vger.kernel.org, Michael S. Tsirkin, John Fastabend
In-Reply-To: <CAMEtUuzkwVSetLKb0AK7U=iB5ArhAhs-0vx9cX+WeZ2Fz04kqg@mail.gmail.com>
On Sat, 2013-11-16 at 23:31 -0800, Alexei Starovoitov wrote:
> In case of VMs sending gso packets over tap and tunnel in the host,
> ip_forward is not in the picture.
>
I was specifically answering to #2 which should use ip forwarding, of
course. Note that my patch was a POC : We have many other places where
the typical MTU check is simply disabled as soon as skb is GSO.
> when host mtu doesn't account for overhead of tunnel, the neat trick
> we can do is to decrease gso_size while adding tunnel header.
That would be very broken to change gso_size, this breaks DF flag
semantic. You need to send an ICMP, and the sender will take appropriate
action.
GRO + GSO request that forwarded segments are the same than incoming
ones. It's not like a proxy that can chose to aggregate as it wants.
^ permalink raw reply
* Re: [PATCH ] net_sched: actions - Add default lookup
From: Jamal Hadi Salim @ 2013-11-17 16:23 UTC (permalink / raw)
To: David Miller, eric.dumazet; +Cc: netdev, ebiederm, alexander.h.duyck
In-Reply-To: <5288E4C2.9090405@mojatatu.com>
[-- Attachment #1: Type: text/plain, Size: 696 bytes --]
Follow up to that patch would be this one.
I also want to do the same for the walker, but will wait
response.
cheers,
jamal
On 11/17/13 10:46, Jamal Hadi Salim wrote:
> On 11/03/13 23:17, David Miller wrote:
>> From: David Miller <davem@davemloft.net>
>> Date: Sun, 03 Nov 2013 23:12:32 -0500 (EST)
>>
>>> Right, and BUG() if we try to register and action with a NULL .lookup
>>> member.
>>
>> I return an error, BUG() is too harsh.
>>
>
> Sorry - I was distracted, but have time now.
> I wanted to fix this then send the other patches - but confused.
> We are setting the default if someone registers an action with
> a NULL .lookup. Why do we want to return an error?
>
> cheers,
> jamal
>
[-- Attachment #2: p-act2 --]
[-- Type: text/plain, Size: 3153 bytes --]
commit 926db1197d055ee933265ad4d5f5ae2392bcc654
Author: Jamal Hadi Salim <hadi@mojatatu.com>
Date: Sun Nov 17 11:20:16 2013 -0500
Use default lookup functions. Users are free to override when needed.
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 3a4c0ca..4225a93 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -585,7 +585,6 @@ static struct tc_action_ops act_csum_ops = {
.act = tcf_csum,
.dump = tcf_csum_dump,
.cleanup = tcf_csum_cleanup,
- .lookup = tcf_hash_search,
.init = tcf_csum_init,
.walk = tcf_generic_walker
};
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index fd2b3cf..15851da 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -206,7 +206,6 @@ static struct tc_action_ops act_gact_ops = {
.act = tcf_gact,
.dump = tcf_gact_dump,
.cleanup = tcf_gact_cleanup,
- .lookup = tcf_hash_search,
.init = tcf_gact_init,
.walk = tcf_generic_walker
};
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 60d88b6..1d3e191 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -298,7 +298,6 @@ static struct tc_action_ops act_ipt_ops = {
.act = tcf_ipt,
.dump = tcf_ipt_dump,
.cleanup = tcf_ipt_cleanup,
- .lookup = tcf_hash_search,
.init = tcf_ipt_init,
.walk = tcf_generic_walker
};
@@ -312,7 +311,6 @@ static struct tc_action_ops act_xt_ops = {
.act = tcf_ipt,
.dump = tcf_ipt_dump,
.cleanup = tcf_ipt_cleanup,
- .lookup = tcf_hash_search,
.init = tcf_ipt_init,
.walk = tcf_generic_walker
};
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 977c10e..6cb16ec 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -271,7 +271,6 @@ static struct tc_action_ops act_mirred_ops = {
.act = tcf_mirred,
.dump = tcf_mirred_dump,
.cleanup = tcf_mirred_cleanup,
- .lookup = tcf_hash_search,
.init = tcf_mirred_init,
.walk = tcf_generic_walker
};
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 876f0ef..30c13de 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -308,7 +308,6 @@ static struct tc_action_ops act_nat_ops = {
.act = tcf_nat,
.dump = tcf_nat_dump,
.cleanup = tcf_nat_cleanup,
- .lookup = tcf_hash_search,
.init = tcf_nat_init,
.walk = tcf_generic_walker
};
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 7ed78c9..ab4fc56 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -243,7 +243,6 @@ static struct tc_action_ops act_pedit_ops = {
.act = tcf_pedit,
.dump = tcf_pedit_dump,
.cleanup = tcf_pedit_cleanup,
- .lookup = tcf_hash_search,
.init = tcf_pedit_init,
.walk = tcf_generic_walker
};
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 272d8e9..16a62c3 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -407,7 +407,6 @@ static struct tc_action_ops act_police_ops = {
.act = tcf_act_police,
.dump = tcf_act_police_dump,
.cleanup = tcf_act_police_cleanup,
- .lookup = tcf_hash_search,
.init = tcf_act_police_locate,
.walk = tcf_act_police_walker
};
^ permalink raw reply related
* Re: [PATCH ] net_sched: actions - Add default lookup
From: Jamal Hadi Salim @ 2013-11-17 15:46 UTC (permalink / raw)
To: David Miller, eric.dumazet; +Cc: netdev, ebiederm, alexander.h.duyck
In-Reply-To: <20131103.231729.12647956618590906.davem@davemloft.net>
On 11/03/13 23:17, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Sun, 03 Nov 2013 23:12:32 -0500 (EST)
>
>> Right, and BUG() if we try to register and action with a NULL .lookup
>> member.
>
> I return an error, BUG() is too harsh.
>
Sorry - I was distracted, but have time now.
I wanted to fix this then send the other patches - but confused.
We are setting the default if someone registers an action with
a NULL .lookup. Why do we want to return an error?
cheers,
jamal
^ permalink raw reply
* [PATCH v2] fix possible NULL pointer Oops in fib(6)_rule_suppress
From: Stefan Tomanek @ 2013-11-17 14:45 UTC (permalink / raw)
To: netdev; +Cc: daniel.golle, sergei.shtylyov
In-Reply-To: <5288CB1B.7050806@cogentembedded.com>
Reported-by: Daniel Golle <daniel.golle@gmail.com>
Tested-by: Daniel Golle <daniel.golle@gmail.com>
Signed-off-by: Stefan Tomanek <stefan.tomanek@wertarbyte.de>
---
net/ipv4/fib_rules.c | 5 ++++-
net/ipv6/fib6_rules.c | 6 +++++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index 523be38..f2e1573 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -104,7 +104,10 @@ errout:
static bool fib4_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg)
{
struct fib_result *result = (struct fib_result *) arg->result;
- struct net_device *dev = result->fi->fib_dev;
+ struct net_device *dev = NULL;
+
+ if (result->fi)
+ dev = result->fi->fib_dev;
/* do not accept result if the route does
* not meet the required prefix length
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index e275916..3fd0a57 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -122,7 +122,11 @@ out:
static bool fib6_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg)
{
struct rt6_info *rt = (struct rt6_info *) arg->result;
- struct net_device *dev = rt->rt6i_idev->dev;
+ struct net_device *dev = NULL;
+
+ if (rt->rt6i_idev)
+ dev = rt->rt6i_idev->dev;
+
/* do not accept result if the route does
* not meet the required prefix length
*/
--
1.7.10.4
^ permalink raw reply related
* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Willy Tarreau @ 2013-11-17 14:19 UTC (permalink / raw)
To: Arnaud Ebalard
Cc: Thomas Petazzoni, Cong Wang, edumazet, linux-arm-kernel, netdev
In-Reply-To: <20131113072257.GB10591@1wt.eu>
Hi Arnaud,
[CCing Thomas and removing stable@]
On Wed, Nov 13, 2013 at 08:22:57AM +0100, Willy Tarreau wrote:
> On Tue, Nov 12, 2013 at 04:34:24PM +0100, Arnaud Ebalard wrote:
> > Can you give a pre-3.11.7 kernel a try if you find the time? I started
> > working on RN102 during 3.10-rc cycle but do not remember if I did the
> > first preformance tests on 3.10 or 3.11. And if you find more time,
> > 3.11.7 would be nice too ;-)
>
> Still have not found time for this but I observed something intriguing
> which might possibly match your experience : if I use large enough send
> buffers on the mirabox and receive buffers on the client, then the
> traffic drops for objects larger than 1 MB. I have quickly checked what's
> happening and it's just that there are pauses of up to 8 ms between some
> packets when the TCP send window grows larger than about 200 kB. And
> since there are no drops, there is no reason for the window to shrink.
> I suspect it's exactly related to the issue explained by Eric about the
> timer used to recycle the Tx descriptors. However last time I checked,
> these ones were also processed in the Rx path, which means that the
> ACKs that flow back should have had the same effect as a Tx IRQ (unless
> I'd use asymmetric routing, which was not the case). So there might be
> another issue. Ah, and it only happens with GSO.
I just had a quick look at the driver and I can confirm that Eric is right
about the fact that we use up to two descriptors per GSO segment. Thus, we
can saturate the Tx queue at 532/2 = 266 Tx segments = 388360 bytes (for
1460 MSS). I thought I had seen a tx flush from the rx poll function but I
can't find it so it seems I was wrong, or that I possibly misunderstood
mvneta_poll() the first time I read it. Thus the observed behaviour is
perfectly normal.
With GSO enabled, as soon as the window grows large enough, we can fill
all the Tx descriptors with few segments, then need to wait for 10ms (12
if running at 250 Hz as I am) to flush them, which explains the low speed
I was observing with large windows. When disabling GSO, as much as twice
the number of descriptors can be used, which is enough to fill the wire
in the same time frame. Additionally it's likely that more descriptors
get the time to be sent during that period and that each call to mvneta_tx()
causing a call to mvneta_txq_done() releases some of the previously sent
descriptors, allowing to sustain wire rate.
I wonder if we can call mvneta_txq_done() from the IRQ handler, which would
cause some recycling of the Tx descriptors when receiving the corresponding
ACKs.
Ideally we should enable the Tx IRQ, but I still have no access to this
chip's datasheet despite having asked Marvell several times in one year
(Thomas has it though).
So it is fairly possible that in your case you can't fill the link if you
consume too many descriptors. For example, if your server uses TCP_NODELAY
and sends incomplete segments (which is quite common), it's very easy to
run out of descriptors before the link is full.
I still did not have time to run a new kernel on this device however :-(
Best regards,
Willy
^ permalink raw reply
* Re: [PATCH] fix NULL pointer Oops in fib(6)_rule_suppress
From: Sergei Shtylyov @ 2013-11-17 13:56 UTC (permalink / raw)
To: Stefan Tomanek, netdev; +Cc: Daniel Golle
In-Reply-To: <20131117111409.GB23153@zirkel.wertarbyte.de>
Hello.
On 17-11-2013 15:14, Stefan Tomanek wrote:
> Reported-by: Daniel Golle <daniel.golle@gmail.com>
> Tested-by: Daniel Golle <daniel.golle@gmail.com>
> Signed-off-by: Stefan Tomanek <stefan.tomanek@wertarbyte.de>
> ---
> net/ipv4/fib_rules.c | 4 +++-
> net/ipv6/fib6_rules.c | 5 ++++-
> 2 files changed, 7 insertions(+), 2 deletions(-)
> diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
> index 523be38..4e1c1e5 100644
> --- a/net/ipv4/fib_rules.c
> +++ b/net/ipv4/fib_rules.c
> @@ -104,7 +104,9 @@ errout:
> static bool fib4_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg)
> {
> struct fib_result *result = (struct fib_result *) arg->result;
> - struct net_device *dev = result->fi->fib_dev;
> + struct net_device *dev = NULL;
Please leave empty line after declarations, like it was before your patch.
> + if (result->fi)
> + dev = result->fi->fib_dev;
>
> /* do not accept result if the route does
> * not meet the required prefix length
> diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
> index e275916..43e45b5 100644
> --- a/net/ipv6/fib6_rules.c
> +++ b/net/ipv6/fib6_rules.c
> @@ -122,7 +122,10 @@ out:
> static bool fib6_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg)
> {
> struct rt6_info *rt = (struct rt6_info *) arg->result;
> - struct net_device *dev = rt->rt6i_idev->dev;
> + struct net_device *dev = NULL;
Same here.
> + if (rt->rt6i_idev)
> + dev = rt->rt6i_idev->dev;
> +
WBR, Sergei
^ permalink raw reply
* [PATCH 2/2] 8139too: Allow using the largest possible MTU
From: Alban Bedel @ 2013-11-17 13:05 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, jgarzik, davem
In-Reply-To: <1384693524-20599-1-git-send-email-albeu@free.fr>
This driver allows MTU up to 1518 bytes which is not enought to run
batman-adv. Simply raise the maximum packet size up to the maximum
allowed by the transmit descriptor, 1792 bytes, giving a maximum MTU
of 1774 bytes.
Signed-off-by: Alban Bedel <albeu@free.fr>
---
drivers/net/ethernet/realtek/8139too.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/realtek/8139too.c b/drivers/net/ethernet/realtek/8139too.c
index 9d5d6d1..7be187a 100644
--- a/drivers/net/ethernet/realtek/8139too.c
+++ b/drivers/net/ethernet/realtek/8139too.c
@@ -183,7 +183,7 @@ static int debug = -1;
#define NUM_TX_DESC 4
/* max supported ethernet frame size -- must be at least (dev->mtu+14+4).*/
-#define MAX_ETH_FRAME_SIZE 1536
+#define MAX_ETH_FRAME_SIZE 1792
/* max supported payload size */
#define MAX_ETH_DATA_SIZE (MAX_ETH_FRAME_SIZE - ETH_HLEN - ETH_FCS_LEN)
--
1.8.4.2
^ permalink raw reply related
* [PATCH 1/2] 8139too: Allow setting MTU larger than 1500
From: Alban Bedel @ 2013-11-17 13:05 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, jgarzik, davem
Replace the default ndo_change_mtu callback with one that allow
setting MTU that the driver can handle.
Signed-off-by: Alban Bedel <albeu@free.fr>
---
drivers/net/ethernet/realtek/8139too.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/realtek/8139too.c b/drivers/net/ethernet/realtek/8139too.c
index da5972e..9d5d6d1 100644
--- a/drivers/net/ethernet/realtek/8139too.c
+++ b/drivers/net/ethernet/realtek/8139too.c
@@ -185,6 +185,9 @@ static int debug = -1;
/* max supported ethernet frame size -- must be at least (dev->mtu+14+4).*/
#define MAX_ETH_FRAME_SIZE 1536
+/* max supported payload size */
+#define MAX_ETH_DATA_SIZE (MAX_ETH_FRAME_SIZE - ETH_HLEN - ETH_FCS_LEN)
+
/* Size of the Tx bounce buffers -- must be at least (dev->mtu+14+4). */
#define TX_BUF_SIZE MAX_ETH_FRAME_SIZE
#define TX_BUF_TOT_LEN (TX_BUF_SIZE * NUM_TX_DESC)
@@ -920,11 +923,19 @@ static int rtl8139_set_features(struct net_device *dev, netdev_features_t featur
return 0;
}
+static int rtl8139_change_mtu(struct net_device *dev, int new_mtu)
+{
+ if (new_mtu < 68 || new_mtu > MAX_ETH_DATA_SIZE)
+ return -EINVAL;
+ dev->mtu = new_mtu;
+ return 0;
+}
+
static const struct net_device_ops rtl8139_netdev_ops = {
.ndo_open = rtl8139_open,
.ndo_stop = rtl8139_close,
.ndo_get_stats64 = rtl8139_get_stats64,
- .ndo_change_mtu = eth_change_mtu,
+ .ndo_change_mtu = rtl8139_change_mtu,
.ndo_validate_addr = eth_validate_addr,
.ndo_set_mac_address = rtl8139_set_mac_address,
.ndo_start_xmit = rtl8139_start_xmit,
--
1.8.4.2
^ permalink raw reply related
* [PATCH] fix NULL pointer Oops in fib(6)_rule_suppress
From: Stefan Tomanek @ 2013-11-17 11:14 UTC (permalink / raw)
To: netdev; +Cc: Daniel Golle
Reported-by: Daniel Golle <daniel.golle@gmail.com>
Tested-by: Daniel Golle <daniel.golle@gmail.com>
Signed-off-by: Stefan Tomanek <stefan.tomanek@wertarbyte.de>
---
net/ipv4/fib_rules.c | 4 +++-
net/ipv6/fib6_rules.c | 5 ++++-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index 523be38..4e1c1e5 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -104,7 +104,9 @@ errout:
static bool fib4_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg)
{
struct fib_result *result = (struct fib_result *) arg->result;
- struct net_device *dev = result->fi->fib_dev;
+ struct net_device *dev = NULL;
+ if (result->fi)
+ dev = result->fi->fib_dev;
/* do not accept result if the route does
* not meet the required prefix length
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index e275916..43e45b5 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -122,7 +122,10 @@ out:
static bool fib6_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg)
{
struct rt6_info *rt = (struct rt6_info *) arg->result;
- struct net_device *dev = rt->rt6i_idev->dev;
+ struct net_device *dev = NULL;
+ if (rt->rt6i_idev)
+ dev = rt->rt6i_idev->dev;
+
/* do not accept result if the route does
* not meet the required prefix length
*/
--
1.7.10.4
^ permalink raw reply related
* Re: oops in pskb_expand_head - 3.11.6
From: Michele Baldessari @ 2013-11-17 11:11 UTC (permalink / raw)
To: netdev, jiri; +Cc: Hannes Frederic Sowa
In-Reply-To: <20131116234223.GG16541@order.stressinduktion.org>
Hi Hannes,
On Sun, Nov 17, 2013 at 12:42:23AM +0100, Hannes Frederic Sowa wrote:
> On Sat, Nov 16, 2013 at 11:16:15PM +0000, Michele Baldessari wrote:
> > Two oops like the following were reported in Fedora 19 - kernel 3.11.6:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1015905
>
> I have not followed that issue that closely, but could you try linus tree
> or net-next?
>
> Maybe those two patches improve the situation:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=9037c3579a277f3a23ba476664629fda8c35f7c4
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=6aafeef03b9d9ecf255f3a80ed85ee070260e1ae
>
indeed a current kernel seems to fix the crash (according to the
reporters). I'll see if I manage to bisect exactly.
Thanks,
Michele
--
Michele Baldessari <michele@acksyn.org>
C2A5 9DA3 9961 4FFB E01B D0BC DDD4 DCCB 7515 5C6D
^ permalink raw reply
* Re: [PATCH 0/9] genetlink: reduce ops size and complexity (v2)
From: Johannes Berg @ 2013-11-17 9:12 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-wimax, bsingharora, netfilter-devel,
alex.bluesman.smirnov, dbaryshkov
In-Reply-To: <20131115.205313.1543711974325231353.davem@davemloft.net>
On Fri, 2013-11-15 at 20:53 -0500, David Miller wrote:
> > We could instead register an array of pointers to the groups:
> >
> > static const struct mcast_group *my_groups[] = {
> > &my_foo_mcast_group,
> > ...
> > };
> >
> > and pass this to the family - that'd still be less space (one pointer
> > for each group rather than two in a linked list) and still allow all
> > groups and this array to be const, but it's not quite as big a
> > saving ...
> >
> > Thoughts?
>
> This idea sounds fine. I don't even thing the array indexing is
> odd, especially if we can have named mnenomics for the indices
> or similar.
I posted something yesterday, but forgot to reply here - basically as
you'll see in the patches, I decided that we should pass the
family/group array index instead of passing the global group identifier
- this will also prevent new users of the APIs from abusing them like
quota/dropmonitor unfortunately did.
johannes
^ permalink raw reply
* Re: [PATCH net-next] veth: extend features to support tunneling
From: Alexei Starovoitov @ 2013-11-17 7:31 UTC (permalink / raw)
To: Eric Dumazet
Cc: Or Gerlitz, David Miller, Eric Dumazet, Stephen Hemminger,
netdev@vger.kernel.org, Michael S. Tsirkin, John Fastabend
In-Reply-To: <1384638027.8604.22.camel@edumazet-glaptop2.roam.corp.google.com>
On Sat, Nov 16, 2013 at 1:40 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sat, 2013-11-16 at 23:11 +0200, Or Gerlitz wrote:
>
>> Guys (thanks Eric for the clarification over the other vxlan thread),
>> with the latest networking code (e.g 3.12 or net-next) do you expect
>> notable performance (throughput) difference between these two configs?
>>
>> 1. bridge --> vxlan --> NIC
>> 2. veth --> bridge --> vxlan --> NIC
>>
>> BTW #2 doesn't work when packets start to be large unless I manually
>> decrease the veth device pair MTU. E.g if the NIC MTU is 1500, vxlan
>> advertizes an MTU of 1450 (= 1500 - (14 + 20 + 8 + 8)) and the bridge
>> inherits that, but not the veth device. Should someone/somewhere here
>> generate an ICMP packet which will cause the stack to decreate the
>> path mtu for the neighbour created on the veth device? what about
>> para-virtualized guests which are plugged into this (or any host based
>> tunneling) scheme, e.g in this scheme
>>
>> 3. guest virtio NIC --> vhost --> tap/macvtap --> bridge --> vxlan --> NIC
>>
>> Who/how do we want the guest NIC mtu/path mtu to take into account the
>> tunneling over-head?
>
> I mentioned this problem on another thread : gso packets escape the
> normal mtu checks in ip forwarding.
>
> vi +91 net/ipv4/ip_forward.c
>
> gso_size contains the size of the segment minus all headers.
In case of VMs sending gso packets over tap and tunnel in the host,
ip_forward is not in the picture.
when host mtu doesn't account for overhead of tunnel, the neat trick
we can do is to decrease gso_size while adding tunnel header.
This way when skb_gso_segment() kicks in during tx the packets will be
segmented into host mtu sized packets.
Receiving vm on the other side will be seeing packets of size
guest_mtu - tunnel_header_size,
but imo that's much better than sending ip fragments over vxlan fabric.
It will work for guests sending tcp/udp, but there is no good solution
for icmp other than ip frags.
This trick should work for hw offloaded vxlan, but we yet to
experiment with it on such nic.
^ permalink raw reply
* [PATCH net 4/4] bnx2x: Prevent "timeout waiting for state X"
From: Yuval Mintz @ 2013-11-17 6:59 UTC (permalink / raw)
To: davem, netdev; +Cc: ariele, dmitry, Yuval Mintz
In-Reply-To: <1384671569-30105-1-git-send-email-yuvalmin@broadcom.com>
From: Dmitry Kravkov <dmitry@broadcom.com>
Current driver release rtnl lock in between DCB re-configuration.
As a result, other flows (e.g., mtu config) may enter in between and fail
due to halted tx path for dcb configuration.
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Ariel Elior <ariele@broadcom.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 1 -
drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c | 5 -----
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 9 ++++-----
3 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
index c5e375d..8471478 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
@@ -1376,7 +1376,6 @@ enum {
BNX2X_SP_RTNL_RX_MODE,
BNX2X_SP_RTNL_HYPERVISOR_VLAN,
BNX2X_SP_RTNL_TX_STOP,
- BNX2X_SP_RTNL_TX_RESUME,
};
struct bnx2x_prev_path_list {
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c
index fcf2761..fdace20 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c
@@ -778,11 +778,6 @@ void bnx2x_dcbx_set_params(struct bnx2x *bp, u32 state)
/* ets may affect cmng configuration: reinit it in hw */
bnx2x_set_local_cmng(bp);
-
- set_bit(BNX2X_SP_RTNL_TX_RESUME, &bp->sp_rtnl_state);
-
- schedule_delayed_work(&bp->sp_rtnl_task, 0);
-
return;
case BNX2X_DCBX_STATE_TX_RELEASED:
DP(BNX2X_MSG_DCB, "BNX2X_DCBX_STATE_TX_RELEASED\n");
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 1d799ee..0fb1677 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -5235,18 +5235,18 @@ static void bnx2x_eq_int(struct bnx2x *bp)
case EVENT_RING_OPCODE_STOP_TRAFFIC:
DP(BNX2X_MSG_SP | BNX2X_MSG_DCB, "got STOP TRAFFIC\n");
+ bnx2x_dcbx_set_params(bp, BNX2X_DCBX_STATE_TX_PAUSED);
if (f_obj->complete_cmd(bp, f_obj,
BNX2X_F_CMD_TX_STOP))
break;
- bnx2x_dcbx_set_params(bp, BNX2X_DCBX_STATE_TX_PAUSED);
goto next_spqe;
case EVENT_RING_OPCODE_START_TRAFFIC:
DP(BNX2X_MSG_SP | BNX2X_MSG_DCB, "got START TRAFFIC\n");
+ bnx2x_dcbx_set_params(bp, BNX2X_DCBX_STATE_TX_RELEASED);
if (f_obj->complete_cmd(bp, f_obj,
BNX2X_F_CMD_TX_START))
break;
- bnx2x_dcbx_set_params(bp, BNX2X_DCBX_STATE_TX_RELEASED);
goto next_spqe;
case EVENT_RING_OPCODE_FUNCTION_UPDATE:
@@ -9714,11 +9714,10 @@ sp_rtnl_not_reset:
&bp->sp_rtnl_state))
bnx2x_pf_set_vfs_vlan(bp);
- if (test_and_clear_bit(BNX2X_SP_RTNL_TX_STOP, &bp->sp_rtnl_state))
+ if (test_and_clear_bit(BNX2X_SP_RTNL_TX_STOP, &bp->sp_rtnl_state)) {
bnx2x_dcbx_stop_hw_tx(bp);
-
- if (test_and_clear_bit(BNX2X_SP_RTNL_TX_RESUME, &bp->sp_rtnl_state))
bnx2x_dcbx_resume_hw_tx(bp);
+ }
/* work which needs rtnl lock not-taken (as it takes the lock itself and
* can be called from other contexts as well)
--
1.8.1.227.g44fe835
^ permalink raw reply related
* [PATCH net 1/4] bnx2x: Clean the sp rtnl task upon unload
From: Yuval Mintz @ 2013-11-17 6:59 UTC (permalink / raw)
To: davem, netdev; +Cc: ariele, dmitry, Yuval Mintz
In-Reply-To: <1384671569-30105-1-git-send-email-yuvalmin@broadcom.com>
From: Dmitry Kravkov <dmitry@broadcom.com>
While unloading, bnx2x needs to clean the sp_rtnl_state to prevent
configuration made before the unload to be applied afterwards with
stale values.
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Ariel Elior <ariele@broadcom.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 74d6486..f8cc9c2 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -2959,6 +2959,10 @@ int bnx2x_nic_unload(struct bnx2x *bp, int unload_mode, bool keep_link)
bp->port.pmf = 0;
+ /* clear pending work in rtnl task */
+ bp->sp_rtnl_state = 0;
+ smp_mb();
+
/* Free SKBs, SGEs, TPA pool and driver internals */
bnx2x_free_skbs(bp);
if (CNIC_LOADED(bp))
--
1.8.1.227.g44fe835
^ permalink raw reply related
* [PATCH net 3/4] bnx2x: prevent CFC attention
From: Yuval Mintz @ 2013-11-17 6:59 UTC (permalink / raw)
To: davem, netdev; +Cc: ariele, dmitry, Yuval Mintz
In-Reply-To: <1384671569-30105-1-git-send-email-yuvalmin@broadcom.com>
From: Dmitry Kravkov <dmitry@broadcom.com>
During VF load, prior to sending messages on HW channel to PF the VF
checks its bulletin board to see whether the PF indicated it has closed;
If a closed PF is encountered, the VF skips sending the message.
Due to incorrect return values, there's a possible scenario in which the VF
finishes loading "successfully", while the PF hasn't actually fully configured
FW/HW for the VFs supposed configuration.
Once VF tries to send Tx packets, HW will raise an attention (and FW possibly
will start treat the VF as malicious).
The patch fails the loading process in such a scenario.
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Ariel Elior <ariele@broadcom.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
index 28757df..c421009 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
@@ -128,7 +128,7 @@ static int bnx2x_send_msg2pf(struct bnx2x *bp, u8 *done, dma_addr_t msg_mapping)
if (bp->old_bulletin.valid_bitmap & 1 << CHANNEL_DOWN) {
DP(BNX2X_MSG_IOV, "detecting channel down. Aborting message\n");
*done = PFVF_STATUS_SUCCESS;
- return 0;
+ return -EINVAL;
}
/* Write message address */
--
1.8.1.227.g44fe835
^ permalink raw reply related
* [PATCH net 0/4] bnx2x: Bug fixes patch series
From: Yuval Mintz @ 2013-11-17 6:59 UTC (permalink / raw)
To: davem, netdev; +Cc: ariele, dmitry
Hi Dave,
This series contains several fixes, relating either to SR-IOV flows
or to critical sections protected by the rtnl lock.
Please consider applying these patches to `net'.
Thanks,
Yuval Mintz
^ 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