| Message ID | 20240610050724.2439780-1-aryan.srivastava@alliedtelesis.co.nz (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v0] net: dsa: mv88e6xxx: Add FID map cache | expand |
On Mon, Jun 10, 2024 at 05:07:23PM +1200, Aryan Srivastava 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. How long does the first walk take? Rather than having fid_populated, i wounder if the walk should just be done in mv88e6xxx_vtu_setup() or mv88e6xxx_atu_setup(). Andrew
On Mon, 10 Jun 2024 17:07:23 +1200 Aryan Srivastava <aryan.srivastava@alliedtelesis.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@alliedtelesis.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() ?
Hi Andrew, On Mon, 2024-06-10 at 14:23 +0200, Andrew Lunn wrote: > On Mon, Jun 10, 2024 at 05:07:23PM +1200, Aryan Srivastava 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. > > How long does the first walk take? Rather than having fid_populated, > i > wounder if the walk should just be done in mv88e6xxx_vtu_setup() or > mv88e6xxx_atu_setup(). > > Andrew I agree, will implement this. The first walk does not take long, but would be better served in the setup. Thanks, Aryan.
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@alliedtelesis.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@alliedtelesis.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.
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); + return 0; } @@ -2636,7 +2642,14 @@ static int mv88e6xxx_port_vlan_leave(struct mv88e6xxx_chip *chip, return err; } - return mv88e6xxx_g1_atu_remove(chip, vlan.fid, port, false); + err = mv88e6xxx_g1_atu_remove(chip, vlan.fid, port, false); + if (err) + return err; + + /* Record FID freed in SW FID map */ + bitmap_clear(chip->fid_bitmap, vlan.fid, 1); + + return err; } static int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port, diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h index c54d305a1d83..2abe6f09c8df 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.h +++ b/drivers/net/dsa/mv88e6xxx/chip.h @@ -421,6 +421,10 @@ struct mv88e6xxx_chip { /* Bridge MST to SID mappings */ struct list_head msts; + + /* FID map */ + DECLARE_BITMAP(fid_bitmap, MV88E6XXX_N_FID); + bool fid_populated; }; struct mv88e6xxx_bus_ops {
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@alliedtelesis.co.nz> --- drivers/net/dsa/mv88e6xxx/chip.c | 25 +++++++++++++++++++------ drivers/net/dsa/mv88e6xxx/chip.h | 4 ++++ 2 files changed, 23 insertions(+), 6 deletions(-)