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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <875z8qazq2.fsf@kurt>
Date:   Mon, 07 Sep 2020 08:05:25 +0200
From:   Kurt Kanzenbach <kurt@...utronix.de>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        Rob Herring <robh+dt@...nel.org>, devicetree@...r.kernel.org,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Richard Cochran <richardcochran@...il.com>,
        Kamil Alkhouri <kamil.alkhouri@...offenburg.de>,
        ilias.apalodimas@...aro.org
Subject: 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:
>> 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@...utronix.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.

>
>> +{
>> +	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@...uf>
> 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

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)