diff mbox series

[v2,net-next,1/6] net: dsa: Fix SWITCHDEV_ATTR_ID_PORT_PARENT_ID

Message ID 1510009351-21875-2-git-send-email-andrew@lunn.ch
State Changes Requested, archived
Delegated to: David Miller
Headers show
Series mv88e6xxx broadcast flooding in hardware | expand

Commit Message

Andrew Lunn Nov. 6, 2017, 11:02 p.m. UTC 
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 <andrew@lunn.ch>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
v2: Swap from MAC address to dst->tree
v3: Move variables to eliminate block in case.
    Fix present perfect passive infinitive have and add missing of
---
 net/dsa/slave.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Florian Fainelli Nov. 6, 2017, 11:17 p.m. UTC | #1 
On 11/06/2017 03:02 PM, Andrew Lunn wrote:
> 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 <andrew@lunn.ch>
> Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
diff mbox series

Patch

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 9b75d0ac4092..1179e4cd6701 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -342,11 +342,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->tree);
+		memcpy(&attr->u.ppid.id, &dst->tree, attr->u.ppid.id_len);
 		break;
 	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT:
 		attr->u.brport_flags_support = 0;