| 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: <51C49666.1030407@redhat.com>
Date: Fri, 21 Jun 2013 20:07:34 +0200
From: Nikolay Aleksandrov <nikolay@...hat.com>
To: Veaceslav Falico <vfalico@...hat.com>
CC: netdev@...r.kernel.org, fubar@...ibm.com, andy@...yhouse.net,
davem@...emloft.net, linux@...2.net, nicolas.2p.debian@...e.fr,
rick.jones2@...com, mkubecek@...e.cz
Subject: Re: [PATCH v3 net-next 6/6] bonding: add an option to fail when any
of arp_ip_target is inaccessible
On 06/21/2013 03:11 PM, Veaceslav Falico wrote:
> @@ -590,10 +633,11 @@ static ssize_t bonding_store_arp_targets(struct device *d,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> - __be32 newtarget;
> - int i = 0, ret = -EINVAL;
> struct bonding *bond = to_bond(d);
> - __be32 *targets;
> + struct slave *slave;
> + __be32 newtarget, *targets;
> + unsigned long *targets_rx;
> + int ind, i, j, ret = -EINVAL;
>
> targets = bond->params.arp_targets;
> newtarget = in_aton(buf + 1);
> @@ -611,8 +655,8 @@ static ssize_t bonding_store_arp_targets(struct device *d,
> goto out;
> }
>
> - i = bond_get_targets_ip(targets, 0); /* first free slot */
> - if (i == -1) {
> + ind = bond_get_targets_ip(targets, 0); /* first free slot */
> + if (ind == -1) {
> pr_err("%s: ARP target table is full!\n",
> bond->dev->name);
> goto out;
> @@ -620,7 +664,12 @@ static ssize_t bonding_store_arp_targets(struct device *d,
>
> pr_info("%s: adding ARP target %pI4.\n", bond->dev->name,
> &newtarget);
> - targets[i] = newtarget;
> + /* not to race with bond_arp_rcv */
> + write_lock_bh(&bond->lock);
> + bond_for_each_slave(bond, slave, i)
> + slave->target_last_arp_rx[ind] = jiffies;
> + targets[ind] = newtarget;
> + write_unlock_bh(&bond->lock);
> } else if (buf[0] == '-') {
> if ((newtarget == 0) || (newtarget == htonl(INADDR_BROADCAST))) {
> pr_err("%s: invalid ARP target %pI4 specified for removal\n",
> @@ -628,18 +677,32 @@ static ssize_t bonding_store_arp_targets(struct device *d,
> goto out;
> }
>
> - i = bond_get_targets_ip(targets, newtarget);
> - if (i == -1) {
> - pr_info("%s: unable to remove nonexistent ARP target %pI4.\n",
> + ind = bond_get_targets_ip(targets, newtarget);
> + if (ind == -1) {
> + pr_err("%s: unable to remove nonexistent ARP target %pI4.\n",
> bond->dev->name, &newtarget);
> goto out;
> }
>
> + if (ind == 0 && !targets[1] && bond->params.arp_validate)
> + pr_warn("%s: removing last arp target with arp_validate on\n",
> + bond->dev->name);
> +
> pr_info("%s: removing ARP target %pI4.\n", bond->dev->name,
> &newtarget);
> - for (; (i < BOND_MAX_ARP_TARGETS-1) && targets[i+1]; i++)
> +
> + write_lock_bh(&bond->lock);
> + bond_for_each_slave(bond, slave, i) {
> + targets_rx = slave->target_last_arp_rx;
> + j = ind;
> + for (; (j < BOND_MAX_ARP_TARGETS-1) && targets[j+1]; j++)
> + targets_rx[j] = targets_rx[j+1];
> + targets_rx[j] = 0;
> + }
> + for (i = ind; (i < BOND_MAX_ARP_TARGETS-1) && targets[i+1]; i++)
> targets[i] = targets[i+1];
> targets[i] = 0;
> + write_unlock_bh(&bond->lock);
> } else {
> pr_err("no command found in arp_ip_targets file for bond %s. Use +<addr> or -<addr>.\n",
> bond->dev->name);
> @@ -649,6 +712,7 @@ static ssize_t bonding_store_arp_targets(struct device *d,
>
> ret = count;
> out:
> + rtnl_unlock();
A leftover rtnl_unlock here.
Nik
> return ret;
> }
> static DEVICE_ATTR(arp_ip_target, S_IRUGO | S_IWUSR , bonding_show_arp_targets, bonding_store_arp_targets);
> @@ -1623,6 +1687,7 @@ static struct attribute *per_bond_attrs[] = {
> &dev_attr_mode.attr,
> &dev_attr_fail_over_mac.attr,
> &dev_attr_arp_validate.attr,
> + &dev_attr_arp_all_targets.attr,
> &dev_attr_arp_interval.attr,
> &dev_attr_arp_ip_target.attr,
> &dev_attr_downdelay.attr,
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index 486e532..3fb73cc 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -144,6 +144,7 @@ struct bond_params {
> u8 num_peer_notif;
> int arp_interval;
> int arp_validate;
> + int arp_all_targets;
> int use_carrier;
> int fail_over_mac;
> int updelay;
> @@ -179,6 +180,7 @@ struct slave {
> int delay;
> unsigned long jiffies;
> unsigned long last_arp_rx;
> + unsigned long target_last_arp_rx[BOND_MAX_ARP_TARGETS];
> s8 link; /* one of BOND_LINK_XXXX */
> s8 new_link;
> u8 backup:1, /* indicates backup slave. Value corresponds with
> @@ -322,6 +324,9 @@ static inline bool bond_is_active_slave(struct slave *slave)
> #define BOND_FOM_ACTIVE 1
> #define BOND_FOM_FOLLOW 2
>
> +#define BOND_ARP_TARGETS_ANY 0
> +#define BOND_ARP_TARGETS_ALL 1
> +
> #define BOND_ARP_VALIDATE_NONE 0
> #define BOND_ARP_VALIDATE_ACTIVE (1 << BOND_STATE_ACTIVE)
> #define BOND_ARP_VALIDATE_BACKUP (1 << BOND_STATE_BACKUP)
> @@ -334,11 +339,31 @@ static inline int slave_do_arp_validate(struct bonding *bond,
> return bond->params.arp_validate & (1 << bond_slave_state(slave));
> }
>
> +/* Get the oldest arp which we've received on this slave for bond's
> + * arp_targets.
> + */
> +static inline unsigned long slave_oldest_target_arp_rx(struct bonding *bond,
> + struct slave *slave)
> +{
> + int i = 1;
> + unsigned long ret = slave->target_last_arp_rx[0];
> +
> + for (; (i < BOND_MAX_ARP_TARGETS) && bond->params.arp_targets[i]; i++)
> + if (time_before(slave->target_last_arp_rx[i], ret))
> + ret = slave->target_last_arp_rx[i];
> +
> + return ret;
> +}
> +
> static inline unsigned long slave_last_rx(struct bonding *bond,
> struct slave *slave)
> {
> - if (slave_do_arp_validate(bond, slave))
> - return slave->last_arp_rx;
> + if (slave_do_arp_validate(bond, slave)) {
> + if (bond->params.arp_all_targets == BOND_ARP_TARGETS_ALL)
> + return slave_oldest_target_arp_rx(bond, slave);
> + else
> + return slave->last_arp_rx;
> + }
>
> return slave->dev->last_rx;
> }
> @@ -486,6 +511,7 @@ extern const struct bond_parm_tbl bond_lacp_tbl[];
> 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 arp_all_targets_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[];
>
--
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