[PATCH net-next v3] net: Add sysfs atttribute for max_mtu

[PATCH net-next v3] net: Add sysfs atttribute for max_mtu

[Shradha Gupta]

For drivers like MANA, max_mtu value is populated with the value of
maximum MTU that the underlying hardware can support.
Exposing this attribute as sysfs param, would be helpful in debugging
and customization of config issues with such drivers.

Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 Changes in v3:
 * Removed the min_mtu sysfs attribute as it was not needed
 * Improved the commit message to explain the need for the changes
 * Seperated this patch from other mana attributes requirements.
---
 Documentation/ABI/testing/sysfs-class-net | 8 ++++++++
 net/core/net-sysfs.c                      | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-net b/Documentation/ABI/testing/sysfs-class-net
index ebf21beba846..5dcab8648283 100644
--- a/Documentation/ABI/testing/sysfs-class-net
+++ b/Documentation/ABI/testing/sysfs-class-net
@@ -352,3 +352,11 @@ Description:
 		0  threaded mode disabled for this dev
 		1  threaded mode enabled for this dev
 		== ==================================
+
+What:           /sys/class/net/<iface>/max_mtu
+Date:           April 2024
+KernelVersion:  6.10
+Contact:        netdev@vger.kernel.org
+Description:
+                Indicates the interface's maximum supported MTU value, in
+                bytes, and in decimal format.
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index e3d7a8cfa20b..381becdd73a8 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -114,6 +114,7 @@ NETDEVICE_SHOW_RO(addr_len, fmt_dec);
 NETDEVICE_SHOW_RO(ifindex, fmt_dec);
 NETDEVICE_SHOW_RO(type, fmt_dec);
 NETDEVICE_SHOW_RO(link_mode, fmt_dec);
+NETDEVICE_SHOW_RO(max_mtu, fmt_dec);
 
 static ssize_t iflink_show(struct device *dev, struct device_attribute *attr,
 			   char *buf)
@@ -660,6 +661,7 @@ static struct attribute *net_class_attrs[] __ro_after_init = {
 	&dev_attr_ifalias.attr,
 	&dev_attr_carrier.attr,
 	&dev_attr_mtu.attr,
+	&dev_attr_max_mtu.attr,
 	&dev_attr_flags.attr,
 	&dev_attr_tx_queue_len.attr,
 	&dev_attr_gro_flush_timeout.attr,
-- 
2.34.1

Re: [PATCH net-next v3] net: Add sysfs atttribute for max_mtu

[Ratheesh Kannoth]

On 2024-05-09 at 14:41:23, Shradha Gupta (shradhagupta@linux.microsoft.com) wrote:
> For drivers like MANA, max_mtu value is populated with the value of
> maximum MTU that the underlying hardware can support.
IIUC, this reads dev->mtu. you can read the same using ifconfig, or any thing that uses
SIOCGIFMTU. why do you need to add a new sysfs ?

> Exposing this attribute as sysfs param, would be helpful in debugging
> and customization of config issues with such drivers.
>
> --- a/net/core/net-sysfs.c
> +++ b/net/core/net-sysfs.c
> @@ -114,6 +114,7 @@ NETDEVICE_SHOW_RO(addr_len, fmt_dec);
>  NETDEVICE_SHOW_RO(ifindex, fmt_dec);
>  NETDEVICE_SHOW_RO(type, fmt_dec);
>  NETDEVICE_SHOW_RO(link_mode, fmt_dec);
> +NETDEVICE_SHOW_RO(max_mtu, fmt_dec);
>
>  static ssize_t iflink_show(struct device *dev, struct device_attribute *attr,
>  			   char *buf)
> @@ -660,6 +661,7 @@ static struct attribute *net_class_attrs[] __ro_after_init = {
>  	&dev_attr_ifalias.attr,
>  	&dev_attr_carrier.attr,
>  	&dev_attr_mtu.attr,
> +	&dev_attr_max_mtu.attr,
>  	&dev_attr_flags.attr,
>  	&dev_attr_tx_queue_len.attr,
>  	&dev_attr_gro_flush_timeout.attr,
> --
> 2.34.1
>

Re: [PATCH net-next v3] net: Add sysfs atttribute for max_mtu

[Eric Dumazet]

On Thu, May 9, 2024 at 11:11 AM Shradha Gupta
<shradhagupta@linux.microsoft.com> wrote:
>
> For drivers like MANA, max_mtu value is populated with the value of
> maximum MTU that the underlying hardware can support.
> Exposing this attribute as sysfs param, would be helpful in debugging
> and customization of config issues with such drivers.
>
> Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
>  Changes in v3:
>  * Removed the min_mtu sysfs attribute as it was not needed
>  * Improved the commit message to explain the need for the changes
>  * Seperated this patch from other mana attributes requirements.
> ---
>  Documentation/ABI/testing/sysfs-class-net | 8 ++++++++
>  net/core/net-sysfs.c                      | 2 ++
>  2 files changed, 10 insertions(+)


Sorry, this is a NACK from my side.

Adding sysfs attributes is costly for setups adding/deleting many
netns/devices per second.

RTNL already provides this value.

net/core/rtnetlink.c:1850:          nla_put_u32(skb, IFLA_MAX_MTU,
READ_ONCE(dev->max_mtu)) ||

Re: [PATCH net-next v3] net: Add sysfs atttribute for max_mtu

[Andrew Lunn]

On Thu, May 09, 2024 at 03:12:25PM +0530, Ratheesh Kannoth wrote:
> On 2024-05-09 at 14:41:23, Shradha Gupta (shradhagupta@linux.microsoft.com) wrote:
> > For drivers like MANA, max_mtu value is populated with the value of
> > maximum MTU that the underlying hardware can support.
> IIUC, this reads dev->mtu.

I think you are misunderstanding the code.

> > +NETDEVICE_SHOW_RO(max_mtu, fmt_dec);

/* generate a show function for simple field */
#define NETDEVICE_SHOW(field, format_string)				\
static ssize_t format_##field(const struct net_device *dev, char *buf)	\
{									\
	return sysfs_emit(buf, format_string, dev->field);		\
}									\
static ssize_t field##_show(struct device *dev,				\
			    struct device_attribute *attr, char *buf)	\
{									\
	return netdev_show(dev, attr, buf, format_##field);		\
}									\

#define NETDEVICE_SHOW_RO(field, format_string)				\
NETDEVICE_SHOW(field, format_string);					\
static DEVICE_ATTR_RO(field)

So field is max_mtu, so that dev->field gets expanded to dev->max_mtu.

> you can read the same using ifconfig

We stopped using ifconfig years ago. You actually mean "ip link show"

> or any thing that uses SIOCGIFMTU. why do you need to add a new sysfs ?

SIOCGIFMTU is still implemented, but obsolete, replaced by netlink, as
Eric pointed out.

	Andrew