Subject: Re: [PATCH net 2/3] nfp: flower: prevent re-adding mac index for bonded port

From: Simon Horman <[email-masked]>
Date: Mon, 5 Feb 2024 13:32:03 +0000

On Fri, Feb 02, 2024 at 01:37:18PM +0200, Louis Peens wrote:
> From: Daniel de Villiers <[email-masked]>
> 
> When physical ports are reset (either through link failure or manually
> toggled down and up again) that are slaved to a Linux bond with a tunnel
> endpoint IP address on the bond device, not all tunnel packets arriving
> on the bond port are decapped as expected.
> 
> The bond dev assigns the same MAC address to itself and each of its
> slaves. When toggling a slave device, the same MAC address is therefore
> offloaded to the NFP multiple times with different indexes.
> 
> The issue only occurs when re-adding the shared mac. The
> nfp_tunnel_add_shared_mac() function has a conditional check early on
> that checks if a mac entry already exists and if that mac entry is
> global: (entry && nfp_tunnel_is_mac_idx_global(entry->index)). In the
> case of a bonded device (For example br-ex), the mac index is obtained,
> and no new index is assigned.
> 
> We therefore modify the conditional in nfp_tunnel_add_shared_mac() to
> check if the port belongs to the LAG along with the existing checks to
> prevent a new global mac index from being re-assigned to the slave port.
> 
> Fixes: 20cce8865098 ("nfp: flower: enable MAC address sharing for offloadable devs")
> CC: [email-masked] # 5.1+
> Signed-off-by: Daniel de Villiers <[email-masked]>
> Signed-off-by: Louis Peens <[email-masked]>

Hi Daniel and Louis,

I'd like to encourage you to update the wording of the commit message
to use more inclusive language; I'd suggest describing the patch
in terms of members of a LAG.

The code-change looks good to me.

> ---
>  drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c b/drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c
> index e522845c7c21..0d7d138d6e0d 100644
> --- a/drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c
> +++ b/drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c
> @@ -1084,7 +1084,7 @@ nfp_tunnel_add_shared_mac(struct nfp_app *app, struct net_device *netdev,
>  	u16 nfp_mac_idx = 0;
>  
>  	entry = nfp_tunnel_lookup_offloaded_macs(app, netdev->dev_addr);
> -	if (entry && nfp_tunnel_is_mac_idx_global(entry->index)) {
> +	if (entry && (nfp_tunnel_is_mac_idx_global(entry->index) || netif_is_lag_port(netdev))) {
>  		if (entry->bridge_count ||
>  		    !nfp_flower_is_supported_bridge(netdev)) {
>  			nfp_tunnel_offloaded_macs_inc_ref_and_link(entry,
> -- 
> 2.34.1
> 
>