From: Jay Vosburgh <jay.vosburgh@canonical.com>
To: zyjzyj2000@gmail.com
Cc: mkubecek@suse.cz, vfalico@gmail.com, gospo@cumulusnetworks.com,
netdev@vger.kernel.org, boris.shteinbock@windriver.com,
emil.s.tantilov@intel.com
Subject: Re: [PATCH 1/1] bonding: Use notifiers for slave link state detection
Date: Mon, 25 Jan 2016 16:43:32 -0800 [thread overview]
Message-ID: <7578.1453769012@famine> (raw)
In-Reply-To: <1453371388-30230-2-git-send-email-zyjzyj2000@gmail.com>
<zyjzyj2000@gmail.com> wrote:
>From: Zhu Yanjun <zyjzyj2000@gmail.com>
>
>Bonding will utilize notifier callbacks to detect slave
>link state changes. It is intended to be used with miimon
>set to zero, and does not support the updelay or downdelay
>options to bonding.
>
>Because of link flap from the slave interface, if the notifier
>is NETDEV_UP while the actual link state is down, it is not
>necessary to continue.
>
>Signed-off-by: Jay Vosburgh <jay.vosburgh@canonical.com>
I haven't signed off on this patch.
I've just started some testing, but as before immediately get an
RCU warning; it looks to be coming from bond_miimon_inspect_slave();
[ 316.473050] bond1: Enslaving eth1 as a backup interface with an up link
[ 316.473059]
[ 316.473806] ===============================
[ 316.475630] [ INFO: suspicious RCU usage. ]
[ 316.477519] 4.4.0+ #38 Not tainted
[ 316.479094] -------------------------------
[ 316.480765] drivers/net/bonding/bond_main.c:2024 suspicious rcu_dereference_check() usage!
This is presumably because the "case NETDEV_DOWN" call to
bond_miimon_inspect_slave does not hold RCU. It does hold RTNL, though,
which should be safe for this usage (RTNL mutexes changes to the active
slave). The appended patch on top of the original makes the warning go
away.
I'm still testing the patch and have no comment about its
functionality as yet.
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 9f67948..e3faee9 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2014,14 +2014,14 @@ static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *in
/*-------------------------------- Monitoring -------------------------------*/
-/* called with rcu_read_lock() */
+/* called with rcu_read_lock() or RTNL */
static int bond_miimon_inspect_slave(struct bonding *bond, struct slave *slave,
unsigned long event)
{
int link_state;
bool ignore_updelay;
- ignore_updelay = !rcu_dereference(bond->curr_active_slave);
+ ignore_updelay = !rcu_dereference_rtnl(bond->curr_active_slave);
slave->new_link = BOND_LINK_NOCHANGE;
-J
---
-Jay Vosburgh, jay.vosburgh@canonical.com
next prev parent reply other threads:[~2016-01-26 0:43 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-17 8:03 [PATCH 1/1] bonding: restrict up state in 802.3ad mode zyjzyj2000
2015-12-17 21:57 ` Jay Vosburgh
2015-12-18 4:36 ` zyjzyj2000
2015-12-18 4:36 ` [PATCH 1/1] bonding: delay up state without speed and duplex " zyjzyj2000
2015-12-18 4:54 ` Jay Vosburgh
2015-12-18 13:37 ` Sergei Shtylyov
2015-12-28 8:43 ` [PATCH 1/1] bonding: restrict up state " Michal Kubecek
2015-12-28 9:19 ` zhuyj
2016-01-06 1:26 ` Tantilov, Emil S
2016-01-06 3:05 ` zhuyj
2016-01-07 2:43 ` Tantilov, Emil S
2016-01-07 3:33 ` zhuyj
2016-01-07 5:02 ` Tantilov, Emil S
2016-01-07 6:15 ` zyjzyj2000
2016-01-07 6:22 ` zhuyj
2016-01-07 6:33 ` Jay Vosburgh
2016-01-07 15:27 ` Tantilov, Emil S
2016-01-08 1:28 ` [RFC PATCH net-next] bonding: Use notifiers for slave link state detection Jay Vosburgh
2016-01-08 4:36 ` zhuyj
2016-01-08 6:12 ` Jay Vosburgh
2016-01-08 7:41 ` (unknown), zyjzyj2000
2016-01-08 7:41 ` [PATCH 1/1] bonding: utilize notifier callbacks to detect slave link state changes zyjzyj2000
2016-01-08 10:18 ` zhuyj
2016-01-09 1:35 ` [RFC PATCH net-next] bonding: Use notifiers for slave link state detection Tantilov, Emil S
2016-01-09 2:19 ` Jay Vosburgh
2016-01-11 9:03 ` zhuyj
2016-01-13 2:54 ` zhuyj
2016-01-13 17:03 ` Tantilov, Emil S
2016-01-20 5:13 ` [PATCH 1/1] " zyjzyj2000
2016-01-20 5:13 ` zyjzyj2000
2016-01-21 10:16 ` zyjzyj2000
2016-01-21 10:16 ` zyjzyj2000
2016-01-25 16:37 ` Tantilov, Emil S
2016-01-26 0:43 ` Jay Vosburgh [this message]
2016-01-26 3:19 ` zhuyj
2016-01-26 6:00 ` Jay Vosburgh
2016-01-26 6:26 ` zhuyj
2016-01-26 6:45 ` zhuyj
2016-01-27 20:00 ` Tantilov, Emil S
2016-01-28 8:44 ` zyjzyj2000
2016-01-29 7:05 ` zhuyj
2016-01-25 16:33 ` Tantilov, Emil S
2016-01-25 18:00 ` David Miller
2016-01-25 18:37 ` Tantilov, Emil S
2016-01-08 2:29 ` [PATCH 1/1] bonding: restrict up state in 802.3ad mode zhuyj
2016-01-07 6:53 ` Michal Kubecek
2016-01-07 7:37 ` zhuyj
2016-01-07 7:59 ` Michal Kubecek
2016-01-07 8:35 ` zhuyj
2016-01-07 7:47 ` zhuyj
2016-01-07 18:28 ` Tantilov, Emil S
2016-01-08 6:09 ` zhuyj
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7578.1453769012@famine \
--to=jay.vosburgh@canonical.com \
--cc=boris.shteinbock@windriver.com \
--cc=emil.s.tantilov@intel.com \
--cc=gospo@cumulusnetworks.com \
--cc=mkubecek@suse.cz \
--cc=netdev@vger.kernel.org \
--cc=vfalico@gmail.com \
--cc=zyjzyj2000@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).