From a42c8e33f2044d6b56f167b5506c7e09e5b702c2 Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Thu, 9 Nov 2017 22:29:51 +0100 Subject: [PATCH 1/6] net: dsa: Fix SWITCHDEV_ATTR_ID_PORT_PARENT_ID SWITCHDEV_ATTR_ID_PORT_PARENT_ID is used by the software bridge when determining which ports to flood a packet out. If the packet originated from a switch, it assumes the switch has already flooded the packet out the switches ports, so the bridge should not flood the packet itself out switch ports. Ports on the same switch are expected to return the same parent ID when SWITCHDEV_ATTR_ID_PORT_PARENT_ID is called. DSA gets this wrong with clusters of switches. As far as the software bridge is concerned, the cluster is all one switch. A packet from any switch in the cluster can be assumed to have been flooded as needed out of all ports of the cluster, not just the switch it originated from. Hence all ports of a cluster should return the same parent. The old implementation did not, each switch in the cluster had its own ID. Also wrong was that the ID was not unique if multiple DSA instances are in operation. Use the tree ID as the parent ID, which is the same for all switches in a cluster and unique across switch clusters. Signed-off-by: Andrew Lunn Reviewed-by: Vivien Didelot Reviewed-by: Florian Fainelli Signed-off-by: David S. Miller --- net/dsa/slave.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/net/dsa/slave.c b/net/dsa/slave.c index 35715fda84b2..d6e7a642493b 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -355,11 +355,12 @@ static int dsa_slave_port_attr_get(struct net_device *dev, { struct dsa_port *dp = dsa_slave_to_port(dev); struct dsa_switch *ds = dp->ds; + struct dsa_switch_tree *dst = ds->dst; switch (attr->id) { case SWITCHDEV_ATTR_ID_PORT_PARENT_ID: - attr->u.ppid.id_len = sizeof(ds->index); - memcpy(&attr->u.ppid.id, &ds->index, attr->u.ppid.id_len); + attr->u.ppid.id_len = sizeof(dst->index); + memcpy(&attr->u.ppid.id, &dst->index, attr->u.ppid.id_len); break; case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT: attr->u.brport_flags_support = 0; From 13edbdb6ed8b829ab3198a8c7b978c37184317b5 Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Thu, 9 Nov 2017 22:29:52 +0100 Subject: [PATCH 2/6] net: dsa: {e}dsa: set offload_fwd_mark on received packets The software bridge needs to know if a packet has already been bridged by hardware offload to ports in the same hardware offload, in order that it does not re-flood them, causing duplicates. This is particularly true for broadcast and multicast traffic which the host has requested. By setting offload_fwd_mark in the skb the bridge will only flood to ports in other offloads and other netifs. Set this flag in the DSA and EDSA tag driver. Signed-off-by: Andrew Lunn Signed-off-by: David S. Miller --- net/dsa/tag_dsa.c | 2 ++ net/dsa/tag_edsa.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c index dbbcdafed8c3..cd13cfc542ce 100644 --- a/net/dsa/tag_dsa.c +++ b/net/dsa/tag_dsa.c @@ -141,6 +141,8 @@ static struct sk_buff *dsa_rcv(struct sk_buff *skb, struct net_device *dev, 2 * ETH_ALEN); } + skb->offload_fwd_mark = 1; + return skb; } diff --git a/net/dsa/tag_edsa.c b/net/dsa/tag_edsa.c index f38a626b3a05..4083326b806e 100644 --- a/net/dsa/tag_edsa.c +++ b/net/dsa/tag_edsa.c @@ -160,6 +160,8 @@ static struct sk_buff *edsa_rcv(struct sk_buff *skb, struct net_device *dev, 2 * ETH_ALEN); } + skb->offload_fwd_mark = 1; + return skb; } From cd88646994bc0b537e0e7c89c53e674b58ba22b0 Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Thu, 9 Nov 2017 22:29:53 +0100 Subject: [PATCH 3/6] net: dsa: mv88e6xxx: Fixed port netdev check for VLANs Having the same VLAN on multiple bridges is currently unsupported as an offload. mv88e6xxx_port_check_hw_vlan() is used to ensure that a VLAN is not on multiple bridges when adding a VLAN range to a port. It loops the ports and checks to see if there are ports in a different bridge with the same VLAN. While walking all switch ports, the code was checking if the new port has a netdev slave attached to it. If not, skip checking the port being walked. This seems like a typ0. If the new port does not have a slave, how has a VLAN been added to it in the first place, requiring this check be performed at all? More likely, we should be checking if the port being walked has a slave. Without the port having a slave, it cannot have a VLAN on it, so there is no need to check further for that particular port. Signed-off-by: Andrew Lunn Reviewed-by: Vivien Didelot Signed-off-by: David S. Miller --- 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 09a66d4d9492..420621c759e4 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -1137,7 +1137,7 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port, if (dsa_is_dsa_port(ds, i) || dsa_is_cpu_port(ds, i)) continue; - if (!ds->ports[port].slave) + if (!ds->ports[i].slave) continue; if (vlan.member[i] == From 743fcc283edd5d3a9d61abf96d4443329c826d28 Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Thu, 9 Nov 2017 22:29:54 +0100 Subject: [PATCH 4/6] net: dsa: mv88e6xxx: Print offending port when vlan check fails When testing if a VLAN is one more than one bridge, we print an error message that the VLAN is already in use somewhere else. Print both the new port which would like the VLAN, and the port which already has it, to aid debugging. Signed-off-by: Andrew Lunn Reviewed-by: Vivien Didelot Signed-off-by: David S. Miller --- drivers/net/dsa/mv88e6xxx/chip.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index 420621c759e4..0a4a740ebea6 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -1151,8 +1151,8 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port, if (!dsa_to_port(ds, i)->bridge_dev) continue; - dev_err(ds->dev, "p%d: hw VLAN %d already used by %s\n", - port, vlan.vid, + dev_err(ds->dev, "p%d: hw VLAN %d already used by port %d in %s\n", + port, vlan.vid, i, netdev_name(dsa_to_port(ds, i)->bridge_dev)); err = -EOPNOTSUPP; goto unlock; From a4c93ae1bb675c875c5bee2fa817680cd6588240 Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Thu, 9 Nov 2017 22:29:55 +0100 Subject: [PATCH 5/6] net: dsa: mv88e6xxx: Move mv88e6xxx_port_db_load_purge() This function is going to be needed by a soon to be added new function. Move it earlier so we can avoid a forward declaration. No functional changes. Signed-off-by: Andrew Lunn Reviewed-by: Vivien Didelot Signed-off-by: David S. Miller --- drivers/net/dsa/mv88e6xxx/chip.c | 88 ++++++++++++++++---------------- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index 0a4a740ebea6..263b714a5ae3 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -1208,6 +1208,50 @@ mv88e6xxx_port_vlan_prepare(struct dsa_switch *ds, int port, return 0; } +static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port, + const unsigned char *addr, u16 vid, + u8 state) +{ + struct mv88e6xxx_vtu_entry vlan; + struct mv88e6xxx_atu_entry entry; + int err; + + /* Null VLAN ID corresponds to the port private database */ + if (vid == 0) + err = mv88e6xxx_port_get_fid(chip, port, &vlan.fid); + else + err = mv88e6xxx_vtu_get(chip, vid, &vlan, false); + if (err) + return err; + + entry.state = MV88E6XXX_G1_ATU_DATA_STATE_UNUSED; + ether_addr_copy(entry.mac, addr); + eth_addr_dec(entry.mac); + + err = mv88e6xxx_g1_atu_getnext(chip, vlan.fid, &entry); + if (err) + return err; + + /* Initialize a fresh ATU entry if it isn't found */ + if (entry.state == MV88E6XXX_G1_ATU_DATA_STATE_UNUSED || + !ether_addr_equal(entry.mac, addr)) { + memset(&entry, 0, sizeof(entry)); + ether_addr_copy(entry.mac, addr); + } + + /* Purge the ATU entry only if no port is using it anymore */ + if (state == MV88E6XXX_G1_ATU_DATA_STATE_UNUSED) { + entry.portvec &= ~BIT(port); + if (!entry.portvec) + entry.state = MV88E6XXX_G1_ATU_DATA_STATE_UNUSED; + } else { + entry.portvec |= BIT(port); + entry.state = state; + } + + return mv88e6xxx_g1_atu_loadpurge(chip, vlan.fid, &entry); +} + static int _mv88e6xxx_port_vlan_add(struct mv88e6xxx_chip *chip, int port, u16 vid, u8 member) { @@ -1324,50 +1368,6 @@ unlock: return err; } -static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port, - const unsigned char *addr, u16 vid, - u8 state) -{ - struct mv88e6xxx_vtu_entry vlan; - struct mv88e6xxx_atu_entry entry; - int err; - - /* Null VLAN ID corresponds to the port private database */ - if (vid == 0) - err = mv88e6xxx_port_get_fid(chip, port, &vlan.fid); - else - err = mv88e6xxx_vtu_get(chip, vid, &vlan, false); - if (err) - return err; - - entry.state = MV88E6XXX_G1_ATU_DATA_STATE_UNUSED; - ether_addr_copy(entry.mac, addr); - eth_addr_dec(entry.mac); - - err = mv88e6xxx_g1_atu_getnext(chip, vlan.fid, &entry); - if (err) - return err; - - /* Initialize a fresh ATU entry if it isn't found */ - if (entry.state == MV88E6XXX_G1_ATU_DATA_STATE_UNUSED || - !ether_addr_equal(entry.mac, addr)) { - memset(&entry, 0, sizeof(entry)); - ether_addr_copy(entry.mac, addr); - } - - /* Purge the ATU entry only if no port is using it anymore */ - if (state == MV88E6XXX_G1_ATU_DATA_STATE_UNUSED) { - entry.portvec &= ~BIT(port); - if (!entry.portvec) - entry.state = MV88E6XXX_G1_ATU_DATA_STATE_UNUSED; - } else { - entry.portvec |= BIT(port); - entry.state = state; - } - - return mv88e6xxx_g1_atu_loadpurge(chip, vlan.fid, &entry); -} - static int mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port, const unsigned char *addr, u16 vid) { From 87fa886e1fb7d08a35be2f39d15225d249daeea2 Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Thu, 9 Nov 2017 22:29:56 +0100 Subject: [PATCH 6/6] net: dsa: mv88e6xxx: Flood broadcast frames in hardware By default, the switch does not flood broadcast frames. Instead the broadcast address is unknown in the ATU, so the frame gets forwarded out the cpu port. The software bridge then floods it back to the individual switch ports which are members of the bridge. Add an ATU entry in the switch so that it floods broadcast frames out ports, rather than have the software bridge do it. Also, send a copy out the cpu port and any dsa ports. Rely on the port vectors to prevent broadcast frames leaking between bridges, and separated ports. Additionally, when a VLAN is added, a new FID is allocated. This represents a new table of ATU entries. A broadcast entry is added to the new FID. With offload_fwd_mark being set, the software bridge will not flood the frames it receives back to the switch. Signed-off-by: Andrew Lunn Signed-off-by: David S. Miller --- drivers/net/dsa/mv88e6xxx/chip.c | 33 +++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index 263b714a5ae3..6dd5fdfeafcf 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -1252,6 +1252,29 @@ static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port, return mv88e6xxx_g1_atu_loadpurge(chip, vlan.fid, &entry); } +static int mv88e6xxx_port_add_broadcast(struct mv88e6xxx_chip *chip, int port, + u16 vid) +{ + const char broadcast[6] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; + u8 state = MV88E6XXX_G1_ATU_DATA_STATE_MC_STATIC; + + return mv88e6xxx_port_db_load_purge(chip, port, broadcast, vid, state); +} + +static int mv88e6xxx_broadcast_setup(struct mv88e6xxx_chip *chip, u16 vid) +{ + int port; + int err; + + for (port = 0; port < mv88e6xxx_num_ports(chip); port++) { + err = mv88e6xxx_port_add_broadcast(chip, port, vid); + if (err) + return err; + } + + return 0; +} + static int _mv88e6xxx_port_vlan_add(struct mv88e6xxx_chip *chip, int port, u16 vid, u8 member) { @@ -1264,7 +1287,11 @@ static int _mv88e6xxx_port_vlan_add(struct mv88e6xxx_chip *chip, int port, vlan.member[port] = member; - return mv88e6xxx_vtu_loadpurge(chip, &vlan); + err = mv88e6xxx_vtu_loadpurge(chip, &vlan); + if (err) + return err; + + return mv88e6xxx_broadcast_setup(chip, vid); } static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port, @@ -2049,6 +2076,10 @@ static int mv88e6xxx_setup(struct dsa_switch *ds) if (err) goto unlock; + err = mv88e6xxx_broadcast_setup(chip, 0); + if (err) + goto unlock; + err = mv88e6xxx_pot_setup(chip); if (err) goto unlock;