[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <56e57cf68444b80562e2ee652b58dc11be03ff01.camel@alliedtelesis.co.nz>
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.
Powered by blists - more mailing lists