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

On Oct 14, 陈耀明 wrote:
> Yes, your analysis is correct: with the patch, the flow offloaded at t1
> reprograms the shared UPDMEM entry, so the flow egressing port0 starts
> using port1's L2 address. Our setup happens to carry a single IPv6
> identity per GDM port (the other services are IPv4), which is why the
> patch holds there -- it is a stopgap for that topology, as noted.
> 
> On the second question: in the default DSA configuration user ports
> inherit the conduit address, so the conduit MAC as source is
> indistinguishable from the user port's own. It diverges once user ports
> carry their own addresses (nvmem-assigned or user-configured): the
> egress frame then differs from what the software path would emit, and
> upstream bridges learn the conduit MAC behind a user port. Whether that
> is a bug or a corner case is your call; per-netdev slots would cover
> either way.

I have not tested it yet, but can you please check if the patch available at
[0] fixes the reported issue? Thanks.

Regards,
Lorenzo

[0] https://github.com/LorenzoBianconi/net-next/commit/486a5f13c62985d4333a231cc9aaec714e81fcd2

> 
> Lorenzo Bianconi <lorenzo@kernel.org> 于2026年10月13日周二 23:54写道：
> >
> > > 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 --]