* [PATCH net-next 0/6] prandom fixes/improvements
From: Daniel Borkmann @ 2013-11-11 11:20 UTC (permalink / raw)
To: davem; +Cc: shemminger, fweimer, netdev
Hi Dave,
It would be great if you could still consider this series that fixes and
improves prandom for 3.13. We have sent it to netdev as prandom() originally
came from net/core/utils.c and networking is its main user. For a detailled
description, please see individual patches.
For patch 3 in this series, there will be a minor merge conflict with the
random tree that is for 3.13. See below how to resolve it.
====
Hannes says: on merge with the random tree I would suggest to resolve the
conflict in drivers/char/random.c like this:
if (r->entropy_total > 128) {
r->initialized = 1;
r->entropy_total = 0;
if (r == &nonblocking_pool) {
prandom_reseed_late();
pr_notice("random: %s pool is initialized\n",
r->name);
}
}
So it won't generate a warning if DEBUG_RANDOM_BOOT gets activated.
====
Patch 1 should probably also go to -stable.
Set tested on 32 and 64 bit machines.
Thanks a lot!
Ref. original discussion: http://patchwork.ozlabs.org/patch/289951/
Daniel Borkmann (4):
random32: fix off-by-one in seeding requirement
random32: move rnd_state to linux/random.h
random32: upgrade taus88 generator to taus113 from errata paper
random32: add test cases for taus113 implementation
Hannes Frederic Sowa (2):
random32: add periodic reseeding
random32: add prandom_reseed_late() and call when nonblocking pool
becomes initialized
drivers/char/random.c | 5 +-
include/linux/random.h | 14 +-
include/uapi/linux/random.h | 7 -
lib/Kconfig | 7 +
lib/random32.c | 307 +++++++++++++++++++++++++++++++++++++++-----
5 files changed, 294 insertions(+), 46 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: bridge not getting ip since 3.11.5 and 3.4.66
From: Veaceslav Falico @ 2013-11-11 11:07 UTC (permalink / raw)
To: Mark Trompell; +Cc: Linux-Kernel, netdev
In-Reply-To: <CAEPa5ym2Xz3F19fJJqNawkRLOKk6Ao6oXrzSrHU051V7p=9otA@mail.gmail.com>
On Mon, Nov 11, 2013 at 11:29 AM, Mark Trompell <mark@foresightlinux.org> wrote:
> my bridge br0 doesn't get an ip from dhcp anymore after 3.11.5 and 3.4.66,
> What information would be helpful and required to find out what's going wrong.
CC netdev
First thing would be to provide the network scheme. Do you use vlans?
Which network
cards/drivers are you using? Do you use some kind of virtualization?
Is bonding involved?
>
> Greetings
> Mark
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Best regards,
Veaceslav Falico
^ permalink raw reply
* Re: Oops in rfcomm_sock_getsockopt on net-next as of 20131111
From: Marcel Holtmann @ 2013-11-11 11:05 UTC (permalink / raw)
To: Bjørn Mork
Cc: linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org development,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <874n7j6xei.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
Hi Bjorn,
> I got this when I booted my laptop with todays net-next:
>
> [ 16.064546] BUG: unable to handle kernel paging request at 00000c8bfd080975
> [ 16.064558] IP: [<ffffffffa07f65b3>] rfcomm_sock_getsockopt+0x62/0x251 [rfcomm]
> [ 16.064561] PGD 0
> [ 16.064564] Oops: 0000 [#1] SMP
> [ 16.064637] Modules linked in: rfcomm bnep xt_hl binfmt_misc ip6table_filter ip6_tables x_tables nfsd nfs_acl nfs lockd fscache sunrpc 8021q garp stp llc tun loop fuse iTCO_wdt iTCO_vendor_support snd_hda_codec_conexant arc4 snd_hda_intel snd_hda_codec snd_hwdep snd_pcm_oss snd_mixer_oss iwlmvm mac80211 snd_pcm snd_page_alloc thinkpad_acpi nvram snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq iwlwifi coretemp uvcvideo cdc_mbim cdc_wdm videobuf2_vmalloc videobuf2_memops cdc_ncm kvm_intel videobuf2_core snd_seq_device psmouse usbnet lpc_ich videodev kvm snd_timer evdev serio_raw mfd_core mii cfg80211 i2c_i801 snd soundcore battery ac i915 i2c_algo_bit drm_kms_helper drm i2c_core acpi_cpufreq video wmi processor button btusb bluetooth rfkill ext4 crc16 jbd2 mbcache nbd sg sd_mod crc_
t10dif sr_mod cdrom crct10dif_common ahci libahci microcode libata scsi_mod thermal thermal_sys ehci_pci uhci_hcd ehci_hcd e1000e usbcore ptp pps_core usb_common
> [ 16.064656] CPU: 1 PID: 3153 Comm: bluetoothd Not tainted 3.12.0+ #136
> [ 16.064658] Hardware name: LENOVO 2776LEG/2776LEG, BIOS 6EET55WW (3.15 ) 12/19/2011
> [ 16.064660] task: ffff88022e50a440 ti: ffff8800b78b4000 task.ti: ffff8800b78b4000
> [ 16.064669] RIP: 0010:[<ffffffffa07f65b3>] [<ffffffffa07f65b3>] rfcomm_sock_getsockopt+0x62/0x251 [rfcomm]
> [ 16.064670] RSP: 0018:ffff8800b78b5ed8 EFLAGS: 00010246
> [ 16.064672] RAX: 00000c8bfd080975 RBX: ffff8800b688b140 RCX: 00007fff7ae042f8
> [ 16.064674] RDX: 0000000000000003 RSI: 0000000000000012 RDI: ffff8800b688b140
> [ 16.064676] RBP: ffff8800b78b5f28 R08: 00007fff7ae042fc R09: 00007fff7ae042f8
> [ 16.064678] R10: 00007fff7ae042f8 R11: ffff8800b83ec180 R12: ffff8800b78bc800
> [ 16.064680] R13: 00007fff7ae042f8 R14: 0000000000000003 R15: ffff8800b78bc800
> [ 16.064682] FS: 00007f020d523740(0000) GS:ffff88023b000000(0000) knlGS:0000000000000000
> [ 16.064685] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 16.064687] CR2: 00000c8bfd080975 CR3: 00000000b78a2000 CR4: 00000000000007e0
> [ 16.064688] Stack:
> [ 16.064692] ffff8800b78b5f08 00007fff7ae042fc ffffffff8139b647 00007fff7ae04570
> [ 16.064696] ffff8800b78b5f50 ffff8800b688b140 0000000000000012 0000000000000003
> [ 16.064700] 0000000000000011 00007f020e6abce0 ffff8800b78b5f78 ffffffff812b8215
> [ 16.064701] Call Trace:
> [ 16.064708] [<ffffffff8139b647>] ? sysret_check+0x1b/0x56
> [ 16.064714] [<ffffffff812b8215>] SyS_getsockopt+0x79/0x99
> [ 16.064718] [<ffffffff8139b622>] system_call_fastpath+0x16/0x1b
> [ 16.064756] Code: a0 31 c0 48 c7 c7 08 e0 7f a0 e8 35 45 9f e0 41 83 ff 12 0f 85 1e 01 00 00 4c 8b 7b 20 f6 05 a6 7a 00 00 04 49 8b 87 18 05 00 00 <4c> 8b 20 74 18 4c 89 fa 48 c7 c6 8f b9 7f a0 31 c0 48 c7 c7 30
> [ 16.064763] RIP [<ffffffffa07f65b3>] rfcomm_sock_getsockopt+0x62/0x251 [rfcomm]
> [ 16.064764] RSP <ffff8800b78b5ed8>
> [ 16.064766] CR2: 00000c8bfd080975
> [ 16.064769] ---[ end trace f71c8d4720ff0e6f ]---
>
>
> I am using standard Debian startup scripts AFAIK, doing:
>
> /usr/bin/rfcomm -f /etc/bluetooth/rfcomm.conf bind all
>
> where my rfcomm.conf is completely empty except for comments. The
> reported userspace version is
>
> RFCOMM configuration utility ver 4.99
>
> Let me know if there is any other information you need to debug this
> further.
known issue. Check 0be087f56118b67479b6e1a542d1dcf54fa83615 from bluetooth-next tree. We just have not pushed that through wireless-next and net-next yet.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v2] mac80211: add assoc beacon timeout logic
From: Felipe Contreras @ 2013-11-11 11:01 UTC (permalink / raw)
To: Krishna Chaitanya
Cc: linux-wireless, netdev, John W. Linville, Johannes Berg,
David S. Miller
In-Reply-To: <CABPxzY+MkosJBeQdBFV3u5X=e5io3qKeb6sqd8Fn89awMc2vuA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, Nov 11, 2013 at 12:41 AM, Krishna Chaitanya
<chaitanya.mgit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, Nov 11, 2013 at 3:15 AM, Felipe Contreras
> <felipe.contreras-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> We don't want to be waiting forever for a beacon that will never come,
>> just continue the association.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>
>> The previous version depended on some cleanup patches, plus had some unclear
>> rerun logic.
>>
>> net/mac80211/ieee80211_i.h | 1 +
>> net/mac80211/mlme.c | 32 ++++++++++++++++++++++++++------
>> 2 files changed, 27 insertions(+), 6 deletions(-)
>>
>> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
>> index 611abfc..e1f858d 100644
>> --- a/net/mac80211/ieee80211_i.h
>> +++ b/net/mac80211/ieee80211_i.h
>> @@ -358,6 +358,7 @@ struct ieee80211_mgd_assoc_data {
>> const u8 *supp_rates;
>>
>> unsigned long timeout;
>> + unsigned long beacon_timeout;
>> int tries;
>>
>> u16 capability;
>> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
>> index 86e4ad5..68f76f7 100644
>> --- a/net/mac80211/mlme.c
>> +++ b/net/mac80211/mlme.c
>> @@ -38,6 +38,7 @@
>> #define IEEE80211_ASSOC_TIMEOUT (HZ / 5)
>> #define IEEE80211_ASSOC_TIMEOUT_LONG (HZ / 2)
>> #define IEEE80211_ASSOC_TIMEOUT_SHORT (HZ / 10)
>> +#define IEEE80211_ASSOC_BEACON_TIMEOUT 2 * HZ
>
> Don't you think its too long? 3-4 beacon intervals should suffice.
I don't have an opinion, if you think that's better, that's fine by me.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] mac80211: add assoc beacon timeout logic
From: Felipe Contreras @ 2013-11-11 10:59 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-wireless Mailing List, netdev, John W. Linville,
David S. Miller
In-Reply-To: <1384160932.14334.6.camel@jlt4.sipsolutions.net>
On Mon, Nov 11, 2013 at 3:08 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Sun, 2013-11-10 at 15:45 -0600, Felipe Contreras wrote:
>> We don't want to be waiting forever for a beacon that will never come,
>> just continue the association.
>
> This makes no sense, IMO.
>
> If there really are no beacons at all, then nothing can work with the AP
> -- things like HT, regulatory/radar, powersave, multiple virtual
> interfaces and many others require beacons.
Well the AP is sending beacons, but they seem to be corrupted,
although the corruption often seems to happen in a place that is not
so important.
No other device at this house seems to have a problem with that, even
the Intel driver in Windows doesn't have a problem, it's just the
driver in Linux.
However, if I apply this patch, I don't notice any issue, it
associates and works fine. Maybe there's some subtle issues with
features I don't personally use, or perhaps there's the occasional
disconnection (although that could be due to something else), but
that's light years away from not associating at all.
I'd say between a) some features not working and b) nothing working at
all, a) is preferred.
If you think it's better that nothing works at all, then wouldn't it
make sense to time out and return an error? Currently we just keep
trying to associate *forever*.
> If the AP is sending beacons but the device isn't receiving them, then
> it's a driver bug and mac80211 shouldn't work around it.
I agree, but I can't seem to convince Intel guys of that. The firmware
is dropping the corrupt beacon frames (although not always), so
there's nothing the driver can do afterwards.
But even if there were not beacons at all (corrupt or otherwise), I
still think waiting *forever* in a loop is not ideal, a) is preferred;
not having all the features, but still somehow work (from my point of
view it's more than somewhat).
--
Felipe Contreras
^ permalink raw reply
* Oops in rfcomm_sock_getsockopt on net-next as of 20131111
From: Bjørn Mork @ 2013-11-11 10:33 UTC (permalink / raw)
To: linux-bluetooth-u79uwXL29TY76Z2rM5mHXA; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA
Hello,
I got this when I booted my laptop with todays net-next:
[ 16.064546] BUG: unable to handle kernel paging request at 00000c8bfd080975
[ 16.064558] IP: [<ffffffffa07f65b3>] rfcomm_sock_getsockopt+0x62/0x251 [rfcomm]
[ 16.064561] PGD 0
[ 16.064564] Oops: 0000 [#1] SMP
[ 16.064637] Modules linked in: rfcomm bnep xt_hl binfmt_misc ip6table_filter ip6_tables x_tables nfsd nfs_acl nfs lockd fscache sunrpc 8021q garp stp llc tun loop fuse iTCO_wdt iTCO_vendor_support snd_hda_codec_conexant arc4 snd_hda_intel snd_hda_codec snd_hwdep snd_pcm_oss snd_mixer_oss iwlmvm mac80211 snd_pcm snd_page_alloc thinkpad_acpi nvram snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq iwlwifi coretemp uvcvideo cdc_mbim cdc_wdm videobuf2_vmalloc videobuf2_memops cdc_ncm kvm_intel videobuf2_core snd_seq_device psmouse usbnet lpc_ich videodev kvm snd_timer evdev serio_raw mfd_core mii cfg80211 i2c_i801 snd soundcore battery ac i915 i2c_algo_bit drm_kms_helper drm i2c_core acpi_cpufreq video wmi processor button btusb bluetooth rfkill ext4 crc16 jbd2 mbcache nbd sg sd_mod crc_t10dif sr_mod cdrom crct10dif_common ahci libahci microcode libata scsi_mod thermal thermal_sys ehci_pci uhci_hcd ehci_hcd e1000e usbcore ptp pps_core usb_common
[ 16.064656] CPU: 1 PID: 3153 Comm: bluetoothd Not tainted 3.12.0+ #136
[ 16.064658] Hardware name: LENOVO 2776LEG/2776LEG, BIOS 6EET55WW (3.15 ) 12/19/2011
[ 16.064660] task: ffff88022e50a440 ti: ffff8800b78b4000 task.ti: ffff8800b78b4000
[ 16.064669] RIP: 0010:[<ffffffffa07f65b3>] [<ffffffffa07f65b3>] rfcomm_sock_getsockopt+0x62/0x251 [rfcomm]
[ 16.064670] RSP: 0018:ffff8800b78b5ed8 EFLAGS: 00010246
[ 16.064672] RAX: 00000c8bfd080975 RBX: ffff8800b688b140 RCX: 00007fff7ae042f8
[ 16.064674] RDX: 0000000000000003 RSI: 0000000000000012 RDI: ffff8800b688b140
[ 16.064676] RBP: ffff8800b78b5f28 R08: 00007fff7ae042fc R09: 00007fff7ae042f8
[ 16.064678] R10: 00007fff7ae042f8 R11: ffff8800b83ec180 R12: ffff8800b78bc800
[ 16.064680] R13: 00007fff7ae042f8 R14: 0000000000000003 R15: ffff8800b78bc800
[ 16.064682] FS: 00007f020d523740(0000) GS:ffff88023b000000(0000) knlGS:0000000000000000
[ 16.064685] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 16.064687] CR2: 00000c8bfd080975 CR3: 00000000b78a2000 CR4: 00000000000007e0
[ 16.064688] Stack:
[ 16.064692] ffff8800b78b5f08 00007fff7ae042fc ffffffff8139b647 00007fff7ae04570
[ 16.064696] ffff8800b78b5f50 ffff8800b688b140 0000000000000012 0000000000000003
[ 16.064700] 0000000000000011 00007f020e6abce0 ffff8800b78b5f78 ffffffff812b8215
[ 16.064701] Call Trace:
[ 16.064708] [<ffffffff8139b647>] ? sysret_check+0x1b/0x56
[ 16.064714] [<ffffffff812b8215>] SyS_getsockopt+0x79/0x99
[ 16.064718] [<ffffffff8139b622>] system_call_fastpath+0x16/0x1b
[ 16.064756] Code: a0 31 c0 48 c7 c7 08 e0 7f a0 e8 35 45 9f e0 41 83 ff 12 0f 85 1e 01 00 00 4c 8b 7b 20 f6 05 a6 7a 00 00 04 49 8b 87 18 05 00 00 <4c> 8b 20 74 18 4c 89 fa 48 c7 c6 8f b9 7f a0 31 c0 48 c7 c7 30
[ 16.064763] RIP [<ffffffffa07f65b3>] rfcomm_sock_getsockopt+0x62/0x251 [rfcomm]
[ 16.064764] RSP <ffff8800b78b5ed8>
[ 16.064766] CR2: 00000c8bfd080975
[ 16.064769] ---[ end trace f71c8d4720ff0e6f ]---
I am using standard Debian startup scripts AFAIK, doing:
/usr/bin/rfcomm -f /etc/bluetooth/rfcomm.conf bind all
where my rfcomm.conf is completely empty except for comments. The
reported userspace version is
RFCOMM configuration utility ver 4.99
Let me know if there is any other information you need to debug this
further.
Bjørn
^ permalink raw reply
* Re: [PATCH net-next 02/13] driver: net: remove unnecessary skb NULL check before calling dev_kfree_skb_irq
From: Govindarajulu Varadarajan @ 2013-11-11 10:31 UTC (permalink / raw)
To: David Miller
Cc: govindarajulu90-Re5JQEeQqe8AvxtiuMwx3w,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
schwidefsky-tA70FqPdS9bQT0dZR+AlfA,
linville-2XuSBdqkA4R54TAoqtyWWQ,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, IvDoorn-Re5JQEeQqe8AvxtiuMwx3w,
sbhatewara-pghWNbHTmq7QT0dZR+AlfA, samuel-jcdQHdrhKHMdnm+yROfE0A,
chas-vT06rRrALxcmhCb6mdbn6A, roland-DgEjT+Ai2ygdnm+yROfE0A,
isdn-iHCpqvpFUx0uJkBD2foKsQ, jcliburn-Re5JQEeQqe8AvxtiuMwx3w,
benve-FYB4Gu1CFyUAvxtiuMwx3w, ssujith-FYB4Gu1CFyUAvxtiuMwx3w,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
jesse.brandeburg-ral2JQCrhuEAvxtiuMwx3w,
shahed.shaikh-h88ZbnxC6KDQT0dZR+AlfA, joe-6d6DIl74uiNBDgjK7y7TUQ,
apw-Z7WLFzj8eWMS+FvcfC7Uqw
In-Reply-To: <20131104.151230.1978898006990867916.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Mon, 4 Nov 2013, David Miller wrote:
> From: Govindarajulu Varadarajan <govindarajulu90-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Date: Sat, 2 Nov 2013 19:17:43 +0530
>
>> @@ -1030,10 +1030,8 @@ static void ni65_xmit_intr(struct net_device *dev,int csr0)
>> }
>>
>> #ifdef XMT_VIA_SKB
>> - if(p->tmd_skb[p->tmdlast]) {
>> - dev_kfree_skb_irq(p->tmd_skb[p->tmdlast]);
>> - p->tmd_skb[p->tmdlast] = NULL;
>> - }
>> + dev_kfree_skb_irq(p->tmd_skb[p->tmdlast]);
>> + p->tmd_skb[p->tmdlast] = NULL;
>> #endif
>
> I absolutely disagree with this kind of change.
>
> There is a non-trivial cost for NULL'ing out that array entry
> unconditionally. It's a dirtied cache line and this is in the
> fast path of TX SKB reclaim of this driver.
>
> You've made several changes of this kind.
>
> And it sort-of shows that the places that do check for NULL,
> are getting something in return for that test, namely avoidance
> of an unnecessary cpu store in the fast path of the driver.
>
True, in case of dev_kfree_skb_irq. If you look at patch 06-12, at many
places we do
if (s->skb) {
dev_kfree_skb_any(s->skb);
s->skb = NULL)
}
This is in fast path. If the code is not running in hardirq,
dev_kfree_skb_any calls dev_kfree_skb. Which again check if skb is NULL.
So we are checking if skb is null twice. That is what this patch is
trying to fix. (sorry I should have mentioned this in cover letter).
I am not sure if you have read my previous mail. I am pasting it below.
>> On Sun, 3 Nov 2013, Brandeburg, Jesse wrote:
>> Thanks for this work, I'm a little concerned that there is a
>> non-trivial
>> overhead to this patch.
>>
>> when doing (for example in the Intel drivers):
>> if (s->skb) {
>> dev_kfree_skb(s->skb);
>> s->skb = NULL;
>> }
>>
>
>In current code, dev_kfree_skb is NULL safe. Which means skb is
>checked for NULL inside dev_kfree_skb. dev_kfree_skb_any is also NULL safe
>when the code is running in non-hardirq.
>
>Lets consider two cases
>
>1. skb is not NULL:
> * Without my patch:
> In the code above, we check for skb!=NULL twice. (once
> before calling dev_kfree_skb, once by dev_kfree_skb). And
> then we do assignment.
> * With this patch:
> we check for skb!=NULL once, And then we do assignment.
>
> To fix the twice NULL check, we either have to remove the check
> which is inside dev_kfree_skb (1). Or do whats done in this
> patch.
>
> (1) is not an option because a lot of kernel code already
> assumes that dev_kfree_skb is NULL safe.
>
>2. skb is NULL:
> * Without this patch:
> One if statement is executed.
> * With this patch:
> One if statement and one assignment is executed.
>
> From my observation most of the dev_kfree_skb calls are from
> e1000_unmap_and_free_tx_resource, e1000_put_txbuf,
> atl1_clean_tx_ring, alx_free_txbuf etc. in clean up functions.
>
> Is is quite unlikely thats skb is NULL. So it comes down to one extra
> if-branching statement or one extra assignment. I would prefer extra
> assignment to branching statement. In my opinion extra assignment is
> very little price we pay.
>
> //govind
Another way to solve the double NULL check is to define a new function
something like this
dev_kfree_skb_NULL(struct sk_buff **skb)
{
if(*skb) {
free_skb(*skb);
*skb=NULL;
}
}
and use this if you want to free a skb and make it NULL.
Is this approach better?
//govind
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [REGRESSION,BISECTED] 3.11.7,3.12: GbE iface rate drops to few KB/s
From: Arnaud Ebalard @ 2013-11-11 10:27 UTC (permalink / raw)
To: Eric Dumazet, David S. Miller, Greg Kroah-Hartman
Cc: netdev, stable, linux-arm-kernel
Hi, [resending (previous msg did not hit netdev); also fixed subject]
I decided to upgrade the kernel on one of my ReadyNAS 102 from 3.11.1 to
3.11.7. The device is based on Marvell Armada 370 SoC and uses mvneta
driver. Mine runs Debian armel unstable but I can confirm the issue also
happens on a debian harmhf unstable.
Doing some scp transfers of files located on the NAS (1000baseT-FD on
both side), I noticed the transfers rate is ridiculously slow (280KB/s).
I did the same test with a 3.12 kernel and got the same results,
i.e. AFAICT the bug also exist upstream.
So, I decided to go to hell and start digging a bit: I run a 'git bisect'
session on stable tree from 3.11.1 (known good) to 3.11.7 (known
bad). The results are given below.
I decided to reboot on my old 3.11.1 kernel and do 20 files transfers
of a 1GB file located on the NAS to my laptop via scp. I took the 20+
minutes and let them all finish: each transfer took between 1min5s and
1min7s (around 16MB/s, the limitation in that case being the crypto part).
I rebooted again and did the exact same thing on the 3.11.7 and after
the completion of the first file transfer in 1m6s (16MB/s), the second
one gave me that:
arno@small:~scp RN102:/tmp/random /dev/null
random 0% 1664KB 278.9KB/s 1:05:37 ETA^C
And it continued that way for the remaining transfers (i did ^c after
some seconds to restart the transfer when the rate was low):
$ for k in $(seq 1 20) ; do scp RN102:random /dev/null ; done
random 100% 1024MB 15.6MB/s 01:06 ETA^C
random 0% 9856KB 282.2KB/s 1:01:20 ETA^C
random 16% 168MB 563.9KB/s 25:54 ETA^C
random 0% 2816KB 273.5KB/s 1:03:43 ETA^C
random 100% 1024MB 15.5MB/s 01:06
random 1% 17MB 282.3KB/s 1:00:54 ETA^C
random 0% 544KB 259.2KB/s 1:07:23 ETA^C
random 0% 4224KB 277.3KB/s 1:02:45 ETA^C
random 0% 832KB 262.1KB/s 1:06:37 ETA^C
random 0% 3360KB 273.4KB/s 1:03:43 ETA^C
random 0% 3072KB 271.8KB/s 1:04:07 ETA^C
random 0% 832KB 262.1KB/s 1:06:37 ETA^C
random 0% 1408KB 267.0KB/s 1:05:21 ETA^C
random 0% 1120KB 264.7KB/s 1:05:57 ETA
...
To be sure, I did 2 additional reboots, one on each kernel and the
result is consistent. Perfect on 3.11.1 and slow rate most of the time
on 3.11.7 (Both kernel are compiled from a fresh make clean, using the
same config file).
Then, knowing that, I started a git bisect session on stable tree to end
up with the following suspects. I failed to go any further to a single
commit, due to crashes, but I could recompile a kernel w/ debug info and
report what I get if neeeded.
commit dc0791aee672 tcp: do not forget FIN in tcp_shifted_skb() [bad]
commit 18ddf5127c9f tcp: must unclone packets before mangling them
commit 80bd5d8968d8 tcp: TSQ can use a dynamic limit
commit dbeb18b22197 tcp: TSO packets automatic sizing
commit 50704410d014 Linux 3.11.6 [good]
Eric, David, if it has already been reported and fixed, just tell
me. Otherwise, if you have any ideas, I'll be happy to test this
evening.
Cheers,
a+
Just in case it may be useful, here is what ethtool reports on RN102:
# ethtool -i eth0
driver: mvneta
version: 1.0
firmware-version:
bus-info: eth0
supports-statistics: no
supports-test: no
supports-eeprom-access: no
supports-register-dump: no
supports-priv-flags: no
# ethtool -k eth0
Features for eth0:
rx-checksumming: off [fixed]
tx-checksumming: on
tx-checksum-ipv4: on
tx-checksum-ip-generic: off [fixed]
tx-checksum-ipv6: off [fixed]
tx-checksum-fcoe-crc: off [fixed]
tx-checksum-sctp: off [fixed]
scatter-gather: on
tx-scatter-gather: on
tx-scatter-gather-fraglist: off [fixed]
tcp-segmentation-offload: off
tx-tcp-segmentation: off [fixed]
tx-tcp-ecn-segmentation: off [fixed]
tx-tcp6-segmentation: off [fixed]
udp-fragmentation-offload: off [fixed]
generic-segmentation-offload: on
generic-receive-offload: on
large-receive-offload: off [fixed]
rx-vlan-offload: off [fixed]
tx-vlan-offload: off [fixed]
ntuple-filters: off [fixed]
receive-hashing: off [fixed]
highdma: off [fixed]
rx-vlan-filter: off [fixed]
vlan-challenged: off [fixed]
tx-lockless: off [fixed]
netns-local: off [fixed]
tx-gso-robust: off [fixed]
tx-fcoe-segmentation: off [fixed]
tx-gre-segmentation: off [fixed]
tx-udp_tnl-segmentation: off [fixed]
tx-mpls-segmentation: off [fixed]
fcoe-mtu: off [fixed]
tx-nocache-copy: on
loopback: off [fixed]
rx-fcs: off [fixed]
rx-all: off [fixed]
tx-vlan-stag-hw-insert: off [fixed]
rx-vlan-stag-hw-parse: off [fixed]
rx-vlan-stag-filter: off [fixed]
^ permalink raw reply
* Re: [PATCH] net bridge: add null pointer check, fix panic
From: Alexander Y. Fomichev @ 2013-11-11 10:27 UTC (permalink / raw)
To: Eric Dumazet; +Cc: xiaoming gao, stephen, davem, bridge, netdev, linux-kernel
In-Reply-To: <1371713342.3252.371.camel@edumazet-glaptop>
[-- Attachment #1: Type: text/plain, Size: 4775 bytes --]
On Thu, Jun 20, 2013 at 11:29 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2013-06-20 at 15:00 +0800, xiaoming gao wrote:
>
>> HI Eric
>> the problem is as follow:
>> br_del_if()-->del_nbp():
>>
>> list_del_rcu(&p->list);
>> dev->priv_flags &= ~IFF_BRIDGE_PORT;
>>
>> ------>at this point, the nic be deleting still have rx_handler , so , may in br_handle_frame()
>> ------>br_port_exists() will return false,so br_get_port_rcu() will return NULL
>> ------>so in br_handle_frame , there will be a null panic.
>>
>> netdev_rx_handler_unregister(dev);
>> synchronize_net();
>
> This code is no longer like that in current tree.
> Check commit 4a0b5ec12f0ffc3024616e6dc62cf8a04c54edcd
> ("bridge: remove a redundant synchronize_net()")
>
>>
>>
>> i have checked commit 00cfec37484761a44, i think it didn't fix this bug..
>
> I claim adding NULL tests is not needed in the fast path, it was clearly
> stated in the changelog.
>
> I would change the dismantle path to make sure br_get_port_rcu() does
> not return NULL in the fast path, and remove the test on FF_BRIDGE_PORT
> Try something like that :
>
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index 1b8b8b8..2edfb80 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -60,7 +60,7 @@ static int br_pass_frame_up(struct sk_buff *skb)
> int br_handle_frame_finish(struct sk_buff *skb)
> {
> const unsigned char *dest = eth_hdr(skb)->h_dest;
> - struct net_bridge_port *p = br_port_get_rcu(skb->dev);
> + struct net_bridge_port *p = __br_port_get_rcu(skb->dev);
> struct net_bridge *br;
> struct net_bridge_fdb_entry *dst;
> struct net_bridge_mdb_entry *mdst;
> @@ -68,7 +68,7 @@ int br_handle_frame_finish(struct sk_buff *skb)
> bool unicast = true;
> u16 vid = 0;
>
> - if (!p || p->state == BR_STATE_DISABLED)
> + if (p->state == BR_STATE_DISABLED)
> goto drop;
>
> if (!br_allowed_ingress(p->br, nbp_get_vlan_info(p), skb, &vid))
> @@ -142,7 +142,7 @@ drop:
> /* note: already called with rcu_read_lock */
> static int br_handle_local_finish(struct sk_buff *skb)
> {
> - struct net_bridge_port *p = br_port_get_rcu(skb->dev);
> + struct net_bridge_port *p = __br_port_get_rcu(skb->dev);
> u16 vid = 0;
>
> br_vlan_get_tag(skb, &vid);
> @@ -172,7 +172,7 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
> if (!skb)
> return RX_HANDLER_CONSUMED;
>
> - p = br_port_get_rcu(skb->dev);
> + p = __br_port_get_rcu(skb->dev);
>
> if (unlikely(is_link_local_ether_addr(dest))) {
> /*
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 3be89b3..9fdd467 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -184,6 +184,11 @@ struct net_bridge_port
>
> #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
>
> +static inline struct net_bridge_port *__br_port_get_rcu(const struct net_device *dev)
> +{
> + return rcu_dereference(dev->rx_handler_data);
> +}
> +
> static inline struct net_bridge_port *br_port_get_rcu(const struct net_device *dev)
> {
> struct net_bridge_port *port =
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
> I claim adding NULL tests is not needed in the fast path, it was clearly
> stated in the changelog.
Hello,
This commit makes trouble for current STP.
Two days ago i tried to switch to 3.10.18 and i've caught "bad magic"
on uninitialized br->lock:
./net/bridge/br_stp_bpdu.c +158 in br_stp_rcv (trace attached):
p = br_port_get_rcu(dev);
br = p->br;
spin_lock(&br->lock); <- here
-----------------------------------------------
Bisect pointed to this commit
(linux-stable: 960b8e5018a552f62cfbc0dfe94be7b6ba178f13)
(mainline 716ec052d2280d511e10e90ad54a86f5b5d4dcc2)
As far as i can see this happens when:
a) bridge module had been loaded but there was no bridge interface,
br->lock had not been initialized.
b) interface had been in promiscuous mod (tcpdump)
c) stp broadcasts 01:80:c2:00:00:00 coming to this iface
(llc_rcv drops PACKET_OTHERHOST to protect us in promiscuous mode
but seems like not a broadcasts)
d) and finally rx_handler_data had been initialised for this interface
( by macvlan in my case)
It seems like STP needs its own IFF_BRIDGE_PORT check.
probably an easiest option to check it in br_stp_rcv as before (or
probably in llc_rcv)...
--
Best regards.
Alexander Y. Fomichev <git.user@gmail.com>
[-- Attachment #2: br_stp_rcv-spin_lock.trace --]
[-- Type: application/octet-stream, Size: 5983 bytes --]
[ 1761.045223] BUG: soft lockup - CPU#6 stuck for 23s! [md1_resync:1597]
[ 1761.051701] Modules linked in: bridge stp llc macvlan ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 xt_multiport xt_conntrack nf_conntrack iptable_filter ip_tables [last unloaded: xt_owner]
[ 1761.068794] irq event stamp: 229014772
[ 1761.072564] hardirqs last enabled at (229014772): [<ffffffff8103e492>] vprintk_emit+0x1b2/0x460
[ 1761.081420] hardirqs last disabled at (229014771): [<ffffffff8103e324>] vprintk_emit+0x44/0x460
[ 1761.090191] softirqs last enabled at (229014700): [<ffffffff810444d8>] __do_softirq+0x178/0x2a0
[ 1761.099042] softirqs last disabled at (229014753): [<ffffffff816aa44c>] call_softirq+0x1c/0x30
[ 1761.107729] CPU: 6 PID: 1597 Comm: md1_resync Not tainted 3.10.17 #13
[ 1761.114199] Hardware name: Supermicro X9DRG-HF/X9DRG-HF, BIOS 1.0c 08/22/2012
[ 1761.121368] task: ffff88084f080000 ti: ffff88084f07a000 task.ti: ffff88084f07a000
[ 1761.128908] RIP: 0010:[<ffffffff81322c57>] [<ffffffff81322c57>] delay_tsc+0x37/0x60
[ 1761.136727] RSP: 0018:ffff88107fc03ad0 EFLAGS: 00000206
[ 1761.142073] RAX: 0000000000dc56cb RBX: ffffffff816a065c RCX: 0000000000000001
[ 1761.149250] RDX: 00000000001daa0a RSI: 0000000000dc569f RDI: 0000000000000001
[ 1761.156418] RBP: ffff88107fc03ad0 R08: 0000000000000006 R09: 0000000000000000
[ 1761.163577] R10: 0000000000000002 R11: 0000000000000002 R12: ffff88107fc03a48
[ 1761.170746] R13: ffffffff816a9d2f R14: ffff88107fc03ad0 R15: 000000002a552e2e
[ 1761.177903] FS: 0000000000000000(0000) GS:ffff88107fc00000(0000) knlGS:0000000000000000
[ 1761.186050] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1761.191826] CR2: 0000000001e18000 CR3: 0000000001974000 CR4: 00000000000407e0
[ 1761.199000] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1761.206158] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 1761.213316] Stack:
[ 1761.216583] ffff88107fc03ae0 ffffffff81322b5f ffff88107fc03b10 ffffffff816950a4
[ 1761.224124] 0000000000007465 ffff8810527a8000 ffff8810527a8000 ffff88084e96e000
[ 1761.231666] ffff88107fc03b30 ffffffff8132b344 ffff88104b5c1180 ffff8810527a8000
[ 1761.239214] Call Trace:
[ 1761.241686] <IRQ>
[ 1761.243619] [<ffffffff81322b5f>] __delay+0xf/0x20
[ 1761.248693] [<ffffffff816950a4>] __spin_lock_debug+0x43/0x7b
[ 1761.254474] [<ffffffff8132b344>] do_raw_spin_lock+0x44/0xc0
[ 1761.260158] [<ffffffff8169f929>] _raw_spin_lock+0x39/0x40
[ 1761.265681] [<ffffffffa005b3da>] ? br_stp_rcv+0x8a/0x3f0 [bridge]
[ 1761.271902] [<ffffffffa005b3da>] br_stp_rcv+0x8a/0x3f0 [bridge]
[ 1761.277941] [<ffffffffa000e160>] ? stp_proto_register+0xb0/0xb0 [stp]
[ 1761.284495] [<ffffffff816888e3>] ? printk+0x4d/0x4f
[ 1761.289494] [<ffffffff81044219>] ? local_bh_enable+0x89/0xf0
[ 1761.295275] [<ffffffffa000e160>] ? stp_proto_register+0xb0/0xb0 [stp]
[ 1761.301834] [<ffffffffa000e160>] ? stp_proto_register+0xb0/0xb0 [stp]
[ 1761.308416] [<ffffffffa000e248>] stp_pdu_rcv+0xe8/0xea0 [stp]
[ 1761.314299] [<ffffffffa000e160>] ? stp_proto_register+0xb0/0xb0 [stp]
[ 1761.320868] [<ffffffffa003e661>] llc_rcv+0x371/0x4e0 [llc]
[ 1761.326486] [<ffffffff815ee646>] __netif_receive_skb_core+0x5c6/0x700
[ 1761.333046] [<ffffffff815ee17a>] ? __netif_receive_skb_core+0xfa/0x700
[ 1761.339694] [<ffffffff810a098f>] ? __lock_release+0x6f/0x100
[ 1761.345469] [<ffffffff815f23e1>] __netif_receive_skb+0x21/0x70
[ 1761.351421] [<ffffffff815f25f3>] netif_receive_skb+0x23/0xf0
[ 1761.357203] [<ffffffff8139f220>] ? dma_issue_pending_all+0xb0/0xe0
[ 1761.363495] [<ffffffff815f36e8>] napi_gro_receive+0xe8/0x140
[ 1761.369283] [<ffffffff814ee0f8>] igb_clean_rx_irq+0x148/0x1c0
[ 1761.375143] [<ffffffff814ee1ce>] igb_poll+0x5e/0xa0
[ 1761.380162] [<ffffffff815f2a29>] net_rx_action+0x139/0x250
[ 1761.385768] [<ffffffff81044409>] ? __do_softirq+0xa9/0x2a0
[ 1761.391375] [<ffffffff81044450>] __do_softirq+0xf0/0x2a0
[ 1761.396828] [<ffffffff816aa44c>] call_softirq+0x1c/0x30
[ 1761.402183] [<ffffffff81003e7d>] do_softirq+0x8d/0xc0
[ 1761.407349] [<ffffffff8104479e>] irq_exit+0xae/0xd0
[ 1761.412352] [<ffffffff816aa993>] do_IRQ+0x63/0xe0
[ 1761.417179] [<ffffffff816a05af>] common_interrupt+0x6f/0x6f
[ 1761.422887] <EOI>
[ 1761.424821] [<ffffffff811072e1>] ? get_page_from_freelist+0x241/0x5d0
[ 1761.431610] [<ffffffff811081c2>] __alloc_pages_nodemask+0x182/0xa50
[ 1761.437989] [<ffffffff8109fe61>] ? __lock_acquire+0x3f1/0xa80
[ 1761.443848] [<ffffffff8106fbe3>] ? finish_task_switch+0x83/0xf0
[ 1761.449905] [<ffffffff8109fe61>] ? __lock_acquire+0x3f1/0xa80
[ 1761.455799] [<ffffffff812ffae6>] ? ioc_lookup_icq+0x86/0xf0
[ 1761.461486] [<ffffffff811998c1>] ? bio_alloc_bioset+0x161/0x1c0
[ 1761.467536] [<ffffffff8114239a>] alloc_pages_current+0xba/0x170
[ 1761.473568] [<ffffffff815663a4>] r10buf_pool_alloc+0x174/0x2c0
[ 1761.479531] [<ffffffff8110241e>] mempool_alloc+0x5e/0x160
[ 1761.485044] [<ffffffff815676c6>] sync_request+0x916/0x1180
[ 1761.490650] [<ffffffff81570af1>] ? is_mddev_idle+0x141/0x170
[ 1761.496439] [<ffffffff815709b5>] ? is_mddev_idle+0x5/0x170
[ 1761.502039] [<ffffffff81573c98>] md_do_sync+0x548/0xd10
[ 1761.507385] [<ffffffff8106fbe3>] ? finish_task_switch+0x83/0xf0
[ 1761.513418] [<ffffffff81063b60>] ? __init_waitqueue_head+0x60/0x60
[ 1761.519720] [<ffffffff81570d8d>] md_thread+0x11d/0x170
[ 1761.524972] [<ffffffff8109e91d>] ? trace_hardirqs_on+0xd/0x10
[ 1761.530840] [<ffffffff81570c70>] ? md_register_thread+0xf0/0xf0
[ 1761.536879] [<ffffffff8106304b>] kthread+0xdb/0xe0
[ 1761.541795] [<ffffffff81062f70>] ? __init_kthread_worker+0x70/0x70
[ 1761.548094] [<ffffffff816a8f5c>] ret_from_fork+0x7c/0xb0
[ 1761.553530] [<ffffffff81062f70>] ? __init_kthread_worker+0x70/0x70
[ 1761.559829] Code: 04 25 1c b0 00 00 0f 1f 00 0f ae e8 0f 31 89 c6 eb 11 66 90 f3 90 65 8b 0c 25 1c b0 00 00 41 39 c8 75 12 0f 1f 00 0f ae e8 0f 31 <89> c2 29 f2 39 fa 72 e1 5d c3 29 c6 01 f7 0f 1f 00 0f ae e8 0f
[-- Attachment #3: fix_stp_bridge_uninitialized.patch --]
[-- Type: application/x-download, Size: 477 bytes --]
^ permalink raw reply
* Re: spurious ip forwarding inside namespaces
From: Maciej Żenczykowski @ 2013-11-11 10:01 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Nicolas Dichtel, David S. Miller, Linux NetDev
In-Reply-To: <CAMEtUuz70qyDgB1-EH9xPK71PagxwHFVCzmrnwC2uWum_gxxFg@mail.gmail.com>
I didn't really understand what he was trying to fix (seemed like a
userspace misconfiguration issue to me…), but it seems pretty clear
that this is not the right way to go about it.
Ignoring destination mac address is not the right way to go about doing things.
^ permalink raw reply
* Re: [PATCH v2] mac80211: add assoc beacon timeout logic
From: Johannes Berg @ 2013-11-11 9:08 UTC (permalink / raw)
To: Felipe Contreras
Cc: linux-wireless, netdev, John W. Linville, David S. Miller
In-Reply-To: <1384119945-31213-1-git-send-email-felipe.contreras@gmail.com>
On Sun, 2013-11-10 at 15:45 -0600, Felipe Contreras wrote:
> We don't want to be waiting forever for a beacon that will never come,
> just continue the association.
This makes no sense, IMO.
If there really are no beacons at all, then nothing can work with the AP
-- things like HT, regulatory/radar, powersave, multiple virtual
interfaces and many others require beacons.
If the AP is sending beacons but the device isn't receiving them, then
it's a driver bug and mac80211 shouldn't work around it.
johannes
^ permalink raw reply
* Re: [PATCH] ipv6: match those routes that have different metirc
From: Duan Jiong @ 2013-11-11 8:29 UTC (permalink / raw)
To: hannes; +Cc: David Miller, netdev
In-Reply-To: <20131111073923.GB27030@order.stressinduktion.org>
于 2013年11月11日 15:39, Hannes Frederic Sowa 写道:
> On Mon, Nov 11, 2013 at 03:16:24PM +0800, Duan Jiong wrote:
>> 于 2013年11月11日 14:07, Hannes Frederic Sowa 写道:
>>> On Mon, Nov 11, 2013 at 11:00:53AM +0800, Duan Jiong wrote:
>>>> Now the kernel only match those routes that have the same
>>>> metirc, so if those routes are non-reachable, then the packets
>>>> can't be sent out.
>>>>
>>>> But according to the rfc 4191 section 3.2, if the best route
>>>> points to a non-reachable route, the next best route should be
>>>> consulted. So the kernel should not only match those routes that
>>>> have minimum metric, and should also match others.
>>>
>>> I am not so keen on chaning how metrics get handled. Please note that
>>> metrics are a pure linux-centric concept and are not standardized. I bet
>>> some people depend on how these work and have the same semantic as IPv4
>>> ones because they use the same code in their routing daemon backends.
>>>
>>
>> I have a question. If we only match those routes that have lowest metric, others
>> will never be used, so why the other routes are inserted to the router table?
>
> Metrics could get used by software which manages dynamic interfaces, e.g.
> vpn/ppp software, to make sure they have priority over the current routing
> settings. If we don't respect metrics there, this could lead to security
> problems. (if the interface vanishes, the other route gets active again).
>
> It could also be used by dynamic routing software, e.g. is-is, ospf, bgp and
> does reflect the admins choice where traffic should get routed. Some routing
> daemon apply the whole routing table to the fib. If we don't respect metrics
> there, they could lose money, because maybe they pay for the traffic.
> They really get interesting if you have more than one routing protocol active
> at the same time. ;)
>
Thanks for your reply. I think i don't fully understand the metric, and this
patch should be ignored!
I am sorry that bothers your.
Thanks,
Duan
^ permalink raw reply
* Re: TCP performance regression
From: Sujith Manoharan @ 2013-11-11 8:19 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1384152853.16391.19.camel@edumazet-glaptop2.roam.corp.google.com>
Eric Dumazet wrote:
> Hmm... apparently ath9k uses :
>
> #define ATH_AMPDU_LIMIT_MAX (64 * 1024 - 1)
This is the maximum AMPDU size, specified in the
802.11 standard.
> And mentions a 4ms time frame :
>
> max_4ms_framelen = ATH_AMPDU_LIMIT_MAX;
The 4ms limitation is a FCC limitation and is used
for regulatory compliance.
> So prior to "tcp: TSQ can use a dynamic limit", the ~128KB bytes TCP
> could queue per TCP socket on qdisc/NIC would happen to please ath9k
Ok.
> ath9k can set rts_aggr_limit to 8*1024 :
>
> if (AR_SREV_9160_10_OR_LATER(ah) || AR_SREV_9100(ah))
> pCap->rts_aggr_limit = ATH_AMPDU_LIMIT_MAX;
> else
> pCap->rts_aggr_limit = (8 * 1024);
The RTS limit is required for some old chips which had HW bugs and
the above code is a workaround.
> A timer would be definitely needed, and it should be rather small (1 or
> 2 ms)
>
> If TCP socket is application limited, it seems ath9k can delay the last
> block by a too long time.
I am not really clear on how this regression can be fixed in the driver
since the majority of the transmission/aggregation logic is present in the
TX completion path.
Sujith
^ permalink raw reply
* Re: [PATCH] usbnet: fix race condition caused spinlock bad magic issue
From: Ingo Molnar @ 2013-11-11 8:23 UTC (permalink / raw)
To: wangbiao
Cc: oneukum, netdev, linux-usb, linux-kernel, akpm, mingo,
a.p.zijlstra, rusty, william.douglas
In-Reply-To: <1384139315.2179.9.camel@wangbiao>
* wangbiao <biao.wang@intel.com> wrote:
> @@ -1448,8 +1448,10 @@ static void usbnet_bh (unsigned long param)
>
> // waiting for all pending urbs to complete?
> if (dev->wait) {
> + wait_queue_head_t *wait_d = dev->wait;
> if ((dev->txq.qlen + dev->rxq.qlen + dev->done.qlen) == 0) {
> - wake_up (dev->wait);
> + if (wait_d)
> + wake_up(wait_d);
> }
>
> // or are we maybe short a few urbs?
1)
Nit: the scope of 'wait_d' is unnecessarily broad, it could be moved to
the block that uses it.
2)
Also, the changelog mentions that dev->wait can race - it would be nice to
add to the changelog what exact synchronization mechanism protects
usbnet_terminate_urbs() and usbnet_bh() from seeing/modifying that value
at once - as the code was clearly written without such interaction in
mind.
> @@ -1602,6 +1604,7 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
> init_timer (&dev->delay);
> mutex_init (&dev->phy_mutex);
> mutex_init(&dev->interrupt_mutex);
> + init_waitqueue_head(&unlink_wakeup);
3)
Can that runtime initialization be avoided by using
DECLARE_WAIT_QUEUE_HEAD()?
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH] usbnet: fix race condition caused spinlock bad magic issue
From: Oliver Neukum @ 2013-11-11 7:44 UTC (permalink / raw)
To: wangbiao
Cc: netdev, linux-usb, linux-kernel, akpm, mingo, a.p.zijlstra, rusty,
william.douglas
In-Reply-To: <1384139315.2179.9.camel@wangbiao>
On Mon, 2013-11-11 at 11:08 +0800, wangbiao wrote:
> From: wang, biao <biao.wang@intel.com>
> Date: Mon, 11 Nov 2013 10:23:40 +0800
> Subject: [PATCH] usbnet: fix race condition caused spinlock bad magic issue
>
> there is race between usbnet_terminate_urbs and usbnet_bh, when
> unlink_wakeup used in usbnet_bh, it may be already freed and used by
> other function as unlink_wakeup was a local var on stack.
Hi,
the debugging is good, but the fix is not good.
If you use a global variable, you can init the head
only once, that is on module load, not during probe()
Doing so can mess up operations with multiple modules
as a hotplug happens.
Regards
Oliver
^ permalink raw reply
* Re: [PATCH] ipv6: match those routes that have different metirc
From: Hannes Frederic Sowa @ 2013-11-11 7:39 UTC (permalink / raw)
To: Duan Jiong; +Cc: David Miller, netdev
In-Reply-To: <52808448.3020206@cn.fujitsu.com>
On Mon, Nov 11, 2013 at 03:16:24PM +0800, Duan Jiong wrote:
> 于 2013年11月11日 14:07, Hannes Frederic Sowa 写道:
> > On Mon, Nov 11, 2013 at 11:00:53AM +0800, Duan Jiong wrote:
> >> Now the kernel only match those routes that have the same
> >> metirc, so if those routes are non-reachable, then the packets
> >> can't be sent out.
> >>
> >> But according to the rfc 4191 section 3.2, if the best route
> >> points to a non-reachable route, the next best route should be
> >> consulted. So the kernel should not only match those routes that
> >> have minimum metric, and should also match others.
> >
> > I am not so keen on chaning how metrics get handled. Please note that
> > metrics are a pure linux-centric concept and are not standardized. I bet
> > some people depend on how these work and have the same semantic as IPv4
> > ones because they use the same code in their routing daemon backends.
> >
>
> I have a question. If we only match those routes that have lowest metric, others
> will never be used, so why the other routes are inserted to the router table?
Metrics could get used by software which manages dynamic interfaces, e.g.
vpn/ppp software, to make sure they have priority over the current routing
settings. If we don't respect metrics there, this could lead to security
problems. (if the interface vanishes, the other route gets active again).
It could also be used by dynamic routing software, e.g. is-is, ospf, bgp and
does reflect the admins choice where traffic should get routed. Some routing
daemon apply the whole routing table to the fib. If we don't respect metrics
there, they could lose money, because maybe they pay for the traffic.
They really get interesting if you have more than one routing protocol active
at the same time. ;)
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH] ipv6: match those routes that have different metirc
From: Duan Jiong @ 2013-11-11 7:16 UTC (permalink / raw)
To: hannes; +Cc: David Miller, netdev
In-Reply-To: <20131111060753.GA27030@order.stressinduktion.org>
于 2013年11月11日 14:07, Hannes Frederic Sowa 写道:
> On Mon, Nov 11, 2013 at 11:00:53AM +0800, Duan Jiong wrote:
>> Now the kernel only match those routes that have the same
>> metirc, so if those routes are non-reachable, then the packets
>> can't be sent out.
>>
>> But according to the rfc 4191 section 3.2, if the best route
>> points to a non-reachable route, the next best route should be
>> consulted. So the kernel should not only match those routes that
>> have minimum metric, and should also match others.
>
> I am not so keen on chaning how metrics get handled. Please note that
> metrics are a pure linux-centric concept and are not standardized. I bet
> some people depend on how these work and have the same semantic as IPv4
> ones because they use the same code in their routing daemon backends.
>
I have a question. If we only match those routes that have lowest metric, others
will never be used, so why the other routes are inserted to the router table?
Thanks,
Duan
> Also, I think it is better targetted for net-next, which is currently closed.
> So we have some time to discuss. ;)
>
> Greetings,
>
> Hannes
>
>
^ permalink raw reply
* Re: [PATCH v3 net-next] xfrm: Add check to prevent un-complete key manager
From: Fan Du @ 2013-11-11 7:11 UTC (permalink / raw)
To: baker.kernel; +Cc: davem, herbert, steffen.klassert, netdev, linux-kernel
In-Reply-To: <1384151951-13549-1-git-send-email-baker.kernel@gmail.com>
On 2013年11月11日 14:39, baker.kernel@gmail.com wrote:
> From: Baker Zhang<Baker.kernel@gmail.com>
>
> "acquire" and "compile_policy" callbacks are necessary for a key manager.
>
> Signed-off-by: Baker Zhang<Baker.kernel@gmail.com>
> ---
> Thanks for all reply.
>
> V1:
> For current kernel source, there is no problem.
>
> In our vpn product, we need a xfrm_km in kernel module
> to monitor the xfrm state change.
> thus, the 'acquire' and 'compile_policy' may be NULL.
>
> So I think we should do the check before use it.
>
> V2:
> Align the continuation line according the networking coding style.
>
> V3:
> Add check to prevent un-complete key manager at register time.
>
> net/xfrm/xfrm_state.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index b9c3f9e..178283e 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -1806,6 +1806,9 @@ static DEFINE_SPINLOCK(xfrm_km_lock);
>
> int xfrm_register_km(struct xfrm_mgr *km)
> {
> + if (km->acquire == NULL || km->compile_policy == NULL)
"acquire" is a MUST, "compile_policy" is not a necessity.
From the fist commit log, you probably add functionality providing SA state changes
in your private key manager, which current implementation does not. Maybe it's worthwhile
to elaborate the missing functionality than add those checking, because both key manage
(pfkeyv2/netlink) in use has "acquire" and "compile_policy" at the same time.
--
浮沉随浪只记今朝笑
--fan
^ permalink raw reply
* Re: TCP performance regression
From: Eric Dumazet @ 2013-11-11 6:54 UTC (permalink / raw)
To: Sujith Manoharan; +Cc: netdev
In-Reply-To: <21120.29720.673157.151074@gargle.gargle.HOWL>
On Mon, 2013-11-11 at 11:37 +0530, Sujith Manoharan wrote:
> Eric Dumazet wrote:
> > I am afraid this commit shows bugs in various network drivers.
> >
> > All drivers doing TX completion using a timer are buggy.
> >
> > Holding skb 10 ms before TX completion is totally wrong and must be fixed.
> >
> > If really NIC is not able to trigger an interrupt after TX completion, then
> > driver should call skb_orphan() in its ndo_start_xmit()
>
> 802.11 AMPDU formation is done in the TX completion path in ath9k.
>
> Incoming frames are added to a software queue and the TX completion
> tasklet checks if enough frames are available to form an aggregate and
> if so, forms new aggregates and transmits them.
>
Hmm... apparently ath9k uses :
#define ATH_AMPDU_LIMIT_MAX (64 * 1024 - 1)
And mentions a 4ms time frame :
max_4ms_framelen = ATH_AMPDU_LIMIT_MAX;
So prior to "tcp: TSQ can use a dynamic limit", the ~128KB bytes TCP
could queue per TCP socket on qdisc/NIC would happen to please ath9k
ath9k can set rts_aggr_limit to 8*1024 :
if (AR_SREV_9160_10_OR_LATER(ah) || AR_SREV_9100(ah))
pCap->rts_aggr_limit = ATH_AMPDU_LIMIT_MAX;
else
pCap->rts_aggr_limit = (8 * 1024);
> There is no timer involved, but the completion routine is rather heavy.
> Many wireless drivers handle 802.11 aggregation in this way:
> ath9k, ath9k_htc, ath10k etc.
>
A timer would be definitely needed, and it should be rather small (1 or
2 ms)
If TCP socket is application limited, it seems ath9k can delay the last
block by a too long time.
^ permalink raw reply
* Re: spurious ip forwarding inside namespaces
From: Alexei Starovoitov @ 2013-11-11 6:51 UTC (permalink / raw)
To: Maciej Żenczykowski; +Cc: Nicolas Dichtel, David S. Miller, Linux NetDev
In-Reply-To: <CAHo-Oow6h1LRNw5DMb4+6bdwy31WGOTNokKe=C5cj40dDhTyQQ@mail.gmail.com>
On Sun, Nov 10, 2013 at 9:12 PM, Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
> I've already suggested a month or so back that that patch should be reverted.
Isaku's email (author of offending patch) was bouncing whole week.
Didn't have time last week to come up with a good fix.
Reverting fixes 'spurious ip forwarding', but Isaku's original problem
needs to be fixed somehow.
May be going through loopback should set pkt_type?
Alexei
> On Tue, Nov 5, 2013 at 8:32 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>> Hi All,
>>
>> commit 06a23fe31c
>> "core/dev: set pkt_type after eth_type_trans() in dev_forward_skb()"
>> and refactoring 64261f230a
>> "dev: move skb_scrub_packet() after eth_type_trans()"
>>
>> are forcing pkt_type to be PACKET_HOST when skb traverses veth.
>>
>> which means that ip forwarding will kick in inside netns
>> even if skb->eth->h_dest != dev->dev_addr
>>
>> The way we hit the problem is the following:
>> create 4 or more of netns with veth and attach them to a bridge.
>> disable learning on a bridge (via setageing 0), so it will flood
>> packets to all namespaces.
>> Since ip forwarding and icmp redirect by default is on,
>> single icmp request packet from one netns creates a storm of packets,
>> since every netns receives it and incorrectly does ip forward on it
>> and also incorrectly sends icmp redirect which are multiplied once
>> again by flooding in the bridge.
>> ping -c1 generates 1 outgoing packet and ~13K incoming packets with
>> just 4 netns.
>> Running any tcp traffic between such namespaces becomes impossible
>> and ksoftirqd cpu usage goes to 100% on all cpus.
>>
>> Please suggest how to fix this cleanly.
>>
>> Thanks
>> Alexei
>> --
>> 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: [PATCH v2] mac80211: add assoc beacon timeout logic
From: Krishna Chaitanya @ 2013-11-11 6:41 UTC (permalink / raw)
To: Felipe Contreras
Cc: linux-wireless, netdev, John W. Linville, Johannes Berg,
David S. Miller
In-Reply-To: <1384119945-31213-1-git-send-email-felipe.contreras-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Mon, Nov 11, 2013 at 3:15 AM, Felipe Contreras
<felipe.contreras-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> We don't want to be waiting forever for a beacon that will never come,
> just continue the association.
>
> Signed-off-by: Felipe Contreras <felipe.contreras-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>
> The previous version depended on some cleanup patches, plus had some unclear
> rerun logic.
>
> net/mac80211/ieee80211_i.h | 1 +
> net/mac80211/mlme.c | 32 ++++++++++++++++++++++++++------
> 2 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index 611abfc..e1f858d 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -358,6 +358,7 @@ struct ieee80211_mgd_assoc_data {
> const u8 *supp_rates;
>
> unsigned long timeout;
> + unsigned long beacon_timeout;
> int tries;
>
> u16 capability;
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 86e4ad5..68f76f7 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -38,6 +38,7 @@
> #define IEEE80211_ASSOC_TIMEOUT (HZ / 5)
> #define IEEE80211_ASSOC_TIMEOUT_LONG (HZ / 2)
> #define IEEE80211_ASSOC_TIMEOUT_SHORT (HZ / 10)
> +#define IEEE80211_ASSOC_BEACON_TIMEOUT 2 * HZ
Don't you think its too long? 3-4 beacon intervals should suffice.
> #define IEEE80211_ASSOC_MAX_TRIES 3
>
> static int max_nullfunc_tries = 2;
> @@ -3475,6 +3476,24 @@ void ieee80211_mgd_conn_tx_status(struct ieee80211_sub_if_data *sdata,
> ieee80211_queue_work(&local->hw, &sdata->work);
> }
>
> +static int check_beacon(struct ieee80211_sub_if_data *sdata)
> +{
> + struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
> + struct ieee80211_mgd_assoc_data *assoc_data = ifmgd->assoc_data;
> +
> + if (!assoc_data->need_beacon || ifmgd->have_beacon)
> + return true;
> +
> + if (time_after(jiffies, assoc_data->beacon_timeout)) {
> + sdata_info(sdata, "no beacon from %pM\n", assoc_data->bss->bssid);
> + return true;
> + }
> +
> + assoc_data->timeout = TU_TO_EXP_TIME(assoc_data->bss->beacon_interval);
> + run_again(sdata, assoc_data->timeout);
> + return false;
> +}
> +
> void ieee80211_sta_work(struct ieee80211_sub_if_data *sdata)
> {
> struct ieee80211_local *local = sdata->local;
> @@ -3533,12 +3552,12 @@ void ieee80211_sta_work(struct ieee80211_sub_if_data *sdata)
>
> if (ifmgd->assoc_data && ifmgd->assoc_data->timeout_started &&
> time_after(jiffies, ifmgd->assoc_data->timeout)) {
> - if ((ifmgd->assoc_data->need_beacon && !ifmgd->have_beacon) ||
> - ieee80211_do_assoc(sdata)) {
> - struct cfg80211_bss *bss = ifmgd->assoc_data->bss;
> -
> - ieee80211_destroy_assoc_data(sdata, false);
> - cfg80211_assoc_timeout(sdata->dev, bss);
> + if (check_beacon(sdata)) {
> + if (ieee80211_do_assoc(sdata)) {
> + struct cfg80211_bss *bss = ifmgd->assoc_data->bss;
> + ieee80211_destroy_assoc_data(sdata, false);
> + cfg80211_assoc_timeout(sdata->dev, bss);
> + }
> }
> } else if (ifmgd->assoc_data && ifmgd->assoc_data->timeout_started)
> run_again(sdata, ifmgd->assoc_data->timeout);
> @@ -4335,6 +4354,7 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data *sdata,
> sdata_info(sdata, "waiting for beacon from %pM\n",
> ifmgd->bssid);
> assoc_data->timeout = TU_TO_EXP_TIME(req->bss->beacon_interval);
> + assoc_data->beacon_timeout = jiffies + IEEE80211_ASSOC_BEACON_TIMEOUT;
> assoc_data->timeout_started = true;
> assoc_data->need_beacon = true;
> } else if (beacon_ies) {
> --
> 1.8.4.2+fc1
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3 net-next] xfrm: Add check to prevent un-complete key manager
From: baker.kernel @ 2013-11-11 6:39 UTC (permalink / raw)
To: davem, herbert, steffen.klassert; +Cc: netdev, linux-kernel, Baker Zhang
In-Reply-To: <1384093556-8987-1-git-send-email-baker.kernel@gmail.com>
From: Baker Zhang <Baker.kernel@gmail.com>
"acquire" and "compile_policy" callbacks are necessary for a key manager.
Signed-off-by: Baker Zhang <Baker.kernel@gmail.com>
---
Thanks for all reply.
V1:
For current kernel source, there is no problem.
In our vpn product, we need a xfrm_km in kernel module
to monitor the xfrm state change.
thus, the 'acquire' and 'compile_policy' may be NULL.
So I think we should do the check before use it.
V2:
Align the continuation line according the networking coding style.
V3:
Add check to prevent un-complete key manager at register time.
net/xfrm/xfrm_state.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index b9c3f9e..178283e 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1806,6 +1806,9 @@ static DEFINE_SPINLOCK(xfrm_km_lock);
int xfrm_register_km(struct xfrm_mgr *km)
{
+ if (km->acquire == NULL || km->compile_policy == NULL)
+ return -EINVAL;
+
spin_lock_bh(&xfrm_km_lock);
list_add_tail_rcu(&km->list, &xfrm_km_list);
spin_unlock_bh(&xfrm_km_lock);
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH] ipv6: protect for_each_sk_fl_rcu in mem_check with rcu_read_lock_bh
From: David Miller @ 2013-11-11 6:26 UTC (permalink / raw)
To: hannes; +Cc: netdev, florent.fourcot
In-Reply-To: <20131108182621.GA404@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Fri, 8 Nov 2013 19:26:21 +0100
> Fixes a suspicious rcu derference warning.
>
> Cc: Florent Fourcot <florent.fourcot@enst-bretagne.fr>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Looks good, applied and queued up for -stable.
^ permalink raw reply
* [PATCH] xtsonic: add missing platform_set_drvdata() in xtsonic_probe()
From: Wei Yongjun @ 2013-11-11 6:17 UTC (permalink / raw)
To: davem, wfp5p, gregkh, michael.opdenacker, joe; +Cc: yongjun_wei, netdev
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Add missing platform_set_drvdata() in xtsonic_probe(), otherwise
calling platform_get_drvdata() in xtsonic_device_remove() may
returns NULL.
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
drivers/net/ethernet/natsemi/xtsonic.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/natsemi/xtsonic.c b/drivers/net/ethernet/natsemi/xtsonic.c
index 4da172a..7007d21 100644
--- a/drivers/net/ethernet/natsemi/xtsonic.c
+++ b/drivers/net/ethernet/natsemi/xtsonic.c
@@ -264,6 +264,7 @@ int xtsonic_probe(struct platform_device *pdev)
lp = netdev_priv(dev);
lp->device = &pdev->dev;
+ platform_set_drvdata(pdev, dev);
SET_NETDEV_DEV(dev, &pdev->dev);
netdev_boot_setup_check(dev);
^ permalink raw reply related
* [PATCH] macmace: add missing platform_set_drvdata() in mace_probe()
From: Wei Yongjun @ 2013-11-11 6:16 UTC (permalink / raw)
To: davem, wfp5p, gregkh, joe; +Cc: yongjun_wei, netdev
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Add missing platform_set_drvdata() in mace_probe(), otherwise
calling platform_get_drvdata() in mac_mace_device_remove() may
returns NULL.
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
drivers/net/ethernet/apple/macmace.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/apple/macmace.c b/drivers/net/ethernet/apple/macmace.c
index 4ce8ceb..58a200d 100644
--- a/drivers/net/ethernet/apple/macmace.c
+++ b/drivers/net/ethernet/apple/macmace.c
@@ -211,6 +211,7 @@ static int mace_probe(struct platform_device *pdev)
mp = netdev_priv(dev);
mp->device = &pdev->dev;
+ platform_set_drvdata(pdev, dev);
SET_NETDEV_DEV(dev, &pdev->dev);
dev->base_addr = (u32)MACE_BASE;
^ 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