linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: netdev@vger.kernel.org
Cc: jiri@mellanox.com, petr@mellanox.com, idosch@mellanox.com,
	privat@egil-hjelmeland.no, Woojung.Huh@microchip.com,
	tristram.ha@microchip.com,
	Florian Fainelli <f.fainelli@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@savoirfairelinux.com>,
	"David S. Miller" <davem@davemloft.net>,
	linux-kernel@vger.kernel.org (open list)
Subject: [PATCH RFC] net: dsa: Make switches VLAN aware when enslaved into a bridge
Date: Wed, 24 Oct 2018 12:36:57 -0700	[thread overview]
Message-ID: <20181024193657.24012-1-f.fainelli@gmail.com> (raw)

Commit 2ea7a679ca2a ("net: dsa: Don't add vlans when vlan filtering is
disabled") changed the behavior of DSA switches when the switch ports
are enslaved into the bridge and only pushed the VLAN configuration down
to the switch if the bridge is configured with VLAN filtering enabled.

This is unfortunately wrong, because what vlan_filtering configures is a
policy on the acceptance of VLAN tagged frames with an unknown VID.

vlan_filtering=0 means a frame with a VLAN tag that is not part of the
VLAN table should be allowed to ingress the switch, and vlan_fltering=1
would reject that frame.

Fixes: 2ea7a679ca2a ("net: dsa: Don't add vlans when vlan filtering is disabled")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Andrew,

I checked with Jiri and he confirmed that our interpretention of
vlan_filtering in DSA was incorrect and that it does denote whether the
switch should be doing VID ingress policy checking.

You mentioned in the commit message some problems without being too
specific about them which is why I am putting the same checks in
mv88e6xxx in order not to break your use cases. Let me know if you want
to drop that hunk entirely.

Thanks!

 drivers/net/dsa/mv88e6xxx/chip.c |  7 ++++++-
 net/dsa/port.c                   | 10 ++--------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 8da3d39e3218..df411e776911 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1684,13 +1684,14 @@ static int _mv88e6xxx_port_vlan_add(struct mv88e6xxx_chip *chip, int port,
 static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
 				    const struct switchdev_obj_port_vlan *vlan)
 {
+	struct dsa_port *dp = &ds->ports[i];
 	struct mv88e6xxx_chip *chip = ds->priv;
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
 	u8 member;
 	u16 vid;
 
-	if (!chip->info->max_vid)
+	if (!chip->info->max_vid || br_vlan_enabled(dp->bridge_dev))
 		return;
 
 	if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
@@ -1751,12 +1752,16 @@ static int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
 				   const struct switchdev_obj_port_vlan *vlan)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
+	struct dsa_port *dp = &ds->ports[i];
 	u16 pvid, vid;
 	int err = 0;
 
 	if (!chip->info->max_vid)
 		return -EOPNOTSUPP;
 
+	if (br_vlan_enabled(dp->bridge_dev))
+		return 0;
+
 	mutex_lock(&chip->reg_lock);
 
 	err = mv88e6xxx_port_get_pvid(chip, port, &pvid);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index ed0595459df1..111d7cfc8982 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -255,10 +255,7 @@ int dsa_port_vlan_add(struct dsa_port *dp,
 	if (netif_is_bridge_master(vlan->obj.orig_dev))
 		return -EOPNOTSUPP;
 
-	if (br_vlan_enabled(dp->bridge_dev))
-		return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
-
-	return 0;
+	return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
 }
 
 int dsa_port_vlan_del(struct dsa_port *dp,
@@ -273,10 +270,7 @@ int dsa_port_vlan_del(struct dsa_port *dp,
 	if (netif_is_bridge_master(vlan->obj.orig_dev))
 		return -EOPNOTSUPP;
 
-	if (br_vlan_enabled(dp->bridge_dev))
-		return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
-
-	return 0;
+	return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
 }
 
 static struct phy_device *dsa_port_get_phy_device(struct dsa_port *dp)
-- 
2.17.1


             reply	other threads:[~2018-10-24 19:37 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-24 19:36 Florian Fainelli [this message]
2018-10-24 22:10 ` [PATCH RFC] net: dsa: Make switches VLAN aware when enslaved into a bridge Florian Fainelli
2018-10-26 15:10 ` Ido Schimmel
2018-10-26 23:16   ` Florian Fainelli

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=20181024193657.24012-1-f.fainelli@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=Woojung.Huh@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=idosch@mellanox.com \
    --cc=jiri@mellanox.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=petr@mellanox.com \
    --cc=privat@egil-hjelmeland.no \
    --cc=tristram.ha@microchip.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;
as well as URLs for NNTP newsgroup(s).