linux-kernel - Re: [PATCH v0] net: dsa: mv88e6xxx: Add FID map cache

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Wed, 19 Jun 2024 23:22:31 +0000
From: Aryan Srivastava <Aryan.Srivastava@...iedtelesis.co.nz>
To: "kabel@...nel.org" <kabel@...nel.org>
CC: "andrew@...n.ch" <andrew@...n.ch>, "olteanv@...il.com"
	<olteanv@...il.com>, "davem@...emloft.net" <davem@...emloft.net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"f.fainelli@...il.com" <f.fainelli@...il.com>, "kuba@...nel.org"
	<kuba@...nel.org>
Subject: Re: [PATCH v0] net: dsa: mv88e6xxx: Add FID map cache

Hi Marek,
On Tue, 2024-06-11 at 09:50 +0200, Marek Behún wrote:
> On Mon, 10 Jun 2024 17:07:23 +1200
> Aryan Srivastava <aryan.srivastava@...iedtelesis.co.nz> wrote:
> 
> > Add a cached FID bitmap. This mitigates the need to
> > walk all VTU entries to find the next free FID.
> > 
> > Walk VTU once, then store read FID map into bitmap. Use
> > and manipulate this bitmap from now on, instead of re-reading
> > HW for the FID map.
> > 
> > The repeatedly VTU walks are costly can result in taking ~40 mins
> > if ~4000 vlans are added. Caching the FID map reduces this time
> > to <2 mins.
> > 
> > Signed-off-by: Aryan Srivastava
> > <aryan.srivastava@...iedtelesis.co.nz>
> > ---
> >  drivers/net/dsa/mv88e6xxx/chip.c | 25 +++++++++++++++++++------
> >  drivers/net/dsa/mv88e6xxx/chip.h |  4 ++++
> >  2 files changed, 23 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c
> > b/drivers/net/dsa/mv88e6xxx/chip.c
> > index e5bac87941f6..91816e3e35ed 100644
> > --- a/drivers/net/dsa/mv88e6xxx/chip.c
> > +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> > @@ -1815,14 +1815,17 @@ int mv88e6xxx_fid_map(struct mv88e6xxx_chip
> > *chip, unsigned long *fid_bitmap)
> >  
> >  static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16
> > *fid)
> >  {
> > -       DECLARE_BITMAP(fid_bitmap, MV88E6XXX_N_FID);
> >         int err;
> >  
> > -       err = mv88e6xxx_fid_map(chip, fid_bitmap);
> > -       if (err)
> > -               return err;
> > +       if (!chip->fid_populated) {
> > +               err = mv88e6xxx_fid_map(chip, chip->fid_bitmap);
> > +               if (err)
> > +                       return err;
> >  
> > -       *fid = find_first_zero_bit(fid_bitmap, MV88E6XXX_N_FID);
> > +               chip->fid_populated = true;
> > +       }
> > +
> > +       *fid = find_first_zero_bit(chip->fid_bitmap,
> > MV88E6XXX_N_FID);
> >         if (unlikely(*fid >= mv88e6xxx_num_databases(chip)))
> >                 return -ENOSPC;
> >  
> > @@ -2529,6 +2532,9 @@ static int mv88e6xxx_port_vlan_join(struct
> > mv88e6xxx_chip *chip, int port,
> >                          port, vid);
> >         }
> >  
> > +       /* Record FID used in SW FID map */
> > +       bitmap_set(chip->fid_bitmap, vlan.fid, 1);
> > +
> 
> wouldn't it make more sense to do this bit setting in
> mv88e6xxx_atu_new() and clearingin a new function
> mv88e6xxx_atu_delete/drop() ?
The reason I did not do this is to ensure that we only clear/set the SW
bit if the HW ops pass, i.e. I want to make sure calls like
mv88e6xxx_vtu_loadpurge and mv88e6xxx_g1_atu_remove pass before editing
the SW cache.

Thanks,
Aryan.