netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v5 0/3] net: dsa: mv88e6xxx: fix IPv6
@ 2019-02-20 23:35 Florian Fainelli
  2019-02-20 23:35 ` [PATCH net-next v5 1/3] net: dsa: add support for bridge flags Florian Fainelli
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Florian Fainelli @ 2019-02-20 23:35 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, andrew, vivien.didelot, davem, rmk+kernel,
	hkallweit1

We have had some emails in private over this issue, this is my current
patch set rebased on top of net-next which provides working IPv6 (and
probably other protocols as well) over mv88e6xxx DSA switches.

The problem comes down to mv88e6xxx defaulting to not flood unknown
unicast and multicast datagrams, as they would be by dumb switches,
and as the Linux bridge code does by default.

There is also the issue of IPv6 over a vlan that is transparent to the
bridge; the multicast querier will not reach inside the vlan, and so
the switch can not learn about multicast routing within the vlan.

These flood settings can be disabled via the Linux bridge code if it's
desired to make the switch behave more like a managed switch, eg, by
enabling the multicast querier.  However, the multicast querier
defaults to being disabled which effectively means that by default,
mv88e6xxx switches block all multicast traffic.  This is at odds with
the Linux bridge documentation, and the defaults that the Linux bridge
code adopts.

So, this patch set adds DSA support for Linux bridge flags, adds
mv88e6xxx support for the unicast and multicast flooding flags, and
lastly enables flooding of these frames by default to match the
Linux bridge defaults.

Russell King (3):
  net: dsa: add support for bridge flags
  net: dsa: mv88e6xxx: add support for bridge flags
  net: dsa: enable flooding for bridge ports

 drivers/net/dsa/mv88e6xxx/chip.c | 17 ++++++++++++++++
 include/net/dsa.h                |  2 ++
 net/dsa/dsa_priv.h               |  2 ++
 net/dsa/port.c                   | 33 +++++++++++++++++++++++++++++---
 net/dsa/slave.c                  |  9 +++++++++
 5 files changed, 60 insertions(+), 3 deletions(-)


v2: fix a couple of compile errors in patch 2 and patch 3 (oops).
v3: change interface between core DSA and drivers
v4: fix comments from v3
v5: fix build failure in patch #1
-- 
2.17.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH net-next v5 1/3] net: dsa: add support for bridge flags
  2019-02-20 23:35 [PATCH net-next v5 0/3] net: dsa: mv88e6xxx: fix IPv6 Florian Fainelli
@ 2019-02-20 23:35 ` Florian Fainelli
  2019-02-20 23:35 ` [PATCH net-next v5 2/3] net: dsa: mv88e6xxx: " Florian Fainelli
  2019-02-20 23:35 ` [PATCH net-next v5 3/3] net: dsa: enable flooding for bridge ports Florian Fainelli
  2 siblings, 0 replies; 4+ messages in thread
From: Florian Fainelli @ 2019-02-20 23:35 UTC (permalink / raw)
  To: netdev
  Cc: Russell King, Florian Fainelli, andrew, vivien.didelot, davem,
	hkallweit1

From: Russell King <rmk+kernel@armlinux.org.uk>

The Linux bridge implementation allows various properties of the bridge
to be controlled, such as flooding unknown unicast and multicast frames.
This patch adds the necessary DSA infrastructure to allow the Linux
bridge support to control these properties for DSA switches.

Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
[florian: Add missing dp and ds variables declaration to fix build]
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/net/dsa.h  |  2 ++
 net/dsa/dsa_priv.h |  2 ++
 net/dsa/port.c     | 17 +++++++++++++++++
 net/dsa/slave.c    |  9 +++++++++
 4 files changed, 30 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 7f2a668ef2cc..2c2c10812814 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -400,6 +400,8 @@ struct dsa_switch_ops {
 	void	(*port_stp_state_set)(struct dsa_switch *ds, int port,
 				      u8 state);
 	void	(*port_fast_age)(struct dsa_switch *ds, int port);
+	int	(*port_egress_floods)(struct dsa_switch *ds, int port,
+				      bool unicast, bool multicast);
 
 	/*
 	 * VLAN support
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 1f4972dab9f2..f4f99ec29f5d 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -160,6 +160,8 @@ int dsa_port_mdb_add(const struct dsa_port *dp,
 		     struct switchdev_trans *trans);
 int dsa_port_mdb_del(const struct dsa_port *dp,
 		     const struct switchdev_obj_port_mdb *mdb);
+int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
+			  struct switchdev_trans *trans);
 int dsa_port_vlan_add(struct dsa_port *dp,
 		      const struct switchdev_obj_port_vlan *vlan,
 		      struct switchdev_trans *trans);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 2d7e01b23572..6df29bddf37e 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -177,6 +177,23 @@ int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock,
 	return dsa_port_notify(dp, DSA_NOTIFIER_AGEING_TIME, &info);
 }
 
+int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
+			  struct switchdev_trans *trans)
+{
+	struct dsa_switch *ds = dp->ds;
+	int port = dp->index;
+	int err = 0;
+
+	if (switchdev_trans_ph_prepare(trans))
+		return 0;
+
+	if (ds->ops->port_egress_floods)
+		err = ds->ops->port_egress_floods(ds, port, flags & BR_FLOOD,
+						  flags & BR_MCAST_FLOOD);
+
+	return err;
+}
+
 int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 		     u16 vid)
 {
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 2e5e7c04821b..85dc68611002 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -295,6 +295,9 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
 	case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
 		ret = dsa_port_ageing_time(dp, attr->u.ageing_time, trans);
 		break;
+	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
+		ret = dsa_port_bridge_flags(dp, attr->u.brport_flags, trans);
+		break;
 	default:
 		ret = -EOPNOTSUPP;
 		break;
@@ -381,9 +384,15 @@ static int dsa_slave_get_port_parent_id(struct net_device *dev,
 static int dsa_slave_port_attr_get(struct net_device *dev,
 				   struct switchdev_attr *attr)
 {
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
+
 	switch (attr->id) {
 	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT:
 		attr->u.brport_flags_support = 0;
+		if (ds->ops->port_egress_floods)
+			attr->u.brport_flags_support |= BR_FLOOD |
+							BR_MCAST_FLOOD;
 		break;
 	default:
 		return -EOPNOTSUPP;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH net-next v5 2/3] net: dsa: mv88e6xxx: add support for bridge flags
  2019-02-20 23:35 [PATCH net-next v5 0/3] net: dsa: mv88e6xxx: fix IPv6 Florian Fainelli
  2019-02-20 23:35 ` [PATCH net-next v5 1/3] net: dsa: add support for bridge flags Florian Fainelli
@ 2019-02-20 23:35 ` Florian Fainelli
  2019-02-20 23:35 ` [PATCH net-next v5 3/3] net: dsa: enable flooding for bridge ports Florian Fainelli
  2 siblings, 0 replies; 4+ messages in thread
From: Florian Fainelli @ 2019-02-20 23:35 UTC (permalink / raw)
  To: netdev; +Cc: Russell King, andrew, vivien.didelot, davem, hkallweit1

From: Russell King <rmk+kernel@armlinux.org.uk>

Add support for the bridge flags to Marvell 88e6xxx bridges, allowing
the multicast and unicast flood properties to be controlled.  These
can be controlled on a per-port basis via commands such as:

	bridge link set dev lan1 flood on|off
	bridge link set dev lan1 mcast_flood on|off

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 32e7af5caa69..cc7ce06b6d58 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4692,6 +4692,22 @@ static int mv88e6xxx_port_mdb_del(struct dsa_switch *ds, int port,
 	return err;
 }
 
+static int mv88e6xxx_port_egress_floods(struct dsa_switch *ds, int port,
+					 bool unicast, bool multicast)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	int err = -EOPNOTSUPP;
+
+	mutex_lock(&chip->reg_lock);
+	if (chip->info->ops->port_set_egress_floods)
+		err = chip->info->ops->port_set_egress_floods(chip, port,
+							      unicast,
+							      multicast);
+	mutex_unlock(&chip->reg_lock);
+
+	return err;
+}
+
 static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 #if IS_ENABLED(CONFIG_NET_DSA_LEGACY)
 	.probe			= mv88e6xxx_drv_probe,
@@ -4719,6 +4735,7 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.set_ageing_time	= mv88e6xxx_set_ageing_time,
 	.port_bridge_join	= mv88e6xxx_port_bridge_join,
 	.port_bridge_leave	= mv88e6xxx_port_bridge_leave,
+	.port_egress_floods	= mv88e6xxx_port_egress_floods,
 	.port_stp_state_set	= mv88e6xxx_port_stp_state_set,
 	.port_fast_age		= mv88e6xxx_port_fast_age,
 	.port_vlan_filtering	= mv88e6xxx_port_vlan_filtering,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH net-next v5 3/3] net: dsa: enable flooding for bridge ports
  2019-02-20 23:35 [PATCH net-next v5 0/3] net: dsa: mv88e6xxx: fix IPv6 Florian Fainelli
  2019-02-20 23:35 ` [PATCH net-next v5 1/3] net: dsa: add support for bridge flags Florian Fainelli
  2019-02-20 23:35 ` [PATCH net-next v5 2/3] net: dsa: mv88e6xxx: " Florian Fainelli
@ 2019-02-20 23:35 ` Florian Fainelli
  2 siblings, 0 replies; 4+ messages in thread
From: Florian Fainelli @ 2019-02-20 23:35 UTC (permalink / raw)
  To: netdev; +Cc: Russell King, andrew, vivien.didelot, davem, hkallweit1

From: Russell King <rmk+kernel@armlinux.org.uk>

Switches work by learning the MAC address for each attached station by
monitoring traffic from each station.  When a station sends a packet,
the switch records which port the MAC address is connected to.

With IPv4 networking, before communication commences with a neighbour,
an ARP packet is broadcasted to all stations asking for the MAC address
corresponding with the IPv4.  The desired station responds with an ARP
reply, and the ARP reply causes the switch to learn which port the
station is connected to.

With IPv6 networking, the situation is rather different.  Rather than
broadcasting ARP packets, a "neighbour solicitation" is multicasted
rather than broadcasted.  This multicast needs to reach the intended
station in order for the neighbour to be discovered.

Once a neighbour has been discovered, and entered into the sending
stations neighbour cache, communication can restart at a point later
without sending a new neighbour solicitation, even if the entry in
the neighbour cache is marked as stale.  This can be after the MAC
address has expired from the forwarding cache of the DSA switch -
when that occurs, there is a long pause in communication.

Our DSA implementation for mv88e6xxx switches disables flooding of
multicast and unicast frames for bridged ports.  As per the above
description, this is fine for IPv4 networking, since the broadcasted
ARP queries will be sent to and received by all stations on the same
network.  However, this breaks IPv6 very badly - blocking neighbour
solicitations and later causing connections to stall.

The defaults that the Linux bridge code expect from bridges are for
unknown unicast and unknown multicast frames to be flooded to all ports
on the bridge, which is at odds to the defaults adopted by our DSA
implementation for mv88e6xxx switches.

This commit enables by default flooding of both unknown unicast and
unknown multicast frames whenever a port is added to a bridge, and
disables the flooding when a port leaves the bridge.  This means that
mv88e6xxx DSA switches now behave as per the bridge(8) man page, and
IPv6 works flawlessly through such a switch.

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 net/dsa/port.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/net/dsa/port.c b/net/dsa/port.c
index 6df29bddf37e..7bc2a5ad95c6 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -105,16 +105,23 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br)
 	};
 	int err;
 
-	/* Here the port is already bridged. Reflect the current configuration
-	 * so that drivers can program their chips accordingly.
+	/* Set the flooding mode before joining the port in the switch */
+	err = dsa_port_bridge_flags(dp, BR_FLOOD | BR_MCAST_FLOOD, NULL);
+	if (err)
+		return err;
+
+	/* Here the interface is already bridged. Reflect the current
+	 * configuration so that drivers can program their chips accordingly.
 	 */
 	dp->bridge_dev = br;
 
 	err = dsa_port_notify(dp, DSA_NOTIFIER_BRIDGE_JOIN, &info);
 
 	/* The bridging is rolled back on error */
-	if (err)
+	if (err) {
+		dsa_port_bridge_flags(dp, 0, NULL);
 		dp->bridge_dev = NULL;
+	}
 
 	return err;
 }
@@ -137,6 +144,9 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
 	if (err)
 		pr_err("DSA: failed to notify DSA_NOTIFIER_BRIDGE_LEAVE\n");
 
+	/* Port is leaving the bridge, disable flooding */
+	dsa_port_bridge_flags(dp, 0, NULL);
+
 	/* Port left the bridge, put in BR_STATE_DISABLED by the bridge layer,
 	 * so allow it to be in BR_STATE_FORWARDING to be kept functional
 	 */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-02-20 23:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-20 23:35 [PATCH net-next v5 0/3] net: dsa: mv88e6xxx: fix IPv6 Florian Fainelli
2019-02-20 23:35 ` [PATCH net-next v5 1/3] net: dsa: add support for bridge flags Florian Fainelli
2019-02-20 23:35 ` [PATCH net-next v5 2/3] net: dsa: mv88e6xxx: " Florian Fainelli
2019-02-20 23:35 ` [PATCH net-next v5 3/3] net: dsa: enable flooding for bridge ports Florian Fainelli

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).