Re: [PATCH v2 rfc 8/8] net: dsa: Fix SWITCHDEV_ATTR_ID_PORT_PARENT_ID
> >Yes, you are correct. I will change this. > > A switch cluster should be tied to the same dsa_switch_tree though, > can we use dst->tree as an unique identifier? Yes, that is what Vivien was suggesting. Andrew
Re: [PATCH v2 rfc 8/8] net: dsa: Fix SWITCHDEV_ATTR_ID_PORT_PARENT_ID
On September 6, 2017 8:08:25 AM PDT, Andrew Lunn wrote: >> > Use the MAC address of the master interface as the parent ID. This >is >> > the same for all switches in a cluster, and should be unique if >there >> > are multiple clusters. >> >> That is not correct. Support for multiple CPU ports is coming and in >> this case, you can have two CPU host interfaces wired to two switch >> ports of the same tree. So two different master MAC addresses. > >Yes, you are correct. I will change this. A switch cluster should be tied to the same dsa_switch_tree though, can we use dst->tree as an unique identifier? -- Florian
Re: [PATCH v2 rfc 8/8] net: dsa: Fix SWITCHDEV_ATTR_ID_PORT_PARENT_ID
> > Use the MAC address of the master interface as the parent ID. This is > > the same for all switches in a cluster, and should be unique if there > > are multiple clusters. > > That is not correct. Support for multiple CPU ports is coming and in > this case, you can have two CPU host interfaces wired to two switch > ports of the same tree. So two different master MAC addresses. Yes, you are correct. I will change this. Andrew
Re: [PATCH v2 rfc 8/8] net: dsa: Fix SWITCHDEV_ATTR_ID_PORT_PARENT_ID
Hi Andrew, Andrew Lunn writes: > 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 of been flooded as needed out > 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 MAC address of the master interface as the parent ID. This is > the same for all switches in a cluster, and should be unique if there > are multiple clusters. That is not correct. Support for multiple CPU ports is coming and in this case, you can have two CPU host interfaces wired to two switch ports of the same tree. So two different master MAC addresses. Only the tree ID assigned by DSA core is unique to a given tree. Thanks, Vivien
[PATCH v2 rfc 8/8] 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 of been flooded as needed out
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 MAC address of the master interface as the parent ID. This is
the same for all switches in a cluster, and should be unique if there
are multiple clusters.
Signed-off-by: Andrew Lunn
---
net/dsa/slave.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 2e07be149415..d2744b0dad6e 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -374,13 +374,15 @@ static int dsa_slave_port_attr_get(struct net_device *dev,
struct switchdev_attr *attr)
{
struct dsa_slave_priv *p = netdev_priv(dev);
- struct dsa_switch *ds = p->dp->ds;
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);
+ case SWITCHDEV_ATTR_ID_PORT_PARENT_ID: {
+ struct net_device *master = dsa_master_netdev(p);
+
+ attr->u.ppid.id_len = ETH_ALEN;
+ ether_addr_copy(attr->u.ppid.id, master->dev_addr);
break;
+ }
case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT:
attr->u.brport_flags_support = 0;
break;
--
2.14.1
