| 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: <4AB3E03B.3070205@free.fr>
Date: Fri, 18 Sep 2009 21:32:11 +0200
From: Nicolas de Pesloüan
<nicolas.2p.debian@...e.fr>
To: Jiri Pirko <jpirko@...hat.com>
CC: netdev@...r.kernel.org, davem@...emloft.net, fubar@...ibm.com,
bonding-devel@...ts.sourceforge.net
Subject: Re: [PATCH net-next-2.6] bonding: introduce primary_reselect option
Jiri Pirko a écrit :
> (updated 3)
>
> In some cases there is not desirable to switch back to primary interface when
> it's link recovers and rather stay with currently active one. We need to avoid
> packetloss as much as we can in some cases. This is solved by introducing
> primary_reselect option. Note that enslaved primary slave is set as current
> active no matter what.
>
> Signed-off-by: Jiri Pirko <jpirko@...hat.com>
>
> diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
> index d5181ce..fd650e0 100644
> --- a/Documentation/networking/bonding.txt
> +++ b/Documentation/networking/bonding.txt
> @@ -614,6 +614,32 @@ primary
>
> The primary option is only valid for active-backup mode.
>
> +primary_reselect
> +
> + Specifies the behavior of the current active slave when the primary was
> + down and comes back up. This option is designed to prevent
> + flip-flopping between the primary slave and other slaves. The possible
> + values and their respective effects are:
> +
> + always or 0 (default)
> +
> + The primary slave becomes the active slave whenever it comes
> + back up.
> +
> + better or 1
> +
> + The primary slave becomes the active slave when it comes back
> + up, if the speed and duplex of the primary slave is better
> + than the speed and duplex of the current active slave.
> +
> + failure or 2
> +
> + The primary slave becomes the active slave only if the current
> + active slave fails and the primary slave is up.
> +
> + When no slave are active, if the primary comes back up, it becomes the
> + active slave, regardless of the value of primary_reselect.
> +
> updelay
>
> Specifies the time, in milliseconds, to wait before enabling a
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 699bfdd..1127361 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -94,6 +94,7 @@ static int downdelay;
> static int use_carrier = 1;
> static char *mode;
> static char *primary;
> +static char *primary_reselect;
> static char *lacp_rate;
> static char *ad_select;
> static char *xmit_hash_policy;
> @@ -126,6 +127,13 @@ MODULE_PARM_DESC(mode, "Mode of operation : 0 for balance-rr, "
> "6 for balance-alb");
> module_param(primary, charp, 0);
> MODULE_PARM_DESC(primary, "Primary network device to use");
> +module_param(primary_reselect, charp, 0);
> +MODULE_PARM_DESC(primary_reselect, "Reselect primary slave "
> + "once it comes up; "
> + "0 for always (default), "
> + "1 for only if speed of primary is not "
> + "better, "
> + "2 for never");
You should remove "not" for option value 1 and use the word failure for option
value 2.
MODULE_PARM_DESC(primary_reselect, "Reselect primary slave "
"once it comes up; "
"0 for always (default), "
"1 for only if speed of primary is "
"better, "
"2 for only on active slave "
"failure");
Apart from this small detail, this sounds good for me.
> module_param(lacp_rate, charp, 0);
> MODULE_PARM_DESC(lacp_rate, "LACPDU tx rate to request from 802.3ad partner "
> "(slow/fast)");
> @@ -200,6 +208,13 @@ const struct bond_parm_tbl fail_over_mac_tbl[] = {
> { NULL, -1},
> };
>
> +const struct bond_parm_tbl pri_reselect_tbl[] = {
> +{ "always", BOND_PRI_RESELECT_ALWAYS},
> +{ "better", BOND_PRI_RESELECT_BETTER},
> +{ "failure", BOND_PRI_RESELECT_FAILURE},
> +{ NULL, -1},
> +};
> +
> struct bond_parm_tbl ad_select_tbl[] = {
> { "stable", BOND_AD_STABLE},
> { "bandwidth", BOND_AD_BANDWIDTH},
> @@ -1070,6 +1085,25 @@ out:
>
> }
>
> +static bool bond_should_change_active(struct bonding *bond)
> +{
> + struct slave *prim = bond->primary_slave;
> + struct slave *curr = bond->curr_active_slave;
> +
> + if (!prim || !curr || curr->link != BOND_LINK_UP)
> + return true;
> + if (bond->force_primary) {
> + bond->force_primary = false;
> + return true;
> + }
> + if (bond->params.primary_reselect == BOND_PRI_RESELECT_BETTER &&
> + (prim->speed < curr->speed ||
> + (prim->speed == curr->speed && prim->duplex <= curr->duplex)))
> + return false;
> + if (bond->params.primary_reselect == BOND_PRI_RESELECT_FAILURE)
> + return false;
> + return true;
> +}
>
> /**
> * find_best_interface - select the best available slave to be the active one
> @@ -1094,7 +1128,8 @@ static struct slave *bond_find_best_slave(struct bonding *bond)
> }
>
> if ((bond->primary_slave) &&
> - bond->primary_slave->link == BOND_LINK_UP) {
> + bond->primary_slave->link == BOND_LINK_UP &&
> + bond_should_change_active(bond)) {
> new_active = bond->primary_slave;
> }
>
> @@ -1675,8 +1710,10 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>
> if (USES_PRIMARY(bond->params.mode) && bond->params.primary[0]) {
> /* if there is a primary slave, remember it */
> - if (strcmp(bond->params.primary, new_slave->dev->name) == 0)
> + if (strcmp(bond->params.primary, new_slave->dev->name) == 0) {
> bond->primary_slave = new_slave;
> + bond->force_primary = true;
> + }
> }
>
> write_lock_bh(&bond->curr_slave_lock);
> @@ -4643,7 +4680,7 @@ int bond_parse_parm(const char *buf, const struct bond_parm_tbl *tbl)
>
> static int bond_check_params(struct bond_params *params)
> {
> - int arp_validate_value, fail_over_mac_value;
> + int arp_validate_value, fail_over_mac_value, primary_reselect_value;
>
> /*
> * Convert string parameters.
> @@ -4942,6 +4979,20 @@ static int bond_check_params(struct bond_params *params)
> primary = NULL;
> }
>
> + if (primary && primary_reselect) {
> + primary_reselect_value = bond_parse_parm(primary_reselect,
> + pri_reselect_tbl);
> + if (primary_reselect_value == -1) {
> + pr_err(DRV_NAME
> + ": Error: Invalid primary_reselect \"%s\"\n",
> + primary_reselect ==
> + NULL ? "NULL" : primary_reselect);
> + return -EINVAL;
> + }
> + } else {
> + primary_reselect_value = BOND_PRI_RESELECT_ALWAYS;
> + }
> +
> if (fail_over_mac) {
> fail_over_mac_value = bond_parse_parm(fail_over_mac,
> fail_over_mac_tbl);
> @@ -4973,6 +5024,7 @@ static int bond_check_params(struct bond_params *params)
> params->use_carrier = use_carrier;
> params->lacp_fast = lacp_fast;
> params->primary[0] = 0;
> + params->primary_reselect = primary_reselect_value;
> params->fail_over_mac = fail_over_mac_value;
>
> if (primary) {
> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
> index 6044e12..42c44f2 100644
> --- a/drivers/net/bonding/bond_sysfs.c
> +++ b/drivers/net/bonding/bond_sysfs.c
> @@ -1212,6 +1212,61 @@ static DEVICE_ATTR(primary, S_IRUGO | S_IWUSR,
> bonding_show_primary, bonding_store_primary);
>
> /*
> + * Show and set the primary_reselect flag.
> + */
> +static ssize_t bonding_show_primary_reselect(struct device *d,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct bonding *bond = to_bond(d);
> +
> + return sprintf(buf, "%s %d\n",
> + pri_reselect_tbl[bond->params.primary_reselect].modename,
> + bond->params.primary_reselect);
> +}
> +
> +static ssize_t bonding_store_primary_reselect(struct device *d,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int new_value, ret = count;
> + struct bonding *bond = to_bond(d);
> +
> + if (!rtnl_trylock())
> + return restart_syscall();
> +
> + new_value = bond_parse_parm(buf, pri_reselect_tbl);
> + if (new_value < 0) {
> + pr_err(DRV_NAME
> + ": %s: Ignoring invalid primary_reselect value %.*s.\n",
> + bond->dev->name,
> + (int) strlen(buf) - 1, buf);
> + ret = -EINVAL;
> + goto out;
> + } else {
> + bond->params.primary_reselect = new_value;
> + pr_info(DRV_NAME ": %s: setting primary_reselect to %s (%d).\n",
> + bond->dev->name, pri_reselect_tbl[new_value].modename,
> + new_value);
> + if (new_value == BOND_PRI_RESELECT_ALWAYS ||
> + new_value == BOND_PRI_RESELECT_BETTER) {
> + bond->force_primary = true;
> + 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);
> + }
> + }
> +out:
> + rtnl_unlock();
> + return ret;
> +}
> +static DEVICE_ATTR(primary_reselect, S_IRUGO | S_IWUSR,
> + bonding_show_primary_reselect,
> + bonding_store_primary_reselect);
> +
> +/*
> * Show and set the use_carrier flag.
> */
> static ssize_t bonding_show_carrier(struct device *d,
> @@ -1500,6 +1555,7 @@ static struct attribute *per_bond_attrs[] = {
> &dev_attr_num_unsol_na.attr,
> &dev_attr_miimon.attr,
> &dev_attr_primary.attr,
> + &dev_attr_primary_reselect.attr,
> &dev_attr_use_carrier.attr,
> &dev_attr_active_slave.attr,
> &dev_attr_mii_status.attr,
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index 6824771..b5b1530 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -131,6 +131,7 @@ struct bond_params {
> int lacp_fast;
> int ad_select;
> char primary[IFNAMSIZ];
> + int primary_reselect;
> __be32 arp_targets[BOND_MAX_ARP_TARGETS];
> };
>
> @@ -190,6 +191,7 @@ struct bonding {
> struct slave *curr_active_slave;
> struct slave *current_arp_slave;
> struct slave *primary_slave;
> + bool force_primary;
> s32 slave_cnt; /* never change this value outside the attach/detach wrappers */
> rwlock_t lock;
> rwlock_t curr_slave_lock;
> @@ -258,6 +260,10 @@ static inline bool bond_is_lb(const struct bonding *bond)
> || bond->params.mode == BOND_MODE_ALB;
> }
>
> +#define BOND_PRI_RESELECT_ALWAYS 0
> +#define BOND_PRI_RESELECT_BETTER 1
> +#define BOND_PRI_RESELECT_FAILURE 2
> +
> #define BOND_FOM_NONE 0
> #define BOND_FOM_ACTIVE 1
> #define BOND_FOM_FOLLOW 2
> @@ -348,6 +354,7 @@ extern const struct bond_parm_tbl bond_mode_tbl[];
> extern const struct bond_parm_tbl xmit_hashtype_tbl[];
> extern const struct bond_parm_tbl arp_validate_tbl[];
> extern const struct bond_parm_tbl fail_over_mac_tbl[];
> +extern const struct bond_parm_tbl pri_reselect_tbl[];
> extern struct bond_parm_tbl ad_select_tbl[];
>
> #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
>
Nicolas.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists