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: 5335 bytes --]

> The issue is reproducible in the upstream codebase, without any of the
> out-of-tree PON code -- the PON setup is just where I first hit it.
> 
> Reproduction with mainline code only (airoha eth + the mt7530 MMIO DSA
> driver; the "airoha,en7581-switch" binding and driver are mainline,
> the DT node just needs to be added to a board file):
> 
> - AN7581 board, switch node with its CPU port on GDM1. Use a DSA user
>   port as a routed WAN interface with its own MAC address.
> 
> airoha_get_dsa_port() replaces the egress netdev with the conduit, so
> routed IPv6 FOE entries carry smac_id = conduit port id, and the PPE
> resolves the egress source MAC through UPDMEM[conduit]. That slot is
> only programmed by airoha_set_macaddr(), which never runs when a DSA
> slave address changes, so offloaded IPv6 frames leave the port with
> the conduit's MAC instead of the user port's. IPv4 is unaffected,
> since HNAPT entries store the MACs inline. The same shape appears on a
> GDM port with two "airoha,eth-port" children carrying different MAC
> LSBs, which the recent multi-SerDES series allows by design
> (REG_FE_MAC_LMIN/LMAX); for that variant the exposure starts with the
> series introducing multiple net_devices per GDM port.
> 
> The posted patch only reprograms the shared per-port slot at offload
> time, so a later address change on any netdev of that port re-breaks
> offloaded IPv6 flows. It is a temporary patch for this setup; the
> final solution is up to the maintainers.

I do not think the case you described is fixed with your patch. Let's consider
the following case:

- time t0: you offload a flow rule for traffic egressing dsa port0
- time t1: you offload a flow rule for traffic egressing dsa port1

Since the index written in REG_UPDMEM_CTRL() subfield is still the conduit port
ID, you are egressing traffic from port0 with L2 address of port1. Am I right,
or am I missing something?
Moreover, is it a bug if we use the conduit port L2 address as source mac
address for IPv6 traffic?

Regards,
Lorenzo

> 
> A proper fix would probably give each netdev its own UPDMEM entry: the
> table is indexed by a 4-bit address (16 entries) while the FOE
> smac_id field is 5-bit with 0xf reserved for "keep original MAC", so
> there is room beyond the port ids. Slots could be programmed from
> airoha_set_macaddr() (always under RTNL, as you noted) plus a
> netdevice notifier for DSA slaves, and foe_entry_prepare() would only
> reference the slot -- removing both the race and the extra locking the
> current approach needs.
> 
> Lorenzo Bianconi <lorenzo@kernel.org> 于2026年10月13日周二 15:36写道：
> >
> > > 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 --]