Message-Id: <20241211021851.1442842-1-ushankar@purestorage.com>
Date: Tue, 10 Dec 2024 19:18:52 -0700
From: Uday Shankar <ushankar@...estorage.com>
To: Breno Leitao <leitao@...ian.org>,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>
Cc: netdev@...r.kernel.org,
	Uday Shankar <ushankar@...estorage.com>
Subject: [PATCH] netconsole: allow selection of egress interface via MAC address

Currently, netconsole has two methods of configuration - kernel command
line parameter and configfs. The former interface allows for netconsole
activation earlier during boot, so it is preferred for debugging issues
which arise before userspace is up/the configfs interface can be used.
The kernel command line parameter syntax requires specifying the egress
interface name. This requirement makes it hard to use for a couple
reasons:
- The egress interface name can be hard or impossible to predict. For
  example, installing a new network card in a system can change the
  interface names assigned by the kernel.
- When constructing the kernel parameter, one may have trouble
  determining the original (kernel-assigned) name of the interface
  (which is the name that should be given to netconsole) if some stable
  interface naming scheme is in effect. A human can usually look at
  kernel logs to determine the original name, but this is very painful
  if automation is constructing the parameter.

For these reasons, allow selection of the egress interface via MAC
address. To maintain parity between interfaces, the local_mac entry in
configfs is also made read-write and can be used to select the local
interface, though this use case is less interesting than the one
highlighted above.

Signed-off-by: Uday Shankar <ushankar@...estorage.com>
---
 Documentation/networking/netconsole.rst |  8 +++-
 drivers/net/netconsole.c                | 50 +++++++++++++++++++++----
 include/linux/netpoll.h                 |  7 ++++
 net/core/netpoll.c                      | 49 +++++++++++++++++++-----
 4 files changed, 94 insertions(+), 20 deletions(-)

diff --git a/Documentation/networking/netconsole.rst b/Documentation/networking/netconsole.rst
index d55c2a22ec7a..c2e269dc8d75 100644
--- a/Documentation/networking/netconsole.rst
+++ b/Documentation/networking/netconsole.rst
@@ -45,7 +45,7 @@ following format::
 	r             if present, prepend kernel version (release) to the message
 	src-port      source for UDP packets (defaults to 6665)
 	src-ip        source IP to use (interface address)
-	dev           network interface (eth0)
+	dev           network interface name (eth0) or MAC address
 	tgt-port      port for logging agent (6666)
 	tgt-ip        IP address for logging agent
 	tgt-macaddr   ethernet MAC address for logging agent (broadcast)
@@ -62,6 +62,10 @@ or using IPv6::
 
  insmod netconsole netconsole=@/,@fd00:1:2:3::1/
 
+or using a MAC address to select the egress interface::
+
+   linux netconsole=4444@...0.0.1/ab:cd:ef:12:34:56,9353@...0.0.2/12:34:56:78:9a:bc
+
 It also supports logging to multiple remote agents by specifying
 parameters for the multiple agents separated by semicolons and the
 complete string enclosed in "quotes", thusly::
@@ -133,7 +137,7 @@ The interface exposes these parameters of a netconsole target to userspace:
 	remote_port	Remote agent's UDP port			(read-write)
 	local_ip	Source IP address to use		(read-write)
 	remote_ip	Remote agent's IP address		(read-write)
-	local_mac	Local interface's MAC address		(read-only)
+	local_mac	Local interface's MAC address		(read-write)
 	remote_mac	Remote agent's MAC address		(read-write)
 	==============  =================================       ============
 
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 4ea44a2f48f7..865c43a97f70 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -113,7 +113,7 @@ static struct console netconsole_ext;
  *		remote_port	(read-write)
  *		local_ip	(read-write)
  *		remote_ip	(read-write)
- *		local_mac	(read-only)
+ *		local_mac	(read-write)
  *		remote_mac	(read-write)
  */
 struct netconsole_target {
@@ -211,6 +211,8 @@ static struct netconsole_target *alloc_and_init(void)
 
 	nt->np.name = "netconsole";
 	strscpy(nt->np.dev_name, "eth0", IFNAMSIZ);
+	/* the "don't use" or N/A value for this field */
+	eth_broadcast_addr(nt->np.local_mac);
 	nt->np.local_port = 6665;
 	nt->np.remote_port = 6666;
 	eth_broadcast_addr(nt->np.remote_mac);
@@ -360,10 +362,7 @@ static ssize_t remote_ip_show(struct config_item *item, char *buf)
 
 static ssize_t local_mac_show(struct config_item *item, char *buf)
 {
-	struct net_device *dev = to_target(item)->np.dev;
-	static const u8 bcast[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
-
-	return sysfs_emit(buf, "%pM\n", dev ? dev->dev_addr : bcast);
+	return sysfs_emit(buf, "%pM\n", to_target(item)->np.local_mac);
 }
 
 static ssize_t remote_mac_show(struct config_item *item, char *buf)
@@ -511,11 +510,41 @@ static ssize_t dev_name_store(struct config_item *item, const char *buf,
 
 	strscpy(nt->np.dev_name, buf, IFNAMSIZ);
 	trim_newline(nt->np.dev_name, IFNAMSIZ);
+	/* the "don't use" or N/A value for this field */
+	eth_broadcast_addr(nt->np.local_mac);
 
 	mutex_unlock(&dynamic_netconsole_mutex);
 	return strnlen(buf, count);
 }
 
+static ssize_t local_mac_store(struct config_item *item, const char *buf,
+			       size_t count)
+{
+	struct netconsole_target *nt = to_target(item);
+	u8 local_mac[ETH_ALEN];
+	ssize_t ret = -EINVAL;
+
+	mutex_lock(&dynamic_netconsole_mutex);
+	if (nt->enabled) {
+		pr_err("target (%s) is enabled, disable to update parameters\n",
+		       config_item_name(&nt->group.cg_item));
+		goto out_unlock;
+	}
+
+	if (!mac_pton(buf, local_mac))
+		goto out_unlock;
+	if (buf[3 * ETH_ALEN - 1] && buf[3 * ETH_ALEN - 1] != '\n')
+		goto out_unlock;
+	memcpy(nt->np.local_mac, local_mac, ETH_ALEN);
+	/* force use of local_mac for device lookup */
+	nt->np.dev_name[0] = '\0';
+
+	ret = strnlen(buf, count);
+out_unlock:
+	mutex_unlock(&dynamic_netconsole_mutex);
+	return ret;
+}
+
 static ssize_t local_port_store(struct config_item *item, const char *buf,
 		size_t count)
 {
@@ -839,7 +868,7 @@ CONFIGFS_ATTR(, local_port);
 CONFIGFS_ATTR(, remote_port);
 CONFIGFS_ATTR(, local_ip);
 CONFIGFS_ATTR(, remote_ip);
-CONFIGFS_ATTR_RO(, local_mac);
+CONFIGFS_ATTR(, local_mac);
 CONFIGFS_ATTR(, remote_mac);
 CONFIGFS_ATTR(, release);
 
@@ -1001,8 +1030,9 @@ static int netconsole_netdev_event(struct notifier_block *this,
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
 	bool stopped = false;
 
-	if (!(event == NETDEV_CHANGENAME || event == NETDEV_UNREGISTER ||
-	      event == NETDEV_RELEASE || event == NETDEV_JOIN))
+	if (!(event == NETDEV_CHANGENAME || event == NETDEV_CHANGEADDR ||
+	      event == NETDEV_UNREGISTER || event == NETDEV_RELEASE ||
+	      event == NETDEV_JOIN))
 		goto done;
 
 	mutex_lock(&target_cleanup_list_lock);
@@ -1014,6 +1044,10 @@ static int netconsole_netdev_event(struct notifier_block *this,
 			case NETDEV_CHANGENAME:
 				strscpy(nt->np.dev_name, dev->name, IFNAMSIZ);
 				break;
+			case NETDEV_CHANGEADDR:
+				memcpy(nt->np.local_mac, dev->dev_addr,
+				       ETH_ALEN);
+				break;
 			case NETDEV_RELEASE:
 			case NETDEV_JOIN:
 			case NETDEV_UNREGISTER:
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index b34301650c47..e5cdb4d40f13 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -25,7 +25,14 @@ union inet_addr {
 struct netpoll {
 	struct net_device *dev;
 	netdevice_tracker dev_tracker;
+	/*
+	 * Either dev_name or local_mac can be used to specify the local
+	 * interface - dev_name will be used if it is nonempty, else
+	 * local_mac is used. Once netpoll_setup returns successfully,
+	 * both fields are populated.
+	 */
 	char dev_name[IFNAMSIZ];
+	u8 local_mac[ETH_ALEN];
 	const char *name;
 
 	union inet_addr local_ip, remote_ip;
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 2e459b9d88eb..485093387b9f 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -501,7 +501,8 @@ void netpoll_print_options(struct netpoll *np)
 		np_info(np, "local IPv6 address %pI6c\n", &np->local_ip.in6);
 	else
 		np_info(np, "local IPv4 address %pI4\n", &np->local_ip.ip);
-	np_info(np, "interface '%s'\n", np->dev_name);
+	np_info(np, "interface name '%s'\n", np->dev_name);
+	np_info(np, "local ethernet address '%pM'\n", np->local_mac);
 	np_info(np, "remote port %d\n", np->remote_port);
 	if (np->ipv6)
 		np_info(np, "remote IPv6 address %pI6c\n", &np->remote_ip.in6);
@@ -570,11 +571,20 @@ int netpoll_parse_options(struct netpoll *np, char *opt)
 	cur++;
 
 	if (*cur != ',') {
-		/* parse out dev name */
+		/* parse out dev_name or local_mac */
 		if ((delim = strchr(cur, ',')) == NULL)
 			goto parse_failed;
 		*delim = 0;
-		strscpy(np->dev_name, cur, sizeof(np->dev_name));
+		if (!strchr(cur, ':')) {
+			strscpy(np->dev_name, cur, sizeof(np->dev_name));
+			eth_broadcast_addr(np->local_mac);
+		} else {
+			if (!mac_pton(cur, np->local_mac)) {
+				goto parse_failed;
+			}
+			/* force use of local_mac for device lookup */
+			np->dev_name[0] = '\0';
+		}
 		cur = delim;
 	}
 	cur++;
@@ -660,6 +670,7 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
 
 	np->dev = ndev;
 	strscpy(np->dev_name, ndev->name, IFNAMSIZ);
+	memcpy(np->local_mac, ndev->dev_addr, ETH_ALEN);
 	npinfo->netpoll = np;
 
 	/* last thing to do is link it to the net device structure */
@@ -674,29 +685,46 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
 }
 EXPORT_SYMBOL_GPL(__netpoll_setup);
 
+/* upper bound on length of %pM output */
+#define MAX_MAC_ADDR_LEN (4 * ETH_ALEN)
+
+static char *local_dev(struct netpoll *np, char *buf)
+{
+	if (np->dev_name[0]) {
+		return np->dev_name;
+	}
+
+	snprintf(buf, MAX_MAC_ADDR_LEN, "%pM", np->local_mac);
+	return buf;
+}
+
 int netpoll_setup(struct netpoll *np)
 {
 	struct net_device *ndev = NULL;
 	bool ip_overwritten = false;
 	struct in_device *in_dev;
 	int err;
+	char buf[MAX_MAC_ADDR_LEN];
 
 	skb_queue_head_init(&np->skb_pool);
 
 	rtnl_lock();
+	struct net *net = current->nsproxy->net_ns;
 	if (np->dev_name[0]) {
-		struct net *net = current->nsproxy->net_ns;
 		ndev = __dev_get_by_name(net, np->dev_name);
+	} else if (is_valid_ether_addr(np->local_mac)) {
+		ndev = dev_getbyhwaddr_rcu(net, ARPHRD_ETHER, np->local_mac);
 	}
 	if (!ndev) {
-		np_err(np, "%s doesn't exist, aborting\n", np->dev_name);
+		np_err(np, "%s doesn't exist, aborting\n", local_dev(np, buf));
 		err = -ENODEV;
 		goto unlock;
 	}
 	netdev_hold(ndev, &np->dev_tracker, GFP_KERNEL);
 
 	if (netdev_master_upper_dev_get(ndev)) {
-		np_err(np, "%s is a slave device, aborting\n", np->dev_name);
+		np_err(np, "%s is a slave device, aborting\n",
+		       local_dev(np, buf));
 		err = -EBUSY;
 		goto put;
 	}
@@ -704,7 +732,8 @@ int netpoll_setup(struct netpoll *np)
 	if (!netif_running(ndev)) {
 		unsigned long atmost;
 
-		np_info(np, "device %s not up yet, forcing it\n", np->dev_name);
+		np_info(np, "device %s not up yet, forcing it\n",
+			local_dev(np, buf));
 
 		err = dev_open(ndev, NULL);
 
@@ -738,7 +767,7 @@ int netpoll_setup(struct netpoll *np)
 			if (!ifa) {
 put_noaddr:
 				np_err(np, "no IP address for %s, aborting\n",
-				       np->dev_name);
+				       local_dev(np, buf));
 				err = -EDESTADDRREQ;
 				goto put;
 			}
@@ -769,13 +798,13 @@ int netpoll_setup(struct netpoll *np)
 			}
 			if (err) {
 				np_err(np, "no IPv6 address for %s, aborting\n",
-				       np->dev_name);
+				       local_dev(np, buf));
 				goto put;
 			} else
 				np_info(np, "local IPv6 %pI6c\n", &np->local_ip.in6);
 #else
 			np_err(np, "IPv6 is not supported %s, aborting\n",
-			       np->dev_name);
+			       local_dev(np, buf));
 			err = -EINVAL;
 			goto put;
 #endif
-- 
2.34.1

Powered by blists - more mailing lists