forked from Minki/linux
net: dsa: mv88e6xxx: isolate the ATU databases of standalone and bridged ports
Similar to commit6087175b79("net: dsa: mt7530: use independent VLAN learning on VLAN-unaware bridges"), software forwarding between an unoffloaded LAG port (a bonding interface with an unsupported policy) and a mv88e6xxx user port directly under a bridge is broken. We adopt the same strategy, which is to make the standalone ports not find any ATU entry learned on a bridge port. Theory: the mv88e6xxx ATU is looked up by FID and MAC address. There are as many FIDs as VIDs (4096). The FID is derived from the VID when possible (the VTU maps a VID to a FID), with a fallback to the port based default FID value when not (802.1Q Mode is disabled on the port, or the classified VID isn't present in the VTU). The mv88e6xxx driver makes the following use of FIDs and VIDs: - the port's DefaultVID (to which untagged & pvid-tagged packets get classified) is 0 and is absent from the VTU, so this kind of packets is processed in FID 0, the default FID assigned by mv88e6xxx_setup_port. - every time a bridge VLAN is created, mv88e6xxx_port_vlan_join() -> mv88e6xxx_atu_new() associates a FID with that VID which increases linearly starting from 1. Like this: bridge vlan add dev lan0 vid 100 # FID 1 bridge vlan add dev lan1 vid 100 # still FID 1 bridge vlan add dev lan2 vid 1024 # FID 2 The FID allocation made by the driver is sub-optimal for the following reasons: (a) A standalone port has a DefaultPVID of 0 and a default FID of 0 too. A VLAN-unaware bridged port has a DefaultPVID of 0 and a default FID of 0 too. The difference is that the bridged ports may learn ATU entries, while the standalone port has the requirement that it must not, and must not find them either. Standalone ports must not use the same FID as ports belonging to a bridge. All standalone ports can use the same FID, since the ATU will never have an entry in that FID. (b) Multiple VLAN-unaware bridges will all use a DefaultPVID of 0 and a default FID of 0 on all their ports. The FDBs will not be isolated between these bridges. Every VLAN-unaware bridge must use the same FID on all its ports, different from the FID of other bridge ports. (c) Each bridge VLAN uses a unique FID which is useful for Independent VLAN Learning, but the same VLAN ID on multiple VLAN-aware bridges will result in the same FID being used by mv88e6xxx_atu_new(). The correct behavior is for VLAN 1 in br0 to have a different FID compared to VLAN 1 in br1. This patch cannot fix all the above. Traditionally the DSA framework did not care about this, and the reality is that DSA core involvement is needed for the aforementioned issues to be solved. The only thing we can solve here is an issue which does not require API changes, and that is issue (a), aka use a different FID for standalone ports vs ports under VLAN-unaware bridges. The first step is deciding what VID and FID to use for standalone ports, and what VID and FID for bridged ports. The 0/0 pair for standalone ports is what they used up till now, let's keep using that. For bridged ports, there are 2 cases: - VLAN-aware ports will never end up using the port default FID, because packets will always be classified to a VID in the VTU or dropped otherwise. The FID is the one associated with the VID in the VTU. - On VLAN-unaware ports, we _could_ leave their DefaultVID (pvid) at zero (just as in the case of standalone ports), and just change the port's default FID from 0 to a different number (say 1). However, Tobias points out that there is one more requirement to cater to: cross-chip bridging. The Marvell DSA header does not carry the FID in it, only the VID. So once a packet crosses a DSA link, if it has a VID of zero it will get classified to the default FID of that cascade port. Relying on a port default FID for upstream cascade ports results in contradictions: a default FID of 0 breaks ATU isolation of bridged ports on the downstream switch, a default FID of 1 breaks standalone ports on the downstream switch. So not only must standalone ports have different FIDs compared to bridged ports, they must also have different DefaultVID values. IEEE 802.1Q defines two reserved VID values: 0 and 4095. So we simply choose 4095 as the DefaultVID of ports belonging to VLAN-unaware bridges, and VID 4095 maps to FID 1. For the xmit operation to look up the same ATU database, we need to put VID 4095 in DSA tags sent to ports belonging to VLAN-unaware bridges too. All shared ports are configured to map this VID to the bridging FID, because they are members of that VLAN in the VTU. Shared ports don't need to have 802.1QMode enabled in any way, they always parse the VID from the DSA header, they don't need to look at the 802.1Q header. We install VID 4095 to the VTU in mv88e6xxx_setup_port(), with the mention that mv88e6xxx_vtu_setup() which was located right below that call was flushing the VTU so those entries wouldn't be preserved. So we need to relocate the VTU flushing prior to the port initialization during ->setup(). Also note that this is why it is safe to assume that VID 4095 will get associated with FID 1: the user ports haven't been created, so there is no avenue for the user to create a bridge VLAN which could otherwise race with the creation of another FID which would otherwise use up the non-reserved FID value of 1. [ Currently mv88e6xxx_port_vlan_join() doesn't have the option of specifying a preferred FID, it always calls mv88e6xxx_atu_new(). ] mv88e6xxx_port_db_load_purge() is the function to access the ATU for FDB/MDB entries, and it used to determine the FID to use for VLAN-unaware FDB entries (VID=0) using mv88e6xxx_port_get_fid(). But the driver only called mv88e6xxx_port_set_fid() once, during probe, so no surprises, the port FID was always 0, the call to get_fid() was redundant. As much as I would have wanted to not touch that code, the logic is broken when we add a new FID which is not the port-based default. Now the port-based default FID only corresponds to standalone ports, and FDB/MDB entries belong to the bridging service. So while in the future, when the DSA API will support FDB isolation, we will have to figure out the FID based on the bridge number, for now there's a single bridging FID, so hardcode that. Lastly, the tagger needs to check, when it is transmitting a VLAN untagged skb, whether it is sending it towards a bridged or a standalone port. When we see it is bridged we assume the bridge is VLAN-unaware. Not because it cannot be VLAN-aware but: - if we are transmitting from a VLAN-aware bridge we are likely doing so using TX forwarding offload. That code path guarantees that skbs have a vlan hwaccel tag in them, so we would not enter the "else" branch of the "if (skb->protocol == htons(ETH_P_8021Q))" condition. - if we are transmitting on behalf of a VLAN-aware bridge but with no TX forwarding offload (no PVT support, out of space in the PVT, whatever), we would indeed be transmitting with VLAN 4095 instead of the bridge device's pvid. However we would be injecting a "From CPU" frame, and the switch won't learn from that - it only learns from "Forward" frames. So it is inconsequential for address learning. And VLAN 4095 is absolutely enough for the frame to exit the switch, since we never remove that VLAN from any port. Fixes:57e661aae6("net: dsa: mv88e6xxx: Link aggregation support") Reported-by: Tobias Waldekranz <tobias@waldekranz.com> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Vladimir Oltean
Jakub Kicinski
parent
8b6836d824
commit
5bded8259e
@ -11153,6 +11153,7 @@ S: Maintained
|
||||
F: Documentation/devicetree/bindings/net/dsa/marvell.txt
|
||||
F: Documentation/networking/devlink/mv88e6xxx.rst
|
||||
F: drivers/net/dsa/mv88e6xxx/
|
||||
F: include/linux/dsa/mv88e6xxx.h
|
||||
F: include/linux/platform_data/mv88e6xxx.h
|
||||
|
||||
MARVELL ARMADA 3700 PHY DRIVERS
|
||||
|
||||
@ -12,6 +12,7 @@
|
||||
|
||||
#include <linux/bitfield.h>
|
||||
#include <linux/delay.h>
|
||||
#include <linux/dsa/mv88e6xxx.h>
|
||||
#include <linux/etherdevice.h>
|
||||
#include <linux/ethtool.h>
|
||||
#include <linux/if_bridge.h>
|
||||
@ -1681,13 +1682,17 @@ static int mv88e6xxx_port_commit_pvid(struct mv88e6xxx_chip *chip, int port)
|
||||
{
|
||||
struct dsa_port *dp = dsa_to_port(chip->ds, port);
|
||||
struct mv88e6xxx_port *p = &chip->ports[port];
|
||||
u16 pvid = MV88E6XXX_VID_STANDALONE;
|
||||
bool drop_untagged = false;
|
||||
u16 pvid = 0;
|
||||
int err;
|
||||
|
||||
if (dp->bridge_dev && br_vlan_enabled(dp->bridge_dev)) {
|
||||
pvid = p->bridge_pvid.vid;
|
||||
drop_untagged = !p->bridge_pvid.valid;
|
||||
if (dp->bridge_dev) {
|
||||
if (br_vlan_enabled(dp->bridge_dev)) {
|
||||
pvid = p->bridge_pvid.vid;
|
||||
drop_untagged = !p->bridge_pvid.valid;
|
||||
} else {
|
||||
pvid = MV88E6XXX_VID_BRIDGED;
|
||||
}
|
||||
}
|
||||
|
||||
err = mv88e6xxx_port_set_pvid(chip, port, pvid);
|
||||
@ -1754,11 +1759,15 @@ static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
|
||||
u16 fid;
|
||||
int err;
|
||||
|
||||
/* Null VLAN ID corresponds to the port private database */
|
||||
/* Ports have two private address databases: one for when the port is
|
||||
* standalone and one for when the port is under a bridge and the
|
||||
* 802.1Q mode is disabled. When the port is standalone, DSA wants its
|
||||
* address database to remain 100% empty, so we never load an ATU entry
|
||||
* into a standalone port's database. Therefore, translate the null
|
||||
* VLAN ID into the port's database used for VLAN-unaware bridging.
|
||||
*/
|
||||
if (vid == 0) {
|
||||
err = mv88e6xxx_port_get_fid(chip, port, &fid);
|
||||
if (err)
|
||||
return err;
|
||||
fid = MV88E6XXX_FID_BRIDGED;
|
||||
} else {
|
||||
err = mv88e6xxx_vtu_get(chip, vid, &vlan);
|
||||
if (err)
|
||||
@ -2434,7 +2443,16 @@ static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
|
||||
int err;
|
||||
|
||||
mv88e6xxx_reg_lock(chip);
|
||||
|
||||
err = mv88e6xxx_bridge_map(chip, br);
|
||||
if (err)
|
||||
goto unlock;
|
||||
|
||||
err = mv88e6xxx_port_commit_pvid(chip, port);
|
||||
if (err)
|
||||
goto unlock;
|
||||
|
||||
unlock:
|
||||
mv88e6xxx_reg_unlock(chip);
|
||||
|
||||
return err;
|
||||
@ -2444,11 +2462,20 @@ static void mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port,
|
||||
struct net_device *br)
|
||||
{
|
||||
struct mv88e6xxx_chip *chip = ds->priv;
|
||||
int err;
|
||||
|
||||
mv88e6xxx_reg_lock(chip);
|
||||
|
||||
if (mv88e6xxx_bridge_map(chip, br) ||
|
||||
mv88e6xxx_port_vlan_map(chip, port))
|
||||
dev_err(ds->dev, "failed to remap in-chip Port VLAN\n");
|
||||
|
||||
err = mv88e6xxx_port_commit_pvid(chip, port);
|
||||
if (err)
|
||||
dev_err(ds->dev,
|
||||
"port %d failed to restore standalone pvid: %pe\n",
|
||||
port, ERR_PTR(err));
|
||||
|
||||
mv88e6xxx_reg_unlock(chip);
|
||||
}
|
||||
|
||||
@ -2894,6 +2921,20 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
|
||||
if (err)
|
||||
return err;
|
||||
|
||||
/* Associate MV88E6XXX_VID_BRIDGED with MV88E6XXX_FID_BRIDGED in the
|
||||
* ATU by virtue of the fact that mv88e6xxx_atu_new() will pick it as
|
||||
* the first free FID after MV88E6XXX_FID_STANDALONE. This will be used
|
||||
* as the private PVID on ports under a VLAN-unaware bridge.
|
||||
* Shared (DSA and CPU) ports must also be members of it, to translate
|
||||
* the VID from the DSA tag into MV88E6XXX_FID_BRIDGED, instead of
|
||||
* relying on their port default FID.
|
||||
*/
|
||||
err = mv88e6xxx_port_vlan_join(chip, port, MV88E6XXX_VID_BRIDGED,
|
||||
MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_UNTAGGED,
|
||||
false);
|
||||
if (err)
|
||||
return err;
|
||||
|
||||
if (chip->info->ops->port_set_jumbo_size) {
|
||||
err = chip->info->ops->port_set_jumbo_size(chip, port, 10218);
|
||||
if (err)
|
||||
@ -2966,7 +3007,7 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
|
||||
* database, and allow bidirectional communication between the
|
||||
* CPU and DSA port(s), and the other ports.
|
||||
*/
|
||||
err = mv88e6xxx_port_set_fid(chip, port, 0);
|
||||
err = mv88e6xxx_port_set_fid(chip, port, MV88E6XXX_FID_STANDALONE);
|
||||
if (err)
|
||||
return err;
|
||||
|
||||
@ -3156,6 +3197,10 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
|
||||
}
|
||||
}
|
||||
|
||||
err = mv88e6xxx_vtu_setup(chip);
|
||||
if (err)
|
||||
goto unlock;
|
||||
|
||||
/* Setup Switch Port Registers */
|
||||
for (i = 0; i < mv88e6xxx_num_ports(chip); i++) {
|
||||
if (dsa_is_unused_port(ds, i))
|
||||
@ -3185,10 +3230,6 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
|
||||
if (err)
|
||||
goto unlock;
|
||||
|
||||
err = mv88e6xxx_vtu_setup(chip);
|
||||
if (err)
|
||||
goto unlock;
|
||||
|
||||
err = mv88e6xxx_pvt_setup(chip);
|
||||
if (err)
|
||||
goto unlock;
|
||||
|
||||
@ -21,6 +21,9 @@
|
||||
#define EDSA_HLEN 8
|
||||
#define MV88E6XXX_N_FID 4096
|
||||
|
||||
#define MV88E6XXX_FID_STANDALONE 0
|
||||
#define MV88E6XXX_FID_BRIDGED 1
|
||||
|
||||
/* PVT limits for 4-bit port and 5-bit switch */
|
||||
#define MV88E6XXX_MAX_PVT_SWITCHES 32
|
||||
#define MV88E6XXX_MAX_PVT_PORTS 16
|
||||
|
||||
13
include/linux/dsa/mv88e6xxx.h
Normal file
13
include/linux/dsa/mv88e6xxx.h
Normal file
@ -0,0 +1,13 @@
|
||||
/* SPDX-License-Identifier: GPL-2.0
|
||||
* Copyright 2021 NXP
|
||||
*/
|
||||
|
||||
#ifndef _NET_DSA_TAG_MV88E6XXX_H
|
||||
#define _NET_DSA_TAG_MV88E6XXX_H
|
||||
|
||||
#include <linux/if_vlan.h>
|
||||
|
||||
#define MV88E6XXX_VID_STANDALONE 0
|
||||
#define MV88E6XXX_VID_BRIDGED (VLAN_N_VID - 1)
|
||||
|
||||
#endif
|
||||
@ -45,6 +45,7 @@
|
||||
* 6 6 2 2 4 2 N
|
||||
*/
|
||||
|
||||
#include <linux/dsa/mv88e6xxx.h>
|
||||
#include <linux/etherdevice.h>
|
||||
#include <linux/list.h>
|
||||
#include <linux/slab.h>
|
||||
@ -164,16 +165,21 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
|
||||
dsa_header[2] &= ~0x10;
|
||||
}
|
||||
} else {
|
||||
struct net_device *br = dp->bridge_dev;
|
||||
u16 vid;
|
||||
|
||||
vid = br ? MV88E6XXX_VID_BRIDGED : MV88E6XXX_VID_STANDALONE;
|
||||
|
||||
skb_push(skb, DSA_HLEN + extra);
|
||||
dsa_alloc_etype_header(skb, DSA_HLEN + extra);
|
||||
|
||||
/* Construct untagged DSA tag. */
|
||||
/* Construct DSA header from untagged frame. */
|
||||
dsa_header = dsa_etype_header_pos_tx(skb) + extra;
|
||||
|
||||
dsa_header[0] = (cmd << 6) | tag_dev;
|
||||
dsa_header[1] = tag_port << 3;
|
||||
dsa_header[2] = 0;
|
||||
dsa_header[3] = 0;
|
||||
dsa_header[2] = vid >> 8;
|
||||
dsa_header[3] = vid & 0xff;
|
||||
}
|
||||
|
||||
return skb;
|
||||
|
||||
Loading…
Reference in New Issue
Block a user