netdev - [PATCH v2 net-next 2/4] net: dsa: automatically bring user ports down when master goes down

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]

Message-Id: <20210203160823.2163194-3-olteanv@gmail.com>
Date:   Wed,  3 Feb 2021 18:08:21 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org
Cc:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>
Subject: [PATCH v2 net-next 2/4] net: dsa: automatically bring user ports down when master goes down

From: Vladimir Oltean <vladimir.oltean@....com>

This is not fixing any actual bug that I know of, but having a DSA
interface that is up even when its lower (master) interface is down is
one of those things that just do not sound right.

Yes, DSA checks if the master is up before actually bringing the
user interface up, but nobody prevents bringing the master interface
down immediately afterwards... Then the user ports would attempt
dev_queue_xmit on an interface that is down, and wonder what's wrong.

This patch prevents that from happening. NETDEV_GOING_DOWN is the
notification emitted _before_ the master actually goes down, and we are
protected by the rtnl_mutex, so all is well.

$ ip link set eno2 down
[  763.672211] mscc_felix 0000:00:00.5 swp0: Link is Down
[  763.880137] mscc_felix 0000:00:00.5 swp1: Link is Down
[  764.078773] mscc_felix 0000:00:00.5 swp2: Link is Down
[  764.197106] mscc_felix 0000:00:00.5 swp3: Link is Down
[  764.299384] fsl_enetc 0000:00:00.2 eno2: Link is Down

For those of you reading this because you were doing switch testing
such as latency measurements for autonomously forwarded traffic, and you
needed a controlled environment with no extra packets sent by the
network stack, this patch breaks that, because now the user ports go
down too, which may shut down the PHY etc. But please don't do it like
that, just do instead:

tc qdisc add dev eno2 clsact
tc filter add dev eno2 egress flower action drop

Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
Reviewed-by: Andrew Lunn <andrew@...n.ch>
Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
---
Changes in v2:
Fix typo: !dsa_is_user_port -> dsa_is_user_port.

 net/dsa/slave.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 4616bd7c8684..aa7bd223073c 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -2084,6 +2084,36 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb,
 		err = dsa_port_lag_change(dp, info->lower_state_info);
 		return notifier_from_errno(err);
 	}
+	case NETDEV_GOING_DOWN: {
+		struct dsa_port *dp, *cpu_dp;
+		struct dsa_switch_tree *dst;
+		int err = 0;
+
+		if (!netdev_uses_dsa(dev))
+			return NOTIFY_DONE;
+
+		cpu_dp = dev->dsa_ptr;
+		dst = cpu_dp->ds->dst;
+
+		list_for_each_entry(dp, &dst->ports, list) {
+			if (dsa_is_user_port(dp->ds, dp->index)) {
+				struct net_device *slave = dp->slave;
+
+				if (!(slave->flags & IFF_UP))
+					continue;
+
+				err = dev_change_flags(slave,
+						       slave->flags & ~IFF_UP,
+						       NULL);
+				if (err)
+					break;
+			}
+		}
+
+		return notifier_from_errno(err);
+	}
+	default:
+		break;
 	}

 	return NOTIFY_DONE;
-- 
2.25.1