| 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: <AANLkTilkujZ-XaPFNtlLp_9el6ABC7ICcxFIL13hDzWM@mail.gmail.com>
Date: Tue, 18 May 2010 08:57:05 -0400
From: Andy Gospodarek <andy@...yhouse.net>
To: Jay Vosburgh <fubar@...ibm.com>
Cc: Neil Horman <nhorman@...driver.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next-2.6 2/2] bonding: allow user-controlled output
slave selection
On Mon, May 17, 2010 at 9:21 PM, Jay Vosburgh <fubar@...ibm.com> wrote:
> Jay Vosburgh <fubar@...ibm.com> wrote:
> [...]
>> For your patch, I'm exploring the idea of not setting
>>IFF_SLAVE_INACTIVE on "inactive" slaves for an "all_slaves_active"
>>option (I think that's a more descriptive name than "keep_all") instead
>>of adding a new KEEP_ALL flag bit to priv_flags. Did you consider this
>>methodology and exclude it for some reason?
>
> Following up to myself, I coded up approximately what I was
> talking about. This doesn't require the extra priv_flag, and the sysfs
> _store is a little more complicated, but this appears to work (testing
> with ping -f after clearing the switch's MAC table to induce traffic
> flooding). I didn't change the option name from "keep_all" here, but as
> far as the functionality goes, this seems to do what I think you want it
> to.
>
I can test it here to be sure, but overall this seems like a fine
approach. The IFF_SLAVE_INACTIVE doesn't seem to be used for much
other than duplicate suppression at this point, so it seems like a
logical choice.
As for the original name 'keep_all,' I tried to use something that
user would find easy to understand. Obviously not all users think
alike. :-)
> -J
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 5e12462..c80d2ff 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -106,6 +106,7 @@ static int arp_interval = BOND_LINK_ARP_INTERV;
> static char *arp_ip_target[BOND_MAX_ARP_TARGETS];
> static char *arp_validate;
> static char *fail_over_mac;
> +static int keep_all = 0;
> static struct bond_params bonding_defaults;
>
> module_param(max_bonds, int, 0);
> @@ -155,6 +156,9 @@ module_param(arp_validate, charp, 0);
> MODULE_PARM_DESC(arp_validate, "validate src/dst of ARP probes: none (default), active, backup or all");
> module_param(fail_over_mac, charp, 0);
> MODULE_PARM_DESC(fail_over_mac, "For active-backup, do not set all slaves to the same MAC. none (default), active or follow");
> +module_param(keep_all, int, 0);
> +MODULE_PARM_DESC(keep_all, "Keep all frames received on an interface"
> + "0 for never (default), 1 for always.");
>
> /*----------------------------- Global variables ----------------------------*/
>
> @@ -4756,6 +4760,13 @@ static int bond_check_params(struct bond_params *params)
> }
> }
>
> + if ((keep_all != 0) && (keep_all != 1)) {
> + pr_warning("Warning: keep_all module parameter (%d), "
> + "not of valid value (0/1), so it was set to "
> + "0\n", keep_all);
> + keep_all = 0;
> + }
> +
> /* reset values for TLB/ALB */
> if ((bond_mode == BOND_MODE_TLB) ||
> (bond_mode == BOND_MODE_ALB)) {
> @@ -4926,6 +4937,7 @@ static int bond_check_params(struct bond_params *params)
> params->primary[0] = 0;
> params->primary_reselect = primary_reselect_value;
> params->fail_over_mac = fail_over_mac_value;
> + params->keep_all = keep_all;
>
> if (primary) {
> strncpy(params->primary, primary, IFNAMSIZ);
> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
> index b8bec08..73b3c03 100644
> --- a/drivers/net/bonding/bond_sysfs.c
> +++ b/drivers/net/bonding/bond_sysfs.c
> @@ -339,7 +339,6 @@ out:
>
> static DEVICE_ATTR(slaves, S_IRUGO | S_IWUSR, bonding_show_slaves,
> bonding_store_slaves);
> -
> /*
> * Show and set the bonding mode. The bond interface must be down to
> * change the mode.
> @@ -1472,6 +1471,59 @@ static ssize_t bonding_show_ad_partner_mac(struct device *d,
> }
> static DEVICE_ATTR(ad_partner_mac, S_IRUGO, bonding_show_ad_partner_mac, NULL);
>
> +/*
> + * Show and set the keep_all flag.
> + */
> +static ssize_t bonding_show_keep(struct device *d,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct bonding *bond = to_bond(d);
> +
> + return sprintf(buf, "%d\n", bond->params.keep_all);
> +}
> +
> +static ssize_t bonding_store_keep(struct device *d,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int i, new_value, ret = count;
> + struct bonding *bond = to_bond(d);
> + struct slave *slave;
> +
> + if (sscanf(buf, "%d", &new_value) != 1) {
> + pr_err("%s: no keep_all value specified.\n",
> + bond->dev->name);
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + if (new_value == bond->params.keep_all)
> + goto out;
> +
> + if ((new_value == 0) || (new_value == 1)) {
> + bond->params.keep_all = new_value;
> + } else {
> + pr_info("%s: Ignoring invalid keep_all value %d.\n",
> + bond->dev->name, new_value);
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + bond_for_each_slave(bond, slave, i) {
> + if (slave->state == BOND_STATE_BACKUP) {
> + if (new_value)
> + slave->dev->priv_flags &= ~IFF_SLAVE_INACTIVE;
> + else
> + slave->dev->priv_flags |= IFF_SLAVE_INACTIVE;
> + }
> + }
> +out:
> + return count;
> +}
> +static DEVICE_ATTR(keep_all, S_IRUGO | S_IWUSR,
> + bonding_show_keep, bonding_store_keep);
> +
>
>
> static struct attribute *per_bond_attrs[] = {
> @@ -1499,6 +1551,7 @@ static struct attribute *per_bond_attrs[] = {
> &dev_attr_ad_actor_key.attr,
> &dev_attr_ad_partner_key.attr,
> &dev_attr_ad_partner_mac.attr,
> + &dev_attr_keep_all.attr,
> NULL,
> };
>
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index 2aa3367..ef7969b 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -131,6 +131,7 @@ struct bond_params {
> char primary[IFNAMSIZ];
> int primary_reselect;
> __be32 arp_targets[BOND_MAX_ARP_TARGETS];
> + int keep_all;
> };
>
> struct bond_parm_tbl {
> @@ -291,7 +292,8 @@ static inline void bond_set_slave_inactive_flags(struct slave *slave)
> struct bonding *bond = netdev_priv(slave->dev->master);
> if (!bond_is_lb(bond))
> slave->state = BOND_STATE_BACKUP;
> - slave->dev->priv_flags |= IFF_SLAVE_INACTIVE;
> + if (!bond->params.keep_all)
> + slave->dev->priv_flags |= IFF_SLAVE_INACTIVE;
> if (slave_do_arp_validate(bond, slave))
> slave->dev->priv_flags |= IFF_SLAVE_NEEDARP;
> }
>
>
> ---
> -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