| Message ID | 20250204-netconsole-v2-1-5ef5eb5f6056@purestorage.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | netconsole: allow selection of egress interface via MAC address | expand |
On Tue, 2025-02-04 at 14:41 -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. True ... but is "MAC_ADDR_LEN" really an appropriate name for that? Just plain reading the subject, my first thought was why you weren't going to use ETH_ALEN. Would seem nicer to me, at least, to have something indicating a *string* in there, e.g. MAC_ADDR_STR_LEN or so. johannes
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 86ab4a42769a49eebe5dd6f01dafafc6c86ec54f..78c143e65a3f337929f91f70b77e5bc88365eea7 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -675,7 +675,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_LEN] && buf[MAC_ADDR_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..43608e45c58aa96a505d82733de1b24ef8d18a1b 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_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..4a6c0d0e6dc90a138bfbb402d401d41f59c021f8 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_LEN) return -EINVAL; if (!mac_pton(buf, mac)) diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h index 8a9792a6427ad9cf58b50c79cbfe185615800dcb..afb2fdc0af653626034ae5e19309c8427272a828 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_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..0aedd4854d85ba89dbf0e7451b3f6b197cfac1de 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_LEN) < MAC_ADDR_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..29f45382367e4f6cceb0e0b5f04db1c408da712e 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_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_LEN + 1); } LINK_STA_OPS(addr);
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> --- 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 | 5 +++-- 6 files changed, 10 insertions(+), 8 deletions(-)