Netdev List
 help / color / mirror / Atom feed
* Re: No traffic with Marvell switch and latest linux-next
From: Andrew Lunn @ 2019-02-17 15:57 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Florian Fainelli, Russell King - ARM Linux,
	netdev@vger.kernel.org
In-Reply-To: <9f2ca596-4310-7b55-1be6-bc1db1789edd@gmail.com>

> In linux-next from Feb 15th this patch is included already.

So why is port 8 not clearing its interrupt?

Maybe put a printk in m88e1121_did_interrupt(),
marvell_ack_interrupt(), and marvell_config_intr() and see if they are
getting called.

	Andrew

^ permalink raw reply

* Re: No traffic with Marvell switch and latest linux-next
From: Heiner Kallweit @ 2019-02-17 16:01 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Florian Fainelli, Russell King - ARM Linux,
	netdev@vger.kernel.org
In-Reply-To: <20190217155709.GD5968@lunn.ch>

On 17.02.2019 16:57, Andrew Lunn wrote:
>> In linux-next from Feb 15th this patch is included already.
> 
> So why is port 8 not clearing its interrupt?
> 
> Maybe put a printk in m88e1121_did_interrupt(),
> marvell_ack_interrupt(), and marvell_config_intr() and see if they are
> getting called.
> 
I think we're going to mix two things here:
The trace was from 5.0-rc6, there the phylib fix isn't included yet.

> 	Andrew
> 
Heiner

^ permalink raw reply

* [PATCH net] net: dsa: fix lockdep warning
From: Russell King @ 2019-02-17 16:27 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vivien Didelot
  Cc: Heiner Kallweit, David S. Miller, netdev

======================================================
WARNING: possible circular locking dependency detected
4.20.0+ #302 Not tainted
------------------------------------------------------
systemd-udevd/160 is trying to acquire lock:
edea6080 (&chip->reg_lock){+.+.}, at: __setup_irq+0x640/0x704

but task is already holding lock:
edff0340 (&desc->request_mutex){+.+.}, at: __setup_irq+0xa0/0x704

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #1 (&desc->request_mutex){+.+.}:
       mutex_lock_nested+0x1c/0x24
       __setup_irq+0xa0/0x704
       request_threaded_irq+0xd0/0x150
       mv88e6xxx_probe+0x41c/0x694 [mv88e6xxx]
       mdio_probe+0x2c/0x54
       really_probe+0x200/0x2c4
       driver_probe_device+0x5c/0x174
       __driver_attach+0xd8/0xdc
       bus_for_each_dev+0x58/0x7c
       bus_add_driver+0xe4/0x1f0
       driver_register+0x7c/0x110
       mdio_driver_register+0x24/0x58
       do_one_initcall+0x74/0x2e8
       do_init_module+0x60/0x1d0
       load_module+0x1968/0x1ff4
       sys_finit_module+0x8c/0x98
       ret_fast_syscall+0x0/0x28
       0xbedf2ae8

-> #0 (&chip->reg_lock){+.+.}:
       __mutex_lock+0x50/0x8b8
       mutex_lock_nested+0x1c/0x24
       __setup_irq+0x640/0x704
       request_threaded_irq+0xd0/0x150
       mv88e6xxx_g2_irq_setup+0xcc/0x1b4 [mv88e6xxx]
       mv88e6xxx_probe+0x44c/0x694 [mv88e6xxx]
       mdio_probe+0x2c/0x54
       really_probe+0x200/0x2c4
       driver_probe_device+0x5c/0x174
       __driver_attach+0xd8/0xdc
       bus_for_each_dev+0x58/0x7c
       bus_add_driver+0xe4/0x1f0
       driver_register+0x7c/0x110
       mdio_driver_register+0x24/0x58
       do_one_initcall+0x74/0x2e8
       do_init_module+0x60/0x1d0
       load_module+0x1968/0x1ff4
       sys_finit_module+0x8c/0x98
       ret_fast_syscall+0x0/0x28
       0xbedf2ae8

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(&desc->request_mutex);
                               lock(&chip->reg_lock);
                               lock(&desc->request_mutex);
  lock(&chip->reg_lock);

 *** DEADLOCK ***

2 locks held by systemd-udevd/160:
 #0: ee040868 (&dev->mutex){....}, at: __driver_attach+0x70/0xdc
 #1: edff0340 (&desc->request_mutex){+.+.}, at: __setup_irq+0xa0/0x704

stack backtrace:
CPU: 1 PID: 160 Comm: systemd-udevd Not tainted 4.20.0+ #302
Hardware name: Marvell Armada 380/385 (Device Tree)
[<c0019638>] (unwind_backtrace) from [<c0014888>] (show_stack+0x10/0x14)
[<c0014888>] (show_stack) from [<c07f54e0>] (dump_stack+0x9c/0xd4)
[<c07f54e0>] (dump_stack) from [<c0088afc>] (print_circular_bug+0x284/0x2d8)
[<c0088afc>] (print_circular_bug) from [<c0086b5c>] (__lock_acquire+0x15d4/0x19b8)
[<c0086b5c>] (__lock_acquire) from [<c0087828>] (lock_acquire+0xc4/0x1dc)
[<c0087828>] (lock_acquire) from [<c080fd88>] (__mutex_lock+0x50/0x8b8)
[<c080fd88>] (__mutex_lock) from [<c0810678>] (mutex_lock_nested+0x1c/0x24)
[<c0810678>] (mutex_lock_nested) from [<c009e060>] (__setup_irq+0x640/0x704)
[<c009e060>] (__setup_irq) from [<c009e2e0>] (request_threaded_irq+0xd0/0x150)
[<c009e2e0>] (request_threaded_irq) from [<bf0ce978>] (mv88e6xxx_g2_irq_setup+0xcc/0x1b4 [mv88e6xxx])
[<bf0ce978>] (mv88e6xxx_g2_irq_setup [mv88e6xxx]) from [<bf0c7ab0>] (mv88e6xxx_probe+0x44c/0x694 [mv88e6xxx])
[<bf0c7ab0>] (mv88e6xxx_probe [mv88e6xxx]) from [<c050d420>] (mdio_probe+0x2c/0x54)
[<c050d420>] (mdio_probe) from [<c0496eac>] (really_probe+0x200/0x2c4)
[<c0496eac>] (really_probe) from [<c0497140>] (driver_probe_device+0x5c/0x174)
[<c0497140>] (driver_probe_device) from [<c0497330>] (__driver_attach+0xd8/0xdc)
[<c0497330>] (__driver_attach) from [<c0495494>] (bus_for_each_dev+0x58/0x7c)
[<c0495494>] (bus_for_each_dev) from [<c04963d4>] (bus_add_driver+0xe4/0x1f0)
[<c04963d4>] (bus_add_driver) from [<c0498038>] (driver_register+0x7c/0x110)
[<c0498038>] (driver_register) from [<c050d338>] (mdio_driver_register+0x24/0x58)
[<c050d338>] (mdio_driver_register) from [<c000afdc>] (do_one_initcall+0x74/0x2e8)
[<c000afdc>] (do_one_initcall) from [<c00d4994>] (do_init_module+0x60/0x1d0)
[<c00d4994>] (do_init_module) from [<c00d39e0>] (load_module+0x1968/0x1ff4)
mvneta f1034000.ethernet eth2: requesting inband/2500base-x, 00200,0000a440
[<c00d39e0>] (load_module) from [<c00d4248>] (sys_finit_module+0x8c/0x98)
[<c00d4248>] (sys_finit_module) from [<c0009000>] (ret_fast_syscall+0x0/0x28)
Exception stack(0xedfe5fa8 to 0xedfe5ff0)
5fa0:                   00020000 00000000 0000000b b6f2a4b5 00000000 00b8fc70
5fc0: 00020000 00000000 00000000 0000017b 00b995a0 00020000 00000000 00b8fc70
5fe0: bedf2af8 bedf2ae8 b6f242ac b6e83d70

This is caused by the locking order inversion in mv88e6xxx_probe:

        mutex_lock(&chip->reg_lock);
        if (chip->irq > 0)
                err = mv88e6xxx_g1_irq_setup(chip);
        else
                err = mv88e6xxx_irq_poll_setup(chip);
        mutex_unlock(&chip->reg_lock);

Here, we take chip->reg_lock, and then call into mv88e6xxx_g1_irq_setup()
which then calls request_threaded_irq(), taking the request_mutex.

However, when we request the g2 interrupt, we call request_threaded_irq()
again, which takes the request_mutex, which then goes on to call
chip_bus_lock().  This comes through to mv88e6xxx_g1_irq_bus_lock,
which then tries to grab chip->reg_lock.  This results in the two locks
being taken together in differing orders, provoking lockdep to warn.

Move the mutex_lock()/unlock() for reg_lock inside
mv88e6xxx_g1_irq_free_common() and mv88e6xxx_g1_irq_setup_common(), where
we actually access registers, thereby avoiding holding it while calling
request_threaded_irq() or setting up the delayed work.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 24fb6a685039..801442195a04 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -349,9 +349,11 @@ static void mv88e6xxx_g1_irq_free_common(struct mv88e6xxx_chip *chip)
 	int irq, virq;
 	u16 mask;
 
+	mutex_lock(&chip->reg_lock);
 	mv88e6xxx_g1_read(chip, MV88E6XXX_G1_CTL1, &mask);
 	mask &= ~GENMASK(chip->g1_irq.nirqs, 0);
 	mv88e6xxx_g1_write(chip, MV88E6XXX_G1_CTL1, mask);
+	mutex_unlock(&chip->reg_lock);
 
 	for (irq = 0; irq < chip->g1_irq.nirqs; irq++) {
 		virq = irq_find_mapping(chip->g1_irq.domain, irq);
@@ -369,9 +371,7 @@ static void mv88e6xxx_g1_irq_free(struct mv88e6xxx_chip *chip)
 	 */
 	free_irq(chip->irq, chip);
 
-	mutex_lock(&chip->reg_lock);
 	mv88e6xxx_g1_irq_free_common(chip);
-	mutex_unlock(&chip->reg_lock);
 }
 
 static int mv88e6xxx_g1_irq_setup_common(struct mv88e6xxx_chip *chip)
@@ -392,6 +392,7 @@ static int mv88e6xxx_g1_irq_setup_common(struct mv88e6xxx_chip *chip)
 	chip->g1_irq.chip = mv88e6xxx_g1_irq_chip;
 	chip->g1_irq.masked = ~0;
 
+	mutex_lock(&chip->reg_lock);
 	err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_CTL1, &mask);
 	if (err)
 		goto out_mapping;
@@ -406,6 +407,7 @@ static int mv88e6xxx_g1_irq_setup_common(struct mv88e6xxx_chip *chip)
 	err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_STS, &reg);
 	if (err)
 		goto out_disable;
+	mutex_unlock(&chip->reg_lock);
 
 	return 0;
 
@@ -414,6 +416,7 @@ static int mv88e6xxx_g1_irq_setup_common(struct mv88e6xxx_chip *chip)
 	mv88e6xxx_g1_write(chip, MV88E6XXX_G1_CTL1, mask);
 
 out_mapping:
+	mutex_unlock(&chip->reg_lock);
 	for (irq = 0; irq < 16; irq++) {
 		virq = irq_find_mapping(chip->g1_irq.domain, irq);
 		irq_dispose_mapping(virq);
@@ -479,9 +482,7 @@ static void mv88e6xxx_irq_poll_free(struct mv88e6xxx_chip *chip)
 	kthread_cancel_delayed_work_sync(&chip->irq_poll_work);
 	kthread_destroy_worker(chip->kworker);
 
-	mutex_lock(&chip->reg_lock);
 	mv88e6xxx_g1_irq_free_common(chip);
-	mutex_unlock(&chip->reg_lock);
 }
 
 int mv88e6xxx_wait(struct mv88e6xxx_chip *chip, int addr, int reg, u16 mask)
@@ -4718,12 +4719,10 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	 * the PHYs will link their interrupts to these interrupt
 	 * controllers
 	 */
-	mutex_lock(&chip->reg_lock);
 	if (chip->irq > 0)
 		err = mv88e6xxx_g1_irq_setup(chip);
 	else
 		err = mv88e6xxx_irq_poll_setup(chip);
-	mutex_unlock(&chip->reg_lock);
 
 	if (err)
 		goto out;
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next v2 0/3] net: dsa: mv88e6xxx: fix IPv6
From: Russell King - ARM Linux admin @ 2019-02-17 16:31 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vivien Didelot; +Cc: David S. Miller, netdev
In-Reply-To: <20190217142414.cjtmpi5y2l5rtdlb@shell.armlinux.org.uk>

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.

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.

 drivers/net/dsa/mv88e6xxx/chip.c | 45 ++++++++++++++++++++++++++++++++++------
 include/net/dsa.h                |  3 +++
 net/dsa/dsa_priv.h               |  2 ++
 net/dsa/port.c                   | 15 ++++++++++++++
 net/dsa/slave.c                  |  6 ++++++
 5 files changed, 65 insertions(+), 6 deletions(-)

v2: fix a couple of compile errors in patch 2 and patch 3 (oops).
-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* [PATCH net-next v2 1/3] net: dsa: add support for bridge flags
From: Russell King @ 2019-02-17 16:32 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vivien Didelot
  Cc: Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <20190217163114.yomawlljyxlqy3ob@shell.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.

We implement this by providing two new methods: one to get the switch-
wide support bitmask, and another to set the properties.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 include/net/dsa.h  |  3 +++
 net/dsa/dsa_priv.h |  2 ++
 net/dsa/port.c     | 15 +++++++++++++++
 net/dsa/slave.c    |  6 ++++++
 4 files changed, 26 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 7f2a668ef2cc..6cc1222aa2ca 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -400,6 +400,9 @@ 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_bridge_flags)(struct dsa_switch *ds, int port,
+				     unsigned long);
+	unsigned long (*bridge_flags_support)(struct dsa_switch *ds);
 
 	/*
 	 * 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..eb745041b1d9 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -177,6 +177,21 @@ 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;
+
+	if (switchdev_trans_ph_prepare(trans))
+		return ds->ops->port_bridge_flags ? 0 : -EOPNOTSUPP;
+
+	if (ds->ops->port_bridge_flags)
+		ds->ops->port_bridge_flags(ds, port, flags);
+
+	return 0;
+}
+
 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..af53cdd14a29 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;
@@ -384,6 +387,9 @@ static int dsa_slave_port_attr_get(struct net_device *dev,
 	switch (attr->id) {
 	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT:
 		attr->u.brport_flags_support = 0;
+		if (ds->ops->bridge_flags_support)
+			attr->u.brport_flags_support |=
+				ds->ops->bridge_flags_support(ds);
 		break;
 	default:
 		return -EOPNOTSUPP;
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next v2 2/3] net: dsa: mv88e6xxx: add support for bridge flags
From: Russell King @ 2019-02-17 16:32 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vivien Didelot
  Cc: Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <20190217163114.yomawlljyxlqy3ob@shell.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

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 32e7af5caa69..72db6e74be48 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4692,6 +4692,38 @@ static int mv88e6xxx_port_mdb_del(struct dsa_switch *ds, int port,
 	return err;
 }
 
+static int mv88e6xxx_port_bridge_flags(struct dsa_switch *ds, int port,
+				       unsigned long flags)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	bool unicast, multicast;
+	int ret = -EOPNOTSUPP;
+
+	unicast = dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port) ||
+		flags & BR_FLOOD;
+	multicast = flags & BR_MCAST_FLOOD;
+
+	mutex_lock(&chip->reg_lock);
+	if (chip->info->ops->port_set_egress_floods)
+		ret = chip->info->ops->port_set_egress_floods(chip, port,
+							      unicast,
+							      multicast);
+	mutex_unlock(&chip->reg_lock);
+
+	return ret;
+}
+
+static unsigned long mv88e6xxx_bridge_flags_support(struct dsa_switch *ds)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	unsigned long support = 0;
+
+	if (chip->info->ops->port_set_egress_floods)
+		support |= BR_FLOOD | BR_MCAST_FLOOD;
+
+	return support;
+}
+
 static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 #if IS_ENABLED(CONFIG_NET_DSA_LEGACY)
 	.probe			= mv88e6xxx_drv_probe,
@@ -4719,6 +4751,8 @@ 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_bridge_flags	= mv88e6xxx_port_bridge_flags,
+	.bridge_flags_support	= mv88e6xxx_bridge_flags_support,
 	.port_stp_state_set	= mv88e6xxx_port_stp_state_set,
 	.port_fast_age		= mv88e6xxx_port_fast_age,
 	.port_vlan_filtering	= mv88e6xxx_port_vlan_filtering,
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next v2 3/3] net: dsa: mv88e6xxx: default to multicast and unicast flooding
From: Russell King @ 2019-02-17 16:32 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vivien Didelot
  Cc: Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <20190217163114.yomawlljyxlqy3ob@shell.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 has defaulted to having
multicast and unicast flooding disabled.  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 that
unknown unicast frames and unknown multicast frames are flooded to
all stations, 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.  This means that mv88e6xxx DSA switches now
behave as per the bridge(8) man page, and IPv6 works flawlessly through
such a switch.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 72db6e74be48..c1bcd13af13f 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2143,14 +2143,13 @@ static int mv88e6xxx_setup_message_port(struct mv88e6xxx_chip *chip, int port)
 
 static int mv88e6xxx_setup_egress_floods(struct mv88e6xxx_chip *chip, int port)
 {
-	struct dsa_switch *ds = chip->ds;
-	bool flood;
-
-	/* Upstream ports flood frames with unknown unicast or multicast DA */
-	flood = dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port);
+	/* Linux bridges are expected to flood unknown multicast and
+	 * unicast frames to all ports - as per the defaults specified
+	 * in the iproute2 bridge(8) man page. Not doing this causes
+	 * stalls and failures with IPv6 over Marvell bridges. */
 	if (chip->info->ops->port_set_egress_floods)
 		return chip->info->ops->port_set_egress_floods(chip, port,
-							       flood, flood);
+							       true, true);
 
 	return 0;
 }
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH net-next 3/3] net: dsa: mv88e6xxx: defautl to multicast and unicast flooding
From: Russell King - ARM Linux admin @ 2019-02-17 16:34 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vivien Didelot; +Cc: David S. Miller, netdev
In-Reply-To: <20190217142716.gsrq5k2gnw3hsnhu@shell.armlinux.org.uk>

On Sun, Feb 17, 2019 at 02:27:16PM +0000, Russell King - ARM Linux admin wrote:
> On Sun, Feb 17, 2019 at 02:25:17PM +0000, Russell King wrote:
> > 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 has defaulted to having
> > multicast and unicast flooding disabled.  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 that
> > unknown unicast frames and unknown multicast frames are flooded to
> > all stations, 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.  This means that mv88e6xxx DSA switches now
> > behave as per the bridge(8) man page, and IPv6 works flawlessly through
> > such a switch.
> 
> Note that there is the open question whether this affects the case where
> each port is used as a separate network interface: that case has not yet
> been tested.

I've checked with a mv88e6131 on the clearfog gt8k board.  lan1
connected to my lan with plenty of traffic on, and configured as
part of a bridge.  lan2 connected to the zii board, but not part
of the bridge.  Monitoring lan2 from the zii board shows no traffic
that was received from lan1.

So it looks fine.

> 
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/net/dsa/mv88e6xxx/chip.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> > index b75a865a293d..eb5e3d88374f 100644
> > --- a/drivers/net/dsa/mv88e6xxx/chip.c
> > +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> > @@ -2144,13 +2144,14 @@ static int mv88e6xxx_setup_message_port(struct mv88e6xxx_chip *chip, int port)
> >  static int mv88e6xxx_setup_egress_floods(struct mv88e6xxx_chip *chip, int port)
> >  {
> >  	struct dsa_switch *ds = chip->ds;
> > -	bool flood;
> >  
> > -	/* Upstream ports flood frames with unknown unicast or multicast DA */
> > -	flood = dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port);
> > +	/* Linux bridges are expected to flood unknown multicast and
> > +	 * unicast frames to all ports - as per the defaults specified
> > +	 * in the iproute2 bridge(8) man page. Not doing this causes
> > +	 * stalls and failures with IPv6 over Marvell bridges. */
> >  	if (chip->info->ops->port_set_egress_floods)
> >  		return chip->info->ops->port_set_egress_floods(chip, port,
> > -							       flood, flood);
> > +							       true, true);
> >  
> >  	return 0;
> >  }
> > -- 
> > 2.7.4
> > 
> > 
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* Re: No traffic with Marvell switch and latest linux-next
From: Heiner Kallweit @ 2019-02-17 16:40 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Andrew Lunn, Florian Fainelli, netdev@vger.kernel.org
In-Reply-To: <291e7622-4402-e58f-503c-ffc7c6b2f055@gmail.com>

On 17.02.2019 16:50, Heiner Kallweit wrote:
> On 17.02.2019 16:40, Russell King - ARM Linux admin wrote:
>> On Sun, Feb 17, 2019 at 04:34:32PM +0100, Heiner Kallweit wrote:
>>> When testing latest linux-next on the ZII DTU I face the issue that no
>>> traffic is flowing over the switch ports, even though in dmesg
>>> everything looks good. Also PHY properly establishes the link.
>>>
>>> With 4.20.10 I don't have the issue and with 5.0-rc6 also not.
>>> However on 5.0-rc6 I got the following, also number of network
>>> interrupts seems to be very high (few minutes after boot).
>>> Any idea what's going on?
>>>
>>> Andrew, IIRC you recently fixed some interrupt-related issue:
>>> 7ae710f9f8b2 ("gpio: vf610: Mask all GPIO interrupts")
>>> But the description doesn't seem to match this trace.
>>
>> I have a fix for the trace you have below, but it has nothing to do
>> with no traffic.  I'll send it out shortly.
>>
>> Which protocol are you using (ipv4 or ipv6)?  Have you setup a
>> bridge device containing the ports you wish to switch network
>> traffic.  Without a bridge device, DSA will by default treat each
>> port as a separate port.  The other thing that gets people is the
>> ethernet interface connected to the DSA switch must be up _before_
>> bringing up any of the switch ports.
>>
> ipv4, a simple ping. No bridge. Device is connected to a switch
> that is always on.
> 
> Technical environment and userspace is always the same, so it seems
> to be the kernel version.
> 
There haven't been that many changes to mv88e8xxx since 5.0-rc6.
I reverted 7c0db24cc431 ("dsa: mv88e6xxx: Ensure all pending interrupts
are handled prior to exit") who looked like a candidate and bingo:
network is working again. Obviously something is wrong with this patch.

>>>
>>> irq 56: nobody cared (try booting with the "irqpoll" option)
>>> CPU: 0 PID: 577 Comm: irq/38-400d1000 Not tainted 5.0.0-rc6 #1
>>> Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
>>> [<8010c898>] (unwind_backtrace) from [<8010ad98>] (show_stack+0x10/0x14)
>>> [<8010ad98>] (show_stack) from [<80149660>] (__report_bad_irq+0x38/0xb0)
>>> [<80149660>] (__report_bad_irq) from [<80149478>] (note_interrupt+0x10c/0x294)
>>> [<80149478>] (note_interrupt) from [<80149cac>] (handle_nested_irq+0xd8/0xf4)
>>> [<80149cac>] (handle_nested_irq) from [<80384a64>] (mv88e6xxx_g2_irq_thread_fn+0x90/0xc0)
>>> [<80384a64>] (mv88e6xxx_g2_irq_thread_fn) from [<80149c60>] (handle_nested_irq+0x8c/0xf4)
>>> [<80149c60>] (handle_nested_irq) from [<8037ccd0>] (mv88e6xxx_g1_irq_thread_work+0x98/0xcc)
>>> [<8037ccd0>] (mv88e6xxx_g1_irq_thread_work) from [<80147ff4>] (irq_thread_fn+0x1c/0x78)
>>> [<80147ff4>] (irq_thread_fn) from [<80148280>] (irq_thread+0x124/0x1cc)
>>> [<80148280>] (irq_thread) from [<8012f8e8>] (kthread+0x140/0x148)
>>> [<8012f8e8>] (kthread) from [<801010e8>] (ret_from_fork+0x14/0x2c)
>>> Exception stack(0x9f6c7fb0 to 0x9f6c7ff8)
>>> 7fa0:                                     00000000 00000000 00000000 00000000
>>> 7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>> 7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
>>> handlers:
>>> [<b09c70df>] irq_default_primary_handler threaded [<44d6803f>] phy_interrupt
>>> Disabling IRQ #56
>>>
>>>
>>> 36:    2030566   mscm-ir  79 Edge      400d1000.ethernet
>>>  38:    1010437  gpio-vf610   2 Level     400d1000.ethernet-1:00
>>>  42:          0  mv88e6xxx-g1   3 Edge      mv88e6xxx-g1-atu-prob
>>>  44:          0  mv88e6xxx-g1   5 Edge      mv88e6xxx-g1-vtu-prob
>>>  46:    1010435  mv88e6xxx-g1   7 Edge      mv88e6xxx-g2
>>>  49:          0  mv88e6xxx-g2   1 Edge      mv88e6xxx-1:01
>>>  53:          0  mv88e6xxx-g2   5 Edge      mv88e6xxx-1:05
>>>  54:          0  mv88e6xxx-g2   6 Edge      mv88e6xxx-1:06
>>>  56:     100000  mv88e6xxx-g2   8 Edge      mv88e6xxx-1:08
>>>  63:          0  mv88e6xxx-g2  15 Edge      mv88e6xxx-watchdog
>>>
>>> Heiner
>>>
>>
> 


^ permalink raw reply

* Re: [PATCH net] net: dsa: fix lockdep warning
From: Andrew Lunn @ 2019-02-17 16:46 UTC (permalink / raw)
  To: Russell King
  Cc: Florian Fainelli, Vivien Didelot, Heiner Kallweit,
	David S. Miller, netdev
In-Reply-To: <E1gvPHw-0008OD-To@rmk-PC.armlinux.org.uk>

On Sun, Feb 17, 2019 at 04:27:32PM +0000, Russell King wrote:
> ======================================================
> WARNING: possible circular locking dependency detected
> 4.20.0+ #302 Not tainted
> ------------------------------------------------------

Hi Russell

Thanks for turning this into a proper patch. I had just started to try
to reproduce this and confirm your fix. I will add a tested-by once i
do.

Thanks
	Andrew

^ permalink raw reply

* Re: [PATCH net] net: dsa: fix lockdep warning
From: Russell King - ARM Linux admin @ 2019-02-17 16:51 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Florian Fainelli, Vivien Didelot, Heiner Kallweit,
	David S. Miller, netdev
In-Reply-To: <20190217164629.GE5968@lunn.ch>

On Sun, Feb 17, 2019 at 05:46:29PM +0100, Andrew Lunn wrote:
> On Sun, Feb 17, 2019 at 04:27:32PM +0000, Russell King wrote:
> > ======================================================
> > WARNING: possible circular locking dependency detected
> > 4.20.0+ #302 Not tainted
> > ------------------------------------------------------
> 
> Hi Russell
> 
> Thanks for turning this into a proper patch. I had just started to try
> to reproduce this and confirm your fix. I will add a tested-by once i
> do.

Do you have a clearfog board?  If so, just add:

+               interrupt-parent = <&gpio0>;
+               interrupts = <23 IRQ_TYPE_LEVEL_LOW>;

to the DSA switch definitions in
arch/arm/boot/dts/armada-388-clearfog.dts

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* Re: No traffic with Marvell switch and latest linux-next
From: Andrew Lunn @ 2019-02-17 16:57 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Russell King - ARM Linux admin, Florian Fainelli,
	netdev@vger.kernel.org
In-Reply-To: <fed19307-e002-a76b-79e1-a76b292c3f2c@gmail.com>

> There haven't been that many changes to mv88e8xxx since 5.0-rc6.
> I reverted 7c0db24cc431 ("dsa: mv88e6xxx: Ensure all pending interrupts
> are handled prior to exit") who looked like a candidate and bingo:
> network is working again. Obviously something is wrong with this patch.

O.K. I tested it on an edge interrupt system, but not a level
interrupt. I wounder if it is related to that somehow? DTU should be
using level interrupts.

       Andrew

^ permalink raw reply

* Re: [PATCH net] net: dsa: fix lockdep warning
From: Andrew Lunn @ 2019-02-17 17:00 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Florian Fainelli, Vivien Didelot, Heiner Kallweit,
	David S. Miller, netdev
In-Reply-To: <20190217165140.3zyy24vi6mier2tt@shell.armlinux.org.uk>

On Sun, Feb 17, 2019 at 04:51:40PM +0000, Russell King - ARM Linux admin wrote:
> On Sun, Feb 17, 2019 at 05:46:29PM +0100, Andrew Lunn wrote:
> > On Sun, Feb 17, 2019 at 04:27:32PM +0000, Russell King wrote:
> > > ======================================================
> > > WARNING: possible circular locking dependency detected
> > > 4.20.0+ #302 Not tainted
> > > ------------------------------------------------------
> > 
> > Hi Russell
> > 
> > Thanks for turning this into a proper patch. I had just started to try
> > to reproduce this and confirm your fix. I will add a tested-by once i
> > do.
> 
> Do you have a clearfog board?  If so, just add:
> 
> +               interrupt-parent = <&gpio0>;
> +               interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
> 
> to the DSA switch definitions in
> arch/arm/boot/dts/armada-388-clearfog.dts

Hi Russell

I'm using a different board, but i have a similar interrupt
configuration. I just expect that turning on LOCKDEP should be enough.
I don't think EPROBE_DEFFER is playing a part here.

	Andrew

^ permalink raw reply

* Re: [PATCH net] net: dsa: fix lockdep warning
From: Florian Fainelli @ 2019-02-17 17:03 UTC (permalink / raw)
  To: Russell King, Andrew Lunn, Vivien Didelot
  Cc: Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <E1gvPHw-0008OD-To@rmk-PC.armlinux.org.uk>

Hi Russell,

On 2/17/2019 8:27 AM, Russell King wrote:
> ======================================================
> WARNING: possible circular locking dependency detected
> 4.20.0+ #302 Not tainted
> ------------------------------------------------------
> systemd-udevd/160 is trying to acquire lock:
> edea6080 (&chip->reg_lock){+.+.}, at: __setup_irq+0x640/0x704
> 
> but task is already holding lock:
> edff0340 (&desc->request_mutex){+.+.}, at: __setup_irq+0xa0/0x704
> 
> which lock already depends on the new lock.

Since this is specific to mv88e6xxx, in case you resubmit, can you put
that in the commit subject as well? Thanks!
-- 
Florian

^ permalink raw reply

* Re: [PATCH net] net: dsa: fix lockdep warning
From: Russell King - ARM Linux admin @ 2019-02-17 17:03 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Florian Fainelli, Vivien Didelot, Heiner Kallweit,
	David S. Miller, netdev
In-Reply-To: <20190217170024.GG5968@lunn.ch>

On Sun, Feb 17, 2019 at 06:00:24PM +0100, Andrew Lunn wrote:
> On Sun, Feb 17, 2019 at 04:51:40PM +0000, Russell King - ARM Linux admin wrote:
> > On Sun, Feb 17, 2019 at 05:46:29PM +0100, Andrew Lunn wrote:
> > > On Sun, Feb 17, 2019 at 04:27:32PM +0000, Russell King wrote:
> > > > ======================================================
> > > > WARNING: possible circular locking dependency detected
> > > > 4.20.0+ #302 Not tainted
> > > > ------------------------------------------------------
> > > 
> > > Hi Russell
> > > 
> > > Thanks for turning this into a proper patch. I had just started to try
> > > to reproduce this and confirm your fix. I will add a tested-by once i
> > > do.
> > 
> > Do you have a clearfog board?  If so, just add:
> > 
> > +               interrupt-parent = <&gpio0>;
> > +               interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
> > 
> > to the DSA switch definitions in
> > arch/arm/boot/dts/armada-388-clearfog.dts
> 
> Hi Russell
> 
> I'm using a different board, but i have a similar interrupt
> configuration. I just expect that turning on LOCKDEP should be enough.
> I don't think EPROBE_DEFFER is playing a part here.

Yep, lockdep should be enough.  In case it does matter, I have DSA
built as modules here.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* Re: No traffic with Marvell switch and latest linux-next
From: Heiner Kallweit @ 2019-02-17 17:06 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Russell King - ARM Linux admin, Florian Fainelli,
	netdev@vger.kernel.org
In-Reply-To: <20190217165730.GF5968@lunn.ch>

On 17.02.2019 17:57, Andrew Lunn wrote:
>> There haven't been that many changes to mv88e8xxx since 5.0-rc6.
>> I reverted 7c0db24cc431 ("dsa: mv88e6xxx: Ensure all pending interrupts
>> are handled prior to exit") who looked like a candidate and bingo:
>> network is working again. Obviously something is wrong with this patch.
> 
> O.K. I tested it on an edge interrupt system, but not a level
> interrupt. I wounder if it is related to that somehow? DTU should be
> using level interrupts.
> 
Sorry, I may have been too fast with this statement. With this patch
reverted it worked, but now I have a build with this patch still included,
and it works too. Need to dig deeper ..

>        Andrew
> 

Heiner

^ permalink raw reply

* Re: No traffic with Marvell switch and latest linux-next
From: Andrew Lunn @ 2019-02-17 17:10 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Russell King - ARM Linux admin, Florian Fainelli,
	netdev@vger.kernel.org
In-Reply-To: <c7050314-8af4-b65f-778d-92b9d4a7cc27@gmail.com>

> Sorry, I may have been too fast with this statement. With this patch
> reverted it worked, but now I have a build with this patch still included,
> and it works too. Need to dig deeper ..

Hi Heiner

Watch out for boot vs reboot, and when rebooting if port 8 had link or
not before you reboot.

    Andrew

^ permalink raw reply

* Re: [BUG] [FIX] net: dsa: oops in br_vlan_enabled
From: Florian Fainelli @ 2019-02-17 17:13 UTC (permalink / raw)
  To: Frank Wunderlich, netdev, Andrew Lunn, Vivien Didelot
In-Reply-To: <trinity-782c45c1-ef94-4340-8def-f949a4a929a9-1550334156901@3c-app-gmx-bs35>

Hi Frank,

On 2/16/2019 8:22 AM, Frank Wunderlich wrote:
> Hi,
> 
> i've found an oops in 4.19.23/10, seems to be fixed anyhow in 5.0 (also works in 4.14.101)
> 
> root@bpi-r2:~# ip link add link lan0 name lan0.5 type vlan id 5
> root@bpi-r2:~# ip addr add 192.168.5.200/24 brd 192.168.5.255 dev lan0.5
> root@bpi-r2:~# ip link set dev lan0 up
> root@bpi-r2:~# ip link set dev lan0.5 up

So that these steps don't involve a bridge, and because we don't (yet)
implment ndo_vlan_rx_{add,kill}_vid() netdevice_ops, there is no VLAN
programming, it's all software

> 
> 12: lan0.5@lan0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state LOWERLAYERDOWN group default qlen 1000
>     link/ether 02:02:02:02:02:02 brd ff:ff:ff:ff:ff:ff
>     inet 192.168.5.200/24 brd 192.168.5.255 scope global lan0.5
>        valid_lft forever preferred_lft forever
> 
> root@bpi-r2:~# brctl addbr bridge_name
> root@bpi-r2:~# brctl addif bridge_name lan0.5

Unless you changed the bridge to have VLAN filtering/awareness with:

echo 1 > /sys/class/net/bridge_name/bridge/vlan_filtering

There will be no VLAN configuration/objects pushed to DSA since commit
2ea7a679ca2abd251c1ec03f20508619707e1749 ("net: dsa: Don't add vlans
when vlan filtering is disabled") so I am not sure how you got into that
situation because prior to pushing a VLAN object, the port must be part
of a bridge, so the steps typically look like:

- port_bridge_join which assigns dp->bridge_dev
- port_vlan_add

Does your 4.19.23 kernel somehow change how VLAN objects are pushed down
the switch driver?

> [  352.057128] bridge_name: port 1(lan0.5) entered blocking state
> [  352.063065] bridge_name: port 1(lan0.5) entered disabled state
> [  352.069181] device lan0.5 entered promiscuous mode
> [  352.074018] device lan0 entered promiscuous mode
> [  352.078906] Unable to handle kernel NULL pointer dereference at virtual address 00000558
> ...
> [  352.493085] [<bf0fde88>] (br_vlan_enabled [bridge]) from [<bf12c234>] (dsa_port_vlan_add+0x60/0xbc [dsa_core])
> [  352.503050] [<bf12c234>] (dsa_port_vlan_add [dsa_core]) from [<bf12cb64>] (dsa_slave_port_obj_add+0x4c/0x50 [dsa_core])
> [  352.513776] [<bf12cb64>] (dsa_slave_port_obj_add [dsa_core]) from [<c0b4e2d4>] (__switchdev_port_obj_add+0x50/0xc4)
> [  352.524138] [<c0b4e2d4>] (__switchdev_port_obj_add) from [<c0b4e324>] (__switchdev_port_obj_add+0xa0/0xc4)
> [  352.533721] [<c0b4e324>] (__switchdev_port_obj_add) from [<c0b4e3a8>] (switchdev_port_obj_add_now+0x60/0x130)
> [  352.543562] [<c0b4e3a8>] (switchdev_port_obj_add_now) from [<c0b4e7e4>] (switchdev_port_obj_add+0x44/0x190)
> [  352.553284] [<c0b4e7e4>] (switchdev_port_obj_add) from [<bf1013d0>] (br_switchdev_port_vlan_add+0x60/0x7c [bridge])
> [  352.563733] [<bf1013d0>] (br_switchdev_port_vlan_add [bridge]) from [<bf0ff250>] (__vlan_add+0xb0/0x620 [bridge])
> [  352.574007] [<bf0ff250>] (__vlan_add [bridge]) from [<bf0ffd04>] (nbp_vlan_add+0xc4/0x150 [bridge])
> [  352.583073] [<bf0ffd04>] (nbp_vlan_add [bridge]) from [<bf0ffec4>] (nbp_vlan_init+0x134/0x164 [bridge])
> [  352.592482] [<bf0ffec4>] (nbp_vlan_init [bridge]) from [<bf0edd4c>] (br_add_if+0x40c/0x5fc [bridge])
> [  352.601632] [<bf0edd4c>] (br_add_if [bridge]) from [<bf0eeb14>] (add_del_if+0x6c/0x80 [bridge])
> [  352.610351] [<bf0eeb14>] (add_del_if [bridge]) from [<bf0ef5b0>] (br_dev_ioctl+0x7c/0x9c [bridge])
> [  352.619290] [<bf0ef5b0>] (br_dev_ioctl [bridge]) from [<c09583d4>] (dev_ifsioc+0x184/0x324)
> [  352.627582] [<c09583d4>] (dev_ifsioc) from [<c09589e8>] (dev_ioctl+0x32c/0x5cc)
> [  352.634837] [<c09589e8>] (dev_ioctl) from [<c090913c>] (sock_ioctl+0x3bc/0x580)
> 
> 
> since my 4.19.23 kernel is modified a bit i tried with 4.19.10 without my net modifications and it is still reproducable with steps above (create a vlan on dsa-user-port and then use it in a bridge)
> 
> i fixed it with these changes:
> 
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index ed0595459df1..962887752ae8 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -255,8 +255,9 @@ 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);
> +       printk(KERN_ALERT "DEBUG: Passed %s %d 0x%x \n",__FUNCTION__,__LINE__,(unsigned int)dp->bridge_dev);
> +       if (!dp->bridge_dev || br_vlan_enabled(dp->bridge_dev))
> +               return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
>  
>         return 0;
>  }
> @@ -273,7 +274,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))
> +       if (!dp->bridge_dev || br_vlan_enabled(dp->bridge_dev))
>                 return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
>  
>         return 0;
> 
> i've found in a Patch from florian/vivien: https://www.mail-archive.com/netdev@vger.kernel.org/msg281415.html
> 
> Strange that 5.0-rc1 does not crash,because these 2 code-sections are unchanged: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/dsa/port.c#n255 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/bridge/br_vlan.c#n788
> 
> maybe you know why only 4.19 is affected...
> 
> regards Frank
> 

-- 
Florian

^ permalink raw reply

* Re: [PATCH net] net: dsa: fix lockdep warning
From: Andrew Lunn @ 2019-02-17 17:55 UTC (permalink / raw)
  To: Russell King
  Cc: Florian Fainelli, Vivien Didelot, Heiner Kallweit,
	David S. Miller, netdev
In-Reply-To: <E1gvPHw-0008OD-To@rmk-PC.armlinux.org.uk>

On Sun, Feb 17, 2019 at 04:27:32PM +0000, Russell King wrote:
> ======================================================
> WARNING: possible circular locking dependency detected
> 4.20.0+ #302 Not tainted
> ------------------------------------------------------
> systemd-udevd/160 is trying to acquire lock:
> edea6080 (&chip->reg_lock){+.+.}, at: __setup_irq+0x640/0x704
> 
> but task is already holding lock:
> edff0340 (&desc->request_mutex){+.+.}, at: __setup_irq+0xa0/0x704
> 
> which lock already depends on the new lock.
> 
> the existing dependency chain (in reverse order) is:
> 
> -> #1 (&desc->request_mutex){+.+.}:
>        mutex_lock_nested+0x1c/0x24
>        __setup_irq+0xa0/0x704
>        request_threaded_irq+0xd0/0x150
>        mv88e6xxx_probe+0x41c/0x694 [mv88e6xxx]

> -> #0 (&chip->reg_lock){+.+.}:
>        __mutex_lock+0x50/0x8b8
>        mutex_lock_nested+0x1c/0x24
>        __setup_irq+0x640/0x704
>        request_threaded_irq+0xd0/0x150
>        mv88e6xxx_g2_irq_setup+0xcc/0x1b4 [mv88e6xxx]
>        mv88e6xxx_probe+0x44c/0x694 [mv88e6xxx]
>        mdio_probe+0x2c/0x54
> 
> other info that might help us debug this:
> 
>  Possible unsafe locking scenario:
> 
>        CPU0                    CPU1
>        ----                    ----
>   lock(&desc->request_mutex);
>                                lock(&chip->reg_lock);
>                                lock(&desc->request_mutex);
>   lock(&chip->reg_lock);

Hi Russell

I failed to reproduce it on a Freescale system. Which made me take a
closer look at the above. This is a false positive.

In #1 we are requesting the GPIO interrupt. In #2 we are requesting
the chained interrupt from the mv88e6xxx global 1 interrupt handler.
So these are different desc->request_mutex. The Freescale VF610 GPIO
driver uses gpiochip_irqchip_add(), which creates a lock class for the
GPIO. The marvell gpio-mvebu driver does not create a lock class.  So
when i test on Freescale, lockdep can tell they are different mutex,
but on clearfog it cannot.

So i think the real fix is probably two fold, although just doing one
is sufficient:

1) Add lock classes to gpio-mvebu, by call irq_set_lockdep_class()
2) Add lock classes to chip.c global 1, by calling irq_set_lockdep_class()

There is probably more value in 1) since the mvebu gpio driver is much
more widely used than the mv88e6xxx driver.

     Andrew

^ permalink raw reply

* Re: [PATCH net] mlxsw: __mlxsw_sp_port_headroom_set(): Fix a use of local variable
From: David Miller @ 2019-02-17 18:14 UTC (permalink / raw)
  To: idosch; +Cc: netdev, jiri, petrm, mlxsw
In-Reply-To: <20190217071809.19295-1-idosch@mellanox.com>

From: Ido Schimmel <idosch@mellanox.com>
Date: Sun, 17 Feb 2019 07:18:41 +0000

> From: Petr Machata <petrm@mellanox.com>
> 
> The function-local variable "delay" enters the loop interpreted as delay
> in bits. However, inside the loop it gets overwritten by the result of
> mlxsw_sp_pg_buf_delay_get(), and thus leaves the loop as quantity in
> cells. Thus on second and further loop iterations, the headroom for a
> given priority is configured with a wrong size.
> 
> Fix by introducing a loop-local variable, delay_cells. Rename thres to
> thres_cells for consistency.
> 
> Fixes: f417f04da589 ("mlxsw: spectrum: Refactor port buffer configuration")
> Signed-off-by: Petr Machata <petrm@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>

Applied and queued up for -stable, thanks Ido.

^ permalink raw reply

* [PATCH] net: dsa: mv88e6xxx: Add lockdep classes to fix false positive splat
From: Andrew Lunn @ 2019-02-17 18:11 UTC (permalink / raw)
  To: Russell King; +Cc: netdev, Vivien Didelot, Andrew Lunn

The following false positive lockdep splat has been observed.

======================================================
WARNING: possible circular locking dependency detected
4.20.0+ #302 Not tainted
------------------------------------------------------
systemd-udevd/160 is trying to acquire lock:
edea6080 (&chip->reg_lock){+.+.}, at: __setup_irq+0x640/0x704

but task is already holding lock:
edff0340 (&desc->request_mutex){+.+.}, at: __setup_irq+0xa0/0x704

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #1 (&desc->request_mutex){+.+.}:
       mutex_lock_nested+0x1c/0x24
       __setup_irq+0xa0/0x704
       request_threaded_irq+0xd0/0x150
       mv88e6xxx_probe+0x41c/0x694 [mv88e6xxx]
       mdio_probe+0x2c/0x54
       really_probe+0x200/0x2c4
       driver_probe_device+0x5c/0x174
       __driver_attach+0xd8/0xdc
       bus_for_each_dev+0x58/0x7c
       bus_add_driver+0xe4/0x1f0
       driver_register+0x7c/0x110
       mdio_driver_register+0x24/0x58
       do_one_initcall+0x74/0x2e8
       do_init_module+0x60/0x1d0
       load_module+0x1968/0x1ff4
       sys_finit_module+0x8c/0x98
       ret_fast_syscall+0x0/0x28
       0xbedf2ae8

-> #0 (&chip->reg_lock){+.+.}:
       __mutex_lock+0x50/0x8b8
       mutex_lock_nested+0x1c/0x24
       __setup_irq+0x640/0x704
       request_threaded_irq+0xd0/0x150
       mv88e6xxx_g2_irq_setup+0xcc/0x1b4 [mv88e6xxx]
       mv88e6xxx_probe+0x44c/0x694 [mv88e6xxx]
       mdio_probe+0x2c/0x54
       really_probe+0x200/0x2c4
       driver_probe_device+0x5c/0x174
       __driver_attach+0xd8/0xdc
       bus_for_each_dev+0x58/0x7c
       bus_add_driver+0xe4/0x1f0
       driver_register+0x7c/0x110
       mdio_driver_register+0x24/0x58
       do_one_initcall+0x74/0x2e8
       do_init_module+0x60/0x1d0
       load_module+0x1968/0x1ff4
       sys_finit_module+0x8c/0x98
       ret_fast_syscall+0x0/0x28
       0xbedf2ae8

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(&desc->request_mutex);
                               lock(&chip->reg_lock);
                               lock(&desc->request_mutex);
  lock(&chip->reg_lock);

&desc->request_mutex refer to two different mutex. #1 is the GPIO for
the chip interrupt. #2 is the chained interrupt between global 1 and
global 2.

Add lockdep classes to the GPIO interrupt to avoid this.

Reported-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---

Hi Russell

Does this fix it for you on Clearfog?

drivers/net/dsa/mv88e6xxx/chip.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 32e7af5caa69..936d53a92144 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -442,12 +442,20 @@ static int mv88e6xxx_g1_irq_setup_common(struct mv88e6xxx_chip *chip)
 
 static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip)
 {
+	static struct lock_class_key lock_key;
+	static struct lock_class_key request_key;
 	int err;
 
 	err = mv88e6xxx_g1_irq_setup_common(chip);
 	if (err)
 		return err;
 
+	/* These lock classes tells lockdep that global 1 irqs are in
+	 * a different category than their parent GPIO, so it won't
+	 * report false recursion.
+	 */
+	irq_set_lockdep_class(chip->irq, &lock_key, &request_key);
+
 	err = request_threaded_irq(chip->irq, NULL,
 				   mv88e6xxx_g1_irq_thread_fn,
 				   IRQF_ONESHOT | IRQF_SHARED,
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH net-next v3 0/4] net: phy: add and use genphy_c45_an_config_an
From: David Miller @ 2019-02-17 18:28 UTC (permalink / raw)
  To: hkallweit1; +Cc: andrew, f.fainelli, linux, netdev
In-Reply-To: <46898fe1-8924-0da0-1bbd-b54c220ff8d1@gmail.com>

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sun, 17 Feb 2019 10:27:43 +0100

> This series adds genphy_c45_an_config_an() and uses it in the
> marvell10g diver. In addition patch 4 aligns the aneg configuration
> with what is done in genphy_config_aneg().
> 
> v2:
> - in patch 2 changed function name to genphy_c45_an_config_aneg
> - in patch 3 add a comment regarding 1000BaseT vendor registers
> 
> v3:
> - rebase patch 3

Series applied.

For patches #1 and #4 there was some fuzz which I resolved.

Did you really rebase this series? :-)

^ permalink raw reply

* Re: [PATCH net-next 0/2] tracepoints in neighbor subsystem
From: David Miller @ 2019-02-17 18:34 UTC (permalink / raw)
  To: roopa; +Cc: netdev, dsa
In-Reply-To: <1550164511-21195-1-git-send-email-roopa@cumulusnetworks.com>

From: Roopa Prabhu <roopa@cumulusnetworks.com>
Date: Thu, 14 Feb 2019 09:15:09 -0800

> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> 
> Roopa Prabhu (2):
>   trace: events: add a few neigh tracepoints
>   neigh: hook tracepoints in neigh update code

Series applied, thanks.

Maybe put some actual text in this intro posting next time, explaining
what the series is doing, how it is doing it, and why?

Thanks.

^ permalink raw reply

* Re: [PATCH net-next v5] ipmr: ip6mr: Create new sockopt to clear mfc cache or vifs
From: David Miller @ 2019-02-17 18:37 UTC (permalink / raw)
  To: callum.sinclair
  Cc: kuznet, yoshfuji, nikolay, netdev, linux-kernel, nicolas.dichtel
In-Reply-To: <20190214024418.21490-2-callum.sinclair@alliedtelesis.co.nz>

From: Callum Sinclair <callum.sinclair@alliedtelesis.co.nz>
Date: Thu, 14 Feb 2019 15:44:18 +1300

> +#define MRT_FLUSH_VIFS	4	/* Flush multicast vifs */
> +#define MRT_FLUSH_VIFS_STATIC	8	/* Flush static multicast vifs */

Please rename to MRT_FLUSH_MIFS* as suggested by Nicolas and respin.

Thanks.

^ permalink raw reply

* Re: ax25: fix possible use-after-free
From: Eric Dumazet @ 2019-02-17 18:39 UTC (permalink / raw)
  To: f6bvp, netdev
In-Reply-To: <20190216233349.7977096b@draws-f6bvp>



On 02/16/2019 02:33 PM, f6bvp wrote:
> 
> Patch applied successfully on Linux draws-f6bvp 4.14.79-v7+ #1159 SMP Sun Nov 4 17:50:20 GMT 2018 armv7l GNU/Linux
> 
> However ax25_route_lock_use and ax25_route_lock_unuse() are not declared and compile failed.
> 
> make : on entre dans le répertoire « /usr/src/linux-headers-4.14.79-v7+ »
>   CC [M]  /usr/src/linux-4.14.y/net/ax25/ax25_ip.o
> /usr/src/linux-4.14.y/net/ax25/ax25_ip.c: In function ‘ax25_ip_xmit’:
> /usr/src/linux-4.14.y/net/ax25/ax25_ip.c:117:2: error: implicit declaration of function ‘ax25_route_lock_use’ [-Werror=implicit-function-declaration]
>   ax25_route_lock_use();
>   ^~~~~~~~~~~~~~~~~~~
> /usr/src/linux-4.14.y/net/ax25/ax25_ip.c:211:2: error: implicit declaration of function ‘ax25_route_lock_unuse’ [-Werror=implicit-function-declaration]
>   ax25_route_lock_unuse();
>   ^~~~~~~~~~~~~~~~~~~~~
> cc1: some warnings being treated as errors
> scripts/Makefile.build:328 : la recette pour la cible « /usr/src/linux-4.14.y/net/ax25/ax25_ip.o » a echouee
> make[1]: *** [/usr/src/linux-4.14.y/net/ax25/ax25_ip.o] Erreur 1
> Makefile:1527 : la recette pour la cible « _module_/usr/src/linux-4.14.y/net/ax25 » a echouee
> make: *** [_module_/usr/src/linux-4.14.y/net/ax25] Erreur 2
> make : on quitte le repertoire « /usr/src/linux-headers-4.14.79-v7+ »
> 
> 
> Bernard, f6bvp
> 

Hi Bernard

Are you sure you applied the patch correctly/completely ?

git cherry-pick worked without conflicts on top of 4.14.100


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox