* [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
* Re: bridge not getting ip since 3.11.5 and 3.4.66
From: Mark Trompell @ 2013-11-11 12:52 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: Linux-Kernel, netdev
In-Reply-To: <CAE3RKNng9A6Bp7wa4+6iCprwEQpzGueagLc8iob+aFabJW4nnw@mail.gmail.com>
On Mon, Nov 11, 2013 at 12:07 PM, Veaceslav Falico <veaceslav@falico.eu> wrote:
> 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?
>
Actually this is my desktop machine using kvm for a virtual machine
that uses eth0 which is connected to the bridge
which is used as interface for the host.
$ ip addr
2. eth0: <BROADCAST,MULTICAST,UP,10000> mtu 1500 qdisc pfifo_fast
master br0 qlen 1000
...
3. br0: <BROADCAST,MULTICAST,UP,10000> mtu 1500 qdisc noqueue
...
Anything else?
Greetings
Mark
^ permalink raw reply
* Re: [PATCH net-next v3 5/10] bonding: create bond_first_slave_rcu()
From: Veaceslav Falico @ 2013-11-11 12:59 UTC (permalink / raw)
To: Ding Tianhong
Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Netdev
In-Reply-To: <5280CF59.5090202@huawei.com>
On Mon, Nov 11, 2013 at 08:36:41PM +0800, Ding Tianhong wrote:
>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.
The whole point in netdev_adjacent functions was to hide it from the users
who wanted to use it directly. See
http://www.spinics.net/lists/netdev/msg248026.html
for reference. Please try to avoid that.
>
>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
* Re: [PATCH net-next v3 0/10] bonding: rebuild the lock use for bond monitor
From: Veaceslav Falico @ 2013-11-11 13:06 UTC (permalink / raw)
To: Ding Tianhong
Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Netdev
In-Reply-To: <5280CF34.20703@huawei.com>
On Mon, Nov 11, 2013 at 08:36:04PM +0800, Ding Tianhong wrote:
>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.
Boot-test *with the same parameters as before* gave me the following
trace[1], which is inevitable in case of mode 1 bonding. So that you've
either ignored this warning or didn't actually test mode 1, even though
your last patchset was reverted because of a regression in the same mode.
How was this tested?
And btw - net-next is closed.
[1]:
[ 13.847032] bonding: bond0: link status definitely up for interface eth2.
[ 13.848732] bonding: bond0: making interface eth2 the new active one.
[ 13.850429] device eth2 entered promiscuous mode
[ 13.852168]
[ 13.853833] ===============================
[ 13.855410] [ INFO: suspicious RCU usage. ]
[ 13.857017] 3.12.0-bond+ #314 Tainted: G I
[ 13.858690] -------------------------------
[ 13.860404] drivers/net/bonding/bond_main.c:818 suspicious rcu_dereference_check() usage!
[ 13.862006]
[ 13.862006] other info that might help us debug this:
[ 13.862006]
[ 13.866334]
[ 13.866334] rcu_scheduler_active = 1, debug_locks = 0
[ 13.869296] 4 locks held by kworker/u8:3/57:
[ 13.870841] #0: (%s#4){.+.+..}, at: [<ffffffff810cfec9>] process_one_work+0x189/0x580
[ 13.872353] #1: ((&(&bond->arp_work)->work)){+.+...}, at: [<ffffffff810cfec9>] process_one_work+0x189/0x580
[ 13.873967] #2: (rtnl_mutex){+.+.+.}, at: [<ffffffff8169e765>] rtnl_trylock+0x15/0x20
[ 13.875569] #3: (&bond->curr_slave_lock){++.+..}, at: [<ffffffffa00b922e>] bond_ab_arp_commit+0x12e/0x200 [bonding]
[ 13.877167]
[ 13.877167] stack backtrace:
[ 13.880287] CPU: 1 PID: 57 Comm: kworker/u8:3 Tainted: G I 3.12.0-bond+ #314
[ 13.882011] Hardware name: Hewlett-Packard HP xw4600 Workstation/0AA0h, BIOS 786F3 v01.15 08/28/2008
[ 13.883585] Workqueue: bond0 bond_activebackup_arp_mon [bonding]
[ 13.885011] 0000000000000001 ffff880079e89be8 ffffffff817a9df8 0000000000000002
[ 13.886564] ffff880079e80000 ffff880079e89c18 ffffffff81128d23 ffff8800790d4b40
[ 13.888179] ffff88007980a400 ffff8800790d4bb8 ffff8800790d4b40 ffff880079e89c38
[ 13.889837] Call Trace:
[ 13.891417] [<ffffffff817a9df8>] dump_stack+0x59/0x81
[ 13.892881] [<ffffffff81128d23>] lockdep_rcu_suspicious+0x103/0x140
[ 13.894290] [<ffffffffa00b8b61>] bond_should_notify_peers+0xb1/0x110 [bonding]
[ 13.895686] [<ffffffffa00b8e59>] bond_change_active_slave+0x299/0x370 [bonding]
[ 13.897118] [<ffffffffa00b9027>] bond_select_active_slave+0xf7/0x1d0 [bonding]
[ 13.898672] [<ffffffffa00b9236>] bond_ab_arp_commit+0x136/0x200 [bonding]
[ 13.900165] [<ffffffffa00bb98d>] bond_activebackup_arp_mon+0x10d/0x340 [bonding]
[ 13.901709] [<ffffffffa00bb8d3>] ? bond_activebackup_arp_mon+0x53/0x340 [bonding]
[ 13.903125] [<ffffffff810cff3a>] process_one_work+0x1fa/0x580
[ 13.904554] [<ffffffff810cfec9>] ? process_one_work+0x189/0x580
[ 13.906023] [<ffffffff810d231f>] worker_thread+0x11f/0x3a0
[ 13.907506] [<ffffffff810d2200>] ? manage_workers+0x170/0x170
[ 13.908931] [<ffffffff810dbdfe>] kthread+0xee/0x100
[ 13.910327] [<ffffffff8112d99b>] ? __lock_release+0x13b/0x1b0
[ 13.911677] [<ffffffff810dbd10>] ? __init_kthread_worker+0x70/0x70
[ 13.913082] [<ffffffff817ba16c>] ret_from_fork+0x7c/0xb0
[ 13.914478] [<ffffffff810dbd10>] ? __init_kthread_worker+0x70/0x70
[ 13.915860] bonding: bond0: first active interface up!
[ 13.917294] bridge0: port 1(bond0) entered forwarding state
[ 13.918632] bridge0: port 1(bond0) entered forwarding state
[ 14.017018] bonding: bond0: link status definitely up for interface eth0.
>
>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
* Re: [PATCHv2] net: wireless: iwlwifi: remove minor dead code
From: Johannes Berg @ 2013-11-11 13:41 UTC (permalink / raw)
To: Michal Nazarewicz
Cc: Sergei Shtylyov, Emmanuel Grumbach, John W. Linville,
Intel Linux Wireless, linux-wireless, netdev, linux-kernel
In-Reply-To: <87iow080zv.fsf@mina86.com>
On Sun, 2013-11-10 at 21:17 +0100, Michal Nazarewicz wrote:
> inta is checked to be zero in a IRQ_NONE branch so afterwards it
> cannot be zero as it is never modified.
no signed-off-by
> @@ -1150,7 +1149,14 @@ static irqreturn_t iwl_pcie_isr(int irq, void *data)
> * or due to sporadic interrupts thrown from our NIC. */
> if (!inta) {
> IWL_DEBUG_ISR(trans, "Ignore interrupt, inta == 0\n");
> - goto none;
> + /* re-enable interrupts here since we don't have anything to
> + * service. only Re-enable if disabled by irq and no
> + * schedules tasklet.
> + */
> + if (test_bit(STATUS_INT_ENABLED, &trans_pcie->status) &&
> + !trans_pcie->inta)
> + iwl_enable_interrupts(trans);
> + return IRQ_NONE;
I don't really think duplicating this is really an improvement?
johannes
^ permalink raw reply
* Re: [PATCHv2] net: wireless: iwlwifi: remove minor dead code
From: Johannes Berg @ 2013-11-11 13:42 UTC (permalink / raw)
To: Michal Nazarewicz
Cc: Sergei Shtylyov, Emmanuel Grumbach, John W. Linville,
Intel Linux Wireless, linux-wireless, netdev, linux-kernel
In-Reply-To: <1384177312.14334.12.camel@jlt4.sipsolutions.net>
On Mon, 2013-11-11 at 14:41 +0100, Johannes Berg wrote:
> > @@ -1150,7 +1149,14 @@ static irqreturn_t iwl_pcie_isr(int irq, void *data)
> > * or due to sporadic interrupts thrown from our NIC. */
> > if (!inta) {
> > IWL_DEBUG_ISR(trans, "Ignore interrupt, inta == 0\n");
> > - goto none;
> > + /* re-enable interrupts here since we don't have anything to
> > + * service. only Re-enable if disabled by irq and no
> > + * schedules tasklet.
> > + */
> > + if (test_bit(STATUS_INT_ENABLED, &trans_pcie->status) &&
> > + !trans_pcie->inta)
> > + iwl_enable_interrupts(trans);
> > + return IRQ_NONE;
>
> I don't really think duplicating this is really an improvement?
Err, sorry, not paying attention.
johannes
^ permalink raw reply
* Re: [PATCH net-next 3/6] random32: add prandom_reseed_late() and call when nonblocking pool becomes initialized
From: Theodore Ts'o @ 2013-11-11 13:43 UTC (permalink / raw)
To: Daniel Borkmann
Cc: davem, shemminger, fweimer, netdev, Hannes Frederic Sowa,
Eric Dumazet
In-Reply-To: <2ea03f60bb65429cbe5d74a6d356fde3eefcf06c.1384160397.git.dborkman@redhat.com>
On Mon, Nov 11, 2013 at 12:20:34PM +0100, Daniel Borkmann wrote:
> 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.
Acked-by: "Theodore Ts'o" <tytso@mit.edu>
I wasn't cc'ed on the full series (I didn't see the 0/3 or the 4/6
messages) but there are two other things that you might want to
consider.
1) I'm pretty sure, but it would be good to get netdev confirmation,
that the call to get_random_bytes() in
net/mac80211/rc80211_minstrel.c's init_sample_table() can be replaced
by calls to prandom_u32().
That is, I don't believe cryptographic strength randomness is needed
--- which is good, because my debugging indicates on a test system
indicates that it gets called so early that there is typically less
than two dozen bits of entropy collected in the non-blocking pool
before it calls get_random_bytes(). If we can move away from using
get_random_bytes(), those two dozen bits of entropy can be used to
make sure the urandom pool is initialized much more quickly.
2) Since the minstrel code apparently uses this information for
initializing a machine learning algorithm for backoff purposes, I
suspect it might be good if the numbers it gets are different from
machine to machine --- and right now prandom_init() does not mix in
any kind of personalization information, so calls to prandom_u32()
will be the same across machines until it gets initialized from the
/dev/random subsysem.
Currently, the way we get personlization information which uniquifies
the randomness in early boot is via add_device_randomness(). Yes,
some of the function names are a bit misleading; maybe we should try
to fix this at some point. So perhaps we should add a hook to
add_device_randomness() so that each time it gets called, if the
random32.c state hasn't been strongly initialized by the call to
prandom_reseed_late(), we also use that information add some per-host
uniqueness into prandom32.c. (Note: I'd prefer that we do this via
some interface other than get_random_bytes(), so we don't end up
draining entropy from the non_blocking pool, and thus delay the point
where we can strongly initialize the non_blocking pool, and thus
strongly initialize prandom32.c)
Does this make sense to folks?
Cheers,
- Ted
^ permalink raw reply
* IPv6: Blackhole route support partial ?
From: Kamala R @ 2013-11-11 13:55 UTC (permalink / raw)
To: davem; +Cc: linux-kernel, netdev
Hi,
On adding IPv6 blackhole routes, ICMP unreachable messages are being
sent back to source. According to the definition, packets destined to
a blackhole address must be dropped silently.
I applied the patch submitted to the 3.7 kernel that indicates that it
supports blackhole and prohibit routes correctly. However, the patch
only sets the error code and route type correctly, so the show command
displays the appropriate output.
It seems to me that the input and output function pointers of the dst
variable, which determine packet processing, need to be set to
dst_discard. This would enable correct behaviour for blackhole routes.
Am I on the right path here ?
Thanks and Regards,
Kamala
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox