bonding: add an option to specify a delay between peer notifications
authorVincent Bernat <vincent@bernat.ch>
Tue, 2 Jul 2019 17:43:54 +0000 (19:43 +0200)
committerDavid S. Miller <davem@davemloft.net>
Thu, 4 Jul 2019 19:30:48 +0000 (12:30 -0700)
Currently, gratuitous ARP/ND packets are sent every `miimon'
milliseconds. This commit allows a user to specify a custom delay
through a new option, `peer_notif_delay'.

Like for `updelay' and `downdelay', this delay should be a multiple of
`miimon' to avoid managing an additional work queue. The configuration
logic is copied from `updelay' and `downdelay'. However, the default
value cannot be set using a module parameter: Netlink or sysfs should
be used to configure this feature.

When setting `miimon' to 100 and `peer_notif_delay' to 500, we can
observe the 500 ms delay is respected:

    20:30:19.354693 ARP, Request who-has 203.0.113.10 tell 203.0.113.10, length 28
    20:30:19.874892 ARP, Request who-has 203.0.113.10 tell 203.0.113.10, length 28
    20:30:20.394919 ARP, Request who-has 203.0.113.10 tell 203.0.113.10, length 28
    20:30:20.914963 ARP, Request who-has 203.0.113.10 tell 203.0.113.10, length 28

In bond_mii_monitor(), I have tried to keep the lock logic readable.
The change is due to the fact we cannot rely on a notification to
lower the value of `bond->send_peer_notif' as `NETDEV_NOTIFY_PEERS' is
only triggered once every N times, while we need to decrement the
counter each time.

iproute2 also needs to be updated to be able to specify this new
attribute through `ip link'.

Signed-off-by: Vincent Bernat <vincent@bernat.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/bonding/bond_main.c
drivers/net/bonding/bond_netlink.c
drivers/net/bonding/bond_options.c
drivers/net/bonding/bond_procfs.c
drivers/net/bonding/bond_sysfs.c
include/net/bond_options.h
include/net/bonding.h
include/uapi/linux/if_link.h
tools/include/uapi/linux/if_link.h

index 84168455aded96dfd85b310841dee2a0d917b580..302499ae05e6c5cba4ee37640ebce748b1aae839 100644 (file)
@@ -796,6 +796,8 @@ static bool bond_should_notify_peers(struct bonding *bond)
                   slave ? slave->dev->name : "NULL");
 
        if (!slave || !bond->send_peer_notif ||
+           bond->send_peer_notif %
+           max(1, bond->params.peer_notif_delay) != 0 ||
            !netif_carrier_ok(bond->dev) ||
            test_bit(__LINK_STATE_LINKWATCH_PENDING, &slave->dev->state))
                return false;
@@ -886,15 +888,18 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
 
                        if (netif_running(bond->dev)) {
                                bond->send_peer_notif =
-                                       bond->params.num_peer_notif;
+                                       bond->params.num_peer_notif *
+                                       max(1, bond->params.peer_notif_delay);
                                should_notify_peers =
                                        bond_should_notify_peers(bond);
                        }
 
                        call_netdevice_notifiers(NETDEV_BONDING_FAILOVER, bond->dev);
-                       if (should_notify_peers)
+                       if (should_notify_peers) {
+                               bond->send_peer_notif--;
                                call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
                                                         bond->dev);
+                       }
                }
        }
 
@@ -2279,6 +2284,7 @@ static void bond_mii_monitor(struct work_struct *work)
        struct bonding *bond = container_of(work, struct bonding,
                                            mii_work.work);
        bool should_notify_peers = false;
+       bool commit;
        unsigned long delay;
        struct slave *slave;
        struct list_head *iter;
@@ -2289,12 +2295,19 @@ static void bond_mii_monitor(struct work_struct *work)
                goto re_arm;
 
        rcu_read_lock();
-
        should_notify_peers = bond_should_notify_peers(bond);
-
-       if (bond_miimon_inspect(bond)) {
+       commit = !!bond_miimon_inspect(bond);
+       if (bond->send_peer_notif) {
+               rcu_read_unlock();
+               if (rtnl_trylock()) {
+                       bond->send_peer_notif--;
+                       rtnl_unlock();
+               }
+       } else {
                rcu_read_unlock();
+       }
 
+       if (commit) {
                /* Race avoidance with bond_close cancel of workqueue */
                if (!rtnl_trylock()) {
                        delay = 1;
@@ -2308,8 +2321,7 @@ static void bond_mii_monitor(struct work_struct *work)
                bond_miimon_commit(bond);
 
                rtnl_unlock();  /* might sleep, hold no other locks */
-       } else
-               rcu_read_unlock();
+       }
 
 re_arm:
        if (bond->params.miimon)
@@ -3065,10 +3077,6 @@ static int bond_master_netdev_event(unsigned long event,
        case NETDEV_REGISTER:
                bond_create_proc_entry(event_bond);
                break;
-       case NETDEV_NOTIFY_PEERS:
-               if (event_bond->send_peer_notif)
-                       event_bond->send_peer_notif--;
-               break;
        default:
                break;
        }
@@ -4691,6 +4699,7 @@ static int bond_check_params(struct bond_params *params)
        params->arp_all_targets = arp_all_targets_value;
        params->updelay = updelay;
        params->downdelay = downdelay;
+       params->peer_notif_delay = 0;
        params->use_carrier = use_carrier;
        params->lacp_fast = lacp_fast;
        params->primary[0] = 0;
index b24cce48ae35cabd2af2bb53412c89de5d736738..a259860a72086371a01f943b820476ffd1237329 100644 (file)
@@ -108,6 +108,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = {
        [IFLA_BOND_AD_ACTOR_SYSTEM]     = { .type = NLA_BINARY,
                                            .len  = ETH_ALEN },
        [IFLA_BOND_TLB_DYNAMIC_LB]      = { .type = NLA_U8 },
+       [IFLA_BOND_PEER_NOTIF_DELAY]    = { .type = NLA_U32 },
 };
 
 static const struct nla_policy bond_slave_policy[IFLA_BOND_SLAVE_MAX + 1] = {
@@ -215,6 +216,14 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
                if (err)
                        return err;
        }
+       if (data[IFLA_BOND_PEER_NOTIF_DELAY]) {
+               int delay = nla_get_u32(data[IFLA_BOND_PEER_NOTIF_DELAY]);
+
+               bond_opt_initval(&newval, delay);
+               err = __bond_opt_set(bond, BOND_OPT_PEER_NOTIF_DELAY, &newval);
+               if (err)
+                       return err;
+       }
        if (data[IFLA_BOND_USE_CARRIER]) {
                int use_carrier = nla_get_u8(data[IFLA_BOND_USE_CARRIER]);
 
@@ -494,6 +503,7 @@ static size_t bond_get_size(const struct net_device *bond_dev)
                nla_total_size(sizeof(u16)) + /* IFLA_BOND_AD_USER_PORT_KEY */
                nla_total_size(ETH_ALEN) + /* IFLA_BOND_AD_ACTOR_SYSTEM */
                nla_total_size(sizeof(u8)) + /* IFLA_BOND_TLB_DYNAMIC_LB */
+               nla_total_size(sizeof(u32)) +   /* IFLA_BOND_PEER_NOTIF_DELAY */
                0;
 }
 
@@ -536,6 +546,10 @@ static int bond_fill_info(struct sk_buff *skb,
                        bond->params.downdelay * bond->params.miimon))
                goto nla_put_failure;
 
+       if (nla_put_u32(skb, IFLA_BOND_PEER_NOTIF_DELAY,
+                       bond->params.downdelay * bond->params.miimon))
+               goto nla_put_failure;
+
        if (nla_put_u8(skb, IFLA_BOND_USE_CARRIER, bond->params.use_carrier))
                goto nla_put_failure;
 
index 0d852fe9da7c192161720b26f64060802b7bdd3c..ddb3916d3506bec2fe317973054a32ea5c4255ad 100644 (file)
@@ -24,6 +24,8 @@ static int bond_option_updelay_set(struct bonding *bond,
                                   const struct bond_opt_value *newval);
 static int bond_option_downdelay_set(struct bonding *bond,
                                     const struct bond_opt_value *newval);
+static int bond_option_peer_notif_delay_set(struct bonding *bond,
+                                           const struct bond_opt_value *newval);
 static int bond_option_use_carrier_set(struct bonding *bond,
                                       const struct bond_opt_value *newval);
 static int bond_option_arp_interval_set(struct bonding *bond,
@@ -424,6 +426,13 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
                .desc = "Number of peer notifications to send on failover event",
                .values = bond_num_peer_notif_tbl,
                .set = bond_option_num_peer_notif_set
+       },
+       [BOND_OPT_PEER_NOTIF_DELAY] = {
+               .id = BOND_OPT_PEER_NOTIF_DELAY,
+               .name = "peer_notif_delay",
+               .desc = "Delay between each peer notification on failover event, in milliseconds",
+               .values = bond_intmax_tbl,
+               .set = bond_option_peer_notif_delay_set
        }
 };
 
@@ -841,6 +850,9 @@ static int bond_option_miimon_set(struct bonding *bond,
        if (bond->params.downdelay)
                netdev_dbg(bond->dev, "Note: Updating downdelay (to %d) since it is a multiple of the miimon value\n",
                           bond->params.downdelay * bond->params.miimon);
+       if (bond->params.peer_notif_delay)
+               netdev_dbg(bond->dev, "Note: Updating peer_notif_delay (to %d) since it is a multiple of the miimon value\n",
+                          bond->params.peer_notif_delay * bond->params.miimon);
        if (newval->value && bond->params.arp_interval) {
                netdev_dbg(bond->dev, "MII monitoring cannot be used with ARP monitoring - disabling ARP monitoring...\n");
                bond->params.arp_interval = 0;
@@ -864,52 +876,59 @@ static int bond_option_miimon_set(struct bonding *bond,
        return 0;
 }
 
-/* Set up and down delays. These must be multiples of the
- * MII monitoring value, and are stored internally as the multiplier.
- * Thus, we must translate to MS for the real world.
+/* Set up, down and peer notification delays. These must be multiples
+ * of the MII monitoring value, and are stored internally as the
+ * multiplier. Thus, we must translate to MS for the real world.
  */
-static int bond_option_updelay_set(struct bonding *bond,
-                                  const struct bond_opt_value *newval)
+static int _bond_option_delay_set(struct bonding *bond,
+                                 const struct bond_opt_value *newval,
+                                 const char *name,
+                                 int *target)
 {
        int value = newval->value;
 
        if (!bond->params.miimon) {
-               netdev_err(bond->dev, "Unable to set up delay as MII monitoring is disabled\n");
+               netdev_err(bond->dev, "Unable to set %s as MII monitoring is disabled\n",
+                          name);
                return -EPERM;
        }
        if ((value % bond->params.miimon) != 0) {
-               netdev_warn(bond->dev, "up delay (%d) is not a multiple of miimon (%d), updelay rounded to %d ms\n",
+               netdev_warn(bond->dev,
+                           "%s (%d) is not a multiple of miimon (%d), value rounded to %d ms\n",
+                           name,
                            value, bond->params.miimon,
                            (value / bond->params.miimon) *
                            bond->params.miimon);
        }
-       bond->params.updelay = value / bond->params.miimon;
-       netdev_dbg(bond->dev, "Setting up delay to %d\n",
-                  bond->params.updelay * bond->params.miimon);
+       *target = value / bond->params.miimon;
+       netdev_dbg(bond->dev, "Setting %s to %d\n",
+                  name,
+                  *target * bond->params.miimon);
 
        return 0;
 }
 
+static int bond_option_updelay_set(struct bonding *bond,
+                                  const struct bond_opt_value *newval)
+{
+       return _bond_option_delay_set(bond, newval, "up delay",
+                                     &bond->params.updelay);
+}
+
 static int bond_option_downdelay_set(struct bonding *bond,
                                     const struct bond_opt_value *newval)
 {
-       int value = newval->value;
-
-       if (!bond->params.miimon) {
-               netdev_err(bond->dev, "Unable to set down delay as MII monitoring is disabled\n");
-               return -EPERM;
-       }
-       if ((value % bond->params.miimon) != 0) {
-               netdev_warn(bond->dev, "down delay (%d) is not a multiple of miimon (%d), delay rounded to %d ms\n",
-                           value, bond->params.miimon,
-                           (value / bond->params.miimon) *
-                           bond->params.miimon);
-       }
-       bond->params.downdelay = value / bond->params.miimon;
-       netdev_dbg(bond->dev, "Setting down delay to %d\n",
-                  bond->params.downdelay * bond->params.miimon);
+       return _bond_option_delay_set(bond, newval, "down delay",
+                                     &bond->params.downdelay);
+}
 
-       return 0;
+static int bond_option_peer_notif_delay_set(struct bonding *bond,
+                                           const struct bond_opt_value *newval)
+{
+       int ret = _bond_option_delay_set(bond, newval,
+                                        "peer notification delay",
+                                        &bond->params.peer_notif_delay);
+       return ret;
 }
 
 static int bond_option_use_carrier_set(struct bonding *bond,
index 9f7d83e827c37361d9856c5124655f157bf7ef49..fd5c9cbe45b1be1af68252ac48aa68bd52004df3 100644 (file)
@@ -104,6 +104,8 @@ static void bond_info_show_master(struct seq_file *seq)
                   bond->params.updelay * bond->params.miimon);
        seq_printf(seq, "Down Delay (ms): %d\n",
                   bond->params.downdelay * bond->params.miimon);
+       seq_printf(seq, "Peer Notification Delay (ms): %d\n",
+                  bond->params.peer_notif_delay * bond->params.miimon);
 
 
        /* ARP information */
index 94214eaf53c50d4a8464ed2592ea305e85591bf5..2d615a93685e8d4b4fdbcfa423a3d143bba71e72 100644 (file)
@@ -327,6 +327,18 @@ static ssize_t bonding_show_updelay(struct device *d,
 static DEVICE_ATTR(updelay, 0644,
                   bonding_show_updelay, bonding_sysfs_store_option);
 
+static ssize_t bonding_show_peer_notif_delay(struct device *d,
+                                            struct device_attribute *attr,
+                                            char *buf)
+{
+       struct bonding *bond = to_bond(d);
+
+       return sprintf(buf, "%d\n",
+                      bond->params.peer_notif_delay * bond->params.miimon);
+}
+static DEVICE_ATTR(peer_notif_delay, 0644,
+                  bonding_show_peer_notif_delay, bonding_sysfs_store_option);
+
 /* Show the LACP interval. */
 static ssize_t bonding_show_lacp(struct device *d,
                                 struct device_attribute *attr,
@@ -718,6 +730,7 @@ static struct attribute *per_bond_attrs[] = {
        &dev_attr_arp_ip_target.attr,
        &dev_attr_downdelay.attr,
        &dev_attr_updelay.attr,
+       &dev_attr_peer_notif_delay.attr,
        &dev_attr_lacp_rate.attr,
        &dev_attr_ad_select.attr,
        &dev_attr_xmit_hash_policy.attr,
index 2a05cc349018ef73d86abdf395f9c419990577bf..9d382f2f0bc57b611b10b90dacd36060dd2980d3 100644 (file)
@@ -63,6 +63,7 @@ enum {
        BOND_OPT_AD_ACTOR_SYSTEM,
        BOND_OPT_AD_USER_PORT_KEY,
        BOND_OPT_NUM_PEER_NOTIF_ALIAS,
+       BOND_OPT_PEER_NOTIF_DELAY,
        BOND_OPT_LAST
 };
 
index 676e7fae05a3b5c418f809cd0252f2e2885d3623..f7fe45689142150231a74247f35811ce4dd77600 100644 (file)
@@ -123,6 +123,7 @@ struct bond_params {
        int fail_over_mac;
        int updelay;
        int downdelay;
+       int peer_notif_delay;
        int lacp_fast;
        unsigned int min_links;
        int ad_select;
index 6f75bda2c2d7089704362cad6aa4ac8fd0fd2bd2..4a8c02cafa9a1220cda77bc2dcf923bac9ab1fce 100644 (file)
@@ -636,6 +636,7 @@ enum {
        IFLA_BOND_AD_USER_PORT_KEY,
        IFLA_BOND_AD_ACTOR_SYSTEM,
        IFLA_BOND_TLB_DYNAMIC_LB,
+       IFLA_BOND_PEER_NOTIF_DELAY,
        __IFLA_BOND_MAX,
 };
 
index 5b225ff63b483145878e977cf0a1be7d1fe53cd9..7d113a9602f06801b948ebeaff2fa4bbbbed2ce5 100644 (file)
@@ -636,6 +636,7 @@ enum {
        IFLA_BOND_AD_USER_PORT_KEY,
        IFLA_BOND_AD_ACTOR_SYSTEM,
        IFLA_BOND_TLB_DYNAMIC_LB,
+       IFLA_BOND_PEER_NOTIF_DELAY,
        __IFLA_BOND_MAX,
 };