Subject: Re: [net-next,v22,2/7] net: mtip: The L2 switch driver for imx287

From: Łukasz Majewski <[email-masked]>
Date: Wed, 4 Feb 2026 10:24:52 +0100

Hi Jakub,

> On Tue, 3 Feb 2026 21:19:55 +0100 Łukasz Majewski wrote:
> > > > +static void mtip_write_atable(struct switch_enet_private *fep,
> > > > int index,
> > > > +			      u32 write_lo, u32 write_hi)
> > > > +{
> > > > +	struct addr_table64b_entry __iomem *atable_base =
> > > > +		fep->hwentry->mtip_table64b_entry;
> > > > +
> > > > +	writel(write_lo, &atable_base[index].lo);
> > > > +	writel(write_hi, &atable_base[index].hi);
> > > > +}      
> > > 
> > > Can these functions race with concurrent access? Looking at the
> > > callers, mtip_write_atable is called from two different paths:
> > > 
> > > 1. Static entry updates: mtip_config_switch ->
> > > esw_mac_addr_static -> mtip_update_atable_static ->
> > > mtip_write_atable (no lock held)
> > > 
> > > 2. Dynamic entry updates: timer callback -> mtip_mgnt_timer ->
> > >    mtip_atable_dynamicms_learn_migration ->
> > > mtip_update_atable_dynamic1 -> mtip_write_atable (learn_lock held)
> > > 
> > > The learn_lock only protects the dynamic entry path. The static
> > > entry path runs during link changes (mtip_switch_restart called
> > > from mtip_adjust_link) without lock protection.
> > > 
> > > Both paths can access the same hash block in the address table
> > > (determined by GET_BLOCK_PTR(hash)). If the timer fires during a
> > > link change callback, both can concurrently access the table,
> > > potentially causing torn reads (reading .lo from one entry
> > > version and .hi from another) or torn writes (the entry is in an
> > > inconsistent state between the two writel calls).
> > > 
> > > Would extending learn_lock to protect all address table access
> > > work, or is a separate hw_lock needed for hardware register
> > > access? 
> > 
> > This is handled in another way:
> > 
> > 1. Partial write is not possible as this IP block handles it in
> > order (with some kind of 'latch' registers):
> > 
> > "VFxxx Controller Reference Manual, Rev. 0, 10/2016"
> > 11.5.4 MAC address lookup table
> > 
> > "Each entry must be written or read with the low address accessed
> > first followed by the high address"
> > 
> > 2. The code for dynamic IP writing will not "touch" the entries for
> > "static" MAC addresses - Figure 11-70 - bit 49 is "Record Type":
> > 	1 - static entry
> > 	0 - dynamic entry
> > 
> > IMHO, we are "safe" here.  
> 
> My reading of the AI's comment was that there is no lock in SW so two
> SW threads can write:
> 
>    Thread A   Thread B
>    write_lo
>               write_lo
>               write_hi
>    write_hi

Ok. Then I can add "atable_access" spin lock to prevent from this
situation.

-- 
Best regards,

Łukasz Majewski