net: bonding: replace dev_trans_start() with the jiffies of the last ARP/NS

The bonding driver piggybacks on time stamps kept by the network stack
for the purpose of the netdev TX watchdog, and this is problematic
because it does not work with NETIF_F_LLTX devices.

It is hard to say why the driver looks at dev_trans_start() of the
slave->dev, considering that this is updated even by non-ARP/NS probes
sent by us, and even by traffic not sent by us at all (for example PTP
on physical slave devices). ARP monitoring in active-backup mode appears
to still work even if we track only the last TX time of actual ARP
probes.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e75acb14d0665550c6f38e8c80b8799e5e438f1b..ab7fdbbc2530f114b3d41a0abc97499ba2f0965d 100644 (file)
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2001,6 +2001,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
        for (i = 0; i < BOND_MAX_ARP_TARGETS; i++)
                new_slave->target_last_arp_rx[i] = new_slave->last_rx;
 
+       new_slave->last_tx = new_slave->last_rx;
+
        if (bond->params.miimon && !bond->params.use_carrier) {
                link_reporting = bond_check_dev_link(bond, slave_dev, 1);
 
@@ -2884,8 +2886,11 @@ static void bond_arp_send(struct slave *slave, int arp_op, __be32 dest_ip,
                return;
        }
 
-       if (bond_handle_vlan(slave, tags, skb))
+       if (bond_handle_vlan(slave, tags, skb)) {
+               slave_update_last_tx(slave);
                arp_xmit(skb);
+       }
+
        return;
 }
 
@@ -3074,8 +3079,7 @@ static int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
                            curr_active_slave->last_link_up))
                bond_validate_arp(bond, slave, tip, sip);
        else if (curr_arp_slave && (arp->ar_op == htons(ARPOP_REPLY)) &&
-                bond_time_in_interval(bond,
-                                      dev_trans_start(curr_arp_slave->dev), 1))
+                bond_time_in_interval(bond, slave_last_tx(curr_arp_slave), 1))
                bond_validate_arp(bond, slave, sip, tip);
 
 out_unlock:
@@ -3103,8 +3107,10 @@ static void bond_ns_send(struct slave *slave, const struct in6_addr *daddr,
        }
 
        addrconf_addr_solict_mult(daddr, &mcaddr);
-       if (bond_handle_vlan(slave, tags, skb))
+       if (bond_handle_vlan(slave, tags, skb)) {
+               slave_update_last_tx(slave);
                ndisc_send_skb(skb, &mcaddr, saddr);
+       }
 }
 
 static void bond_ns_send_all(struct bonding *bond, struct slave *slave)
@@ -3246,8 +3252,7 @@ static int bond_na_rcv(const struct sk_buff *skb, struct bonding *bond,
                            curr_active_slave->last_link_up))
                bond_validate_ns(bond, slave, saddr, daddr);
        else if (curr_arp_slave &&
-                bond_time_in_interval(bond,
-                                      dev_trans_start(curr_arp_slave->dev), 1))
+                bond_time_in_interval(bond, slave_last_tx(curr_arp_slave), 1))
                bond_validate_ns(bond, slave, saddr, daddr);
 
 out:
@@ -3335,12 +3340,12 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
         *       so it can wait
         */
        bond_for_each_slave_rcu(bond, slave, iter) {
-               unsigned long trans_start = dev_trans_start(slave->dev);
+               unsigned long last_tx = slave_last_tx(slave);
 
                bond_propose_link_state(slave, BOND_LINK_NOCHANGE);
 
                if (slave->link != BOND_LINK_UP) {
-                       if (bond_time_in_interval(bond, trans_start, 1) &&
+                       if (bond_time_in_interval(bond, last_tx, 1) &&
                            bond_time_in_interval(bond, slave->last_rx, 1)) {
 
                                bond_propose_link_state(slave, BOND_LINK_UP);
@@ -3365,7 +3370,7 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
                         * when the source ip is 0, so don't take the link down
                         * if we don't know our ip yet
                         */
-                       if (!bond_time_in_interval(bond, trans_start, bond->params.missed_max) ||
+                       if (!bond_time_in_interval(bond, last_tx, bond->params.missed_max) ||
                            !bond_time_in_interval(bond, slave->last_rx, bond->params.missed_max)) {
 
                                bond_propose_link_state(slave, BOND_LINK_DOWN);
@@ -3431,7 +3436,7 @@ re_arm:
  */
 static int bond_ab_arp_inspect(struct bonding *bond)
 {
-       unsigned long trans_start, last_rx;
+       unsigned long last_tx, last_rx;
        struct list_head *iter;
        struct slave *slave;
        int commit = 0;
@@ -3482,9 +3487,9 @@ static int bond_ab_arp_inspect(struct bonding *bond)
                 * - (more than missed_max*delta since receive AND
                 *    the bond has an IP address)
                 */
-               trans_start = dev_trans_start(slave->dev);
+               last_tx = slave_last_tx(slave);
                if (bond_is_active_slave(slave) &&
-                   (!bond_time_in_interval(bond, trans_start, bond->params.missed_max) ||
+                   (!bond_time_in_interval(bond, last_tx, bond->params.missed_max) ||
                     !bond_time_in_interval(bond, last_rx, bond->params.missed_max))) {
                        bond_propose_link_state(slave, BOND_LINK_DOWN);
                        commit++;
@@ -3501,8 +3506,8 @@ static int bond_ab_arp_inspect(struct bonding *bond)
  */
 static void bond_ab_arp_commit(struct bonding *bond)
 {
-       unsigned long trans_start;
        struct list_head *iter;
+       unsigned long last_tx;
        struct slave *slave;
 
        bond_for_each_slave(bond, slave, iter) {
@@ -3511,10 +3516,10 @@ static void bond_ab_arp_commit(struct bonding *bond)
                        continue;
 
                case BOND_LINK_UP:
-                       trans_start = dev_trans_start(slave->dev);
+                       last_tx = slave_last_tx(slave);
                        if (rtnl_dereference(bond->curr_active_slave) != slave ||
                            (!rtnl_dereference(bond->curr_active_slave) &&
-                            bond_time_in_interval(bond, trans_start, 1))) {
+                            bond_time_in_interval(bond, last_tx, 1))) {
                                struct slave *current_arp_slave;
 
                                current_arp_slave = rtnl_dereference(bond->current_arp_slave);

diff --git a/include/net/bonding.h b/include/net/bonding.h
index 6e78d657aa05333a9ccd3df6e9a53210e0da589d..afd606df149adc4b1e18a562fa5f9f71f0e85a47 100644 (file)
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -161,8 +161,9 @@ struct slave {
        struct net_device *dev; /* first - useful for panic debug */
        struct bonding *bond; /* our master */
        int    delay;
-       /* all three in jiffies */
+       /* all 4 in jiffies */
        unsigned long last_link_up;
+       unsigned long last_tx;
        unsigned long last_rx;
        unsigned long target_last_arp_rx[BOND_MAX_ARP_TARGETS];
        s8     link;            /* one of BOND_LINK_XXXX */
@@ -540,6 +541,16 @@ static inline unsigned long slave_last_rx(struct bonding *bond,
        return slave->last_rx;
 }
 
+static inline void slave_update_last_tx(struct slave *slave)
+{
+       WRITE_ONCE(slave->last_tx, jiffies);
+}
+
+static inline unsigned long slave_last_tx(struct slave *slave)
+{
+       return READ_ONCE(slave->last_tx);
+}
+
 #ifdef CONFIG_NET_POLL_CONTROLLER
 static inline netdev_tx_t bond_netpoll_send_skb(const struct slave *slave,
                                         struct sk_buff *skb)