Message ID | 20131107094330.15846.84353.stgit@monster-03.cumulusnetworks.com |
---|---|
State | Deferred, archived |
Delegated to: | David Miller |
Headers | show |
Thu, Nov 07, 2013 at 10:43:30AM CET, sfeldma@cumulusnetworks.com wrote: >Add IFLA_BOND_ARP_IP_TARGET to allow get/set of bonding parameter >arp_ip_target via netlink. > >Signed-off-by: Scott Feldman <sfeldma@cumulusnetworks.com> >--- > drivers/net/bonding/bond_netlink.c | 43 ++++++++++++++- > drivers/net/bonding/bond_options.c | 102 ++++++++++++++++++++++++++++++++++++ > drivers/net/bonding/bond_sysfs.c | 77 +++------------------------ > drivers/net/bonding/bonding.h | 3 + > include/uapi/linux/if_link.h | 1 > 5 files changed, 155 insertions(+), 71 deletions(-) > >diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c >index 9c50165..e4673ba 100644 >--- a/drivers/net/bonding/bond_netlink.c >+++ b/drivers/net/bonding/bond_netlink.c >@@ -29,6 +29,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = { > [IFLA_BOND_DOWNDELAY] = { .type = NLA_U32 }, > [IFLA_BOND_USE_CARRIER] = { .type = NLA_U8 }, > [IFLA_BOND_ARP_INTERVAL] = { .type = NLA_U32 }, >+ [IFLA_BOND_ARP_IP_TARGET] = { .type = NLA_NESTED }, > }; > > static int bond_validate(struct nlattr *tb[], struct nlattr *data[]) >@@ -109,6 +110,21 @@ static int bond_changelink(struct net_device *bond_dev, > if (err) > return err; > } >+ if (data[IFLA_BOND_ARP_IP_TARGET]) { >+ struct nlattr *attr; >+ int rem; >+ >+ err = bond_option_arp_ip_target_clear_all(bond); >+ if (err) >+ return err; >+ Hmm. I think this is not correct. I think you should not clear all. They might be needed to be used but for a brief moment, they disappear. Just set or unset them one by one. >+ nla_for_each_nested(attr, data[IFLA_BOND_ARP_IP_TARGET], rem) { >+ __be32 target = nla_get_u32(attr); >+ err = bond_option_arp_ip_target_set(bond, target); >+ if (err) >+ return err; >+ } >+ } > return 0; > } > >@@ -133,6 +149,8 @@ static size_t bond_get_size(const struct net_device *bond_dev) > nla_total_size(sizeof(u32)) + /* IFLA_BOND_DOWNDELAY */ > nla_total_size(sizeof(u8)) + /* IFLA_BOND_USE_CARRIER */ > nla_total_size(sizeof(u32)) + /* IFLA_BOND_ARP_INTERVAL */ >+ /* IFLA_BOND_ARP_IP_TARGET */ >+ nla_total_size(sizeof(u32)) * BOND_MAX_ARP_TARGETS + > 0; > } > >@@ -141,6 +159,8 @@ static int bond_fill_info(struct sk_buff *skb, > { > struct bonding *bond = netdev_priv(bond_dev); > struct net_device *slave_dev = bond_option_active_slave_get(bond); >+ struct nlattr *targets; >+ int i, targets_added; > > if (nla_put_u8(skb, IFLA_BOND_MODE, bond->params.mode)) > goto nla_put_failure; >@@ -152,10 +172,12 @@ static int bond_fill_info(struct sk_buff *skb, > if (nla_put_u32(skb, IFLA_BOND_MIIMON, bond->params.miimon)) > goto nla_put_failure; > >- if (nla_put_u32(skb, IFLA_BOND_UPDELAY, bond->params.updelay)) >+ if (nla_put_u32(skb, IFLA_BOND_UPDELAY, >+ bond->params.updelay * bond->params.miimon)) > goto nla_put_failure; This bit should probably no be here :) > >- if (nla_put_u32(skb, IFLA_BOND_DOWNDELAY, bond->params.downdelay)) >+ if (nla_put_u32(skb, IFLA_BOND_DOWNDELAY, >+ bond->params.downdelay * bond->params.miimon)) > goto nla_put_failure; This as well. > > if (nla_put_u8(skb, IFLA_BOND_USE_CARRIER, bond->params.use_carrier)) >@@ -164,6 +186,23 @@ static int bond_fill_info(struct sk_buff *skb, > if (nla_put_u32(skb, IFLA_BOND_ARP_INTERVAL, bond->params.arp_interval)) > goto nla_put_failure; > >+ targets = nla_nest_start(skb, IFLA_BOND_ARP_IP_TARGET); >+ if (!targets) >+ goto nla_put_failure; >+ >+ targets_added = 0; >+ for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) { >+ if (bond->params.arp_targets[i]) { >+ nla_put_u32(skb, i, bond->params.arp_targets[i]); >+ targets_added = 1; >+ } >+ } >+ >+ if (targets_added) >+ nla_nest_end(skb, targets); >+ else >+ nla_nest_cancel(skb, targets); >+ > return 0; > > nla_put_failure: >diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c >index c4b60ad..f57f113 100644 >--- a/drivers/net/bonding/bond_options.c >+++ b/drivers/net/bonding/bond_options.c >@@ -302,3 +302,105 @@ int bond_option_arp_interval_set(struct bonding *bond, int arp_interval) > > return 0; > } >+ >+int bond_option_arp_ip_target_set(struct bonding *bond, __be32 target) >+{ >+ __be32 *targets = bond->params.arp_targets; >+ struct list_head *iter; >+ struct slave *slave; >+ int ind; >+ >+ if ((target == 0) || (target == htonl(INADDR_BROADCAST))) { >+ pr_err("%s: invalid ARP target %pI4 specified for addition\n", >+ bond->dev->name, &target); >+ return -EINVAL; >+ } >+ >+ if (bond_get_targets_ip(targets, target) != -1) { /* dup */ >+ pr_err("%s: ARP target %pI4 is already present\n", >+ bond->dev->name, &target); >+ return -EINVAL; >+ } >+ >+ 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); >+ return -EINVAL; >+ } >+ >+ pr_info("%s: adding ARP target %pI4.\n", bond->dev->name, >+ &target); >+ >+ /* not to race with bond_arp_rcv */ >+ write_lock_bh(&bond->lock); >+ bond_for_each_slave(bond, slave, iter) >+ slave->target_last_arp_rx[ind] = jiffies; >+ targets[ind] = target; >+ write_unlock_bh(&bond->lock); >+ >+ return 0; >+} >+ >+int bond_option_arp_ip_target_unset(struct bonding *bond, __be32 target) >+{ >+ __be32 *targets = bond->params.arp_targets; >+ struct list_head *iter; >+ struct slave *slave; >+ unsigned long *targets_rx; >+ int ind, i, j; >+ >+ if ((target == 0) || (target == htonl(INADDR_BROADCAST))) { >+ pr_err("%s: invalid ARP target %pI4 specified for removal\n", >+ bond->dev->name, &target); >+ return -EINVAL; >+ } >+ >+ ind = bond_get_targets_ip(targets, target); >+ if (ind == -1) { >+ pr_err("%s: unable to remove nonexistent ARP target %pI4.\n", >+ bond->dev->name, &target); >+ return -EINVAL; >+ } >+ >+ if (ind == 0 && !targets[1] && bond->params.arp_interval) >+ pr_warn("%s: removing last arp target with arp_interval on\n", >+ bond->dev->name); >+ >+ pr_info("%s: removing ARP target %pI4.\n", bond->dev->name, >+ &target); >+ >+ write_lock_bh(&bond->lock); >+ bond_for_each_slave(bond, slave, iter) { >+ 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); >+ >+ return 0; >+} >+ >+int bond_option_arp_ip_target_clear_all(struct bonding *bond) >+{ >+ __be32 *targets = bond->params.arp_targets; >+ struct list_head *iter; >+ struct slave *slave; >+ int i; >+ >+ write_lock_bh(&bond->lock); >+ bond_for_each_slave(bond, slave, iter) { >+ for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) >+ slave->target_last_arp_rx[i] = 0; >+ } >+ for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) >+ targets[i] = 0; >+ write_unlock_bh(&bond->lock); >+ >+ return 0; >+} >diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c >index 644b745..aa97b12 100644 >--- a/drivers/net/bonding/bond_sysfs.c >+++ b/drivers/net/bonding/bond_sysfs.c >@@ -561,88 +561,27 @@ static ssize_t bonding_store_arp_targets(struct device *d, > const char *buf, size_t count) > { > struct bonding *bond = to_bond(d); >- struct list_head *iter; >- struct slave *slave; >- __be32 newtarget, *targets; >- unsigned long *targets_rx; >- int ind, i, j, ret = -EINVAL; >+ __be32 target; >+ int ret; > > if (!rtnl_trylock()) > return restart_syscall(); > >- targets = bond->params.arp_targets; >- newtarget = in_aton(buf + 1); >+ target = in_aton(buf + 1); > /* look for adds */ > if (buf[0] == '+') { >- if ((newtarget == 0) || (newtarget == htonl(INADDR_BROADCAST))) { >- pr_err("%s: invalid ARP target %pI4 specified for addition\n", >- bond->dev->name, &newtarget); >- goto out; >- } >- >- if (bond_get_targets_ip(targets, newtarget) != -1) { /* dup */ >- pr_err("%s: ARP target %pI4 is already present\n", >- bond->dev->name, &newtarget); >- goto out; >- } >- >- 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; >- } >- >- pr_info("%s: adding ARP target %pI4.\n", bond->dev->name, >- &newtarget); >- /* not to race with bond_arp_rcv */ >- write_lock_bh(&bond->lock); >- bond_for_each_slave(bond, slave, iter) >- slave->target_last_arp_rx[ind] = jiffies; >- targets[ind] = newtarget; >- write_unlock_bh(&bond->lock); >+ ret = bond_option_arp_ip_target_set(bond, target); > } else if (buf[0] == '-') { >- if ((newtarget == 0) || (newtarget == htonl(INADDR_BROADCAST))) { >- pr_err("%s: invalid ARP target %pI4 specified for removal\n", >- bond->dev->name, &newtarget); >- goto out; >- } >- >- 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_interval) >- pr_warn("%s: removing last arp target with arp_interval on\n", >- bond->dev->name); >- >- pr_info("%s: removing ARP target %pI4.\n", bond->dev->name, >- &newtarget); >- >- write_lock_bh(&bond->lock); >- bond_for_each_slave(bond, slave, iter) { >- 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); >+ ret = bond_option_arp_ip_target_unset(bond, target); > } else { > pr_err("no command found in arp_ip_targets file for bond %s. Use +<addr> or -<addr>.\n", > bond->dev->name); > ret = -EPERM; >- goto out; > } > >- ret = count; >-out: >+ if (!ret) >+ ret = count; >+ > rtnl_unlock(); > return ret; > } >diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h >index caaa5e7..e5d215d 100644 >--- a/drivers/net/bonding/bonding.h >+++ b/drivers/net/bonding/bonding.h >@@ -433,6 +433,9 @@ int bond_option_updelay_set(struct bonding *bond, int updelay); > int bond_option_downdelay_set(struct bonding *bond, int downdelay); > int bond_option_use_carrier_set(struct bonding *bond, int use_carrier); > int bond_option_arp_interval_set(struct bonding *bond, int arp_interval); >+int bond_option_arp_ip_target_set(struct bonding *bond, __be32 target); >+int bond_option_arp_ip_target_unset(struct bonding *bond, __be32 target); >+int bond_option_arp_ip_target_clear_all(struct bonding *bond); > struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond); > struct net_device *bond_option_active_slave_get(struct bonding *bond); > >diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h >index 858b09e..58dd318 100644 >--- a/include/uapi/linux/if_link.h >+++ b/include/uapi/linux/if_link.h >@@ -336,6 +336,7 @@ enum { > IFLA_BOND_DOWNDELAY, > IFLA_BOND_USE_CARRIER, > IFLA_BOND_ARP_INTERVAL, >+ IFLA_BOND_ARP_IP_TARGET, > __IFLA_BOND_MAX, > }; > > >-- >To unsubscribe from this list: send the line "unsubscribe netdev" in >the body of a message to majordomo@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Nov 7, 2013, at 12:16 AM, Jiri Pirko <jiri@resnulli.us> wrote: > Thu, Nov 07, 2013 at 10:43:30AM CET, sfeldma@cumulusnetworks.com wrote: >> Add IFLA_BOND_ARP_IP_TARGET to allow get/set of bonding parameter >> arp_ip_target via netlink. >> >> Signed-off-by: Scott Feldman <sfeldma@cumulusnetworks.com> >> --- >> drivers/net/bonding/bond_netlink.c | 43 ++++++++++++++- >> drivers/net/bonding/bond_options.c | 102 ++++++++++++++++++++++++++++++++++++ >> drivers/net/bonding/bond_sysfs.c | 77 +++------------------------ >> drivers/net/bonding/bonding.h | 3 + >> include/uapi/linux/if_link.h | 1 >> 5 files changed, 155 insertions(+), 71 deletions(-) >> >> diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c >> index 9c50165..e4673ba 100644 >> --- a/drivers/net/bonding/bond_netlink.c >> +++ b/drivers/net/bonding/bond_netlink.c >> @@ -29,6 +29,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = { >> [IFLA_BOND_DOWNDELAY] = { .type = NLA_U32 }, >> [IFLA_BOND_USE_CARRIER] = { .type = NLA_U8 }, >> [IFLA_BOND_ARP_INTERVAL] = { .type = NLA_U32 }, >> + [IFLA_BOND_ARP_IP_TARGET] = { .type = NLA_NESTED }, >> }; >> >> static int bond_validate(struct nlattr *tb[], struct nlattr *data[]) >> @@ -109,6 +110,21 @@ static int bond_changelink(struct net_device *bond_dev, >> if (err) >> return err; >> } >> + if (data[IFLA_BOND_ARP_IP_TARGET]) { >> + struct nlattr *attr; >> + int rem; >> + >> + err = bond_option_arp_ip_target_clear_all(bond); >> + if (err) >> + return err; >> + > > Hmm. I think this is not correct. I think you should not clear all. They > might be needed to be used but for a brief moment, they disappear. > > Just set or unset them one by one. That’s a good catch. I should compare existing set with new set and retain entries entries common to both sets (and add/remove uncommon ones). Just to be clear, for netlink, the idea is to push a set of ip targets addrs as a nested attribute and process as a set. This is consistent with arp_ip_target documentation in Documentation/networking/bonding.txt where arp_ip_target is passed as a comma-separated list of addrs. However, it’s not consistent with the sysfs interface where addrs are added/removed one at a time using “+” or “-“ prefix. e.g.: ip link add bond7 type bond mode 1 ip link set dev bond7 type bond arp_ip_target 10.0.0.1,10.0.0.5 ip link set dev bond7 type bond arp_ip_target 10.0.0.1 >> @@ -141,6 +159,8 @@ static int bond_fill_info(struct sk_buff *skb, >> { >> struct bonding *bond = netdev_priv(bond_dev); >> struct net_device *slave_dev = bond_option_active_slave_get(bond); >> + struct nlattr *targets; >> + int i, targets_added; >> >> if (nla_put_u8(skb, IFLA_BOND_MODE, bond->params.mode)) >> goto nla_put_failure; >> @@ -152,10 +172,12 @@ static int bond_fill_info(struct sk_buff *skb, >> if (nla_put_u32(skb, IFLA_BOND_MIIMON, bond->params.miimon)) >> goto nla_put_failure; >> >> - if (nla_put_u32(skb, IFLA_BOND_UPDELAY, bond->params.updelay)) >> + if (nla_put_u32(skb, IFLA_BOND_UPDELAY, >> + bond->params.updelay * bond->params.miimon)) >> goto nla_put_failure; > > This bit should probably no be here :) Drats, that change goes with earlier patch. I’ll fix on patch set resend for issue above. -scott-- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thu, Nov 07, 2013 at 11:56:35AM CET, sfeldma@cumulusnetworks.com wrote: > >On Nov 7, 2013, at 12:16 AM, Jiri Pirko <jiri@resnulli.us> wrote: > >> Thu, Nov 07, 2013 at 10:43:30AM CET, sfeldma@cumulusnetworks.com wrote: >>> Add IFLA_BOND_ARP_IP_TARGET to allow get/set of bonding parameter >>> arp_ip_target via netlink. >>> >>> Signed-off-by: Scott Feldman <sfeldma@cumulusnetworks.com> >>> --- >>> drivers/net/bonding/bond_netlink.c | 43 ++++++++++++++- >>> drivers/net/bonding/bond_options.c | 102 ++++++++++++++++++++++++++++++++++++ >>> drivers/net/bonding/bond_sysfs.c | 77 +++------------------------ >>> drivers/net/bonding/bonding.h | 3 + >>> include/uapi/linux/if_link.h | 1 >>> 5 files changed, 155 insertions(+), 71 deletions(-) >>> >>> diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c >>> index 9c50165..e4673ba 100644 >>> --- a/drivers/net/bonding/bond_netlink.c >>> +++ b/drivers/net/bonding/bond_netlink.c >>> @@ -29,6 +29,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = { >>> [IFLA_BOND_DOWNDELAY] = { .type = NLA_U32 }, >>> [IFLA_BOND_USE_CARRIER] = { .type = NLA_U8 }, >>> [IFLA_BOND_ARP_INTERVAL] = { .type = NLA_U32 }, >>> + [IFLA_BOND_ARP_IP_TARGET] = { .type = NLA_NESTED }, >>> }; >>> >>> static int bond_validate(struct nlattr *tb[], struct nlattr *data[]) >>> @@ -109,6 +110,21 @@ static int bond_changelink(struct net_device *bond_dev, >>> if (err) >>> return err; >>> } >>> + if (data[IFLA_BOND_ARP_IP_TARGET]) { >>> + struct nlattr *attr; >>> + int rem; >>> + >>> + err = bond_option_arp_ip_target_clear_all(bond); >>> + if (err) >>> + return err; >>> + >> >> Hmm. I think this is not correct. I think you should not clear all. They >> might be needed to be used but for a brief moment, they disappear. >> >> Just set or unset them one by one. > >That’s a good catch. I should compare existing set with new set and retain entries entries common to both sets (and add/remove uncommon ones). > >Just to be clear, for netlink, the idea is to push a set of ip targets addrs as a nested attribute and process as a set. This is consistent with arp_ip_target documentation in Documentation/networking/bonding.txt where arp_ip_target is passed as a comma-separated list of addrs. However, it’s not consistent with the sysfs interface where addrs are added/removed one at a time using “+” or “-“ prefix. I believe that the behaviour you described is ok. > >e.g.: > >ip link add bond7 type bond mode 1 >ip link set dev bond7 type bond arp_ip_target 10.0.0.1,10.0.0.5 >ip link set dev bond7 type bond arp_ip_target 10.0.0.1 > > >>> @@ -141,6 +159,8 @@ static int bond_fill_info(struct sk_buff *skb, >>> { >>> struct bonding *bond = netdev_priv(bond_dev); >>> struct net_device *slave_dev = bond_option_active_slave_get(bond); >>> + struct nlattr *targets; >>> + int i, targets_added; >>> >>> if (nla_put_u8(skb, IFLA_BOND_MODE, bond->params.mode)) >>> goto nla_put_failure; >>> @@ -152,10 +172,12 @@ static int bond_fill_info(struct sk_buff *skb, >>> if (nla_put_u32(skb, IFLA_BOND_MIIMON, bond->params.miimon)) >>> goto nla_put_failure; >>> >>> - if (nla_put_u32(skb, IFLA_BOND_UPDELAY, bond->params.updelay)) >>> + if (nla_put_u32(skb, IFLA_BOND_UPDELAY, >>> + bond->params.updelay * bond->params.miimon)) >>> goto nla_put_failure; >> >> This bit should probably no be here :) > >Drats, that change goes with earlier patch. I’ll fix on patch set resend for issue above. > >-scott -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c index 9c50165..e4673ba 100644 --- a/drivers/net/bonding/bond_netlink.c +++ b/drivers/net/bonding/bond_netlink.c @@ -29,6 +29,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = { [IFLA_BOND_DOWNDELAY] = { .type = NLA_U32 }, [IFLA_BOND_USE_CARRIER] = { .type = NLA_U8 }, [IFLA_BOND_ARP_INTERVAL] = { .type = NLA_U32 }, + [IFLA_BOND_ARP_IP_TARGET] = { .type = NLA_NESTED }, }; static int bond_validate(struct nlattr *tb[], struct nlattr *data[]) @@ -109,6 +110,21 @@ static int bond_changelink(struct net_device *bond_dev, if (err) return err; } + if (data[IFLA_BOND_ARP_IP_TARGET]) { + struct nlattr *attr; + int rem; + + err = bond_option_arp_ip_target_clear_all(bond); + if (err) + return err; + + nla_for_each_nested(attr, data[IFLA_BOND_ARP_IP_TARGET], rem) { + __be32 target = nla_get_u32(attr); + err = bond_option_arp_ip_target_set(bond, target); + if (err) + return err; + } + } return 0; } @@ -133,6 +149,8 @@ static size_t bond_get_size(const struct net_device *bond_dev) nla_total_size(sizeof(u32)) + /* IFLA_BOND_DOWNDELAY */ nla_total_size(sizeof(u8)) + /* IFLA_BOND_USE_CARRIER */ nla_total_size(sizeof(u32)) + /* IFLA_BOND_ARP_INTERVAL */ + /* IFLA_BOND_ARP_IP_TARGET */ + nla_total_size(sizeof(u32)) * BOND_MAX_ARP_TARGETS + 0; } @@ -141,6 +159,8 @@ static int bond_fill_info(struct sk_buff *skb, { struct bonding *bond = netdev_priv(bond_dev); struct net_device *slave_dev = bond_option_active_slave_get(bond); + struct nlattr *targets; + int i, targets_added; if (nla_put_u8(skb, IFLA_BOND_MODE, bond->params.mode)) goto nla_put_failure; @@ -152,10 +172,12 @@ static int bond_fill_info(struct sk_buff *skb, if (nla_put_u32(skb, IFLA_BOND_MIIMON, bond->params.miimon)) goto nla_put_failure; - if (nla_put_u32(skb, IFLA_BOND_UPDELAY, bond->params.updelay)) + if (nla_put_u32(skb, IFLA_BOND_UPDELAY, + bond->params.updelay * bond->params.miimon)) goto nla_put_failure; - if (nla_put_u32(skb, IFLA_BOND_DOWNDELAY, bond->params.downdelay)) + if (nla_put_u32(skb, IFLA_BOND_DOWNDELAY, + bond->params.downdelay * bond->params.miimon)) goto nla_put_failure; if (nla_put_u8(skb, IFLA_BOND_USE_CARRIER, bond->params.use_carrier)) @@ -164,6 +186,23 @@ static int bond_fill_info(struct sk_buff *skb, if (nla_put_u32(skb, IFLA_BOND_ARP_INTERVAL, bond->params.arp_interval)) goto nla_put_failure; + targets = nla_nest_start(skb, IFLA_BOND_ARP_IP_TARGET); + if (!targets) + goto nla_put_failure; + + targets_added = 0; + for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) { + if (bond->params.arp_targets[i]) { + nla_put_u32(skb, i, bond->params.arp_targets[i]); + targets_added = 1; + } + } + + if (targets_added) + nla_nest_end(skb, targets); + else + nla_nest_cancel(skb, targets); + return 0; nla_put_failure: diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c index c4b60ad..f57f113 100644 --- a/drivers/net/bonding/bond_options.c +++ b/drivers/net/bonding/bond_options.c @@ -302,3 +302,105 @@ int bond_option_arp_interval_set(struct bonding *bond, int arp_interval) return 0; } + +int bond_option_arp_ip_target_set(struct bonding *bond, __be32 target) +{ + __be32 *targets = bond->params.arp_targets; + struct list_head *iter; + struct slave *slave; + int ind; + + if ((target == 0) || (target == htonl(INADDR_BROADCAST))) { + pr_err("%s: invalid ARP target %pI4 specified for addition\n", + bond->dev->name, &target); + return -EINVAL; + } + + if (bond_get_targets_ip(targets, target) != -1) { /* dup */ + pr_err("%s: ARP target %pI4 is already present\n", + bond->dev->name, &target); + return -EINVAL; + } + + 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); + return -EINVAL; + } + + pr_info("%s: adding ARP target %pI4.\n", bond->dev->name, + &target); + + /* not to race with bond_arp_rcv */ + write_lock_bh(&bond->lock); + bond_for_each_slave(bond, slave, iter) + slave->target_last_arp_rx[ind] = jiffies; + targets[ind] = target; + write_unlock_bh(&bond->lock); + + return 0; +} + +int bond_option_arp_ip_target_unset(struct bonding *bond, __be32 target) +{ + __be32 *targets = bond->params.arp_targets; + struct list_head *iter; + struct slave *slave; + unsigned long *targets_rx; + int ind, i, j; + + if ((target == 0) || (target == htonl(INADDR_BROADCAST))) { + pr_err("%s: invalid ARP target %pI4 specified for removal\n", + bond->dev->name, &target); + return -EINVAL; + } + + ind = bond_get_targets_ip(targets, target); + if (ind == -1) { + pr_err("%s: unable to remove nonexistent ARP target %pI4.\n", + bond->dev->name, &target); + return -EINVAL; + } + + if (ind == 0 && !targets[1] && bond->params.arp_interval) + pr_warn("%s: removing last arp target with arp_interval on\n", + bond->dev->name); + + pr_info("%s: removing ARP target %pI4.\n", bond->dev->name, + &target); + + write_lock_bh(&bond->lock); + bond_for_each_slave(bond, slave, iter) { + 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); + + return 0; +} + +int bond_option_arp_ip_target_clear_all(struct bonding *bond) +{ + __be32 *targets = bond->params.arp_targets; + struct list_head *iter; + struct slave *slave; + int i; + + write_lock_bh(&bond->lock); + bond_for_each_slave(bond, slave, iter) { + for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) + slave->target_last_arp_rx[i] = 0; + } + for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) + targets[i] = 0; + write_unlock_bh(&bond->lock); + + return 0; +} diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index 644b745..aa97b12 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -561,88 +561,27 @@ static ssize_t bonding_store_arp_targets(struct device *d, const char *buf, size_t count) { struct bonding *bond = to_bond(d); - struct list_head *iter; - struct slave *slave; - __be32 newtarget, *targets; - unsigned long *targets_rx; - int ind, i, j, ret = -EINVAL; + __be32 target; + int ret; if (!rtnl_trylock()) return restart_syscall(); - targets = bond->params.arp_targets; - newtarget = in_aton(buf + 1); + target = in_aton(buf + 1); /* look for adds */ if (buf[0] == '+') { - if ((newtarget == 0) || (newtarget == htonl(INADDR_BROADCAST))) { - pr_err("%s: invalid ARP target %pI4 specified for addition\n", - bond->dev->name, &newtarget); - goto out; - } - - if (bond_get_targets_ip(targets, newtarget) != -1) { /* dup */ - pr_err("%s: ARP target %pI4 is already present\n", - bond->dev->name, &newtarget); - goto out; - } - - 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; - } - - pr_info("%s: adding ARP target %pI4.\n", bond->dev->name, - &newtarget); - /* not to race with bond_arp_rcv */ - write_lock_bh(&bond->lock); - bond_for_each_slave(bond, slave, iter) - slave->target_last_arp_rx[ind] = jiffies; - targets[ind] = newtarget; - write_unlock_bh(&bond->lock); + ret = bond_option_arp_ip_target_set(bond, target); } else if (buf[0] == '-') { - if ((newtarget == 0) || (newtarget == htonl(INADDR_BROADCAST))) { - pr_err("%s: invalid ARP target %pI4 specified for removal\n", - bond->dev->name, &newtarget); - goto out; - } - - 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_interval) - pr_warn("%s: removing last arp target with arp_interval on\n", - bond->dev->name); - - pr_info("%s: removing ARP target %pI4.\n", bond->dev->name, - &newtarget); - - write_lock_bh(&bond->lock); - bond_for_each_slave(bond, slave, iter) { - 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); + ret = bond_option_arp_ip_target_unset(bond, target); } else { pr_err("no command found in arp_ip_targets file for bond %s. Use +<addr> or -<addr>.\n", bond->dev->name); ret = -EPERM; - goto out; } - ret = count; -out: + if (!ret) + ret = count; + rtnl_unlock(); return ret; } diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index caaa5e7..e5d215d 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -433,6 +433,9 @@ int bond_option_updelay_set(struct bonding *bond, int updelay); int bond_option_downdelay_set(struct bonding *bond, int downdelay); int bond_option_use_carrier_set(struct bonding *bond, int use_carrier); int bond_option_arp_interval_set(struct bonding *bond, int arp_interval); +int bond_option_arp_ip_target_set(struct bonding *bond, __be32 target); +int bond_option_arp_ip_target_unset(struct bonding *bond, __be32 target); +int bond_option_arp_ip_target_clear_all(struct bonding *bond); struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond); struct net_device *bond_option_active_slave_get(struct bonding *bond); diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index 858b09e..58dd318 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -336,6 +336,7 @@ enum { IFLA_BOND_DOWNDELAY, IFLA_BOND_USE_CARRIER, IFLA_BOND_ARP_INTERVAL, + IFLA_BOND_ARP_IP_TARGET, __IFLA_BOND_MAX, };
Add IFLA_BOND_ARP_IP_TARGET to allow get/set of bonding parameter arp_ip_target via netlink. Signed-off-by: Scott Feldman <sfeldma@cumulusnetworks.com> --- drivers/net/bonding/bond_netlink.c | 43 ++++++++++++++- drivers/net/bonding/bond_options.c | 102 ++++++++++++++++++++++++++++++++++++ drivers/net/bonding/bond_sysfs.c | 77 +++------------------------ drivers/net/bonding/bonding.h | 3 + include/uapi/linux/if_link.h | 1 5 files changed, 155 insertions(+), 71 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html