VLAN filtering with DSA

VLAN filtering with DSA

[Vladimir Oltean]

Hi,

Problem background:

Most DSA switch tags shift the EtherType to the right, causing the
master to not parse the VLAN as VLAN.
However, not all switches do that (example: tail tags), and if the DSA
master has "rx-vlan-filter: on" in ethtool -k, then we have a problem.
Therefore, I was thinking we could populate the VLAN table of the
master, just in case, so that it can work with a VLAN filtering master.
It would look something like this:

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 19b98a7231ec..b8aca2301c59 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -307,9 +307,10 @@ static int dsa_slave_vlan_add(struct net_device *dev,
 			      const struct switchdev_obj *obj,
 			      struct switchdev_trans *trans)
 {
+	struct net_device *master = dsa_slave_to_master(dev);
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 	struct switchdev_obj_port_vlan vlan;
-	int err;
+	int vid, err;
 
 	if (obj->orig_dev != dev)
 		return -EOPNOTSUPP;
@@ -336,6 +337,12 @@ static int dsa_slave_vlan_add(struct net_device *dev,
 	if (err)
 		return err;
 
+	for (vid = vlan.vid_begin; vid <= vlan.vid_end; vid++) {
+		err = vlan_vid_add(master, htons(ETH_P_8021Q), vid);
+		if (err)
+			return err;
+	}
+
 	return 0;
 }
 
@@ -379,8 +386,10 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
 static int dsa_slave_vlan_del(struct net_device *dev,
 			      const struct switchdev_obj *obj)
 {
+	struct net_device *master = dsa_slave_to_master(dev);
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 	struct switchdev_obj_port_vlan *vlan;
+	int vid, err;
 
 	if (obj->orig_dev != dev)
 		return -EOPNOTSUPP;
@@ -396,7 +405,14 @@ static int dsa_slave_vlan_del(struct net_device *dev,
 	/* Do not deprogram the CPU port as it may be shared with other user
 	 * ports which can be members of this VLAN as well.
 	 */
-	return dsa_port_vlan_del(dp, vlan);
+	err = dsa_port_vlan_del(dp, vlan);
+	if (err)
+		return err;
+
+	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++)
+		vlan_vid_del(master, htons(ETH_P_8021Q), vid);
+
+	return 0;
 }
 
 static int dsa_slave_port_obj_del(struct net_device *dev,
@@ -1241,6 +1257,7 @@ static int dsa_slave_get_ts_info(struct net_device *dev,
 static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
 				     u16 vid)
 {
+	struct net_device *master = dsa_slave_to_master(dev);
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 	struct switchdev_obj_port_vlan vlan = {
 		.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
@@ -1294,12 +1311,13 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
 	if (ret)
 		return ret;
 
-	return 0;
+	return vlan_vid_add(master, proto, vid);
 }
 
 static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
 				      u16 vid)
 {
+	struct net_device *master = dsa_slave_to_master(dev);
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 	struct switchdev_obj_port_vlan vlan = {
 		.vid_begin = vid,
@@ -1332,7 +1350,13 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
 	/* Do not deprogram the CPU port as it may be shared with other user
 	 * ports which can be members of this VLAN as well.
 	 */
-	return dsa_port_vlan_del(dp, &vlan);
+	ret = dsa_port_vlan_del(dp, &vlan);
+	if (ret)
+		return ret;
+
+	vlan_vid_del(master, proto, vid);
+
+	return 0;
 }
 
 struct dsa_hw_port {
-- 
2.25.1


Ok, now the problem.
This works, except when the master is another DSA switch.
You see, I hit this -EBUSY condition:

static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
				     u16 vid)
{
	...

	/* Check for a possible bridge VLAN entry now since there is no
	 * need to emulate the switchdev prepare + commit phase.
	 */
	if (dp->bridge_dev) {
		...
		/* br_vlan_get_info() returns -EINVAL or -ENOENT if the
		 * device, respectively the VID is not found, returning
		 * 0 means success, which is a failure for us here.
		 */
		ret = br_vlan_get_info(dp->bridge_dev, vid, &info);
		if (ret == 0)
			return -EBUSY;
	}
}

Introduced by:

commit 061f6a505ac33659eab007731c0f6374df39ab55
Author: Florian Fainelli <f.fainelli@gmail.com>
Date:   Wed Feb 20 14:35:39 2019 -0800

    net: dsa: Add ndo_vlan_rx_{add, kill}_vid implementation

    In order to properly support VLAN filtering being enabled/disabled on a
    bridge, while having other ports being non bridge port members, we need
    to support the ndo_vlan_rx_{add,kill}_vid callbacks in order to make
    sure the non-bridge ports can continue receiving VLAN tags, even when
    the switch is globally configured to do ingress/egress VID checking.

    Since we can call dsa_port_vlan_{add,del} with a bridge_dev pointer
    NULL, we now need to check that in these two functions.

    We specifically deal with two possibly problematic cases:

    - creating a bridge VLAN entry while there is an existing VLAN device
      claiming that same VID

    - creating a VLAN device while there is an existing bridge VLAN entry
      with that VID

    Those are both resolved with returning -EBUSY back to user-space.

    Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Florian, can you please reiterate what is the problem with calling
vlan_vid_add() with a VLAN that is installed by the bridge?

The effect of vlan_vid_add(), to my knowledge, is that the network
interface should add this VLAN to its filtering table, and not drop it.
So why return -EBUSY?

Thanks,
-Vladimir

Re: VLAN filtering with DSA

[Florian Fainelli]

+Ido,

On 9/10/2020 8:07 AM, Vladimir Oltean wrote:
> Florian, can you please reiterate what is the problem with calling
> vlan_vid_add() with a VLAN that is installed by the bridge?
> 
> The effect of vlan_vid_add(), to my knowledge, is that the network
> interface should add this VLAN to its filtering table, and not drop it.
> So why return -EBUSY?

I suppose that if you wanted to have an 802.1Q just for the sake of 
receiving VLAN tagged frames but not have them ingress the to the CPU, 
you could install an 802.1Q upper, but why would you do that unless the 
CPU should also receive that traffic?

The case that I wanted to cover was to avoid the two programming 
interfaces or the same VLAN, and prefer the bridge VLAN management over 
the 802.1Q upper, because once the switch port is in a bridge, that is 
what an user would expect to use.

If you have a bridge that is VLAN aware, it will manage the data and 
control path for us and that is all good since it is capable of dealing 
with VLAN tagged frames.

A non-VLAN aware bridge's data path is only allowed to see untagged 
traffic, so if you wanted somehow to inject untagged traffic into the 
bridge data path, then you would add a 802.1Q upper to that switch port, 
and somehow add that device into the bridge. There is a problem with 
that though, if you have mutliple bridge devices spanning the same 
switch, and you do the same thing on another switch port, with another 
802.1Q upper, I believe you could break isolation between bridges for 
that particular VID.

Most of this was based on discussions we had with Ido and him explaining 
to me how they were doing it in mlxsw.

AFAIR the other case which is that you already have a 802.1Q upper, and 
then you add the switch port to the bridge is permitted and the bridge 
would inherit the VLAN into its local database.

I did not put much thoughts back then into a cascading set-up, so some 
assumptions can certainly be broken, and in fact, are broken today as 
you experimented.
-- 
Florian

Re: VLAN filtering with DSA

[Florian Fainelli]

On 9/10/2020 8:07 AM, Vladimir Oltean wrote:
> Hi,
> 
> Problem background:
> 
> Most DSA switch tags shift the EtherType to the right, causing the
> master to not parse the VLAN as VLAN.
> However, not all switches do that (example: tail tags), and if the DSA
> master has "rx-vlan-filter: on" in ethtool -k, then we have a problem.
> Therefore, I was thinking we could populate the VLAN table of the
> master, just in case, so that it can work with a VLAN filtering master.
> It would look something like this:

Yes, doing what you suggest would make perfect sense for a DSA master 
that is capable of VLAN filtering, I did encounter that problem with 
e1000 and the dsa-loop.c mockup driver while working on a mock-up 802.1Q 
data path.

> 
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 19b98a7231ec..b8aca2301c59 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -307,9 +307,10 @@ static int dsa_slave_vlan_add(struct net_device *dev,
>   			      const struct switchdev_obj *obj,
>   			      struct switchdev_trans *trans)
>   {
> +	struct net_device *master = dsa_slave_to_master(dev);
>   	struct dsa_port *dp = dsa_slave_to_port(dev);
>   	struct switchdev_obj_port_vlan vlan;
> -	int err;
> +	int vid, err;
>   
>   	if (obj->orig_dev != dev)
>   		return -EOPNOTSUPP;
> @@ -336,6 +337,12 @@ static int dsa_slave_vlan_add(struct net_device *dev,
>   	if (err)
>   		return err;
>   
> +	for (vid = vlan.vid_begin; vid <= vlan.vid_end; vid++) {
> +		err = vlan_vid_add(master, htons(ETH_P_8021Q), vid);
> +		if (err)
> +			return err;
> +	}
> +
>   	return 0;
>   }
>   
> @@ -379,8 +386,10 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
>   static int dsa_slave_vlan_del(struct net_device *dev,
>   			      const struct switchdev_obj *obj)
>   {
> +	struct net_device *master = dsa_slave_to_master(dev);
>   	struct dsa_port *dp = dsa_slave_to_port(dev);
>   	struct switchdev_obj_port_vlan *vlan;
> +	int vid, err;
>   
>   	if (obj->orig_dev != dev)
>   		return -EOPNOTSUPP;
> @@ -396,7 +405,14 @@ static int dsa_slave_vlan_del(struct net_device *dev,
>   	/* Do not deprogram the CPU port as it may be shared with other user
>   	 * ports which can be members of this VLAN as well.
>   	 */
> -	return dsa_port_vlan_del(dp, vlan);
> +	err = dsa_port_vlan_del(dp, vlan);
> +	if (err)
> +		return err;
> +
> +	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++)
> +		vlan_vid_del(master, htons(ETH_P_8021Q), vid);
> +
> +	return 0;
>   }
>   
>   static int dsa_slave_port_obj_del(struct net_device *dev,
> @@ -1241,6 +1257,7 @@ static int dsa_slave_get_ts_info(struct net_device *dev,
>   static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
>   				     u16 vid)
>   {
> +	struct net_device *master = dsa_slave_to_master(dev);
>   	struct dsa_port *dp = dsa_slave_to_port(dev);
>   	struct switchdev_obj_port_vlan vlan = {
>   		.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
> @@ -1294,12 +1311,13 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
>   	if (ret)
>   		return ret;
>   
> -	return 0;
> +	return vlan_vid_add(master, proto, vid);
>   }
>   
>   static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
>   				      u16 vid)
>   {
> +	struct net_device *master = dsa_slave_to_master(dev);
>   	struct dsa_port *dp = dsa_slave_to_port(dev);
>   	struct switchdev_obj_port_vlan vlan = {
>   		.vid_begin = vid,
> @@ -1332,7 +1350,13 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
>   	/* Do not deprogram the CPU port as it may be shared with other user
>   	 * ports which can be members of this VLAN as well.
>   	 */
> -	return dsa_port_vlan_del(dp, &vlan);
> +	ret = dsa_port_vlan_del(dp, &vlan);
> +	if (ret)
> +		return ret;
> +
> +	vlan_vid_del(master, proto, vid);
> +
> +	return 0;
>   }
>   
>   struct dsa_hw_port {
> 

-- 
Florian

Re: VLAN filtering with DSA

[Vladimir Oltean]

On Thu, Sep 10, 2020 at 11:42:02AM -0700, Florian Fainelli wrote:
> On 9/10/2020 8:07 AM, Vladimir Oltean wrote:
> Yes, doing what you suggest would make perfect sense for a DSA master that
> is capable of VLAN filtering, I did encounter that problem with e1000 and
> the dsa-loop.c mockup driver while working on a mock-up 802.1Q data path.

Yes, I have another patch where I add those VLANs from tag_8021q.c which
I did not show here.

But if the DSA switch that uses tag_8021q is cascaded to another one,
that's of little use if the upper switch does not propagate that
configuration to its own upstream.

Thanks,
-Vladimir

Re: VLAN filtering with DSA

[Florian Fainelli]

On 9/10/2020 12:01 PM, Vladimir Oltean wrote:
> On Thu, Sep 10, 2020 at 11:42:02AM -0700, Florian Fainelli wrote:
>> On 9/10/2020 8:07 AM, Vladimir Oltean wrote:
>> Yes, doing what you suggest would make perfect sense for a DSA master that
>> is capable of VLAN filtering, I did encounter that problem with e1000 and
>> the dsa-loop.c mockup driver while working on a mock-up 802.1Q data path.
> 
> Yes, I have another patch where I add those VLANs from tag_8021q.c which
> I did not show here.
> 
> But if the DSA switch that uses tag_8021q is cascaded to another one,
> that's of little use if the upper switch does not propagate that
> configuration to its own upstream.

Yes, that would not work. As soon as you have a bridge spanning any of 
those switches, does not the problem go away by virtue of the switch 
port forcing the DSA master/upstream to be in promiscuous mode?
-- 
Florian

Re: VLAN filtering with DSA

[Vladimir Oltean]

On Thu, Sep 10, 2020 at 12:05:17PM -0700, Florian Fainelli wrote:
> On 9/10/2020 12:01 PM, Vladimir Oltean wrote:
> > On Thu, Sep 10, 2020 at 11:42:02AM -0700, Florian Fainelli wrote:
> > > On 9/10/2020 8:07 AM, Vladimir Oltean wrote:
> > > Yes, doing what you suggest would make perfect sense for a DSA master that
> > > is capable of VLAN filtering, I did encounter that problem with e1000 and
> > > the dsa-loop.c mockup driver while working on a mock-up 802.1Q data path.
> > 
> > Yes, I have another patch where I add those VLANs from tag_8021q.c which
> > I did not show here.
> > 
> > But if the DSA switch that uses tag_8021q is cascaded to another one,
> > that's of little use if the upper switch does not propagate that
> > configuration to its own upstream.
> 
> Yes, that would not work. As soon as you have a bridge spanning any of those
> switches, does not the problem go away by virtue of the switch port forcing
> the DSA master/upstream to be in promiscuous mode?

Well, yes, bridged it works but standalone it doesn't. A bit strange if
you ask me.

-Vladimir

Re: VLAN filtering with DSA

[Florian Fainelli]

On 9/10/2020 12:08 PM, Vladimir Oltean wrote:
> On Thu, Sep 10, 2020 at 12:05:17PM -0700, Florian Fainelli wrote:
>> On 9/10/2020 12:01 PM, Vladimir Oltean wrote:
>>> On Thu, Sep 10, 2020 at 11:42:02AM -0700, Florian Fainelli wrote:
>>>> On 9/10/2020 8:07 AM, Vladimir Oltean wrote:
>>>> Yes, doing what you suggest would make perfect sense for a DSA master that
>>>> is capable of VLAN filtering, I did encounter that problem with e1000 and
>>>> the dsa-loop.c mockup driver while working on a mock-up 802.1Q data path.
>>>
>>> Yes, I have another patch where I add those VLANs from tag_8021q.c which
>>> I did not show here.
>>>
>>> But if the DSA switch that uses tag_8021q is cascaded to another one,
>>> that's of little use if the upper switch does not propagate that
>>> configuration to its own upstream.
>>
>> Yes, that would not work. As soon as you have a bridge spanning any of those
>> switches, does not the problem go away by virtue of the switch port forcing
>> the DSA master/upstream to be in promiscuous mode?
> 
> Well, yes, bridged it works but standalone it doesn't. A bit strange if
> you ask me.

Agreed there is no question this should be fixed.
-- 
Florian

Re: VLAN filtering with DSA

[Ido Schimmel]

On Thu, Sep 10, 2020 at 11:41:04AM -0700, Florian Fainelli wrote:
> +Ido,
> 
> On 9/10/2020 8:07 AM, Vladimir Oltean wrote:
> > Florian, can you please reiterate what is the problem with calling
> > vlan_vid_add() with a VLAN that is installed by the bridge?
> > 
> > The effect of vlan_vid_add(), to my knowledge, is that the network
> > interface should add this VLAN to its filtering table, and not drop it.
> > So why return -EBUSY?

Can you clarify when you return -EBUSY? At least in mlxsw we return an
error in case we have a VLAN-aware bridge taking care of some VLAN and
then user space tries to install a VLAN upper with the same VLAN on the
same port. See more below.

> 
> I suppose that if you wanted to have an 802.1Q just for the sake of
> receiving VLAN tagged frames but not have them ingress the to the CPU, you
> could install an 802.1Q upper, but why would you do that unless the CPU
> should also receive that traffic?
> 
> The case that I wanted to cover was to avoid the two programming interfaces
> or the same VLAN, and prefer the bridge VLAN management over the 802.1Q
> upper, because once the switch port is in a bridge, that is what an user
> would expect to use.
> 
> If you have a bridge that is VLAN aware, it will manage the data and control
> path for us and that is all good since it is capable of dealing with VLAN
> tagged frames.
> 
> A non-VLAN aware bridge's data path is only allowed to see untagged traffic,
> so if you wanted somehow to inject untagged traffic into the bridge data
> path, then you would add a 802.1Q upper to that switch port, and somehow add
> that device into the bridge. There is a problem with that though, if you
> have mutliple bridge devices spanning the same switch, and you do the same
> thing on another switch port, with another 802.1Q upper, I believe you could
> break isolation between bridges for that particular VID.

At least in mlxsw this is handled by mapping the two {Port, VID} pairs
into different FIDs, each corresponding to a different bridge instance,
thereby maintaining the isolation.

> 
> Most of this was based on discussions we had with Ido and him explaining to
> me how they were doing it in mlxsw.
> 
> AFAIR the other case which is that you already have a 802.1Q upper, and then
> you add the switch port to the bridge is permitted and the bridge would
> inherit the VLAN into its local database.

If you have swp1 and swp1.10, you can put swp1 in a VLAN-aware bridge
and swp1.10 in a VLAN-unaware bridge. If you add VLAN 10 as part of the
VLAN-aware bridge on swp1, traffic tagged with this VLAN will still be
injected into the stack via swp1.10.

I'm not sure what is the use case for such a configuration and we reject
it in mlxsw.

> 
> I did not put much thoughts back then into a cascading set-up, so some
> assumptions can certainly be broken, and in fact, are broken today as you
> experimented.
> -- 
> Florian

Re: VLAN filtering with DSA

[Vladimir Oltean]

On Fri, Sep 11, 2020 at 04:20:58PM +0300, Ido Schimmel wrote:
> On Thu, Sep 10, 2020 at 11:41:04AM -0700, Florian Fainelli wrote:
> > +Ido,
> >
> > On 9/10/2020 8:07 AM, Vladimir Oltean wrote:
> > > Florian, can you please reiterate what is the problem with calling
> > > vlan_vid_add() with a VLAN that is installed by the bridge?
> > >
> > > The effect of vlan_vid_add(), to my knowledge, is that the network
> > > interface should add this VLAN to its filtering table, and not drop it.
> > > So why return -EBUSY?
>
> Can you clarify when you return -EBUSY? At least in mlxsw we return an
> error in case we have a VLAN-aware bridge taking care of some VLAN and
> then user space tries to install a VLAN upper with the same VLAN on the
> same port. See more below.
>

In the original post Message-ID: <20200910150738.mwhh2i6j2qgacqev@skbuf>
I had copied this piece of code:

static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
				     u16 vid)
{
	...

	/* Check for a possible bridge VLAN entry now since there is no
	 * need to emulate the switchdev prepare + commit phase.
	 */
	if (dp->bridge_dev) {
		...
		/* br_vlan_get_info() returns -EINVAL or -ENOENT if the
		 * device, respectively the VID is not found, returning
		 * 0 means success, which is a failure for us here.
		 */
		ret = br_vlan_get_info(dp->bridge_dev, vid, &info);
		if (ret == 0)
			return -EBUSY;
	}
}

> > Most of this was based on discussions we had with Ido and him explaining to
> > me how they were doing it in mlxsw.
> >
> > AFAIR the other case which is that you already have a 802.1Q upper, and then
> > you add the switch port to the bridge is permitted and the bridge would
> > inherit the VLAN into its local database.
>
> If you have swp1 and swp1.10, you can put swp1 in a VLAN-aware bridge
> and swp1.10 in a VLAN-unaware bridge. If you add VLAN 10 as part of the
> VLAN-aware bridge on swp1, traffic tagged with this VLAN will still be
> injected into the stack via swp1.10.
>
> I'm not sure what is the use case for such a configuration and we reject
> it in mlxsw.

Maybe the problem has to do with the fact that Florian took the
.ndo_vlan_rx_add_vid() callback as a shortcut for deducing that there is
an 8021q upper interface.

Currently there are other places in the network stack that don't really
work with a network interface that has problems with an interface that
has "rx-vlan-filter: on" in ethtool -k. See this discussion with Jiri on
the use of tc-vlan:

https://www.spinics.net/lists/netdev/msg645931.html

So, even though today .ndo_vlan_rx_add_vid() is only called from 8021q,
maybe we should dispel the myth that it's specific to 8021q somehow.

Maybe DSA should start tracking its upper interfaces, after all? Not
convinced though.

Thanks,
-Vladimir

Re: VLAN filtering with DSA

[Vladimir Oltean]

On Fri, Sep 11, 2020 at 07:30:42PM +0300, Vladimir Oltean wrote:
> Currently there are other places in the network stack that don't really
> work with a network interface that has problems with an interface that
> has "rx-vlan-filter: on" in ethtool -k.

Wow, I should learn how to write.
I meant:

Currently there are other places in the network stack that don't really
work with a network interface that has "rx-vlan-filter: on" in
ethtool -k.

Re: VLAN filtering with DSA

[Florian Fainelli]

On 9/11/2020 9:30 AM, Vladimir Oltean wrote:
> On Fri, Sep 11, 2020 at 04:20:58PM +0300, Ido Schimmel wrote:
>> On Thu, Sep 10, 2020 at 11:41:04AM -0700, Florian Fainelli wrote:
>>> +Ido,
>>>
>>> On 9/10/2020 8:07 AM, Vladimir Oltean wrote:
>>>> Florian, can you please reiterate what is the problem with calling
>>>> vlan_vid_add() with a VLAN that is installed by the bridge?
>>>>
>>>> The effect of vlan_vid_add(), to my knowledge, is that the network
>>>> interface should add this VLAN to its filtering table, and not drop it.
>>>> So why return -EBUSY?
>>
>> Can you clarify when you return -EBUSY? At least in mlxsw we return an
>> error in case we have a VLAN-aware bridge taking care of some VLAN and
>> then user space tries to install a VLAN upper with the same VLAN on the
>> same port. See more below.
>>
> 
> In the original post Message-ID: <20200910150738.mwhh2i6j2qgacqev@skbuf>
> I had copied this piece of code:
> 
> static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
> 				     u16 vid)
> {
> 	...
> 
> 	/* Check for a possible bridge VLAN entry now since there is no
> 	 * need to emulate the switchdev prepare + commit phase.
> 	 */
> 	if (dp->bridge_dev) {
> 		...
> 		/* br_vlan_get_info() returns -EINVAL or -ENOENT if the
> 		 * device, respectively the VID is not found, returning
> 		 * 0 means success, which is a failure for us here.
> 		 */
> 		ret = br_vlan_get_info(dp->bridge_dev, vid, &info);
> 		if (ret == 0)
> 			return -EBUSY;
> 	}
> }
> 
>>> Most of this was based on discussions we had with Ido and him explaining to
>>> me how they were doing it in mlxsw.
>>>
>>> AFAIR the other case which is that you already have a 802.1Q upper, and then
>>> you add the switch port to the bridge is permitted and the bridge would
>>> inherit the VLAN into its local database.
>>
>> If you have swp1 and swp1.10, you can put swp1 in a VLAN-aware bridge
>> and swp1.10 in a VLAN-unaware bridge. If you add VLAN 10 as part of the
>> VLAN-aware bridge on swp1, traffic tagged with this VLAN will still be
>> injected into the stack via swp1.10.
>>
>> I'm not sure what is the use case for such a configuration and we reject
>> it in mlxsw.
> 
> Maybe the problem has to do with the fact that Florian took the
> .ndo_vlan_rx_add_vid() callback as a shortcut for deducing that there is
> an 8021q upper interface.

Yes, that was/is definitively the assumption when that code was added, 
and as you indicate below, as of today, only 802.1Q upppers call that NDO.

> 
> Currently there are other places in the network stack that don't really
> work with a network interface that has problems with an interface that
> has "rx-vlan-filter: on" in ethtool -k. See this discussion with Jiri on
> the use of tc-vlan:
> 
> https://www.spinics.net/lists/netdev/msg645931.html
> 
> So, even though today .ndo_vlan_rx_add_vid() is only called from 8021q,
> maybe we should dispel the myth that it's specific to 8021q somehow.
> 
> Maybe DSA should start tracking its upper interfaces, after all? Not
> convinced though.

If we started doing that, it may make sense to refactor the existing 
mlxsw code into something more generic that can be used almost as-is by 
switchdev driver as well as DSA, which ... are switchdev drivers to some 
extent as well.
-- 
Florian