Re: [PATCH net-next v3 08/12] net: dsa: rzn1-a5psw: add FDB support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, May 04, 2022 at 11:29:56AM +0200, Clément Léger wrote:
> +static int a5psw_port_fdb_del(struct dsa_switch *ds, int port,
> +			      const unsigned char *addr, u16 vid,
> +			      struct dsa_db db)
> +{
> +	struct a5psw *a5psw = ds->priv;
> +	union lk_data lk_data = {0};
> +	bool clear = false;
> +	int ret = 0;
> +	u16 entry;
> +	u32 reg;
> +
> +	ether_addr_copy(lk_data.entry.mac, addr);
> +
> +	spin_lock(&a5psw->lk_lock);
> +
> +	ret = a5psw_lk_execute_lookup(a5psw, &lk_data, &entry);
> +	if (ret) {
> +		dev_err(a5psw->dev, "Failed to lookup mac address\n");
> +		goto lk_unlock;
> +	}
> +
> +	lk_data.hi = a5psw_reg_readl(a5psw, A5PSW_LK_DATA_HI);
> +	if (!lk_data.entry.valid) {
> +		dev_err(a5psw->dev, "Tried to remove non-existing entry\n");
> +		ret = -ENOENT;
> +		goto lk_unlock;

These error messages can happen under quite normal use with your hardware.
For example:

ip link add br0 type bridge && ip link set br0 master br0
bridge fdb add dev swp0 00:01:02:03:04:05 vid 1 master static
bridge fdb add dev swp0 00:01:02:03:04:05 vid 2 master static
bridge fdb del dev swp0 00:01:02:03:04:05 vid 2 master static
bridge fdb del dev swp0 00:01:02:03:04:05 vid 1 master static

Because the driver ignores the VID, then as soon as the VID 2 FDB entry
is removed, the VID 1 FDB entry doesn't exist anymore, either.

The above is a bit artificial. More practically, the bridge installs
local FDB entries (MAC address of bridge device, MAC addresses of ports)
once in the VLAN-unaware database (VID 0), and once per VLAN installed
on br0.

When the MAC address of br0 is different from that of the ports, and it
is changed by the user, you'll get these errors too, since those changes
translate into a deletion of the old local FDB entries (installed as FDB
entries towards the CPU port). Do you want the users to see them and
think something is wrong?  I mean, something _is_ wrong (the hardware),
but you have to work with that somehow.

> +	}
> +
> +	lk_data.entry.port_mask &= ~BIT(port);
> +	/* If there is no more port in the mask, clear the entry */
> +	if (lk_data.entry.port_mask == 0)
> +		clear = true;
> +
> +	a5psw_reg_writel(a5psw, A5PSW_LK_DATA_HI, lk_data.hi);
> +
> +	reg = entry;
> +	if (clear)
> +		reg |= A5PSW_LK_ADDR_CTRL_CLEAR;
> +	else
> +		reg |= A5PSW_LK_ADDR_CTRL_WRITE;
> +
> +	ret = a5psw_lk_execute_ctrl(a5psw, &reg);
> +	if (ret)
> +		goto lk_unlock;
> +
> +	/* Decrement LEARNCOUNT */
> +	if (clear) {
> +		reg = A5PSW_LK_LEARNCOUNT_MODE_DEC;
> +		a5psw_reg_writel(a5psw, A5PSW_LK_LEARNCOUNT, reg);
> +	}
> +
> +lk_unlock:
> +	spin_unlock(&a5psw->lk_lock);
> +
> +	return ret;
> +}



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux