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
