[PATCH net-next v5 0/2] netconsole: allow selection of egress interface via MAC address

[PATCH net-next v5 0/2] netconsole: allow selection of egress interface via MAC address

[Uday Shankar]

This series adds support for selecting a netconsole egress interface by
specifying the MAC address (in place of the interface name) in the
boot/module parameter.

Signed-off-by: Uday Shankar <ushankar@purestorage.com>
---
Changes in v5:
- Drop Breno Leitao's patch to add (non-RCU) dev_getbyhwaddr from this
  set since it has landed on net-next (Jakub Kicinski)
- Link to v4: https://lore.kernel.org/r/20250217-netconsole-v4-0-0c681cef71f1@purestorage.com

Changes in v4:
- Incorporate Breno Leitao's patch to add (non-RCU) dev_getbyhwaddr and
  use it (Jakub Kicinski)
- Use MAC_ADDR_STR_LEN in ieee80211_sta_debugfs_add as well (Michal
  Swiatkowski)
- Link to v3: https://lore.kernel.org/r/20250205-netconsole-v3-0-132a31f17199@purestorage.com

Changes in v3:
- Rename MAC_ADDR_LEN to MAC_ADDR_STR_LEN (Johannes Berg)
- Link to v2: https://lore.kernel.org/r/20250204-netconsole-v2-0-5ef5eb5f6056@purestorage.com

---
Uday Shankar (2):
      net, treewide: define and use MAC_ADDR_STR_LEN
      netconsole: allow selection of egress interface via MAC address

 Documentation/networking/netconsole.rst |  6 +++-
 drivers/net/netconsole.c                |  2 +-
 drivers/nvmem/brcm_nvram.c              |  2 +-
 drivers/nvmem/layouts/u-boot-env.c      |  2 +-
 include/linux/if_ether.h                |  3 ++
 include/linux/netpoll.h                 |  6 ++++
 lib/net_utils.c                         |  4 +--
 net/core/netpoll.c                      | 51 +++++++++++++++++++++++++--------
 net/mac80211/debugfs_sta.c              |  7 +++--
 9 files changed, 61 insertions(+), 22 deletions(-)
---
base-commit: 1340461e5168f8a33b2b3e0ed04557742664bee1
change-id: 20250204-netconsole-4c610e2f871c

Best regards,
-- 
Uday Shankar <ushankar@purestorage.com>

[PATCH net-next v5 1/2] net, treewide: define and use MAC_ADDR_STR_LEN

[Uday Shankar]

There are a few places in the tree which compute the length of the
string representation of a MAC address as 3 * ETH_ALEN - 1. Define a
constant for this and use it where relevant. No functionality changes
are expected.

Signed-off-by: Uday Shankar <ushankar@purestorage.com>
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Acked-by: Johannes Berg <johannes@sipsolutions.net>
---
 drivers/net/netconsole.c           | 2 +-
 drivers/nvmem/brcm_nvram.c         | 2 +-
 drivers/nvmem/layouts/u-boot-env.c | 2 +-
 include/linux/if_ether.h           | 3 +++
 lib/net_utils.c                    | 4 +---
 net/mac80211/debugfs_sta.c         | 7 ++++---
 6 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index f77eddf221850fe2778cd479e49c91ad695aba3c..12699831e3c9fdbafd4862a339aea4ef04cf522b 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -721,7 +721,7 @@ static ssize_t remote_mac_store(struct config_item *item, const char *buf,
 
 	if (!mac_pton(buf, remote_mac))
 		goto out_unlock;
-	if (buf[3 * ETH_ALEN - 1] && buf[3 * ETH_ALEN - 1] != '\n')
+	if (buf[MAC_ADDR_STR_LEN] && buf[MAC_ADDR_STR_LEN] != '\n')
 		goto out_unlock;
 	memcpy(nt->np.remote_mac, remote_mac, ETH_ALEN);
 
diff --git a/drivers/nvmem/brcm_nvram.c b/drivers/nvmem/brcm_nvram.c
index b810df727b446b1762a1851750f743e0de6e8788..b4cf245fb2467d281111001bb7ed8db5993a09b2 100644
--- a/drivers/nvmem/brcm_nvram.c
+++ b/drivers/nvmem/brcm_nvram.c
@@ -100,7 +100,7 @@ static int brcm_nvram_read_post_process_macaddr(void *context, const char *id, i
 {
 	u8 mac[ETH_ALEN];
 
-	if (bytes != 3 * ETH_ALEN - 1)
+	if (bytes != MAC_ADDR_STR_LEN)
 		return -EINVAL;
 
 	if (!mac_pton(buf, mac))
diff --git a/drivers/nvmem/layouts/u-boot-env.c b/drivers/nvmem/layouts/u-boot-env.c
index 731e6f4f12b2bf28e4547d128954a095545ad461..436426d4e8f910b51b92f88acddfbb40d374587a 100644
--- a/drivers/nvmem/layouts/u-boot-env.c
+++ b/drivers/nvmem/layouts/u-boot-env.c
@@ -37,7 +37,7 @@ static int u_boot_env_read_post_process_ethaddr(void *context, const char *id, i
 {
 	u8 mac[ETH_ALEN];
 
-	if (bytes != 3 * ETH_ALEN - 1)
+	if (bytes != MAC_ADDR_STR_LEN)
 		return -EINVAL;
 
 	if (!mac_pton(buf, mac))
diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h
index 8a9792a6427ad9cf58b50c79cbfe185615800dcb..61b7335aa037c7232a0caa45572043057c02dde3 100644
--- a/include/linux/if_ether.h
+++ b/include/linux/if_ether.h
@@ -19,6 +19,9 @@
 #include <linux/skbuff.h>
 #include <uapi/linux/if_ether.h>
 
+/* XX:XX:XX:XX:XX:XX */
+#define MAC_ADDR_STR_LEN (3 * ETH_ALEN - 1)
+
 static inline struct ethhdr *eth_hdr(const struct sk_buff *skb)
 {
 	return (struct ethhdr *)skb_mac_header(skb);
diff --git a/lib/net_utils.c b/lib/net_utils.c
index 42bb0473fb22f977409f7a6792bb1340f4e911c3..215cda672fee1b5a029c2b61529c6813c0edab11 100644
--- a/lib/net_utils.c
+++ b/lib/net_utils.c
@@ -7,11 +7,9 @@
 
 bool mac_pton(const char *s, u8 *mac)
 {
-	size_t maxlen = 3 * ETH_ALEN - 1;
 	int i;
 
-	/* XX:XX:XX:XX:XX:XX */
-	if (strnlen(s, maxlen) < maxlen)
+	if (strnlen(s, MAC_ADDR_STR_LEN) < MAC_ADDR_STR_LEN)
 		return false;
 
 	/* Don't dirty result unless string is valid MAC. */
diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index a67a9d3160086ac492d77092a0c8a74d2384b28c..a8948f4d983e5edee45d90ad267582657ed38e38 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -457,11 +457,12 @@ static ssize_t link_sta_addr_read(struct file *file, char __user *userbuf,
 				  size_t count, loff_t *ppos)
 {
 	struct link_sta_info *link_sta = file->private_data;
-	u8 mac[3 * ETH_ALEN + 1];
+	u8 mac[MAC_ADDR_STR_LEN + 2];
 
 	snprintf(mac, sizeof(mac), "%pM\n", link_sta->pub->addr);
 
-	return simple_read_from_buffer(userbuf, count, ppos, mac, 3 * ETH_ALEN);
+	return simple_read_from_buffer(userbuf, count, ppos, mac,
+				       MAC_ADDR_STR_LEN + 1);
 }
 
 LINK_STA_OPS(addr);
@@ -1240,7 +1241,7 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
 	struct ieee80211_local *local = sta->local;
 	struct ieee80211_sub_if_data *sdata = sta->sdata;
 	struct dentry *stations_dir = sta->sdata->debugfs.subdir_stations;
-	u8 mac[3*ETH_ALEN];
+	u8 mac[MAC_ADDR_STR_LEN + 1];
 
 	if (!stations_dir)
 		return;

-- 
2.34.1

[PATCH net-next v5 2/2] netconsole: allow selection of egress interface via MAC address

[Uday Shankar]

Currently, netconsole has two methods of configuration - module
parameter and configfs. The former interface allows for netconsole
activation earlier during boot (by specifying the module parameter on
the kernel command line), so it is preferred for debugging issues which
arise before userspace is up/the configfs interface can be used. The
module 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 module 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 when configuring netconsole using the module parameter. Update
the netconsole documentation with an example of the new syntax.
Selection of egress interface by MAC address via configfs is far less
interesting (since when this interface can be used, one should be able
to easily convert between MAC address and interface name), so it is left
unimplemented.

Signed-off-by: Uday Shankar <ushankar@purestorage.com>
Reviewed-by: Breno Leitao <leitao@debian.org>
Tested-by: Breno Leitao <leitao@debian.org>
---
 Documentation/networking/netconsole.rst |  6 +++-
 include/linux/netpoll.h                 |  6 ++++
 net/core/netpoll.c                      | 51 +++++++++++++++++++++++++--------
 3 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/Documentation/networking/netconsole.rst b/Documentation/networking/netconsole.rst
index 84803c59968a3237012fab821f432eb531aba45c..4b0f32ed5c635dbce594bc09e5d25c7654350779 100644
--- a/Documentation/networking/netconsole.rst
+++ b/Documentation/networking/netconsole.rst
@@ -47,7 +47,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)
@@ -64,6 +64,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@10.0.0.1/22:33:44:55:66:77,9353@10.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::
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index f91e50a76efd4b016381c632456397eea1ea877f..1ade65b59be49cfdcf86ed6e938287b949aa9f58 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -25,7 +25,13 @@ union inet_addr {
 struct netpoll {
 	struct net_device *dev;
 	netdevice_tracker dev_tracker;
+	/*
+	 * Either dev_name or dev_mac can be used to specify the local
+	 * interface - dev_name is used if it is a nonempty string, else
+	 * dev_mac is used.
+	 */
 	char dev_name[IFNAMSIZ];
+	u8 dev_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 62b4041aae1ae8c7dc47c89fb40b14bbd4ad0e0e..64c08b845c92bb2a2165de6dfba95dede2b581db 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->dev_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,18 @@ int netpoll_parse_options(struct netpoll *np, char *opt)
 	cur++;
 
 	if (*cur != ',') {
-		/* parse out dev name */
+		/* parse out dev_name or dev_mac */
 		if ((delim = strchr(cur, ',')) == NULL)
 			goto parse_failed;
 		*delim = 0;
-		strscpy(np->dev_name, cur, sizeof(np->dev_name));
+
+		np->dev_name[0] = '\0';
+		eth_broadcast_addr(np->dev_mac);
+		if (!strchr(cur, ':'))
+			strscpy(np->dev_name, cur, sizeof(np->dev_name));
+		else if (!mac_pton(cur, np->dev_mac))
+			goto parse_failed;
+
 		cur = delim;
 	}
 	cur++;
@@ -679,27 +687,45 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
 }
 EXPORT_SYMBOL_GPL(__netpoll_setup);
 
+/*
+ * Returns a pointer to a string representation of the identifier used
+ * to select the egress interface for the given netpoll instance. buf
+ * must be a buffer of length at least MAC_ADDR_STR_LEN + 1.
+ */
+static char *egress_dev(struct netpoll *np, char *buf)
+{
+	if (np->dev_name[0])
+		return np->dev_name;
+
+	snprintf(buf, MAC_ADDR_STR_LEN, "%pM", np->dev_mac);
+	return buf;
+}
+
 int netpoll_setup(struct netpoll *np)
 {
+	struct net *net = current->nsproxy->net_ns;
+	char buf[MAC_ADDR_STR_LEN + 1];
 	struct net_device *ndev = NULL;
 	bool ip_overwritten = false;
 	struct in_device *in_dev;
 	int err;
 
 	rtnl_lock();
-	if (np->dev_name[0]) {
-		struct net *net = current->nsproxy->net_ns;
+	if (np->dev_name[0])
 		ndev = __dev_get_by_name(net, np->dev_name);
-	}
+	else if (is_valid_ether_addr(np->dev_mac))
+		ndev = dev_getbyhwaddr(net, ARPHRD_ETHER, np->dev_mac);
+
 	if (!ndev) {
-		np_err(np, "%s doesn't exist, aborting\n", np->dev_name);
+		np_err(np, "%s doesn't exist, aborting\n", egress_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",
+		       egress_dev(np, buf));
 		err = -EBUSY;
 		goto put;
 	}
@@ -707,7 +733,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",
+			egress_dev(np, buf));
 
 		err = dev_open(ndev, NULL);
 
@@ -741,7 +768,7 @@ int netpoll_setup(struct netpoll *np)
 			if (!ifa) {
 put_noaddr:
 				np_err(np, "no IP address for %s, aborting\n",
-				       np->dev_name);
+				       egress_dev(np, buf));
 				err = -EDESTADDRREQ;
 				goto put;
 			}
@@ -772,13 +799,13 @@ int netpoll_setup(struct netpoll *np)
 			}
 			if (err) {
 				np_err(np, "no IPv6 address for %s, aborting\n",
-				       np->dev_name);
+				       egress_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);
+			       egress_dev(np, buf));
 			err = -EINVAL;
 			goto put;
 #endif

-- 
2.34.1

Re: [PATCH net-next v5 1/2] net, treewide: define and use MAC_ADDR_STR_LEN

[Breno Leitao]

On Thu, Feb 20, 2025 at 06:29:20PM -0700, Uday Shankar wrote:
> There are a few places in the tree which compute the length of the
> string representation of a MAC address as 3 * ETH_ALEN - 1. Define a
> constant for this and use it where relevant. No functionality changes
> are expected.
> 
> Signed-off-by: Uday Shankar <ushankar@purestorage.com>
> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Acked-by: Johannes Berg <johannes@sipsolutions.net>

Reviwed-by: Breno Leitao <leitao@debian.org>

Re: [PATCH net-next v5 1/2] net, treewide: define and use MAC_ADDR_STR_LEN

[David Laight]

On Thu, 20 Feb 2025 18:29:20 -0700
Uday Shankar <ushankar@purestorage.com> wrote:

> There are a few places in the tree which compute the length of the
> string representation of a MAC address as 3 * ETH_ALEN - 1. Define a
> constant for this and use it where relevant. No functionality changes
> are expected.

The fact that you have to keep adding 1 or 2 is a good indication that
it really isn't a good idea.

	David

> 
> Signed-off-by: Uday Shankar <ushankar@purestorage.com>
> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Acked-by: Johannes Berg <johannes@sipsolutions.net>
> ---
>  drivers/net/netconsole.c           | 2 +-
>  drivers/nvmem/brcm_nvram.c         | 2 +-
>  drivers/nvmem/layouts/u-boot-env.c | 2 +-
>  include/linux/if_ether.h           | 3 +++
>  lib/net_utils.c                    | 4 +---
>  net/mac80211/debugfs_sta.c         | 7 ++++---
>  6 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index f77eddf221850fe2778cd479e49c91ad695aba3c..12699831e3c9fdbafd4862a339aea4ef04cf522b 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -721,7 +721,7 @@ static ssize_t remote_mac_store(struct config_item *item, const char *buf,
>  
>  	if (!mac_pton(buf, remote_mac))
>  		goto out_unlock;
> -	if (buf[3 * ETH_ALEN - 1] && buf[3 * ETH_ALEN - 1] != '\n')
> +	if (buf[MAC_ADDR_STR_LEN] && buf[MAC_ADDR_STR_LEN] != '\n')
>  		goto out_unlock;
>  	memcpy(nt->np.remote_mac, remote_mac, ETH_ALEN);
>  
> diff --git a/drivers/nvmem/brcm_nvram.c b/drivers/nvmem/brcm_nvram.c
> index b810df727b446b1762a1851750f743e0de6e8788..b4cf245fb2467d281111001bb7ed8db5993a09b2 100644
> --- a/drivers/nvmem/brcm_nvram.c
> +++ b/drivers/nvmem/brcm_nvram.c
> @@ -100,7 +100,7 @@ static int brcm_nvram_read_post_process_macaddr(void *context, const char *id, i
>  {
>  	u8 mac[ETH_ALEN];
>  
> -	if (bytes != 3 * ETH_ALEN - 1)
> +	if (bytes != MAC_ADDR_STR_LEN)
>  		return -EINVAL;
>  
>  	if (!mac_pton(buf, mac))
> diff --git a/drivers/nvmem/layouts/u-boot-env.c b/drivers/nvmem/layouts/u-boot-env.c
> index 731e6f4f12b2bf28e4547d128954a095545ad461..436426d4e8f910b51b92f88acddfbb40d374587a 100644
> --- a/drivers/nvmem/layouts/u-boot-env.c
> +++ b/drivers/nvmem/layouts/u-boot-env.c
> @@ -37,7 +37,7 @@ static int u_boot_env_read_post_process_ethaddr(void *context, const char *id, i
>  {
>  	u8 mac[ETH_ALEN];
>  
> -	if (bytes != 3 * ETH_ALEN - 1)
> +	if (bytes != MAC_ADDR_STR_LEN)
>  		return -EINVAL;
>  
>  	if (!mac_pton(buf, mac))
> diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h
> index 8a9792a6427ad9cf58b50c79cbfe185615800dcb..61b7335aa037c7232a0caa45572043057c02dde3 100644
> --- a/include/linux/if_ether.h
> +++ b/include/linux/if_ether.h
> @@ -19,6 +19,9 @@
>  #include <linux/skbuff.h>
>  #include <uapi/linux/if_ether.h>
>  
> +/* XX:XX:XX:XX:XX:XX */
> +#define MAC_ADDR_STR_LEN (3 * ETH_ALEN - 1)
> +
>  static inline struct ethhdr *eth_hdr(const struct sk_buff *skb)
>  {
>  	return (struct ethhdr *)skb_mac_header(skb);
> diff --git a/lib/net_utils.c b/lib/net_utils.c
> index 42bb0473fb22f977409f7a6792bb1340f4e911c3..215cda672fee1b5a029c2b61529c6813c0edab11 100644
> --- a/lib/net_utils.c
> +++ b/lib/net_utils.c
> @@ -7,11 +7,9 @@
>  
>  bool mac_pton(const char *s, u8 *mac)
>  {
> -	size_t maxlen = 3 * ETH_ALEN - 1;
>  	int i;
>  
> -	/* XX:XX:XX:XX:XX:XX */
> -	if (strnlen(s, maxlen) < maxlen)
> +	if (strnlen(s, MAC_ADDR_STR_LEN) < MAC_ADDR_STR_LEN)
>  		return false;
>  
>  	/* Don't dirty result unless string is valid MAC. */
> diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
> index a67a9d3160086ac492d77092a0c8a74d2384b28c..a8948f4d983e5edee45d90ad267582657ed38e38 100644
> --- a/net/mac80211/debugfs_sta.c
> +++ b/net/mac80211/debugfs_sta.c
> @@ -457,11 +457,12 @@ static ssize_t link_sta_addr_read(struct file *file, char __user *userbuf,
>  				  size_t count, loff_t *ppos)
>  {
>  	struct link_sta_info *link_sta = file->private_data;
> -	u8 mac[3 * ETH_ALEN + 1];
> +	u8 mac[MAC_ADDR_STR_LEN + 2];
>  
>  	snprintf(mac, sizeof(mac), "%pM\n", link_sta->pub->addr);
>  
> -	return simple_read_from_buffer(userbuf, count, ppos, mac, 3 * ETH_ALEN);
> +	return simple_read_from_buffer(userbuf, count, ppos, mac,
> +				       MAC_ADDR_STR_LEN + 1);
>  }
>  
>  LINK_STA_OPS(addr);
> @@ -1240,7 +1241,7 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
>  	struct ieee80211_local *local = sta->local;
>  	struct ieee80211_sub_if_data *sdata = sta->sdata;
>  	struct dentry *stations_dir = sta->sdata->debugfs.subdir_stations;
> -	u8 mac[3*ETH_ALEN];
> +	u8 mac[MAC_ADDR_STR_LEN + 1];
>  
>  	if (!stations_dir)
>  		return;
> 

Re: [PATCH net-next v5 1/2] net, treewide: define and use MAC_ADDR_STR_LEN

[Uday Shankar]

On Fri, Feb 21, 2025 at 01:17:50PM +0000, David Laight wrote:
> The fact that you have to keep adding 1 or 2 is a good indication that
> it really isn't a good idea.

Having to adjust by 1 when converting between string lengths and buffer
sizes is super standard in C since strings are NUL-terminated. There are
tons of preexisting examples in the tree.

I agree that the + 2 is a bit of an eyesore but its needed in that case
because that code wants to tack on a newline in addition to a
NUL-terminator. Maybe adding a comment there would help?

In any case, MAC_ADDR_STR_LEN is a much more descriptive name for what
all the changed code is actually doing compared against 3 * ETH_ALEN -
1.

Re: [PATCH net-next v5 1/2] net, treewide: define and use MAC_ADDR_STR_LEN

[Simon Horman]

On Thu, Feb 20, 2025 at 06:29:20PM -0700, Uday Shankar wrote:
> There are a few places in the tree which compute the length of the
> string representation of a MAC address as 3 * ETH_ALEN - 1. Define a
> constant for this and use it where relevant. No functionality changes
> are expected.
> 
> Signed-off-by: Uday Shankar <ushankar@purestorage.com>
> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Acked-by: Johannes Berg <johannes@sipsolutions.net>

Reviewed-by: Simon Horman <horms@verge.net.au>

Re: [PATCH net-next v5 2/2] netconsole: allow selection of egress interface via MAC address

[Simon Horman]

On Thu, Feb 20, 2025 at 06:29:21PM -0700, Uday Shankar wrote:
> Currently, netconsole has two methods of configuration - module
> parameter and configfs. The former interface allows for netconsole
> activation earlier during boot (by specifying the module parameter on
> the kernel command line), so it is preferred for debugging issues which
> arise before userspace is up/the configfs interface can be used. The
> module 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 module 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 when configuring netconsole using the module parameter. Update
> the netconsole documentation with an example of the new syntax.
> Selection of egress interface by MAC address via configfs is far less
> interesting (since when this interface can be used, one should be able
> to easily convert between MAC address and interface name), so it is left
> unimplemented.
> 
> Signed-off-by: Uday Shankar <ushankar@purestorage.com>
> Reviewed-by: Breno Leitao <leitao@debian.org>
> Tested-by: Breno Leitao <leitao@debian.org>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next v5 2/2] netconsole: allow selection of egress interface via MAC address

[Uday Shankar]

On Tue, Feb 25, 2025 at 02:40:35PM +0000, Simon Horman wrote:
> Reviewed-by: Simon Horman <horms@kernel.org>

Hey, since this has gotten quiet for a while, just wanted to confirm
that there's no action needed from my end? Is this in the queue for
net-next?

Re: [PATCH net-next v5 2/2] netconsole: allow selection of egress interface via MAC address

[Simon Horman]

On Fri, Mar 07, 2025 at 01:11:18PM -0700, Uday Shankar wrote:
> On Tue, Feb 25, 2025 at 02:40:35PM +0000, Simon Horman wrote:
> > Reviewed-by: Simon Horman <horms@kernel.org>
> 
> Hey, since this has gotten quiet for a while, just wanted to confirm
> that there's no action needed from my end? Is this in the queue for
> net-next?

Hi Uday,

It seems that this series has been marked as Changes Requested in
patchwork, which may explain the lack of progress. But that designation
doesn't seem correct to me. So let's see if this can move this series
back into the queue for the maintainers.

-- 
pw-bot: under-review

Re: [PATCH net-next v5 2/2] netconsole: allow selection of egress interface via MAC address

[Jakub Kicinski]

On Tue, 11 Mar 2025 12:13:01 +0100 Simon Horman wrote:
> On Fri, Mar 07, 2025 at 01:11:18PM -0700, Uday Shankar wrote:
> > On Tue, Feb 25, 2025 at 02:40:35PM +0000, Simon Horman wrote:  
> > > Reviewed-by: Simon Horman <horms@kernel.org>  
> > 
> > Hey, since this has gotten quiet for a while, just wanted to confirm
> > that there's no action needed from my end? Is this in the queue for
> > net-next?  
> 
> It seems that this series has been marked as Changes Requested in
> patchwork, which may explain the lack of progress. But that designation
> doesn't seem correct to me. So let's see if this can move this series
> back into the queue for the maintainers.

Unclear why the designation was chosen, indeed, but let's get this
reposted per normal process. The posting is a month old.

Re: [PATCH net-next v5 2/2] netconsole: allow selection of egress interface via MAC address

[Uday Shankar]

On Wed, Mar 12, 2025 at 08:37:16PM +0100, Jakub Kicinski wrote:
> On Tue, 11 Mar 2025 12:13:01 +0100 Simon Horman wrote:
> > On Fri, Mar 07, 2025 at 01:11:18PM -0700, Uday Shankar wrote:
> > > On Tue, Feb 25, 2025 at 02:40:35PM +0000, Simon Horman wrote:  
> > > > Reviewed-by: Simon Horman <horms@kernel.org>  
> > > 
> > > Hey, since this has gotten quiet for a while, just wanted to confirm
> > > that there's no action needed from my end? Is this in the queue for
> > > net-next?  
> > 
> > It seems that this series has been marked as Changes Requested in
> > patchwork, which may explain the lack of progress. But that designation
> > doesn't seem correct to me. So let's see if this can move this series
> > back into the queue for the maintainers.
> 
> Unclear why the designation was chosen, indeed, but let's get this
> reposted per normal process. The posting is a month old.

Sounds good, I've posted a v6 which is listed at
https://patchwork.kernel.org/project/netdevbpf/list/?series=943246 as
"new" even though it has several reviews from maintainers. Anything else
for me to do?