Re: [PATCH v5 2/7] net: dsa: Add DSA driver for Hirschmann Hellcreek switches

On Sat Sep 05 2020, Vladimir Oltean wrote:

On Fri, Sep 04, 2020 at 08:27:34AM +0200, Kurt Kanzenbach wrote:

quoted

Add a basic DSA driver for Hirschmann Hellcreek switches. Those switches are
implementing features needed for Time Sensitive Networking (TSN) such as support
for the Time Precision Protocol and various shapers like the Time Aware Shaper.

This driver includes basic support for networking:

 * VLAN handling
 * FDB handling
 * Port statistics
 * STP
 * Phylink

Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
---
+
+/* Default setup for DSA:
+ *  VLAN 2: CPU and Port 1 egress untagged.
+ *  VLAN 3: CPU and Port 2 egress untagged.
+ */
+static int hellcreek_setup_vlan_membership(struct dsa_switch *ds, int port,
+					   bool enabled)

If you use VLAN 2 and 3 for port separation, then how does the driver
deal with the following:

ip link add link swp1 name swp1.100 type vlan id 100
ip link add link swp2 name swp2.100 type vlan id 100

In this case, frames with VLAN 100 shouldn't leak from one port to the
other, will they?

Well, that depends on whether hellcreek_vlan_add() is called for
creating that vlan interfaces. In general: As soon as both ports are
members of the same vlan that traffic is switched.

quoted

+{
+	int upstream = dsa_upstream_port(ds, port);
+	struct switchdev_obj_port_vlan vlan = {
+		.vid_begin = port,
+		.vid_end = port,
+	};
+	int err = 0;
+
+	/* The CPU port is implicitly configured by
+	 * configuring the front-panel ports
+	 */
+	if (!dsa_is_user_port(ds, port))
+		return 0;
+
+	/* Apply vid to port as egress untagged and port vlan id */
+	vlan.flags = BRIDGE_VLAN_INFO_UNTAGGED | BRIDGE_VLAN_INFO_PVID;
+	if (enabled)
+		hellcreek_vlan_add(ds, port, &vlan);
+	else
+		err = hellcreek_vlan_del(ds, port, &vlan);
+	if (err) {
+		dev_err(ds->dev, "Failed to apply VID %d to port %d: %d\n",
+			port, port, err);
+		return err;
+	}
+
+	/* Apply vid to cpu port as well */
+	vlan.flags = BRIDGE_VLAN_INFO_UNTAGGED;
+	if (enabled)
+		hellcreek_vlan_add(ds, upstream, &vlan);
+	else
+		err = hellcreek_vlan_del(ds, upstream, &vlan);
+	if (err) {
+		dev_err(ds->dev, "Failed to apply VID %d to port %d: %d\n",
+			port, port, err);
+		return err;
+	}
+
+	return 0;
+}
+
+static void hellcreek_setup_ingressflt(struct hellcreek *hellcreek, int port,
+				       bool enable)
+{
+	struct hellcreek_port *hellcreek_port = &hellcreek->ports[port];
+	u16 ptcfg;
+
+	mutex_lock(&hellcreek->reg_lock);
+
+	ptcfg = hellcreek_port->ptcfg;
+
+	if (enable)
+		ptcfg |= HR_PTCFG_INGRESSFLT;
+	else
+		ptcfg &= ~HR_PTCFG_INGRESSFLT;
+
+	hellcreek_select_port(hellcreek, port);
+	hellcreek_write(hellcreek, ptcfg, HR_PTCFG);
+	hellcreek_port->ptcfg = ptcfg;
+
+	mutex_unlock(&hellcreek->reg_lock);
+}
+
+static int hellcreek_vlan_filtering(struct dsa_switch *ds, int port,
+				    bool vlan_filtering)

I remember asking in Message-ID: <20200716082935.snokd33kn52ixk5h@skbuf>
whether it would be possible for you to set
ds->configure_vlan_while_not_filtering = true during hellcreek_setup.
Did anything unexpected happen while trying that?

No, that comment got lost.

So looking at the flag: Does it mean the driver can receive vlan
configurations when a bridge without vlan filtering is used? That might
be problematic as this driver uses vlans for the port separation by
default. This is undone when vlan filtering is set to 1 meaning vlan
configurations can be received without any problems.

Thanks,
Kurt