| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <568F8CE3.8040406@gmail.com>
Date: Fri, 8 Jan 2016 18:18:11 +0800
From: zhuyj <zyjzyj2000@...il.com>
To: jay.vosburgh@...onical.com
Cc: emil.s.tantilov@...el.com, mkubecek@...e.cz, vfalico@...il.com,
gospo@...ulusnetworks.com, netdev@...r.kernel.org,
boris.shteinbock@...driver.com
Subject: Re: [PATCH 1/1] bonding: utilize notifier callbacks to detect slave
link state changes
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@...il.com wrote:
> From: Zhu Yanjun <yanjun.zhu@...driver.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@...driver.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
Powered by blists - more mailing lists