netdev - Re: [RFC 3/9] net: dsa: mv88e6xxx: add support for VTU ops

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Mon, 01 Jun 2015 23:50:22 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Vivien Didelot <vivien.didelot@...oirfairelinux.com>,
	netdev@...r.kernel.org
CC:	"David S. Miller" <davem@...emloft.net>,
	Florian Fainelli <f.fainelli@...il.com>,
	Andrew Lunn <andrew@...n.ch>,
	Scott Feldman <sfeldma@...il.com>,
	Jiri Pirko <jiri@...nulli.us>,
	Jerome Oufella <jerome.oufella@...oirfairelinux.com>,
	linux-kernel@...r.kernel.org, kernel@...oirfairelinux.com,
	Chris Healy <cphealy@...il.com>
Subject: Re: [RFC 3/9] net: dsa: mv88e6xxx: add support for VTU ops

Vivien,

On 06/01/2015 06:27 PM, Vivien Didelot wrote:
> This commit implements the port_vlan_add and port_vlan_del functions in
> the dsa_switch_driver structure for Marvell 88E6xxx compatible switches.
>
> This allows to access a switch VLAN Table Unit, and thus define VLANs
> from standard userspace commands such as "bridge vlan".
>
> Signed-off-by: Vivien Didelot <vivien.didelot@...oirfairelinux.com>
> ---

[ ... ]

> +
> +int mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port, u16 vid,
> +			    u16 bridge_flags)
> +{
> +	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> +	struct mv88e6xxx_vtu_entry entry = { 0 };
> +	int prev_vid = vid ? vid - 1 : 4095;
> +	int i, ret;
> +
> +	/* Bringing an interface up adds it to the VLAN 0. Ignore this. */
> +	if (!vid)
> +		return 0;
> +

Me puzzled ;-). I brought this and the fid question up before.
No idea if my e-mail got lost or what happened.

Can you explain why we don't need a configuration for vlan 0 ?

> +	/* The DSA port-based VLAN setup reserves FID 0 to DSA_MAX_PORTS;
> +	 * we will use the next FIDs for 802.1q;
> +	 * thus, forbid the last DSA_MAX_PORTS VLANs.
> +	 */
> +	if (vid > 4095 - DSA_MAX_PORTS)
> +		return -EINVAL;
> +
> +	mutex_lock(&ps->smi_mutex);
> +	ret = _mv88e6xxx_vtu_getnext(ds, prev_vid, &entry);
> +	if (ret < 0)
> +		goto unlock;
> +
> +	/* If the VLAN does not exist, re-initialize the entry for addition */
> +	if (entry.vid != vid || !entry.valid) {
> +		memset(&entry, 0, sizeof(entry));
> +		entry.valid = true;
> +		entry.vid = vid;
> +		entry.fid = DSA_MAX_PORTS + vid;

I brought this up before. No idea if my e-mail got lost or what happened.

We use a fid per port, and a fid per bridge group. With VLANs, this is completely
ignored, ahd there is only a single fid per vlan for the entire switch.

Either per-port fids are unnecessary as well, or something is wrong here,
or I am missing something. Can you explain why we only need a single fid
per vlan, even if we have multiple bridge groups and the same vlan is
configured in all of them ?

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html