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

[Vladimir Oltean <olteanv@gmail.com>]

On Fri, Sep 04, 2020 at 08:27:34AM +0200, Kurt Kanzenbach wrote:
> 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?

> +{
> +	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?

> +{
> +	struct hellcreek *hellcreek = ds->priv;
> +
> +	dev_dbg(hellcreek->dev, "%s VLAN filtering on port %d\n",
> +		vlan_filtering ? "Enable" : "Disable", port);
> +
> +	/* Configure port to drop packages with not known vids */
> +	hellcreek_setup_ingressflt(hellcreek, port, vlan_filtering);
> +
> +	/* Drop DSA vlan membership config. The user can now do it. */
> +	hellcreek_setup_vlan_membership(ds, port, !vlan_filtering);
> +
> +	return 0;
> +}
> +