Re: [PATCH net-next v2] bonding: 802.3ad replace MAC_ADDRESS_EQUAL with __agg_has_partner

From: Hangbin Liu
Date: Fri Feb 23 2024 - 02:48:40 EST


Hi Jones,
On Fri, Feb 23, 2024 at 04:12:00AM +0000, Jones Syue 薛懷宗 wrote:
> They are verifying the same thing: if aggregator has a partner or not.
> Replaces macro with inline function would look more clear to understand.

MAC_ADDRESS_EQUAL and __agg_has_partner are not the same thing.

MAC_ADDRESS_EQUAL() is only same with __agg_has_partner() when verifying
agg partner mac addr with null_mac_addr. The description could be more
accurate.

Since you want to replace the null_mac_addr checking for MAC_ADDRESS_EQUAL().
Maybe we can also replace the null_mac_addr checking in
ad_port_selection_logic(). This should be safe as the
aggregator->partner_system and port->partner_oper.system has been compared.
e.g.

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index f2942e8c6c91..bd46dcb4013c 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -1588,7 +1588,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 */
)
) {

With this the null_mac_addr definition could be removed.

Thanks
Hangbin