| 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: <20090814105208.GD3457@psychotron.englab.brq.redhat.com>
Date: Fri, 14 Aug 2009 12:52:09 +0200
From: Jiri Pirko <jpirko@...hat.com>
To: Jay Vosburgh <fubar@...ibm.com>
Cc: davem@...emloft.net, bonding-devel@...ts.sourceforge.net,
netdev@...r.kernel.org
Subject: Re: [PATCH net-next-2.6] bonding: introduce primary_lazy option
Thu, Aug 13, 2009 at 05:44:55PM CEST, fubar@...ibm.com wrote:
>Jiri Pirko <jpirko@...hat.com> wrote:
>
>>In some cases there is not desirable to switch back to primary interface when
>>it's link recovers and rather stay wiith currently active one. We need to avoid
>>packetloss as much as we can in some cases. This is solved by introducing
>>primary_lazy option. Note that enslaved primary slave is set as current
>>active no matter what.
>
> Are you just looking for a way to insure that, when the bond
>first comes up (is configured at boot, for example), a particular slave
>is the active slave? In that case, why can't an explicit selection be
>made via either ifenslave -c or /sys/class/net/bond0/bonding/active ?
Indeed this can be done this way and I'm aware of it. I'm leaving aside that you
must put this somewhere into init scripts which brings downsides.
But imagine you have bond with 3 slaves:
eth0 eth1 eth2
UP(curr) UP UP
DOWN UP(curr) UP
UP UP(curr) UP
UP DOWN UP(curr)
eth2 ends up being current active but we prefer eth0 (as primary interface).
This is not desirable and is solved by primary_lazy option.
Jirka
>
> -J
>
>>Signed-off-by: Jiri Pirko <jpirko@...hat.com>
>>
>>diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
>>index d5181ce..f1b82af 100644
>>--- a/Documentation/networking/bonding.txt
>>+++ b/Documentation/networking/bonding.txt
>>@@ -614,6 +614,15 @@ primary
>>
>> The primary option is only valid for active-backup mode.
>>
>>+primary_lazy
>>+
>>+ Specifies the behaviour of the primary slave in case of
>>+ it's link recovery has been detected. By default (value 0) the
>>+ primary slave is set as active slave immediately after the link
>>+ recovery. If the value is 1 then current active slave doesn't
>>+ change as long as it's link status doesn't change. This prevents
>>+ the bonding device from flip-flopping.
>>+
>> 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 3bf0cc6..00fbb9d 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 int primary_lazy;
>> static char *lacp_rate;
>> static char *ad_select;
>> static char *xmit_hash_policy;
>>@@ -126,6 +127,9 @@ 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_lazy, int, 0);
>>+MODULE_PARM_DESC(primary_lazy, "Do not set primary slave active once it comes up; "
>>+ "0 for off (default), 1 for on");
>> module_param(lacp_rate, charp, 0);
>> MODULE_PARM_DESC(lacp_rate, "LACPDU tx rate to request from 802.3ad partner "
>> "(slow/fast)");
>>@@ -1067,7 +1071,6 @@ out:
>>
>> }
>>
>>-
>> /**
>> * find_best_interface - select the best available slave to be the active one
>> * @bond: our bonding struct
>>@@ -1097,9 +1100,11 @@ static struct slave *bond_find_best_slave(struct bonding *bond)
>> * and primary_slave that may be up and able to arp
>> */
>> if ((bond->primary_slave) &&
>>- (!bond->params.arp_interval) &&
>>- (IS_UP(bond->primary_slave->dev))) {
>>+ (IS_UP(bond->primary_slave->dev)) &&
>>+ (!(bond->params.primary_lazy && old_active &&
>>+ (IS_UP(old_active->dev))) || bond->force_primary)) {
>> new_active = bond->primary_slave;
>>+ bond->force_primary = false;
>> }
>>
>> /* remember where to stop iterating over the slaves */
>>@@ -1674,8 +1679,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);
>>@@ -2929,7 +2936,9 @@ static int bond_ab_arp_inspect(struct bonding *bond, int delta_in_ticks)
>> */
>> if (bond->primary_slave &&
>> (bond->primary_slave != bond->curr_active_slave) &&
>>- (bond->primary_slave->link == BOND_LINK_UP))
>>+ (bond->primary_slave->link == BOND_LINK_UP) &&
>>+ !(bond->params.primary_lazy && bond->curr_active_slave &&
>>+ bond->curr_active_slave->link == BOND_LINK_UP))
>> commit++;
>>
>> read_unlock(&bond->curr_slave_lock);
>>@@ -3035,7 +3044,9 @@ static void bond_ab_arp_commit(struct bonding *bond, int delta_in_ticks)
>> */
>> if (bond->primary_slave &&
>> (bond->primary_slave != bond->curr_active_slave) &&
>>- (bond->primary_slave->link == BOND_LINK_UP)) {
>>+ (bond->primary_slave->link == BOND_LINK_UP) &&
>>+ !(bond->params.primary_lazy && bond->curr_active_slave &&
>>+ bond->curr_active_slave->link == BOND_LINK_UP)) {
>> write_lock_bh(&bond->curr_slave_lock);
>> bond_change_active_slave(bond, bond->primary_slave);
>> write_unlock_bh(&bond->curr_slave_lock);
>>@@ -4987,6 +4998,17 @@ static int bond_check_params(struct bond_params *params)
>> primary = NULL;
>> }
>>
>>+ if (primary) {
>>+ if ((primary_lazy != 0) && (primary_lazy != 1)) {
>>+ pr_warning(DRV_NAME
>>+ ": Warning: primary_lazy module parameter "
>>+ "(%d), not of valid value (0/1), so it was "
>>+ "set to 0\n",
>>+ primary_lazy);
>>+ primary_lazy = 1;
>>+ }
>>+ }
>>+
>> if (fail_over_mac) {
>> fail_over_mac_value = bond_parse_parm(fail_over_mac,
>> fail_over_mac_tbl);
>>@@ -5018,6 +5040,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_lazy = primary_lazy;
>> 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 55bf34f..573fe82 100644
>>--- a/drivers/net/bonding/bond_sysfs.c
>>+++ b/drivers/net/bonding/bond_sysfs.c
>>@@ -1209,6 +1209,59 @@ static DEVICE_ATTR(primary, S_IRUGO | S_IWUSR,
>> bonding_show_primary, bonding_store_primary);
>>
>> /*
>>+ * Show and set the primary_lazy flag.
>>+ */
>>+static ssize_t bonding_show_primary_lazy(struct device *d,
>>+ struct device_attribute *attr,
>>+ char *buf)
>>+{
>>+ struct bonding *bond = to_bond(d);
>>+
>>+ return sprintf(buf, "%d\n", bond->params.primary_lazy);
>>+}
>>+
>>+static ssize_t bonding_store_primary_lazy(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();
>>+
>>+ if (sscanf(buf, "%d", &new_value) != 1) {
>>+ pr_err(DRV_NAME
>>+ ": %s: no primary_lazy value specified.\n",
>>+ bond->dev->name);
>>+ ret = -EINVAL;
>>+ goto out;
>>+ }
>>+ if ((new_value == 0) || (new_value == 1)) {
>>+ bond->params.primary_lazy = new_value;
>>+ pr_info(DRV_NAME ": %s: Setting primary_lazy to %d.\n",
>>+ bond->dev->name, new_value);
>>+ if (!bond->params.primary_lazy) {
>>+ 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);
>>+ }
>>+ } else {
>>+ pr_info(DRV_NAME
>>+ ": %s: Ignoring invalid primary_lazy value %d.\n",
>>+ bond->dev->name, new_value);
>>+ }
>>+out:
>>+ rtnl_unlock();
>>+ return count;
>>+}
>>+static DEVICE_ATTR(primary_lazy, S_IRUGO | S_IWUSR,
>>+ bonding_show_primary_lazy, bonding_store_primary_lazy);
>>+
>>+/*
>> * Show and set the use_carrier flag.
>> */
>> static ssize_t bonding_show_carrier(struct device *d,
>>@@ -1497,6 +1550,7 @@ static struct attribute *per_bond_attrs[] = {
>> &dev_attr_num_unsol_na.attr,
>> &dev_attr_miimon.attr,
>> &dev_attr_primary.attr,
>>+ &dev_attr_primary_lazy.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 6290a50..ac35c6c 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_lazy;
>> __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;
>
>---
> -Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com
--
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