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

["Marek Behún" <kabel@kernel.org>]

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() ?