On Mon, 2026-06-15 at 16:38 -0700, Jakub Kicinski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> net: sparx5: vcap: add lpm vcap implementation
>
> Add the implementation for the LPM VCAP for lan969x and sparx5. The LPM
> VCAP has a static keyset configuration, and a single lookup dedicated to
> LPM, so this is straight forward, and many functions can use existing
> functionality.
>
> > diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_vcap_impl.c
> > b/drivers/net/ethernet/microchip/sparx5/sparx5_vcap_impl.c
> > index 95b93e46a41dc..e25e759c24f60 100644
> > --- a/drivers/net/ethernet/microchip/sparx5/sparx5_vcap_impl.c
> > +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_vcap_impl.c
> > @@ -1083,6 +1146,46 @@ static void sparx5_vcap_es2_cache_write(struct
> > sparx5 *sparx5,
> > }
> > }
> >
> > +static void sparx5_vcap_lpm_cache_write(struct sparx5 *sparx5,
> > + struct vcap_admin *admin,
> > + enum vcap_selection sel,
> > + u32 start,
> > + u32 count)
> > +{
> > + u32 *keystr, *mskstr, *actstr;
> > + int idx;
> > +
> > + keystr = &admin->cache.keystream[start];
> > + mskstr = &admin->cache.maskstream[start];
> > + actstr = &admin->cache.actionstream[start];
> > +
> > + switch (sel) {
> > + case VCAP_SEL_ENTRY:
> > + for (idx = 0; idx < count; ++idx) {
> > + /* Avoid 'match-off' by setting value & mask */
> > + spx5_wr(keystr[idx] & mskstr[idx], sparx5,
> > + VCAP_SUPER_VCAP_ENTRY_DAT(idx));
>
> [Severity: High]
> Are concurrent writes to the shared VCAP_SUPER_VCAP_ENTRY_DAT registers
> safe here without a global hardware lock across the different VCAP types?
>
> It looks like IS0 and IS2 rules are updated via vcap_add_rule() under the
> rtnl_lock(). However, the LPM rules are updated under the
> sparx5->router->lock inside a workqueue.
>
> Since both execution paths share the Super VCAP hardware block, could
> concurrent updates from TC (IS0/IS2) and routing (LPM) race on these shared
> cache registers, and also on VCAP_SUPER_CTRL in sparx5_vcap_super_update(),
> garbling the hardware command and corrupting the cache memory?
>
Very nice catch, I missed this. This is even reachable before this series
because the debugfs dump also uses these registers, and runs outside of
rtnl.
I will send a fix to net which replaces the per instance locks with a single
vcap_control global mutex.
> > + spx5_wr(~mskstr[idx], sparx5,
> > + VCAP_SUPER_VCAP_MASK_DAT(idx));
> > + }
> > + break;
>
> [ ... ]
>
> > @@ -1379,6 +1523,7 @@ static void sparx5_vcap_update(struct net_device
> > *ndev,
> > struct sparx5 *sparx5 = port->sparx5;
> >
> > switch (admin->vtype) {
> > + case VCAP_TYPE_LPM:
> > case VCAP_TYPE_IS0:
> > case VCAP_TYPE_IS2:
> > sparx5_vcap_super_update(sparx5, cmd, sel, addr);
