Messages in this thread | | | Date | Fri, 31 May 2019 10:31:05 -0400 | From | Vivien Didelot <> | Subject | Re: [PATCH] net: dsa: mv88e6xxx: avoid error message on remove from VLAN 0 |
| |
Hi Nikita,
On Fri, 31 May 2019 10:35:14 +0300, Nikita Yushchenko <nikita.yoush@cogentembedded.com> wrote: > When non-bridged, non-vlan'ed mv88e6xxx port is moving down, error > message is logged: > > failed to kill vid 0081/0 for device eth_cu_1000_4 > > This is caused by call from __vlan_vid_del() with vin set to zero, over > call chain this results into _mv88e6xxx_port_vlan_del() called with > vid=0, and mv88e6xxx_vtu_get() called from there returns -EINVAL. > > On symmetric path moving port up, call goes through > mv88e6xxx_port_vlan_prepare() that calls mv88e6xxx_port_check_hw_vlan() > that returns -EOPNOTSUPP for zero vid. > > This patch changes mv88e6xxx_vtu_get() to also return -EOPNOTSUPP for > zero vid, then this error code is explicitly cleared in > dsa_slave_vlan_rx_kill_vid() and error message is no longer logged. > > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> > --- > drivers/net/dsa/mv88e6xxx/chip.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c > index 28414db979b0..6b77fde5f0e4 100644 > --- a/drivers/net/dsa/mv88e6xxx/chip.c > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > @@ -1392,7 +1392,7 @@ static int mv88e6xxx_vtu_get(struct mv88e6xxx_chip *chip, u16 vid, > int err; > > if (!vid) > - return -EINVAL; > + return -EOPNOTSUPP; > > entry->vid = vid - 1; > entry->valid = false;
I'm not sure that I like the semantic of it, because the driver can actually support VID 0 per-se, only the kernel does not use VLAN 0. Thus I would avoid calling the port_vlan_del() ops for VID 0, directly into the upper DSA layer.
Florian, Andrew, wouldn't the following patch be more adequate?
diff --git a/net/dsa/slave.c b/net/dsa/slave.c index 1e2ae9d59b88..80f228258a92 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -1063,6 +1063,10 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, struct bridge_vlan_info info; int ret;
+ /* VID 0 has a special meaning and is never programmed in hardware */ + if (!vid) + return 0; + /* Check for a possible bridge VLAN entry now since there is no * need to emulate the switchdev prepare + commit phase. */
Thanks, Vivien
| |