Re: [PATCH v4 net-next 09/10] net: dsa: microchip: add support for fdb and mdb management

[Vladimir Oltean]

On Thu, Oct 07, 2021 at 08:41:59PM +0530, Prasanna Vengateshan wrote:
> Support for fdb_add, mdb_add, fdb_del, mdb_del and
> fdb_dump operations. ALU1 and ALU2 are used for fdb operations.
> 
> fdb_add: find any existing entries and update the port map.
> if ALU1 write is failed and attempt to write ALU2.
> If ALU2 is also failed then exit. Clear WRITE_FAIL for both ALU1
> & ALU2.
> 
> fdb_del: find the matching entry and clear the respective port
> in the port map by writing the ALU tables
> 
> fdb_dump: read and dump 2 ALUs upto last entry. ALU_START bit is
> used to find the last entry. If the read is timed out, then pass
> the error message.
> 
> mdb_add: Find the empty slot in ALU and update the port map &
> mac address by writing the ALU
> 
> mdb_del: find the matching entry and delete the respective port
> in port map by writing the ALU
> 
> For MAC address, could not use upper_32_bits() & lower_32_bits()
> as per Vladimir proposal since it gets accessed in terms of 16bits.
> I tried to have common API to get 16bits based on index but shifting
> seems to be straight-forward.
> 
> Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@xxxxxxxxxxxxx>
> ---

Reviewed-by: Vladimir Oltean <olteanv@xxxxxxxxx>

> +static int lan937x_port_fdb_dump(struct dsa_switch *ds, int port,
> +				 dsa_fdb_dump_cb_t *cb, void *data)
> +{

> +			if (alu.port_forward & BIT(port)) {
> +				ret = cb(alu.mac, alu.fid, alu.is_static, data);

A bit strange that you report the FID and not the VID here.

> +				if (ret)
> +					goto exit;
> +			}
> +		} while (lan937x_data & ALU_START);
> +
> +exit:
> +		/* stop ALU search & continue to next ALU if available */
> +		ret = ksz_write32(dev, REG_SW_ALU_CTRL(i), 0);
> +	}
> +
> +	mutex_unlock(&dev->alu_mutex);
> +
> +	return ret;
> +}