public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Vivien Didelot <vivien.didelot@savoirfairelinux.com>,
	netdev@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>, Scott Feldman <sfeldma@gmail.com>,
	Jiri Pirko <jiri@resnulli.us>,
	Jerome Oufella <jerome.oufella@savoirfairelinux.com>,
	linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com,
	Chris Healy <cphealy@gmail.com>
Subject: Re: [RFC 3/9] net: dsa: mv88e6xxx: add support for VTU ops
Date: Mon, 01 Jun 2015 23:50:22 -0700	[thread overview]
Message-ID: <556D522E.90607@roeck-us.net> (raw)
In-Reply-To: <1433208470-25338-4-git-send-email-vivien.didelot@savoirfairelinux.com>

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@savoirfairelinux.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


  reply	other threads:[~2015-06-02  6:50 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-02  1:27 [RFC 0/9] net: dsa: mv88e6352: add 802.1q VLAN support Vivien Didelot
2015-06-02  1:27 ` [RFC 1/9] net: dsa: add basic support for switchdev obj Vivien Didelot
2015-06-02  1:27 ` [RFC 2/9] net: dsa: add basic support for VLAN operations Vivien Didelot
2015-06-02  6:52   ` Guenter Roeck
2015-06-02 14:42   ` Guenter Roeck
2015-06-03  0:45     ` Vivien Didelot
2015-06-02  1:27 ` [RFC 3/9] net: dsa: mv88e6xxx: add support for VTU ops Vivien Didelot
2015-06-02  6:50   ` Guenter Roeck [this message]
2015-06-02  7:44     ` Scott Feldman
2015-06-02 13:41       ` Guenter Roeck
2015-06-02 13:42         ` Andrew Lunn
2015-06-02 14:47           ` Guenter Roeck
2015-06-02 22:31       ` nolan
2015-06-03  6:53         ` Scott Feldman
2015-06-03 14:44           ` Guenter Roeck
2015-06-03 18:42           ` Florian Fainelli
2015-06-04 18:22           ` nolan
2015-06-03  1:39     ` Vivien Didelot
2015-06-03  2:17       ` Guenter Roeck
2015-06-03 14:56         ` Vivien Didelot
2015-06-03 15:39           ` Guenter Roeck
2015-06-02 13:05   ` Andrew Lunn
2015-06-02  1:27 ` [RFC 4/9] net: dsa: mv88e6352: add support for VLAN Vivien Didelot
2015-06-02  1:27 ` [RFC 5/9] net: dsa: mv88e6352: disable mirroring Vivien Didelot
2015-06-02 14:16   ` Guenter Roeck
2015-06-02 14:53     ` Andrew Lunn
2015-06-03  1:12       ` Vivien Didelot
2015-06-03  2:27         ` Guenter Roeck
2015-06-02  1:27 ` [RFC 6/9] net: dsa: mv88e6352: allow egress of unknown multicast Vivien Didelot
2015-06-02 14:20   ` Guenter Roeck
2015-06-03  1:52     ` Vivien Didelot
2015-06-09 22:42       ` Vivien Didelot
2015-06-02  1:27 ` [RFC 7/9] net: dsa: mv88e6352: lock CPU port from learning addresses Vivien Didelot
2015-06-02 14:24   ` Guenter Roeck
2015-06-03  1:06     ` Vivien Didelot
2015-06-03  2:24       ` Guenter Roeck
     [not found]         ` <CAFXsbZo7DAhbUErMfKas_KUtXMHTURgOxwz-GSr=fuAHLWToEQ@mail.gmail.com>
2015-06-03  4:17           ` Guenter Roeck
2015-06-03 20:51           ` Andrew Lunn
2015-06-02  1:27 ` [RFC 8/9] net: dsa: mv88e6352: set port 802.1Q mode to Secure Vivien Didelot
2015-06-02 14:31   ` Guenter Roeck
2015-06-02 23:45     ` Vivien Didelot
2015-06-02 23:28   ` Guenter Roeck
2015-06-02  1:27 ` [RFC 9/9] net: dsa: fix EDSA frame from hwaccel frame Vivien Didelot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=556D522E.90607@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=andrew@lunn.ch \
    --cc=cphealy@gmail.com \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=jerome.oufella@savoirfairelinux.com \
    --cc=jiri@resnulli.us \
    --cc=kernel@savoirfairelinux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=sfeldma@gmail.com \
    --cc=vivien.didelot@savoirfairelinux.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox