Jones Syue 薛懷宗 <jonessyue@xxxxxxxx> wrote: >Replace macro MAC_ADDRESS_EQUAL() for null_mac_addr checking with inline >function__agg_has_partner(). When MAC_ADDRESS_EQUAL() is verifiying >aggregator's partner mac addr with null_mac_addr, means that seeing if >aggregator has a valid partner or not. Using __agg_has_partner() makes it >more clear to understand. > >In ad_port_selection_logic(), since aggregator->partner_system and >port->partner_oper.system has been compared first as a prerequisite, it is >safe to replace the upcoming MAC_ADDRESS_EQUAL() for null_mac_addr checking >with __agg_has_partner(). Ok, I missed this bit when I read the patch this morning, so you can ignore my earlier email's question. Patch looks good to me, glad to see null_mac_addr get the boot. >Delete null_mac_addr, which is not required anymore in bond_3ad.c, since >all references to it are gone. > >Signed-off-by: Jones Syue <jonessyue@xxxxxxxx> Acked-by: Jay Vosburgh <jay.vosburgh@xxxxxxxxxxxxx> -J >--- >v3: > - replace macro with inline function in ad_port_selection_logic() > - delete static variable null_mac_addr in bond_3ad.c > - re-phrase patch description with more precise text > - re-phrase patch description in imperative mood >v2: https://lore.kernel.org/netdev/SI2PR04MB5097AA23EE6799B3E56C0762DC552@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ > - add correct CC list by 'get_maintainer.pl -f .../bonding.rst' >v1: https://lore.kernel.org/netdev/SI2PR04MB50977DA9BB51D9C8FAF6928ADC562@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ >--- > drivers/net/bonding/bond_3ad.c | 14 +++----------- > 1 file changed, 3 insertions(+), 11 deletions(-) > >diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c >index f2942e8c6c91..c6807e473ab7 100644 >--- a/drivers/net/bonding/bond_3ad.c >+++ b/drivers/net/bonding/bond_3ad.c >@@ -82,10 +82,6 @@ enum ad_link_speed_type { > #define MAC_ADDRESS_EQUAL(A, B) \ > ether_addr_equal_64bits((const u8 *)A, (const u8 *)B) > >-static const u8 null_mac_addr[ETH_ALEN + 2] __long_aligned = { >- 0, 0, 0, 0, 0, 0 >-}; >- > static const u16 ad_ticks_per_sec = 1000 / AD_TIMER_INTERVAL; > static const int ad_delta_in_ticks = (AD_TIMER_INTERVAL * HZ) / 1000; > >@@ -1588,7 +1584,7 @@ static void ad_port_selection_logic(struct port *port, bool *update_slave_arr) > (aggregator->partner_system_priority == port->partner_oper.system_priority) && > (aggregator->partner_oper_aggregator_key == port->partner_oper.key) > ) && >- ((!MAC_ADDRESS_EQUAL(&(port->partner_oper.system), &(null_mac_addr)) && /* partner answers */ >+ ((__agg_has_partner(aggregator) && /* partner answers */ > !aggregator->is_individual) /* but is not individual OR */ > ) > ) { >@@ -2036,9 +2032,7 @@ static void ad_enable_collecting(struct port *port) > */ > static void ad_disable_distributing(struct port *port, bool *update_slave_arr) > { >- if (port->aggregator && >- !MAC_ADDRESS_EQUAL(&port->aggregator->partner_system, >- &(null_mac_addr))) { >+ if (port->aggregator && __agg_has_partner(port->aggregator)) { > slave_dbg(port->slave->bond->dev, port->slave->dev, > "Disabling distributing on port %d (LAG %d)\n", > port->actor_port_number, >@@ -2078,9 +2072,7 @@ static void ad_enable_collecting_distributing(struct port *port, > static void ad_disable_collecting_distributing(struct port *port, > bool *update_slave_arr) > { >- if (port->aggregator && >- !MAC_ADDRESS_EQUAL(&(port->aggregator->partner_system), >- &(null_mac_addr))) { >+ if (port->aggregator && __agg_has_partner(port->aggregator)) { > slave_dbg(port->slave->bond->dev, port->slave->dev, > "Disabling port %d (LAG %d)\n", > port->actor_port_number, >-- >2.1.4 >