public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: zhuyj <zyjzyj2000@gmail•com>
To: Jay Vosburgh <jay.vosburgh@canonical•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: Tue, 26 Jan 2016 11:19:45 +0800	[thread overview]
Message-ID: <56A6E5D1.7070807@gmail.com> (raw)
In-Reply-To: <7578.1453769012@famine>

On 01/26/2016 08:43 AM, Jay Vosburgh wrote:
> <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);

Thanks a lot.
Because kernel v4.4 needs this kind of patch, I backport this patch from 
net-next to kernel v4.4.

If it is not appropriate, I will revert this patch.

Best Regards!
Zhu Yanjun

>   
>   	slave->new_link = BOND_LINK_NOCHANGE;
>   
>
> 	-J
>
> ---
> 	-Jay Vosburgh, jay.vosburgh@canonical•com

  reply	other threads:[~2016-01-26  3:19 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
2016-01-26  3:19                                   ` zhuyj [this message]
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=56A6E5D1.7070807@gmail.com \
    --to=zyjzyj2000@gmail$(echo .)com \
    --cc=boris.shteinbock@windriver$(echo .)com \
    --cc=emil.s.tantilov@intel$(echo .)com \
    --cc=gospo@cumulusnetworks$(echo .)com \
    --cc=jay.vosburgh@canonical$(echo .)com \
    --cc=mkubecek@suse$(echo .)cz \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=vfalico@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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