* [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-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
* 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 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
* 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: 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: 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
* [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
* [PATCH net-next 1/6] random32: fix off-by-one in seeding requirement
From: Daniel Borkmann @ 2013-11-11 11:20 UTC (permalink / raw)
To: davem
Cc: shemminger, fweimer, netdev, Stephen Hemminger, Theodore Ts'o,
Hannes Frederic Sowa
In-Reply-To: <cover.1384160397.git.dborkman@redhat.com>
For properly initialising the Tausworthe generator [1], we have
a strict seeding requirement, that is, s1 > 1, s2 > 7, s3 > 15.
Commit 697f8d0348 ("random32: seeding improvement") introduced
a __seed() function that imposes boundary checks proposed by the
errata paper [2] to properly ensure above conditions.
However, we're off by one, as the function is implemented as:
"return (x < m) ? x + m : x;", and called with __seed(X, 1),
__seed(X, 7), __seed(X, 15). Thus, an unwanted seed of 1, 7, 15
would be possible, whereas the lower boundary should actually
be of at least 2, 8, 16, just as GSL does. Fix this, as otherwise
an initialization with an unwanted seed could have the effect
that Tausworthe's PRNG properties cannot not be ensured.
Note that this PRNG is *not* used for cryptography in the kernel.
[1] http://www.iro.umontreal.ca/~lecuyer/myftp/papers/tausme.ps
[2] http://www.iro.umontreal.ca/~lecuyer/myftp/papers/tausme2.ps
Joint work with Hannes Frederic Sowa.
Fixes: 697f8d0348a6 ("random32: seeding improvement")
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: Florian Weimer <fweimer@redhat.com>
Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
include/linux/random.h | 6 +++---
lib/random32.c | 14 +++++++-------
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/linux/random.h b/include/linux/random.h
index 6312dd9..bf9085e 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -50,9 +50,9 @@ static inline void prandom_seed_state(struct rnd_state *state, u64 seed)
{
u32 i = (seed >> 32) ^ (seed << 10) ^ seed;
- state->s1 = __seed(i, 1);
- state->s2 = __seed(i, 7);
- state->s3 = __seed(i, 15);
+ state->s1 = __seed(i, 2);
+ state->s2 = __seed(i, 8);
+ state->s3 = __seed(i, 16);
}
#ifdef CONFIG_ARCH_RANDOM
diff --git a/lib/random32.c b/lib/random32.c
index 52280d5..01e8890 100644
--- a/lib/random32.c
+++ b/lib/random32.c
@@ -141,7 +141,7 @@ void prandom_seed(u32 entropy)
*/
for_each_possible_cpu (i) {
struct rnd_state *state = &per_cpu(net_rand_state, i);
- state->s1 = __seed(state->s1 ^ entropy, 1);
+ state->s1 = __seed(state->s1 ^ entropy, 2);
}
}
EXPORT_SYMBOL(prandom_seed);
@@ -158,9 +158,9 @@ static int __init prandom_init(void)
struct rnd_state *state = &per_cpu(net_rand_state,i);
#define LCG(x) ((x) * 69069) /* super-duper LCG */
- state->s1 = __seed(LCG(i + jiffies), 1);
- state->s2 = __seed(LCG(state->s1), 7);
- state->s3 = __seed(LCG(state->s2), 15);
+ state->s1 = __seed(LCG(i + jiffies), 2);
+ state->s2 = __seed(LCG(state->s1), 8);
+ state->s3 = __seed(LCG(state->s2), 16);
/* "warm it up" */
prandom_u32_state(state);
@@ -187,9 +187,9 @@ static int __init prandom_reseed(void)
u32 seeds[3];
get_random_bytes(&seeds, sizeof(seeds));
- state->s1 = __seed(seeds[0], 1);
- state->s2 = __seed(seeds[1], 7);
- state->s3 = __seed(seeds[2], 15);
+ state->s1 = __seed(seeds[0], 2);
+ state->s2 = __seed(seeds[1], 8);
+ state->s3 = __seed(seeds[2], 16);
/* mix it in */
prandom_u32_state(state);
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 2/6] random32: add periodic reseeding
From: Daniel Borkmann @ 2013-11-11 11:20 UTC (permalink / raw)
To: davem
Cc: shemminger, fweimer, netdev, Hannes Frederic Sowa, Eric Dumazet,
Theodore Ts'o
In-Reply-To: <cover.1384160397.git.dborkman@redhat.com>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
The current Tausworthe PRNG is never reseeded with truly random data after
the first attempt in late_initcall. As this PRNG is used for some critical
random data as e.g. UDP port randomization we should try better and reseed
the PRNG once in a while with truly random data from get_random_bytes().
When we reseed with prandom_seed we now make also sure to throw the first
output away. This suffices the reseeding procedure.
The delay calculation is based on a proposal from Eric Dumazet.
Joint work with Daniel Borkmann.
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
lib/random32.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/lib/random32.c b/lib/random32.c
index 01e8890..165d0a5f 100644
--- a/lib/random32.c
+++ b/lib/random32.c
@@ -142,6 +142,7 @@ void prandom_seed(u32 entropy)
for_each_possible_cpu (i) {
struct rnd_state *state = &per_cpu(net_rand_state, i);
state->s1 = __seed(state->s1 ^ entropy, 2);
+ prandom_u32_state(state);
}
}
EXPORT_SYMBOL(prandom_seed);
@@ -174,6 +175,27 @@ static int __init prandom_init(void)
}
core_initcall(prandom_init);
+static void __prandom_timer(unsigned long dontcare);
+static DEFINE_TIMER(seed_timer, __prandom_timer, 0, 0);
+
+static void __prandom_timer(unsigned long dontcare)
+{
+ u32 entropy;
+
+ get_random_bytes(&entropy, sizeof(entropy));
+ prandom_seed(entropy);
+ /* reseed every ~60 seconds, in [40 .. 80) interval with slack */
+ seed_timer.expires = jiffies + (40 * HZ + (prandom_u32() % (40 * HZ)));
+ add_timer(&seed_timer);
+}
+
+static void prandom_start_seed_timer(void)
+{
+ set_timer_slack(&seed_timer, HZ);
+ seed_timer.expires = jiffies + 40 * HZ;
+ add_timer(&seed_timer);
+}
+
/*
* Generate better values after random number generator
* is fully initialized.
@@ -194,6 +216,7 @@ static int __init prandom_reseed(void)
/* mix it in */
prandom_u32_state(state);
}
+ prandom_start_seed_timer();
return 0;
}
late_initcall(prandom_reseed);
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 3/6] random32: add prandom_reseed_late() and call when nonblocking pool becomes initialized
From: Daniel Borkmann @ 2013-11-11 11:20 UTC (permalink / raw)
To: davem
Cc: shemminger, fweimer, netdev, Hannes Frederic Sowa, Eric Dumazet,
Theodore Ts'o
In-Reply-To: <cover.1384160397.git.dborkman@redhat.com>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
The Tausworthe PRNG is initialized at late_initcall time. At that time the
entropy pool serving get_random_bytes is not filled sufficiently. This
patch adds an additional reseeding step as soon as the nonblocking pool
gets marked as initialized.
On some machines it might be possible that late_initcall gets called after
the pool has been initialized. In this situation we won't reseed again.
(A call to prandom_seed_late blocks later invocations of early reseed
attempts.)
Joint work with Daniel Borkmann.
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
drivers/char/random.c | 5 ++++-
include/linux/random.h | 1 +
lib/random32.c | 23 ++++++++++++++++++++++-
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 7a744d3..4fe5609 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -603,8 +603,11 @@ retry:
if (!r->initialized && nbits > 0) {
r->entropy_total += nbits;
- if (r->entropy_total > 128)
+ if (r->entropy_total > 128) {
r->initialized = 1;
+ if (r == &nonblocking_pool)
+ prandom_reseed_late();
+ }
}
trace_credit_entropy_bits(r->name, nbits, entropy_count,
diff --git a/include/linux/random.h b/include/linux/random.h
index bf9085e..5117ae3 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -29,6 +29,7 @@ unsigned long randomize_range(unsigned long start, unsigned long end, unsigned l
u32 prandom_u32(void);
void prandom_bytes(void *buf, int nbytes);
void prandom_seed(u32 seed);
+void prandom_reseed_late(void);
u32 prandom_u32_state(struct rnd_state *);
void prandom_bytes_state(struct rnd_state *state, void *buf, int nbytes);
diff --git a/lib/random32.c b/lib/random32.c
index 165d0a5f..e048416 100644
--- a/lib/random32.c
+++ b/lib/random32.c
@@ -200,9 +200,18 @@ static void prandom_start_seed_timer(void)
* Generate better values after random number generator
* is fully initialized.
*/
-static int __init prandom_reseed(void)
+static void __prandom_reseed(bool late)
{
int i;
+ unsigned long flags;
+ static bool latch = false;
+ static DEFINE_SPINLOCK(lock);
+
+ /* only allow initial seeding (late == false) once */
+ spin_lock_irqsave(&lock, flags);
+ if (latch && !late)
+ goto out;
+ latch = true;
for_each_possible_cpu(i) {
struct rnd_state *state = &per_cpu(net_rand_state,i);
@@ -216,6 +225,18 @@ static int __init prandom_reseed(void)
/* mix it in */
prandom_u32_state(state);
}
+out:
+ spin_unlock_irqrestore(&lock, flags);
+}
+
+void prandom_reseed_late(void)
+{
+ __prandom_reseed(true);
+}
+
+static int __init prandom_reseed(void)
+{
+ __prandom_reseed(false);
prandom_start_seed_timer();
return 0;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 5/6] random32: upgrade taus88 generator to taus113 from errata paper
From: Daniel Borkmann @ 2013-11-11 11:20 UTC (permalink / raw)
To: davem; +Cc: shemminger, fweimer, netdev, Theodore Ts'o,
Hannes Frederic Sowa
In-Reply-To: <cover.1384160397.git.dborkman@redhat.com>
Since we use prandom*() functions quite often in networking code
i.e. in UDP port selection, netfilter code, etc, upgrade the PRNG
from Pierre L'Ecuyer's original paper "Maximally Equidistributed
Combined Tausworthe Generators", Mathematics of Computation, 65,
213 (1996), 203--213 to the version published in his errata paper [1].
The Tausworthe generator is a maximally-equidistributed generator,
that is fast and has good statistical properties [1].
The version presented there upgrades the 3 state LFSR to a 4 state
LFSR with increased periodicity from about 2^88 to 2^113. The
algorithm is presented in [1] by the very same author who also
designed the original algorithm in [2].
Also, by increasing the state, we make it a bit harder for attackers
to "guess" the PRNGs internal state. See also discussion in [3].
Now, as we use this sort of weak initialization discussed in [3]
only between core_initcall() until late_initcall() time [*] for
prandom32*() users, namely in prandom_init(), it is less relevant
from late_initcall() onwards as we overwrite seeds through
prandom_reseed() anyways with a seed source of higher entropy, that
is, get_random_bytes(). In other words, a exhaustive keysearch of
96 bit would be needed. Now, with the help of this patch, this
state-search increases further to 128 bit. Initialization needs
to make sure that s1 > 1, s2 > 7, s3 > 15, s4 > 127.
taus88 and taus113 algorithm is also part of GSL. I added a test
case in the next patch to verify internal behaviour of this patch
with GSL and ran tests with the dieharder 3.31.1 RNG test suite:
$ dieharder -g 052 -a -m 10 -s 1 -S 4137730333 #taus88
$ dieharder -g 054 -a -m 10 -s 1 -S 4137730333 #taus113
With this seed configuration, in order to compare both, we get
the following differences:
algorithm taus88 taus113
rands/second [**] 1.61e+08 1.37e+08
sts_serial(4, 1st run) WEAK PASSED
sts_serial(9, 2nd run) WEAK PASSED
rgb_lagged_sum(31) WEAK PASSED
We took out diehard_sums test as according to the authors it is
considered broken and unusable [4]. Despite that and the slight
decrease in performance (which is acceptable), taus113 here passes
all 113 tests (only rgb_minimum_distance_5 in WEAK, the rest PASSED).
In general, taus/taus113 is considered "very good" by the authors
of dieharder [5].
The papers [1][2] states a single warm-up step is sufficient by
running quicktaus once on each state to ensure proper initialization
of ~s_{0}:
Our selection of (s) according to Table 1 of [1] row 1 holds the
condition L - k <= r - s, that is,
(32 32 32 32) - (31 29 28 25) <= (25 27 15 22) - (18 2 7 13)
with r = k - q and q = (6 2 13 3) as also stated by the paper.
So according to [2] we are safe with one round of quicktaus for
initialization. However we decided to include the warm-up phase
of the PRNG as done in GSL in every case as a safety net. We also
use the warm up phase to make the output of the RNG easier to
verify by the GSL output.
In prandom_init(), we also mix random_get_entropy() into it, just
like drivers/char/random.c does it, jiffies ^ random_get_entropy().
random-get_entropy() is get_cycles(). xor is entropy preserving so
it is fine if it is not implemented by some architectures.
Note, this PRNG is *not* used for cryptography in the kernel, but
rather as a fast PRNG for various randomizations i.e. in the
networking code, or elsewhere for debugging purposes, for example.
[*]: In order to generate some "sort of pseduo-randomness", since
get_random_bytes() is not yet available for us, we use jiffies and
initialize states s1 - s3 with a simple linear congruential generator
(LCG), that is x <- x * 69069; and derive s2, s3, from the 32bit
initialization from s1. So the above quote from [3] accounts only
for the time from core to late initcall, not afterwards.
[**] Single threaded run on MacBook Air w/ Intel Core i5-3317U
[1] http://www.iro.umontreal.ca/~lecuyer/myftp/papers/tausme2.ps
[2] http://www.iro.umontreal.ca/~lecuyer/myftp/papers/tausme.ps
[3] http://thread.gmane.org/gmane.comp.encryption.general/12103/
[4] http://code.google.com/p/dieharder/source/browse/trunk/libdieharder/diehard_sums.c?spec=svn490&r=490#20
[5] http://www.phy.duke.edu/~rgb/General/dieharder.php
Joint work with Hannes Frederic Sowa.
Cc: Florian Weimer <fweimer@redhat.com>
Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
include/linux/random.h | 11 +++----
lib/random32.c | 80 +++++++++++++++++++++++++++++---------------------
2 files changed, 52 insertions(+), 39 deletions(-)
diff --git a/include/linux/random.h b/include/linux/random.h
index 8ef0b70..4002b3d 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -32,10 +32,10 @@ void prandom_seed(u32 seed);
void prandom_reseed_late(void);
struct rnd_state {
- __u32 s1, s2, s3;
+ __u32 s1, s2, s3, s4;
};
-u32 prandom_u32_state(struct rnd_state *);
+u32 prandom_u32_state(struct rnd_state *state);
void prandom_bytes_state(struct rnd_state *state, void *buf, int nbytes);
/*
@@ -55,9 +55,10 @@ static inline void prandom_seed_state(struct rnd_state *state, u64 seed)
{
u32 i = (seed >> 32) ^ (seed << 10) ^ seed;
- state->s1 = __seed(i, 2);
- state->s2 = __seed(i, 8);
- state->s3 = __seed(i, 16);
+ state->s1 = __seed(i, 2U);
+ state->s2 = __seed(i, 8U);
+ state->s3 = __seed(i, 16U);
+ state->s4 = __seed(i, 128U);
}
#ifdef CONFIG_ARCH_RANDOM
diff --git a/lib/random32.c b/lib/random32.c
index e048416..7b8ed69 100644
--- a/lib/random32.c
+++ b/lib/random32.c
@@ -2,19 +2,19 @@
This is a maximally equidistributed combined Tausworthe generator
based on code from GNU Scientific Library 1.5 (30 Jun 2004)
- x_n = (s1_n ^ s2_n ^ s3_n)
+ lfsr113 version:
- s1_{n+1} = (((s1_n & 4294967294) <<12) ^ (((s1_n <<13) ^ s1_n) >>19))
- s2_{n+1} = (((s2_n & 4294967288) << 4) ^ (((s2_n << 2) ^ s2_n) >>25))
- s3_{n+1} = (((s3_n & 4294967280) <<17) ^ (((s3_n << 3) ^ s3_n) >>11))
+ x_n = (s1_n ^ s2_n ^ s3_n ^ s4_n)
- The period of this generator is about 2^88.
+ s1_{n+1} = (((s1_n & 4294967294) << 18) ^ (((s1_n << 6) ^ s1_n) >> 13))
+ s2_{n+1} = (((s2_n & 4294967288) << 2) ^ (((s2_n << 2) ^ s2_n) >> 27))
+ s3_{n+1} = (((s3_n & 4294967280) << 7) ^ (((s3_n << 13) ^ s3_n) >> 21))
+ s4_{n+1} = (((s4_n & 4294967168) << 13) ^ (((s4_n << 3) ^ s4_n) >> 12))
- From: P. L'Ecuyer, "Maximally Equidistributed Combined Tausworthe
- Generators", Mathematics of Computation, 65, 213 (1996), 203--213.
-
- This is available on the net from L'Ecuyer's home page,
+ The period of this generator is about 2^113 (see erratum paper).
+ From: P. L'Ecuyer, "Maximally Equidistributed Combined Tausworthe
+ Generators", Mathematics of Computation, 65, 213 (1996), 203--213:
http://www.iro.umontreal.ca/~lecuyer/myftp/papers/tausme.ps
ftp://ftp.iro.umontreal.ca/pub/simulation/lecuyer/papers/tausme.ps
@@ -29,7 +29,7 @@
that paper.)
This affects the seeding procedure by imposing the requirement
- s1 > 1, s2 > 7, s3 > 15.
+ s1 > 1, s2 > 7, s3 > 15, s4 > 127.
*/
@@ -52,11 +52,12 @@ u32 prandom_u32_state(struct rnd_state *state)
{
#define TAUSWORTHE(s,a,b,c,d) ((s&c)<<d) ^ (((s <<a) ^ s)>>b)
- state->s1 = TAUSWORTHE(state->s1, 13, 19, 4294967294UL, 12);
- state->s2 = TAUSWORTHE(state->s2, 2, 25, 4294967288UL, 4);
- state->s3 = TAUSWORTHE(state->s3, 3, 11, 4294967280UL, 17);
+ state->s1 = TAUSWORTHE(state->s1, 6U, 13U, 4294967294U, 18U);
+ state->s2 = TAUSWORTHE(state->s2, 2U, 27U, 4294967288U, 2U);
+ state->s3 = TAUSWORTHE(state->s3, 13U, 21U, 4294967280U, 7U);
+ state->s4 = TAUSWORTHE(state->s4, 3U, 12U, 4294967168U, 13U);
- return (state->s1 ^ state->s2 ^ state->s3);
+ return (state->s1 ^ state->s2 ^ state->s3 ^ state->s4);
}
EXPORT_SYMBOL(prandom_u32_state);
@@ -126,6 +127,21 @@ void prandom_bytes(void *buf, int bytes)
}
EXPORT_SYMBOL(prandom_bytes);
+static void prandom_warmup(struct rnd_state *state)
+{
+ /* Calling RNG ten times to satify recurrence condition */
+ prandom_u32_state(state);
+ prandom_u32_state(state);
+ prandom_u32_state(state);
+ prandom_u32_state(state);
+ prandom_u32_state(state);
+ prandom_u32_state(state);
+ prandom_u32_state(state);
+ prandom_u32_state(state);
+ prandom_u32_state(state);
+ prandom_u32_state(state);
+}
+
/**
* prandom_seed - add entropy to pseudo random number generator
* @seed: seed value
@@ -141,8 +157,9 @@ void prandom_seed(u32 entropy)
*/
for_each_possible_cpu (i) {
struct rnd_state *state = &per_cpu(net_rand_state, i);
- state->s1 = __seed(state->s1 ^ entropy, 2);
- prandom_u32_state(state);
+
+ state->s1 = __seed(state->s1 ^ entropy, 2U);
+ prandom_warmup(state);
}
}
EXPORT_SYMBOL(prandom_seed);
@@ -158,18 +175,13 @@ static int __init prandom_init(void)
for_each_possible_cpu(i) {
struct rnd_state *state = &per_cpu(net_rand_state,i);
-#define LCG(x) ((x) * 69069) /* super-duper LCG */
- state->s1 = __seed(LCG(i + jiffies), 2);
- state->s2 = __seed(LCG(state->s1), 8);
- state->s3 = __seed(LCG(state->s2), 16);
-
- /* "warm it up" */
- prandom_u32_state(state);
- prandom_u32_state(state);
- prandom_u32_state(state);
- prandom_u32_state(state);
- prandom_u32_state(state);
- prandom_u32_state(state);
+#define LCG(x) ((x) * 69069U) /* super-duper LCG */
+ state->s1 = __seed(LCG((i + jiffies) ^ random_get_entropy()), 2U);
+ state->s2 = __seed(LCG(state->s1), 8U);
+ state->s3 = __seed(LCG(state->s2), 16U);
+ state->s4 = __seed(LCG(state->s3), 128U);
+
+ prandom_warmup(state);
}
return 0;
}
@@ -215,15 +227,15 @@ static void __prandom_reseed(bool late)
for_each_possible_cpu(i) {
struct rnd_state *state = &per_cpu(net_rand_state,i);
- u32 seeds[3];
+ u32 seeds[4];
get_random_bytes(&seeds, sizeof(seeds));
- state->s1 = __seed(seeds[0], 2);
- state->s2 = __seed(seeds[1], 8);
- state->s3 = __seed(seeds[2], 16);
+ state->s1 = __seed(seeds[0], 2U);
+ state->s2 = __seed(seeds[1], 8U);
+ state->s3 = __seed(seeds[2], 16U);
+ state->s4 = __seed(seeds[3], 128U);
- /* mix it in */
- prandom_u32_state(state);
+ prandom_warmup(state);
}
out:
spin_unlock_irqrestore(&lock, flags);
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 6/6] random32: add test cases for taus113 implementation
From: Daniel Borkmann @ 2013-11-11 11:20 UTC (permalink / raw)
To: davem; +Cc: shemminger, fweimer, netdev, Theodore Ts'o,
Hannes Frederic Sowa
In-Reply-To: <cover.1384160397.git.dborkman@redhat.com>
We generated a battery of 100 test cases from GSL taus113 implemention
and compare the results from a particular seed and a particular
iteration with our implementation in the kernel. We have verified on
32 and 64 bit machines that our taus113 kernel implementation gives
same results as GSL taus113 implementation:
[ 0.147370] prandom: seed boundary self test passed
[ 0.148078] prandom: 100 self tests passed
This is a Kconfig option that is disabled on default, just like the
crc32 init selftests in order to not unnecessary slow down boot process.
We also refactored out prandom_seed_very_weak() as it's now used in
multiple places in order to reduce redundant code.
GSL code we used for generating test cases:
int i, j;
srand(time(NULL));
for (i = 0; i < 100; ++i) {
int iteration = 500 + (rand() % 500);
gsl_rng_default_seed = rand() + 1;
gsl_rng *r = gsl_rng_alloc(gsl_rng_taus113);
printf("\t{ %lu, ", gsl_rng_default_seed);
for (j = 0; j < iteration - 1; ++j)
gsl_rng_get(r);
printf("%u, %lu },\n", iteration, gsl_rng_get(r));
gsl_rng_free(r);
}
Joint work with Hannes Frederic Sowa.
Cc: Florian Weimer <fweimer@redhat.com>
Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
lib/Kconfig | 7 +++
lib/random32.c | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 196 insertions(+), 6 deletions(-)
diff --git a/lib/Kconfig b/lib/Kconfig
index b3c8be0..75485e1 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -189,6 +189,13 @@ config AUDIT_GENERIC
depends on AUDIT && !AUDIT_ARCH
default y
+config RANDOM32_SELFTEST
+ bool "PRNG perform self test on init"
+ default n
+ help
+ This option enables the 32 bit PRNG library functions to perform a
+ self test on initialization.
+
#
# compression support is select'ed if needed
#
diff --git a/lib/random32.c b/lib/random32.c
index 7b8ed69..bc2016a 100644
--- a/lib/random32.c
+++ b/lib/random32.c
@@ -38,6 +38,11 @@
#include <linux/export.h>
#include <linux/jiffies.h>
#include <linux/random.h>
+#include <linux/sched.h>
+
+#ifdef CONFIG_RANDOM32_SELFTEST
+static void __init prandom_state_selftest(void);
+#endif
static DEFINE_PER_CPU(struct rnd_state, net_rand_state);
@@ -142,6 +147,23 @@ static void prandom_warmup(struct rnd_state *state)
prandom_u32_state(state);
}
+static void prandom_seed_very_weak(struct rnd_state *state, u32 seed)
+{
+ /* Note: This sort of seeding is ONLY used in test cases and
+ * during boot at the time from core_initcall until late_initcall
+ * as we don't have a stronger entropy source available yet.
+ * After late_initcall, we reseed entire state, we have to (!),
+ * otherwise an attacker just needs to search 32 bit space to
+ * probe for our internal 128 bit state if he knows a couple
+ * of prandom32 outputs!
+ */
+#define LCG(x) ((x) * 69069U) /* super-duper LCG */
+ state->s1 = __seed(LCG(seed), 2U);
+ state->s2 = __seed(LCG(state->s1), 8U);
+ state->s3 = __seed(LCG(state->s2), 16U);
+ state->s4 = __seed(LCG(state->s3), 128U);
+}
+
/**
* prandom_seed - add entropy to pseudo random number generator
* @seed: seed value
@@ -172,15 +194,14 @@ static int __init prandom_init(void)
{
int i;
+#ifdef CONFIG_RANDOM32_SELFTEST
+ prandom_state_selftest();
+#endif
+
for_each_possible_cpu(i) {
struct rnd_state *state = &per_cpu(net_rand_state,i);
-#define LCG(x) ((x) * 69069U) /* super-duper LCG */
- state->s1 = __seed(LCG((i + jiffies) ^ random_get_entropy()), 2U);
- state->s2 = __seed(LCG(state->s1), 8U);
- state->s3 = __seed(LCG(state->s2), 16U);
- state->s4 = __seed(LCG(state->s3), 128U);
-
+ prandom_seed_very_weak(state, (i + jiffies) ^ random_get_entropy());
prandom_warmup(state);
}
return 0;
@@ -253,3 +274,165 @@ static int __init prandom_reseed(void)
return 0;
}
late_initcall(prandom_reseed);
+
+#ifdef CONFIG_RANDOM32_SELFTEST
+static struct prandom_test1 {
+ u32 seed;
+ u32 result;
+} test1[] = {
+ { 1U, 3484351685U },
+ { 2U, 2623130059U },
+ { 3U, 3125133893U },
+ { 4U, 984847254U },
+};
+
+static struct prandom_test2 {
+ u32 seed;
+ u32 iteration;
+ u32 result;
+} test2[] = {
+ /* Test cases against taus113 from GSL library. */
+ { 931557656U, 959U, 2975593782U },
+ { 1339693295U, 876U, 3887776532U },
+ { 1545556285U, 961U, 1615538833U },
+ { 601730776U, 723U, 1776162651U },
+ { 1027516047U, 687U, 511983079U },
+ { 416526298U, 700U, 916156552U },
+ { 1395522032U, 652U, 2222063676U },
+ { 366221443U, 617U, 2992857763U },
+ { 1539836965U, 714U, 3783265725U },
+ { 556206671U, 994U, 799626459U },
+ { 684907218U, 799U, 367789491U },
+ { 2121230701U, 931U, 2115467001U },
+ { 1668516451U, 644U, 3620590685U },
+ { 768046066U, 883U, 2034077390U },
+ { 1989159136U, 833U, 1195767305U },
+ { 536585145U, 996U, 3577259204U },
+ { 1008129373U, 642U, 1478080776U },
+ { 1740775604U, 939U, 1264980372U },
+ { 1967883163U, 508U, 10734624U },
+ { 1923019697U, 730U, 3821419629U },
+ { 442079932U, 560U, 3440032343U },
+ { 1961302714U, 845U, 841962572U },
+ { 2030205964U, 962U, 1325144227U },
+ { 1160407529U, 507U, 240940858U },
+ { 635482502U, 779U, 4200489746U },
+ { 1252788931U, 699U, 867195434U },
+ { 1961817131U, 719U, 668237657U },
+ { 1071468216U, 983U, 917876630U },
+ { 1281848367U, 932U, 1003100039U },
+ { 582537119U, 780U, 1127273778U },
+ { 1973672777U, 853U, 1071368872U },
+ { 1896756996U, 762U, 1127851055U },
+ { 847917054U, 500U, 1717499075U },
+ { 1240520510U, 951U, 2849576657U },
+ { 1685071682U, 567U, 1961810396U },
+ { 1516232129U, 557U, 3173877U },
+ { 1208118903U, 612U, 1613145022U },
+ { 1817269927U, 693U, 4279122573U },
+ { 1510091701U, 717U, 638191229U },
+ { 365916850U, 807U, 600424314U },
+ { 399324359U, 702U, 1803598116U },
+ { 1318480274U, 779U, 2074237022U },
+ { 697758115U, 840U, 1483639402U },
+ { 1696507773U, 840U, 577415447U },
+ { 2081979121U, 981U, 3041486449U },
+ { 955646687U, 742U, 3846494357U },
+ { 1250683506U, 749U, 836419859U },
+ { 595003102U, 534U, 366794109U },
+ { 47485338U, 558U, 3521120834U },
+ { 619433479U, 610U, 3991783875U },
+ { 704096520U, 518U, 4139493852U },
+ { 1712224984U, 606U, 2393312003U },
+ { 1318233152U, 922U, 3880361134U },
+ { 855572992U, 761U, 1472974787U },
+ { 64721421U, 703U, 683860550U },
+ { 678931758U, 840U, 380616043U },
+ { 692711973U, 778U, 1382361947U },
+ { 677703619U, 530U, 2826914161U },
+ { 92393223U, 586U, 1522128471U },
+ { 1222592920U, 743U, 3466726667U },
+ { 358288986U, 695U, 1091956998U },
+ { 1935056945U, 958U, 514864477U },
+ { 735675993U, 990U, 1294239989U },
+ { 1560089402U, 897U, 2238551287U },
+ { 70616361U, 829U, 22483098U },
+ { 368234700U, 731U, 2913875084U },
+ { 20221190U, 879U, 1564152970U },
+ { 539444654U, 682U, 1835141259U },
+ { 1314987297U, 840U, 1801114136U },
+ { 2019295544U, 645U, 3286438930U },
+ { 469023838U, 716U, 1637918202U },
+ { 1843754496U, 653U, 2562092152U },
+ { 400672036U, 809U, 4264212785U },
+ { 404722249U, 965U, 2704116999U },
+ { 600702209U, 758U, 584979986U },
+ { 519953954U, 667U, 2574436237U },
+ { 1658071126U, 694U, 2214569490U },
+ { 420480037U, 749U, 3430010866U },
+ { 690103647U, 969U, 3700758083U },
+ { 1029424799U, 937U, 3787746841U },
+ { 2012608669U, 506U, 3362628973U },
+ { 1535432887U, 998U, 42610943U },
+ { 1330635533U, 857U, 3040806504U },
+ { 1223800550U, 539U, 3954229517U },
+ { 1322411537U, 680U, 3223250324U },
+ { 1877847898U, 945U, 2915147143U },
+ { 1646356099U, 874U, 965988280U },
+ { 805687536U, 744U, 4032277920U },
+ { 1948093210U, 633U, 1346597684U },
+ { 392609744U, 783U, 1636083295U },
+ { 690241304U, 770U, 1201031298U },
+ { 1360302965U, 696U, 1665394461U },
+ { 1220090946U, 780U, 1316922812U },
+ { 447092251U, 500U, 3438743375U },
+ { 1613868791U, 592U, 828546883U },
+ { 523430951U, 548U, 2552392304U },
+ { 726692899U, 810U, 1656872867U },
+ { 1364340021U, 836U, 3710513486U },
+ { 1986257729U, 931U, 935013962U },
+ { 407983964U, 921U, 728767059U },
+};
+
+static void __init prandom_state_selftest(void)
+{
+ int i, j, errors = 0, runs = 0;
+ bool error = false;
+
+ for (i = 0; i < ARRAY_SIZE(test1); i++) {
+ struct rnd_state state;
+
+ prandom_seed_very_weak(&state, test1[i].seed);
+ prandom_warmup(&state);
+
+ if (test1[i].result != prandom_u32_state(&state))
+ error = true;
+ }
+
+ if (error)
+ pr_warn("prandom: seed boundary self test failed\n");
+ else
+ pr_info("prandom: seed boundary self test passed\n");
+
+ for (i = 0; i < ARRAY_SIZE(test2); i++) {
+ struct rnd_state state;
+
+ prandom_seed_very_weak(&state, test2[i].seed);
+ prandom_warmup(&state);
+
+ for (j = 0; j < test2[i].iteration - 1; j++)
+ prandom_u32_state(&state);
+
+ if (test2[i].result != prandom_u32_state(&state))
+ errors++;
+
+ runs++;
+ cond_resched();
+ }
+
+ if (errors)
+ pr_warn("prandom: %d/%d self tests failed\n", errors, runs);
+ else
+ pr_info("prandom: %d self tests passed\n", runs);
+}
+#endif
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 4/6] random32: move rnd_state to linux/random.h
From: Daniel Borkmann @ 2013-11-11 11:20 UTC (permalink / raw)
To: davem; +Cc: shemminger, fweimer, netdev, Joe Eykholt, Hannes Frederic Sowa
In-Reply-To: <cover.1384160397.git.dborkman@redhat.com>
struct rnd_state got mistakenly pulled into uapi header. It is not
used anywhere and does also not belong there!
Commit 5960164fde ("lib/random32: export pseudo-random number
generator for modules"), the last commit on rnd_state before it
got moved to uapi, says:
This patch moves the definition of struct rnd_state and the inline
__seed() function to linux/random.h. It renames the static __random32()
function to prandom32() and exports it for use in modules.
Hence, the structure was moved from lib/random32.c to linux/random.h
so that it can be used within modules (FCoE-related code in this
case), but not from user space. However, it seems to have been
mistakenly moved to uapi header through the uapi script. Since no-one
should make use of it from the linux headers, move the structure back
to the kernel for internal use, so that it can be modified on demand.
Joint work with Hannes Frederic Sowa.
Cc: Joe Eykholt <jeykholt@cisco.com>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
include/linux/random.h | 4 ++++
include/uapi/linux/random.h | 7 -------
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/include/linux/random.h b/include/linux/random.h
index 5117ae3..8ef0b70 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -31,6 +31,10 @@ void prandom_bytes(void *buf, int nbytes);
void prandom_seed(u32 seed);
void prandom_reseed_late(void);
+struct rnd_state {
+ __u32 s1, s2, s3;
+};
+
u32 prandom_u32_state(struct rnd_state *);
void prandom_bytes_state(struct rnd_state *state, void *buf, int nbytes);
diff --git a/include/uapi/linux/random.h b/include/uapi/linux/random.h
index 7471b5b..fff3528 100644
--- a/include/uapi/linux/random.h
+++ b/include/uapi/linux/random.h
@@ -40,11 +40,4 @@ struct rand_pool_info {
__u32 buf[0];
};
-struct rnd_state {
- __u32 s1, s2, s3;
-};
-
-/* Exported functions */
-
-
#endif /* _UAPI_LINUX_RANDOM_H */
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 0/10] bonding: rebuild the lock use for bond monitor
From: Ding Tianhong @ 2013-11-11 12:36 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev,
Nikolay Aleksandrov
Now the bond slave list is not protected by bond lock, only by RTNL,
but the monitor still use the bond lock to protect the slave list,
it is useless, according to the Veaceslav's opinion, there were
three way to fix the protect problem:
1. add bond_master_upper_dev_link() and bond_upper_dev_unlink()
in bond->lock, but it is unsafe to call call_netdevice_notifiers()
in write lock.
2. remove unused bond->lock for monitor function, only use the exist
rtnl lock(), it will take performance loss in fast path.
3. use RCU to protect the slave list, of course, performance is better,
but in slow path, it is ignored.
obviously the solution 1 is not fit here, I will consider the 2 and 3
solution. My principle is simple, if in fast path, RCU is better,
otherwise in slow path, both is well, but according to the Jay Vosburgh's
opinion, the monitor will loss performace if use RTNL to protect the all
slave list, so remove the bond lock and replace with RCU.
The second problem is the curr_slave_lock for bond, it is too old and
unwanted in many place, because the curr_active_slave would only be
changed in 3 place:
1. enslave slave.
2. release slave.
3. change active slave.
all above were already holding bond lock, RTNL and curr_slave_lock
together, it is tedious and no need to add so mach lock, when change
the curr_active_slave, you have to hold the RTNL and curr_slave_lock
together, and when you read the curr_active_slave, RTNL or curr_slave_lock,
any one of them is no problem.
for the stability, I did not change the logic for the monitor,
all change is clear and simple, I have test the patch set for lockdep,
it work well and stability.
v2. accept the Jay Vosburgh's opinion, remove the RTNL and replace with RCU,
also add some rcu function for bond use, so the patch set reach 10.
v3. accept the Nikolay Aleksandrov's opinion, remove no needed bond_has_slave_rcu(),
add protection for several 3ad mode handler functions and current_arp_slave.
rebuild the bond_first_slave_rcu(), make it more clear.
Best Regards
Ding Tianhong
Ding Tianhong (10):
bonding: remove the no effect lock for bond_select_active_slave()
bonding: rebuild the lock use for bond_mii_monitor()
bonding: rebuild the lock use for bond_alb_monitor()
bonding: rebuild the lock use for bond_loadbalance_arp_mon()
bonding: create bond_first_slave_rcu()
bonding: rebuild the lock use for bond_activebackup_arp_mon()
bonding: rebuild the lock use for bond_3ad_state_machine_handler()
bonding: remove unwanted lock for bond_option_active_slave_set()
bonding: remove unwanted lock for bond enslave and release
bonding: remove unwanted lock for bond_store_primaryxxx()
drivers/net/bonding/bond_3ad.c | 53 +++++++------
drivers/net/bonding/bond_alb.c | 34 +++------
drivers/net/bonding/bond_main.c | 147 ++++++++++++++++---------------------
drivers/net/bonding/bond_options.c | 2 -
drivers/net/bonding/bond_sysfs.c | 4 -
drivers/net/bonding/bonding.h | 9 +++
include/linux/netdevice.h | 16 ++++
net/core/dev.c | 16 ----
8 files changed, 132 insertions(+), 149 deletions(-)
--
1.8.2.1
^ permalink raw reply
* [PATCH net-next v3 2/10] bonding: rebuild the lock use for bond_mii_monitor()
From: Ding Tianhong @ 2013-11-11 12:36 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_mii_monitor() still use bond lock to protect bond slave list,
it is no effect, I have 2 way to fix the problem, move the RTNL to the
top of the function, or add RCU to protect the bond slave list,
according to the Jay Vosburgh's opinion, 10 times one second is a
truely big performance loss if use RTNL to protect the whole monitor,
so I would take the advice and use RCU to protect the bond slave list.
The bond_has_slave() will not protect by anything, there will no things
happen if the slave list is be changed, unless the bond was free, but
it will not happened before the monitor, the bond will closed before
be freed.
The peers notify for the bond will calling curr_active_slave, so
derefence the slave to make sure we will accessing the same slave
if the curr_active_slave changed.
I move the peer notify before the queue_delayed_work(), and obviously
it is no need to lock the RTNL twice if call bond_miimon_commit() and
peer notify together, other path is no logic change, I think the
performance is better than before.
Suggested-by: Nikolay Aleksandrov <nikolay@redhat.com>
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_main.c | 38 ++++++++++++++++++--------------------
1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index f5145ea..ccec68f 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -815,7 +815,7 @@ static struct slave *bond_find_best_slave(struct bonding *bond)
static bool bond_should_notify_peers(struct bonding *bond)
{
- struct slave *slave = bond->curr_active_slave;
+ struct slave *slave = rcu_dereference(bond->curr_active_slave);
pr_debug("bond_should_notify_peers: bond %s slave %s\n",
bond->dev->name, slave ? slave->dev->name : "NULL");
@@ -1919,7 +1919,7 @@ static int bond_miimon_inspect(struct bonding *bond)
ignore_updelay = !bond->curr_active_slave ? true : false;
- bond_for_each_slave(bond, slave, iter) {
+ bond_for_each_slave_rcu(bond, slave, iter) {
slave->new_link = BOND_LINK_NOCHANGE;
link_state = bond_check_dev_link(bond, slave->dev, 0);
@@ -2117,47 +2117,45 @@ void bond_mii_monitor(struct work_struct *work)
bool should_notify_peers = false;
unsigned long delay;
- read_lock(&bond->lock);
-
delay = msecs_to_jiffies(bond->params.miimon);
if (!bond_has_slaves(bond))
goto re_arm;
+ rcu_read_lock();
+
should_notify_peers = bond_should_notify_peers(bond);
if (bond_miimon_inspect(bond)) {
- read_unlock(&bond->lock);
+ rcu_read_unlock();
/* Race avoidance with bond_close cancel of workqueue */
if (!rtnl_trylock()) {
- read_lock(&bond->lock);
delay = 1;
- should_notify_peers = false;
goto re_arm;
}
- read_lock(&bond->lock);
-
bond_miimon_commit(bond);
- read_unlock(&bond->lock);
+ if (should_notify_peers)
+ call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
+ bond->dev);
+
rtnl_unlock(); /* might sleep, hold no other locks */
- read_lock(&bond->lock);
+ } else {
+ rcu_read_unlock();
+ if (should_notify_peers) {
+ if (!rtnl_trylock())
+ goto re_arm;
+ call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
+ bond->dev);
+ rtnl_unlock();
+ }
}
re_arm:
if (bond->params.miimon)
queue_delayed_work(bond->wq, &bond->mii_work, delay);
-
- read_unlock(&bond->lock);
-
- if (should_notify_peers) {
- if (!rtnl_trylock())
- return;
- call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev);
- rtnl_unlock();
- }
}
static bool bond_has_this_ip(struct bonding *bond, __be32 ip)
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next v3 4/10] bonding: rebuild the lock use for bond_loadbalance_arp_mon()
From: Ding Tianhong @ 2013-11-11 12:36 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_loadbalance_arp_mon() use the bond lock to protect the
bond slave list, it is no effect, so I could use RTNL or RCU to
replace it, considering the performance impact, the RCU is more
better here, so the bond lock replace with the RCU.
The bond_select_active_slave() need RTNL and curr_slave_lock
together, but there is no RTNL lock here, so add a rtnl_rtylock.
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_main.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index ccec68f..5df705c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2411,12 +2411,12 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
struct list_head *iter;
int do_failover = 0;
- read_lock(&bond->lock);
-
if (!bond_has_slaves(bond))
goto re_arm;
- oldcurrent = bond->curr_active_slave;
+ rcu_read_lock();
+
+ oldcurrent = ACCESS_ONCE(bond->curr_active_slave);
/* see if any of the previous devices are up now (i.e. they have
* xmt and rcv traffic). the curr_active_slave does not come into
* the picture unless it is null. also, slave->jiffies is not needed
@@ -2425,7 +2425,7 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
* TODO: what about up/down delay in arp mode? it wasn't here before
* so it can wait
*/
- bond_for_each_slave(bond, slave, iter) {
+ bond_for_each_slave_rcu(bond, slave, iter) {
unsigned long trans_start = dev_trans_start(slave->dev);
if (slave->link != BOND_LINK_UP) {
@@ -2487,7 +2487,14 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
bond_arp_send_all(bond, slave);
}
+ rcu_read_unlock();
+
if (do_failover) {
+ /* the bond_select_active_slave must hold RTNL
+ * and curr_slave_lock for write.
+ */
+ if (!rtnl_trylock())
+ goto re_arm;
block_netpoll_tx();
write_lock_bh(&bond->curr_slave_lock);
@@ -2495,14 +2502,13 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
write_unlock_bh(&bond->curr_slave_lock);
unblock_netpoll_tx();
+ rtnl_unlock();
}
re_arm:
if (bond->params.arp_interval)
queue_delayed_work(bond->wq, &bond->arp_work,
msecs_to_jiffies(bond->params.arp_interval));
-
- read_unlock(&bond->lock);
}
/*
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next v3 3/10] bonding: rebuild the lock use for bond_alb_monitor()
From: Ding Tianhong @ 2013-11-11 12:36 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_alb_monitor use bond lock to protect the bond slave list,
it is no effect here, we need to use RTNL or RCU to replace bond lock,
the bond_alb_monitor will called 10 times one second, RTNL may loss
performance here, so the bond lock replace with RCU to protect the
bond slave list, also the RTNL is preserved, the logic of the monitor
did not changed.
Suggested-by: Nikolay Aleksandrov <nikolay@redhat.com>
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_alb.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 1fae915..8a85064 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -816,7 +816,7 @@ static void rlb_rebalance(struct bonding *bond)
for (; hash_index != RLB_NULL_INDEX;
hash_index = client_info->used_next) {
client_info = &(bond_info->rx_hashtbl[hash_index]);
- assigned_slave = rlb_next_rx_slave(bond);
+ assigned_slave = __rlb_next_rx_slave(bond);
if (assigned_slave && (client_info->slave != assigned_slave)) {
client_info->slave = assigned_slave;
client_info->ntt = 1;
@@ -1495,14 +1495,14 @@ void bond_alb_monitor(struct work_struct *work)
struct list_head *iter;
struct slave *slave;
- read_lock(&bond->lock);
-
if (!bond_has_slaves(bond)) {
bond_info->tx_rebalance_counter = 0;
bond_info->lp_counter = 0;
goto re_arm;
}
+ rcu_read_lock();
+
bond_info->tx_rebalance_counter++;
bond_info->lp_counter++;
@@ -1515,7 +1515,7 @@ void bond_alb_monitor(struct work_struct *work)
*/
read_lock(&bond->curr_slave_lock);
- bond_for_each_slave(bond, slave, iter)
+ bond_for_each_slave_rcu(bond, slave, iter)
alb_send_learning_packets(slave, slave->dev->dev_addr);
read_unlock(&bond->curr_slave_lock);
@@ -1528,7 +1528,7 @@ void bond_alb_monitor(struct work_struct *work)
read_lock(&bond->curr_slave_lock);
- bond_for_each_slave(bond, slave, iter) {
+ bond_for_each_slave_rcu(bond, slave, iter) {
tlb_clear_slave(bond, slave, 1);
if (slave == bond->curr_active_slave) {
SLAVE_TLB_INFO(slave).load =
@@ -1552,11 +1552,9 @@ void bond_alb_monitor(struct work_struct *work)
* dev_set_promiscuity requires rtnl and
* nothing else. Avoid race with bond_close.
*/
- read_unlock(&bond->lock);
- if (!rtnl_trylock()) {
- read_lock(&bond->lock);
+ rcu_read_unlock();
+ if (!rtnl_trylock())
goto re_arm;
- }
bond_info->rlb_promisc_timeout_counter = 0;
@@ -1568,7 +1566,7 @@ void bond_alb_monitor(struct work_struct *work)
bond_info->primary_is_promisc = 0;
rtnl_unlock();
- read_lock(&bond->lock);
+ rcu_read_lock();
}
if (bond_info->rlb_rebalance) {
@@ -1590,11 +1588,9 @@ void bond_alb_monitor(struct work_struct *work)
}
}
}
-
+ rcu_read_unlock();
re_arm:
queue_delayed_work(bond->wq, &bond->alb_work, alb_delta_in_ticks);
-
- read_unlock(&bond->lock);
}
/* assumption: called before the slave is attached to the bond
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next v3 8/10] bonding: remove unwanted lock for bond_option_active_slave_set()
From: Ding Tianhong @ 2013-11-11 12:36 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_option_active_slave_set() is always called in RTNL,
the RTNL could protect bond slave list, so remove the unwanted
bond lock.
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_options.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 9a5223c..ae4cd04 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -101,7 +101,6 @@ int bond_option_active_slave_set(struct bonding *bond,
}
block_netpoll_tx();
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
/* check to see if we are clearing active */
@@ -136,7 +135,6 @@ int bond_option_active_slave_set(struct bonding *bond,
}
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
unblock_netpoll_tx();
return ret;
}
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next v3 1/10] bonding: remove the no effect lock for bond_select_active_slave()
From: Ding Tianhong @ 2013-11-11 12:36 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond slave list was no longer protected by bond lock and only
protected by RTNL or RCU, so anywhere that use bond lock to protect
slave list is meaningless.
remove the release and acquire bond lock for bond_select_active_slave().
The curr_active_slave could only be changed in 3 place:
1. enslave slave.
2. release slave.
3. change_active_slave.
all above place were holding bond lock, RTNL and curr_slave_lock
together, it is tedious and meaningless, obviously bond lock is no
need here, but RTNL or curr_slave_lock is needed, so if you want
to access active slave, you have to choose one lock, RTNL or
curr_slave_lock, if RTNL is exist, no need to add curr_slave_lock,
otherwise curr_slave_lock is better, because of the performance.
there are several place calling bond_select_active_slave() and
bond_change_active_slave(), the next step I will clean these place
and remove the no effect lock.
there are some document changed together when update the function.
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_alb.c | 12 +++---------
drivers/net/bonding/bond_main.c | 15 +++------------
2 files changed, 6 insertions(+), 21 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 0287240..1fae915 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -470,7 +470,7 @@ static void rlb_teach_disabled_mac_on_primary(struct bonding *bond, u8 addr[])
/* slave being removed should not be active at this point
*
- * Caller must hold bond lock for read
+ * Caller must hold rtnl.
*/
static void rlb_clear_slave(struct bonding *bond, struct slave *slave)
{
@@ -1680,14 +1680,11 @@ void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char
* If new_slave is NULL, caller must hold curr_slave_lock or
* bond->lock for write.
*
- * If new_slave is not NULL, caller must hold RTNL, bond->lock for
- * read and curr_slave_lock for write. Processing here may sleep, so
- * no other locks may be held.
+ * If new_slave is not NULL, caller must hold RTNL, curr_slave_lock
+ * for write. Processing here may sleep, so no other locks may be held.
*/
void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave)
__releases(&bond->curr_slave_lock)
- __releases(&bond->lock)
- __acquires(&bond->lock)
__acquires(&bond->curr_slave_lock)
{
struct slave *swap_slave;
@@ -1723,7 +1720,6 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
tlb_clear_slave(bond, new_slave, 1);
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
ASSERT_RTNL();
@@ -1749,11 +1745,9 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
/* swap mac address */
alb_swap_mac_addr(swap_slave, new_slave);
alb_fasten_mac_swap(bond, swap_slave, new_slave);
- read_lock(&bond->lock);
} else {
/* set the new_slave to the bond mac address */
alb_set_slave_mac_addr(new_slave, bond->dev->dev_addr);
- read_lock(&bond->lock);
alb_send_learning_packets(new_slave, bond->dev->dev_addr);
}
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 4dd5ee2..f5145ea 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -697,14 +697,12 @@ static void bond_set_dev_addr(struct net_device *bond_dev,
*
* Perform special MAC address swapping for fail_over_mac settings
*
- * Called with RTNL, bond->lock for read, curr_slave_lock for write_bh.
+ * Called with RTNL, curr_slave_lock for write_bh.
*/
static void bond_do_fail_over_mac(struct bonding *bond,
struct slave *new_active,
struct slave *old_active)
__releases(&bond->curr_slave_lock)
- __releases(&bond->lock)
- __acquires(&bond->lock)
__acquires(&bond->curr_slave_lock)
{
u8 tmp_mac[ETH_ALEN];
@@ -715,9 +713,7 @@ static void bond_do_fail_over_mac(struct bonding *bond,
case BOND_FOM_ACTIVE:
if (new_active) {
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
bond_set_dev_addr(bond->dev, new_active->dev);
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
}
break;
@@ -731,7 +727,6 @@ static void bond_do_fail_over_mac(struct bonding *bond,
return;
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
if (old_active) {
memcpy(tmp_mac, new_active->dev->dev_addr, ETH_ALEN);
@@ -761,7 +756,6 @@ static void bond_do_fail_over_mac(struct bonding *bond,
pr_err("%s: Error %d setting MAC of slave %s\n",
bond->dev->name, -rv, new_active->dev->name);
out:
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
break;
default:
@@ -846,8 +840,7 @@ static bool bond_should_notify_peers(struct bonding *bond)
* because it is apparently the best available slave we have, even though its
* updelay hasn't timed out yet.
*
- * If new_active is not NULL, caller must hold bond->lock for read and
- * curr_slave_lock for write_bh.
+ * If new_active is not NULL, caller must hold curr_slave_lock for write_bh.
*/
void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
{
@@ -916,14 +909,12 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
}
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
call_netdevice_notifiers(NETDEV_BONDING_FAILOVER, bond->dev);
if (should_notify_peers)
call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
bond->dev);
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
}
}
@@ -949,7 +940,7 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
* - The primary_slave has got its link back.
* - A slave has got its link back and there's no old curr_active_slave.
*
- * Caller must hold bond->lock for read and curr_slave_lock for write_bh.
+ * Caller must hold curr_slave_lock for write_bh.
*/
void bond_select_active_slave(struct bonding *bond)
{
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next v3 10/10] bonding: remove unwanted lock for bond_store_primaryxxx()
From: Ding Tianhong @ 2013-11-11 12:37 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_select_active_slave() will not release and acquire
bond lock, so it is no need to read the bond lock for them,
and the bond_store_primaryxxx() is in RTNL, remove the
unwanted lock.
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_sysfs.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 47749c9..25ef533 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1068,7 +1068,6 @@ static ssize_t bonding_store_primary(struct device *d,
if (!rtnl_trylock())
return restart_syscall();
block_netpoll_tx();
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
if (!USES_PRIMARY(bond->params.mode)) {
@@ -1108,7 +1107,6 @@ static ssize_t bonding_store_primary(struct device *d,
bond->dev->name, ifname, bond->dev->name);
out:
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
unblock_netpoll_tx();
rtnl_unlock();
@@ -1156,11 +1154,9 @@ static ssize_t bonding_store_primary_reselect(struct device *d,
new_value);
block_netpoll_tx();
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
bond_select_active_slave(bond);
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
unblock_netpoll_tx();
out:
rtnl_unlock();
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next v3 6/10] bonding: rebuild the lock use for bond_activebackup_arp_mon()
From: Ding Tianhong @ 2013-11-11 12:36 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_activebackup_arp_mon() use the bond lock for read to
protect the slave list, it is no effect, and the RTNL is only
called for bond_ab_arp_commit() and peer notify, for the performance
better, use RCU to replace with the bond lock, to the bond slave
list need to called in RCU, add a new bond_first_slave_rcu()
to get the first slave in RCU protection.
In bond_ab_arp_probe(), the bond->current_arp_slave may changd
if bond release slave, just like:
bond_ab_arp_probe() bond_release()
cpu 0 cpu 1
...
if (bond->current_arp_slave...) ...
... bond->current_arp_slave = NULl
bond->current_arp_slave->dev->name ...
So the current_arp_slave need to dereference in the section.
When bond_ab_arp_inspect() and should_notify_peers is true, the
RTNL will called twice, it is a loss of performance, so make the
two RTNL together to avoid performance loss.
Suggested-by: Nikolay Aleksandrov <nikolay@redhat.com>
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_main.c | 49 +++++++++++++++++++++--------------------
1 file changed, 25 insertions(+), 24 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 5df705c..80d5c6a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2517,7 +2517,7 @@ re_arm:
* place for the slave. Returns 0 if no changes are found, >0 if changes
* to link states must be committed.
*
- * Called with bond->lock held for read.
+ * Called with rcu_read_lock hold.
*/
static int bond_ab_arp_inspect(struct bonding *bond)
{
@@ -2526,7 +2526,7 @@ static int bond_ab_arp_inspect(struct bonding *bond)
struct slave *slave;
int commit = 0;
- bond_for_each_slave(bond, slave, iter) {
+ bond_for_each_slave_rcu(bond, slave, iter) {
slave->new_link = BOND_LINK_NOCHANGE;
last_rx = slave_last_rx(bond, slave);
@@ -2588,7 +2588,7 @@ static int bond_ab_arp_inspect(struct bonding *bond)
* Called to commit link state changes noted by inspection step of
* active-backup mode ARP monitor.
*
- * Called with RTNL and bond->lock for read.
+ * Called with RTNL hold.
*/
static void bond_ab_arp_commit(struct bonding *bond)
{
@@ -2663,19 +2663,20 @@ do_failover:
/*
* Send ARP probes for active-backup mode ARP monitor.
*
- * Called with bond->lock held for read.
+ * Called with rcu_read_lock hold.
*/
static void bond_ab_arp_probe(struct bonding *bond)
{
- struct slave *slave, *before = NULL, *new_slave = NULL;
+ struct slave *slave, *before = NULL, *new_slave = NULL,
+ *curr_arp_slave = rcu_dereference(bond->current_arp_slave);
struct list_head *iter;
bool found = false;
read_lock(&bond->curr_slave_lock);
- if (bond->current_arp_slave && bond->curr_active_slave)
+ if (curr_arp_slave && bond->curr_active_slave)
pr_info("PROBE: c_arp %s && cas %s BAD\n",
- bond->current_arp_slave->dev->name,
+ curr_arp_slave->dev->name,
bond->curr_active_slave->dev->name);
if (bond->curr_active_slave) {
@@ -2691,15 +2692,15 @@ static void bond_ab_arp_probe(struct bonding *bond)
* for becoming the curr_active_slave
*/
- if (!bond->current_arp_slave) {
- bond->current_arp_slave = bond_first_slave(bond);
- if (!bond->current_arp_slave)
+ if (!curr_arp_slave) {
+ curr_arp_slave = bond_first_slave_rcu(bond);
+ if (!curr_arp_slave)
return;
}
- bond_set_slave_inactive_flags(bond->current_arp_slave);
+ bond_set_slave_inactive_flags(curr_arp_slave);
- bond_for_each_slave(bond, slave, iter) {
+ bond_for_each_slave_rcu(bond, slave, iter) {
if (!found && !before && IS_UP(slave->dev))
before = slave;
@@ -2722,7 +2723,7 @@ static void bond_ab_arp_probe(struct bonding *bond)
pr_info("%s: backup interface %s is now down.\n",
bond->dev->name, slave->dev->name);
}
- if (slave == bond->current_arp_slave)
+ if (slave == curr_arp_slave)
found = true;
}
@@ -2736,8 +2737,7 @@ static void bond_ab_arp_probe(struct bonding *bond)
bond_set_slave_active_flags(new_slave);
bond_arp_send_all(bond, new_slave);
new_slave->jiffies = jiffies;
- bond->current_arp_slave = new_slave;
-
+ rcu_assign_pointer(bond->current_arp_slave, new_slave);
}
void bond_activebackup_arp_mon(struct work_struct *work)
@@ -2747,17 +2747,17 @@ void bond_activebackup_arp_mon(struct work_struct *work)
bool should_notify_peers = false;
int delta_in_ticks;
- read_lock(&bond->lock);
-
delta_in_ticks = msecs_to_jiffies(bond->params.arp_interval);
if (!bond_has_slaves(bond))
goto re_arm;
+ rcu_read_lock();
+
should_notify_peers = bond_should_notify_peers(bond);
if (bond_ab_arp_inspect(bond)) {
- read_unlock(&bond->lock);
+ rcu_read_unlock();
/* Race avoidance with bond_close flush of workqueue */
if (!rtnl_trylock()) {
@@ -2767,23 +2767,24 @@ void bond_activebackup_arp_mon(struct work_struct *work)
goto re_arm;
}
- read_lock(&bond->lock);
-
bond_ab_arp_commit(bond);
+ if (should_notify_peers) {
+ call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
+ bond->dev);
+ should_notify_peers = false;
+ }
- read_unlock(&bond->lock);
rtnl_unlock();
- read_lock(&bond->lock);
+ rcu_read_lock();
}
bond_ab_arp_probe(bond);
+ rcu_read_unlock();
re_arm:
if (bond->params.arp_interval)
queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
- read_unlock(&bond->lock);
-
if (should_notify_peers) {
if (!rtnl_trylock())
return;
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next v3 7/10] bonding: rebuild the lock use for bond_3ad_state_machine_handler()
From: Ding Tianhong @ 2013-11-11 12:36 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_3ad_state_machine_handler() use the bond lock to protect
the bond slave list and slave port, so as the before patch said,
I remove bond lock and replace with RCU.
There was a lot of function need RCU protect, I have two choice
to make the function in RCU-safe, (1) create new similar functions
and make the bond slave list in RCU. (2) modify the existed functions
and make them in read-side critical section, because the RCU
read-side critical sections may be nested.
I choose (2) because it is no need to create more similar functions.
The nots in the function is still too old, clean up the nots.
Suggested-by: Nikolay Aleksandrov <nikolay@redhat.com>
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_3ad.c | 53 ++++++++++++++++++++++++------------------
1 file changed, 31 insertions(+), 22 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 187b1b7..d935da5 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -147,11 +147,12 @@ static inline struct aggregator *__get_first_agg(struct port *port)
struct bonding *bond = __get_bond_by_port(port);
struct slave *first_slave;
- // If there's no bond for this port, or bond has no slaves
+ /* If there's no bond for this port, or bond has no slaves */
if (bond == NULL)
return NULL;
- first_slave = bond_first_slave(bond);
-
+ rcu_read_lock();
+ first_slave = bond_first_slave_rcu(bond);
+ rcu_read_unlock();
return first_slave ? &(SLAVE_AD_INFO(first_slave).aggregator) : NULL;
}
@@ -702,9 +703,13 @@ static struct aggregator *__get_active_agg(struct aggregator *aggregator)
struct list_head *iter;
struct slave *slave;
- bond_for_each_slave(bond, slave, iter)
- if (SLAVE_AD_INFO(slave).aggregator.is_active)
+ rcu_read_lock();
+ bond_for_each_slave_rcu(bond, slave, iter)
+ if (SLAVE_AD_INFO(slave).aggregator.is_active) {
+ rcu_read_unlock();
return &(SLAVE_AD_INFO(slave).aggregator);
+ }
+ rcu_read_unlock();
return NULL;
}
@@ -1471,7 +1476,8 @@ static void ad_agg_selection_logic(struct aggregator *agg)
active = __get_active_agg(agg);
best = (active && agg_device_up(active)) ? active : NULL;
- bond_for_each_slave(bond, slave, iter) {
+ rcu_read_lock();
+ bond_for_each_slave_rcu(bond, slave, iter) {
agg = &(SLAVE_AD_INFO(slave).aggregator);
agg->is_active = 0;
@@ -1505,7 +1511,7 @@ static void ad_agg_selection_logic(struct aggregator *agg)
active->is_active = 1;
}
- // if there is new best aggregator, activate it
+ /* if there is new best aggregator, activate it */
if (best) {
pr_debug("best Agg=%d; P=%d; a k=%d; p k=%d; Ind=%d; Act=%d\n",
best->aggregator_identifier, best->num_of_ports,
@@ -1516,7 +1522,7 @@ static void ad_agg_selection_logic(struct aggregator *agg)
best->lag_ports, best->slave,
best->slave ? best->slave->dev->name : "NULL");
- bond_for_each_slave(bond, slave, iter) {
+ bond_for_each_slave_rcu(bond, slave, iter) {
agg = &(SLAVE_AD_INFO(slave).aggregator);
pr_debug("Agg=%d; P=%d; a k=%d; p k=%d; Ind=%d; Act=%d\n",
@@ -1526,10 +1532,10 @@ static void ad_agg_selection_logic(struct aggregator *agg)
agg->is_individual, agg->is_active);
}
- // check if any partner replys
+ /* check if any partner replys */
if (best->is_individual) {
pr_warning("%s: Warning: No 802.3ad response from the link partner for any adapters in the bond\n",
- best->slave ? best->slave->bond->dev->name : "NULL");
+ best->slave ? best->slave->bond->dev->name : "NULL");
}
best->is_active = 1;
@@ -1541,7 +1547,7 @@ static void ad_agg_selection_logic(struct aggregator *agg)
best->partner_oper_aggregator_key,
best->is_individual, best->is_active);
- // disable the ports that were related to the former active_aggregator
+ /* disable the ports that were related to the former active_aggregator */
if (active) {
for (port = active->lag_ports; port;
port = port->next_port_in_aggregator) {
@@ -1565,6 +1571,8 @@ static void ad_agg_selection_logic(struct aggregator *agg)
}
}
+ rcu_read_unlock();
+
bond_3ad_set_carrier(bond);
}
@@ -2068,18 +2076,18 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
struct slave *slave;
struct port *port;
- read_lock(&bond->lock);
+ rcu_read_lock();
- //check if there are any slaves
+ /* check if there are any slaves */
if (!bond_has_slaves(bond))
goto re_arm;
- // check if agg_select_timer timer after initialize is timed out
+ /* check if agg_select_timer timer after initialize is timed out */
if (BOND_AD_INFO(bond).agg_select_timer && !(--BOND_AD_INFO(bond).agg_select_timer)) {
- slave = bond_first_slave(bond);
+ slave = bond_first_slave_rcu(bond);
port = slave ? &(SLAVE_AD_INFO(slave).port) : NULL;
- // select the active aggregator for the bond
+ /* select the active aggregator for the bond */
if (port) {
if (!port->slave) {
pr_warning("%s: Warning: bond's first port is uninitialized\n",
@@ -2093,8 +2101,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
bond_3ad_set_carrier(bond);
}
- // for each port run the state machines
- bond_for_each_slave(bond, slave, iter) {
+ /* for each port run the state machines */
+ bond_for_each_slave_rcu(bond, slave, iter) {
port = &(SLAVE_AD_INFO(slave).port);
if (!port->slave) {
pr_warning("%s: Warning: Found an uninitialized port\n",
@@ -2114,7 +2122,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
ad_mux_machine(port);
ad_tx_machine(port);
- // turn off the BEGIN bit, since we already handled it
+ /* turn off the BEGIN bit, since we already handled it */
if (port->sm_vars & AD_PORT_BEGIN)
port->sm_vars &= ~AD_PORT_BEGIN;
@@ -2122,9 +2130,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
}
re_arm:
+ rcu_read_unlock();
queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
-
- read_unlock(&bond->lock);
}
/**
@@ -2303,7 +2310,9 @@ int bond_3ad_set_carrier(struct bonding *bond)
struct aggregator *active;
struct slave *first_slave;
- first_slave = bond_first_slave(bond);
+ rcu_read_lock();
+ first_slave = bond_first_slave_rcu(bond);
+ rcu_read_unlock();
if (!first_slave)
return 0;
active = __get_active_agg(&(SLAVE_AD_INFO(first_slave).aggregator));
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next v3 9/10] bonding: remove unwanted lock for bond enslave and release
From: Ding Tianhong @ 2013-11-11 12:36 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_change_active_slave() and bond_select_active_slave()
do't need bond lock anymore, so remove the unwanted bond lock
for these two functions.
The bond_select_active_slave() will release and acquire
curr_slave_lock, so the curr_slave_lock need to protect
the function.
In bond enslave and bond release, the bond slave list is also
protected by RTNL, so bond lock is no need to exist, remove
the lock and clean the functions.
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_main.c | 27 ++++++---------------------
1 file changed, 6 insertions(+), 21 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index b48ca55..be163e2 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1579,11 +1579,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
bond_set_carrier(bond);
if (USES_PRIMARY(bond->params.mode)) {
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
bond_select_active_slave(bond);
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
}
pr_info("%s: enslaving %s as a%s interface with a%s link.\n",
@@ -1603,19 +1601,13 @@ err_detach:
bond_hw_addr_flush(bond_dev, slave_dev);
vlan_vids_del_by_dev(slave_dev, bond_dev);
- write_lock_bh(&bond->lock);
if (bond->primary_slave == new_slave)
bond->primary_slave = NULL;
if (bond->curr_active_slave == new_slave) {
- bond_change_active_slave(bond, NULL);
- write_unlock_bh(&bond->lock);
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
+ bond_change_active_slave(bond, NULL);
bond_select_active_slave(bond);
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
- } else {
- write_unlock_bh(&bond->lock);
}
slave_disable_netpoll(new_slave);
@@ -1680,20 +1672,16 @@ static int __bond_release_one(struct net_device *bond_dev,
}
block_netpoll_tx();
- write_lock_bh(&bond->lock);
slave = bond_get_slave_by_dev(bond, slave_dev);
if (!slave) {
/* not a slave of this bond */
pr_info("%s: %s not enslaved\n",
bond_dev->name, slave_dev->name);
- write_unlock_bh(&bond->lock);
unblock_netpoll_tx();
return -EINVAL;
}
- write_unlock_bh(&bond->lock);
-
/* release the slave from its bond */
bond->slave_cnt--;
@@ -1711,6 +1699,7 @@ static int __bond_release_one(struct net_device *bond_dev,
*/
bond_3ad_unbind_slave(slave);
}
+ write_unlock_bh(&bond->lock);
pr_info("%s: releasing %s interface %s\n",
bond_dev->name,
@@ -1733,8 +1722,11 @@ static int __bond_release_one(struct net_device *bond_dev,
if (bond->primary_slave == slave)
bond->primary_slave = NULL;
- if (oldcurrent == slave)
+ if (oldcurrent == slave) {
+ write_lock_bh(&bond->curr_slave_lock);
bond_change_active_slave(bond, NULL);
+ write_unlock_bh(&bond->curr_slave_lock);
+ }
if (bond_is_lb(bond)) {
/* Must be called only after the slave has been
@@ -1742,9 +1734,7 @@ static int __bond_release_one(struct net_device *bond_dev,
* has been cleared (if our_slave == old_current),
* but before a new active slave is selected.
*/
- write_unlock_bh(&bond->lock);
bond_alb_deinit_slave(bond, slave);
- write_lock_bh(&bond->lock);
}
if (all) {
@@ -1755,15 +1745,11 @@ static int __bond_release_one(struct net_device *bond_dev,
* is no concern that another slave add/remove event
* will interfere.
*/
- write_unlock_bh(&bond->lock);
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
bond_select_active_slave(bond);
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
- write_lock_bh(&bond->lock);
}
if (!bond_has_slaves(bond)) {
@@ -1778,7 +1764,6 @@ static int __bond_release_one(struct net_device *bond_dev,
}
}
- write_unlock_bh(&bond->lock);
unblock_netpoll_tx();
synchronize_rcu();
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next v3 5/10] bonding: create bond_first_slave_rcu()
From: Ding Tianhong @ 2013-11-11 12:36 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_first_slave_rcu() will be used to instead of bond_first_slave()
in rcu_read_lock().
So move the struct netdev_adjacent to the netdevice.h and make the
bond_first_slave_rcu() could use the struct.
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bonding.h | 9 +++++++++
include/linux/netdevice.h | 16 ++++++++++++++++
net/core/dev.c | 16 ----------------
3 files changed, 25 insertions(+), 16 deletions(-)
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 77a07a1..5d37606 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -91,6 +91,15 @@
netdev_adjacent_get_private(bond_slave_list(bond)->prev) : \
NULL)
+/* Caller must have rcu_read_lock */
+#define bond_first_slave_rcu(bond) \
+ ({struct list_head *__ptr = (bond_slave_list(bond)); \
+ struct list_head *__next = ACCESS_ONCE(__ptr->next); \
+ likely(__ptr != __next) ? \
+ list_entry_rcu(__next, struct netdev_adjacent, list)->private : \
+ NULL; \
+ })
+
#define bond_is_first_slave(bond, pos) (pos == bond_first_slave(bond))
#define bond_is_last_slave(bond, pos) (pos == bond_last_slave(bond))
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 15fa01c..7ca0fc8 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2839,6 +2839,22 @@ extern int netdev_tstamp_prequeue;
extern int weight_p;
extern int bpf_jit_enable;
+struct netdev_adjacent {
+ struct net_device *dev;
+
+ /* upper master flag, there can only be one master device per list */
+ bool master;
+
+ /* counter for the number of times this device was added to us */
+ u16 ref_nr;
+
+ /* private field for the users */
+ void *private;
+
+ struct list_head list;
+ struct rcu_head rcu;
+};
+
bool netdev_has_upper_dev(struct net_device *dev, struct net_device *upper_dev);
bool netdev_has_any_upper_dev(struct net_device *dev);
struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev,
diff --git a/net/core/dev.c b/net/core/dev.c
index 8ffc52e..bcc5001 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4377,22 +4377,6 @@ softnet_break:
goto out;
}
-struct netdev_adjacent {
- struct net_device *dev;
-
- /* upper master flag, there can only be one master device per list */
- bool master;
-
- /* counter for the number of times this device was added to us */
- u16 ref_nr;
-
- /* private field for the users */
- void *private;
-
- struct list_head list;
- struct rcu_head rcu;
-};
-
static struct netdev_adjacent *__netdev_find_adj_rcu(struct net_device *dev,
struct net_device *adj_dev,
struct list_head *adj_list)
--
1.8.2.1
^ 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