bonding: 802.3ad: fix no transmission of LACPDUs

This is caused by the global variable ad_ticks_per_sec being zero as
demonstrated by the reproducer script discussed below. This causes
all timer values in __ad_timer_to_ticks to be zero, resulting
in the periodic timer to never fire.

To reproduce:
Run the script in
`tools/testing/selftests/drivers/net/bonding/bond-break-lacpdu-tx.sh` which
puts bonding into a state where it never transmits LACPDUs.

line 44: ip link add fbond type bond mode 4 miimon 200 \
            xmit_hash_policy 1 ad_actor_sys_prio 65535 lacp_rate fast
setting bond param: ad_actor_sys_prio
given:
    params.ad_actor_system = 0
call stack:
    bond_option_ad_actor_sys_prio()
    -> bond_3ad_update_ad_actor_settings()
       -> set ad.system.sys_priority = bond->params.ad_actor_sys_prio
       -> ad.system.sys_mac_addr = bond->dev->dev_addr; because
            params.ad_actor_system == 0
results:
     ad.system.sys_mac_addr = bond->dev->dev_addr

line 48: ip link set fbond address 52:54:00:3B:7C:A6
setting bond MAC addr
call stack:
    bond->dev->dev_addr = new_mac

line 52: ip link set fbond type bond ad_actor_sys_prio 65535
setting bond param: ad_actor_sys_prio
given:
    params.ad_actor_system = 0
call stack:
    bond_option_ad_actor_sys_prio()
    -> bond_3ad_update_ad_actor_settings()
       -> set ad.system.sys_priority = bond->params.ad_actor_sys_prio
       -> ad.system.sys_mac_addr = bond->dev->dev_addr; because
            params.ad_actor_system == 0
results:
     ad.system.sys_mac_addr = bond->dev->dev_addr

line 60: ip link set veth1-bond down master fbond
given:
    params.ad_actor_system = 0
    params.mode = BOND_MODE_8023AD
    ad.system.sys_mac_addr == bond->dev->dev_addr
call stack:
    bond_enslave
    -> bond_3ad_initialize(); because first slave
       -> if ad.system.sys_mac_addr != bond->dev->dev_addr
          return
results:
     Nothing is run in bond_3ad_initialize() because dev_addr equals
     sys_mac_addr leaving the global ad_ticks_per_sec zero as it is
     never initialized anywhere else.

The if check around the contents of bond_3ad_initialize() is no longer
needed due to commit 5ee14e6d336f ("bonding: 3ad: apply ad_actor settings
changes immediately") which sets ad.system.sys_mac_addr if any one of
the bonding parameters whos set function calls
bond_3ad_update_ad_actor_settings(). This is because if
ad.system.sys_mac_addr is zero it will be set to the current bond mac
address, this causes the if check to never be true.

Fixes: 5ee14e6d336f ("bonding: 3ad: apply ad_actor settings changes immediately")
Signed-off-by: Jonathan Toppins <jtoppins@redhat.com>
Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index d7fb33c078e81a1c8242d5af793b328a19b79bd2..1f0120cbe9e807d6eea3ec49639093e007c8c91c 100644 (file)
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2007,30 +2007,24 @@ void bond_3ad_initiate_agg_selection(struct bonding *bond, int timeout)
  */
 void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution)
 {
-       /* check that the bond is not initialized yet */
-       if (!MAC_ADDRESS_EQUAL(&(BOND_AD_INFO(bond).system.sys_mac_addr),
-                               bond->dev->dev_addr)) {
-
-               BOND_AD_INFO(bond).aggregator_identifier = 0;
-
-               BOND_AD_INFO(bond).system.sys_priority =
-                       bond->params.ad_actor_sys_prio;
-               if (is_zero_ether_addr(bond->params.ad_actor_system))
-                       BOND_AD_INFO(bond).system.sys_mac_addr =
-                           *((struct mac_addr *)bond->dev->dev_addr);
-               else
-                       BOND_AD_INFO(bond).system.sys_mac_addr =
-                           *((struct mac_addr *)bond->params.ad_actor_system);
+       BOND_AD_INFO(bond).aggregator_identifier = 0;
+       BOND_AD_INFO(bond).system.sys_priority =
+               bond->params.ad_actor_sys_prio;
+       if (is_zero_ether_addr(bond->params.ad_actor_system))
+               BOND_AD_INFO(bond).system.sys_mac_addr =
+                   *((struct mac_addr *)bond->dev->dev_addr);
+       else
+               BOND_AD_INFO(bond).system.sys_mac_addr =
+                   *((struct mac_addr *)bond->params.ad_actor_system);
 
-               /* initialize how many times this module is called in one
-                * second (should be about every 100ms)
-                */
-               ad_ticks_per_sec = tick_resolution;
+       /* initialize how many times this module is called in one
+        * second (should be about every 100ms)
+        */
+       ad_ticks_per_sec = tick_resolution;
 
-               bond_3ad_initiate_agg_selection(bond,
-                                               AD_AGGREGATOR_SELECTION_TIMER *
-                                               ad_ticks_per_sec);
-       }
+       bond_3ad_initiate_agg_selection(bond,
+                                       AD_AGGREGATOR_SELECTION_TIMER *
+                                       ad_ticks_per_sec);
 }
 
 /**