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(). > >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> >--- >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 */ I'm not sure this is an equivalent swap, as it is replacing a test for non-empty of the port's partner MAC with a test of the aggregator's partner MAC. In the port case, it is validating that this specific port has received a response from its link partner. In the aggregator case, it's checking that the aggregator as a whole has received response from the link partners of members of the aggregator, which does not include the port under consideration for inclusion into the aggregator. As the port is not yet a member of the aggregator, how is checking the aggregator's MAC for being non-empty an equivalent test to the one it replaces? -J > !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 > --- -Jay Vosburgh, jay.vosburgh@xxxxxxxxxxxxx
