Re: [PATCH net] net: airoha: Keep UPDMEM egress MAC in sync for offloaded IPv6 flows

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2367 bytes --]

> Ooo,  the new patch should be a dedicated leaf lock inside
> airoha_ppe_init_upd_mem().
> --- a/drivers/net/ethernet/airoha/airoha_ppe.c
> +++ b/drivers/net/ethernet/airoha/airoha_ppe.c
> @@ -416,6 +416,19 @@
>    dsa_port);
> 
>   smac_id = port->id;
> + /*
> + * Routed IPv6 entries resolve the egress source MAC
> + * through the UPDMEM table indexed by smac_id. The
> + * egress netdev can legally carry a different address
> + * than the GDM port itself: on PON setups the
> + * authentication MAC lives on the pon device while
> + * each service VLAN routes with its own address. Keep
> + * the table entry in sync with the source address this
> + * flow is actually going to use.

Is the issue occurring just with PON or are you able to trigger the issue even
in the upstream codebase? I guess you should not post a fix for some code that
is not upstream yet. Moreover, in the PON setup, I guess the approach is still
broken if the user change the device mac address.

> + */
> + if (type >= PPE_PKT_TYPE_IPV6_ROUTE_3T)
> + airoha_ppe_init_upd_mem(dev,
> + data->eth.h_source);
>   }
>   }
> 
> @@ -1529,10 +1542,21 @@
> 
>  void airoha_ppe_init_upd_mem(struct airoha_gdm_dev *dev, const u8 *addr)
>  {
> + static DEFINE_MUTEX(updmem_lock);
> +
>   struct airoha_gdm_port *port = dev->port;
>   struct airoha_eth *eth = dev->eth;
>   u32 val;
> 
> + /*
> + * Routed IPv6 entries resolve the egress source MAC through the
> + * UPDMEM table indexed by smac_id; the ndo_open/ndo_set_mac_address
> + * writers hold flow_offload_mutex (QoS paths re-lock via
> + * airoha_set_macaddr()), so this lock also serializes the leaf
> + * against them.
> + */
> + mutex_lock(&updmem_lock);

This is only required adding the above configuration since in the current
codebase airoha_ppe_init_upd_mem() is always running holding RTNL.

Regards,
Lorenzo

> +
>   val = (addr[2] << 24) | (addr[3] << 16) | (addr[4] << 8) | addr[5];
>   airoha_fe_wr(eth, REG_UPDMEM_DATA(0), val);
>   airoha_fe_wr(eth, REG_UPDMEM_CTRL(0),
> @@ -1545,6 +1569,8 @@
>       FIELD_PREP(PPE_UPDMEM_ADDR_MASK, port->id) |
>       FIELD_PREP(PPE_UPDMEM_OFFSET_MASK, 1) |
>       PPE_UPDMEM_WR_MASK | PPE_UPDMEM_REQ_MASK);
> +
> + mutex_unlock(&updmem_lock);
>  }
> 
>  struct airoha_ppe_dev *airoha_ppe_get_dev(struct device *dev)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]