From: zhuyj <zyjzyj2000@gmail•com>
To: jay.vosburgh@canonical•com
Cc: emil.s.tantilov@intel•com, mkubecek@suse•cz, vfalico@gmail•com,
gospo@cumulusnetworks•com, netdev@vger•kernel.org,
boris.shteinbock@windriver•com
Subject: Re: [PATCH 1/1] bonding: utilize notifier callbacks to detect slave link state changes
Date: Fri, 8 Jan 2016 18:18:11 +0800 [thread overview]
Message-ID: <568F8CE3.8040406@gmail.com> (raw)
In-Reply-To: <1452238910-13277-2-git-send-email-zyjzyj2000@gmail.com>
Hi, Jay
I delved into your test patch. I noticed that bond_set_slave_link_state
would call bond_netdev_notify_work.
And your patch is based on netdev notifier.
Will it result into a notifier loop? That is, bonding driver receives
notifier, then bonding driver sends notifier.
In the end, there are more and more notifier.
How do you think about this?
Thanks a lot.
Zhu Yanjun
On 01/08/2016 03:41 PM, zyjzyj2000@gmail•com wrote:
> From: Zhu Yanjun <yanjun.zhu@windriver•com>
>
> This patch modifies bonding to 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. It's not as complicated as it looks; most of the change set is
> to break out the inner loop of bond_miimon_inspect into its own
> function.
>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver•com>
> ---
> drivers/net/bonding/bond_main.c | 154 ++++++++++++++++++++-------------------
> 1 file changed, 78 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 9e0f8a7..9a0e69e 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1977,101 +1977,100 @@ static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *in
> /*-------------------------------- Monitoring -------------------------------*/
>
> /* called with rcu_read_lock() */
> -static int bond_miimon_inspect(struct bonding *bond)
> +static int bond_miimon_inspect_slave(struct bonding *bond, struct slave *slave)
> {
> - int link_state, commit = 0;
> - struct list_head *iter;
> - struct slave *slave;
> + int link_state;
> bool ignore_updelay;
>
> ignore_updelay = !rcu_dereference(bond->curr_active_slave);
>
> - bond_for_each_slave_rcu(bond, slave, iter) {
> - slave->new_link = BOND_LINK_NOCHANGE;
> -
> - link_state = bond_check_dev_link(bond, slave->dev, 0);
> + slave->new_link = BOND_LINK_NOCHANGE;
>
> - switch (slave->link) {
> - case BOND_LINK_UP:
> - if (link_state)
> - continue;
> + link_state = bond_check_dev_link(bond, slave->dev, 0);
>
> - bond_set_slave_link_state(slave, BOND_LINK_FAIL);
> - slave->delay = bond->params.downdelay;
> - if (slave->delay) {
> - netdev_info(bond->dev, "link status down for %sinterface %s, disabling it in %d ms\n",
> - (BOND_MODE(bond) ==
> - BOND_MODE_ACTIVEBACKUP) ?
> - (bond_is_active_slave(slave) ?
> - "active " : "backup ") : "",
> - slave->dev->name,
> - bond->params.downdelay * bond->params.miimon);
> - }
> - /*FALLTHRU*/
> - case BOND_LINK_FAIL:
> - if (link_state) {
> - /* recovered before downdelay expired */
> - bond_set_slave_link_state(slave, BOND_LINK_UP);
> - slave->last_link_up = jiffies;
> - netdev_info(bond->dev, "link status up again after %d ms for interface %s\n",
> - (bond->params.downdelay - slave->delay) *
> - bond->params.miimon,
> - slave->dev->name);
> - continue;
> - }
> + switch (slave->link) {
> + case BOND_LINK_UP:
> + if (link_state)
> + return 0;
>
> - if (slave->delay <= 0) {
> - slave->new_link = BOND_LINK_DOWN;
> - commit++;
> - continue;
> - }
> + bond_set_slave_link_state(slave, BOND_LINK_FAIL);
> + slave->delay = bond->params.downdelay;
> + if (slave->delay) {
> + netdev_info(bond->dev, "link status down for %sinterface %s, disabling it in %d ms\n",
> + (BOND_MODE(bond) == BOND_MODE_ACTIVEBACKUP) ?
> + (bond_is_active_slave(slave) ?
> + "active " : "backup ") : "",
> + slave->dev->name,
> + bond->params.downdelay * bond->params.miimon);
> + }
> + /*FALLTHRU*/
> + case BOND_LINK_FAIL:
> + if (link_state) {
> + /* recovered before downdelay expired */
> + bond_set_slave_link_state(slave, BOND_LINK_UP);
> + slave->last_link_up = jiffies;
> + netdev_info(bond->dev, "link status up again after %d ms for interface %s\n",
> + (bond->params.downdelay - slave->delay) *
> + bond->params.miimon, slave->dev->name);
> + return 0;
> + }
>
> - slave->delay--;
> - break;
> + if (slave->delay <= 0) {
> + slave->new_link = BOND_LINK_DOWN;
> + return 1;
> + }
>
> - case BOND_LINK_DOWN:
> - if (!link_state)
> - continue;
> + slave->delay--;
> + break;
>
> - bond_set_slave_link_state(slave, BOND_LINK_BACK);
> - slave->delay = bond->params.updelay;
> + case BOND_LINK_DOWN:
> + if (!link_state)
> + return 0;
>
> - if (slave->delay) {
> - netdev_info(bond->dev, "link status up for interface %s, enabling it in %d ms\n",
> - slave->dev->name,
> - ignore_updelay ? 0 :
> - bond->params.updelay *
> - bond->params.miimon);
> - }
> - /*FALLTHRU*/
> - case BOND_LINK_BACK:
> - if (!link_state) {
> - bond_set_slave_link_state(slave,
> - BOND_LINK_DOWN);
> - netdev_info(bond->dev, "link status down again after %d ms for interface %s\n",
> - (bond->params.updelay - slave->delay) *
> - bond->params.miimon,
> - slave->dev->name);
> + bond_set_slave_link_state(slave, BOND_LINK_BACK);
> + slave->delay = bond->params.updelay;
>
> - continue;
> - }
> + if (slave->delay) {
> + netdev_info(bond->dev, "link status up for interface %s, enabling it in %d ms\n",
> + slave->dev->name, ignore_updelay ? 0 :
> + bond->params.updelay * bond->params.miimon);
> + }
> + /*FALLTHRU*/
> + case BOND_LINK_BACK:
> + if (!link_state) {
> + bond_set_slave_link_state(slave, BOND_LINK_DOWN);
> + netdev_info(bond->dev, "link status down again after %d ms for interface %s\n",
> + (bond->params.updelay - slave->delay) *
> + bond->params.miimon, slave->dev->name);
>
> - if (ignore_updelay)
> - slave->delay = 0;
> + return 0;
> + }
>
> - if (slave->delay <= 0) {
> - slave->new_link = BOND_LINK_UP;
> - commit++;
> - ignore_updelay = false;
> - continue;
> - }
> + if (ignore_updelay)
> + slave->delay = 0;
>
> - slave->delay--;
> - break;
> + if (slave->delay <= 0) {
> + slave->new_link = BOND_LINK_UP;
> + return 1;
> }
> +
> + slave->delay--;
> + break;
> }
>
> - return commit;
> + return 0;
> +}
> +
> +static int bond_miimon_inspect(struct bonding *bond)
> +{
> + struct list_head *iter;
> + struct slave *slave;
> + int commit = 0;
> +
> + bond_for_each_slave_rcu(bond, slave, iter)
> + commit += bond_miimon_inspect_slave(bond, slave);
> +
> + return commit;
> }
>
> static void bond_miimon_commit(struct bonding *bond)
> @@ -2969,6 +2968,9 @@ static int bond_slave_netdev_event(unsigned long event,
> bond_3ad_adapter_speed_duplex_changed(slave);
> /* Fallthrough */
> case NETDEV_DOWN:
> + if (bond_miimon_inspect_slave(bond, slave))
> + bond_miimon_commit(bond);
> +
> /* Refresh slave-array if applicable!
> * If the setup does not use miimon or arpmon (mode-specific!),
> * then these events will not cause the slave-array to be
next prev parent reply other threads:[~2016-01-08 10:18 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 [this message]
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
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=568F8CE3.8040406@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