[PATCH net-next v2 0/2] Add sysfs attributes for MANA

[PATCH net-next v2 0/2] Add sysfs attributes for MANA

[Shradha Gupta]

These patches include adding sysfs attributes for improving
debuggability on MANA devices.

The first patch consists on max_mtu, min_mtu attributes that are
implemented generically for all devices

The second patch has mana specific attributes max_num_msix and num_ports

Shradha Gupta (2):
  net: Add sysfs atttributes for max_mtu min_mtu
  net: mana: Add new device attributes for mana

 Documentation/ABI/testing/sysfs-class-net     | 16 ++++++++++
 .../net/ethernet/microsoft/mana/gdma_main.c   | 32 +++++++++++++++++++
 net/core/net-sysfs.c                          |  4 +++
 3 files changed, 52 insertions(+)

-- 
2.34.1

[PATCH net-next v2 1/2] net: Add sysfs atttributes for max_mtu min_mtu

[Shradha Gupta]

Add sysfs attributes to read max_mtu and min_mtu value for
network devices

Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
---
 Changes in v2:
 * Created a new patch for generic attributes
---
 Documentation/ABI/testing/sysfs-class-net | 16 ++++++++++++++++
 net/core/net-sysfs.c                      |  4 ++++
 2 files changed, 20 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-net b/Documentation/ABI/testing/sysfs-class-net
index ebf21beba846..f68f3b9be6ec 100644
--- a/Documentation/ABI/testing/sysfs-class-net
+++ b/Documentation/ABI/testing/sysfs-class-net
@@ -352,3 +352,19 @@ 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.
+
+What:           /sys/class/net/<iface>/min_mtu
+Date:           April 2024
+KernelVersion:  6.10
+Contact:        netdev@vger.kernel.org
+Description:
+                Indicates the interface's minimum 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..525b85d47676 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -114,6 +114,8 @@ 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);
+NETDEVICE_SHOW_RO(min_mtu, fmt_dec);
 
 static ssize_t iflink_show(struct device *dev, struct device_attribute *attr,
 			   char *buf)
@@ -671,6 +673,8 @@ static struct attribute *net_class_attrs[] __ro_after_init = {
 	&dev_attr_carrier_up_count.attr,
 	&dev_attr_carrier_down_count.attr,
 	&dev_attr_threaded.attr,
+	&dev_attr_max_mtu.attr,
+	&dev_attr_min_mtu.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(net_class);
-- 
2.34.1

Re: [PATCH net-next v2 1/2] net: Add sysfs atttributes for max_mtu min_mtu

[Jakub Kicinski]

On Wed, 24 Apr 2024 03:33:37 -0700 Shradha Gupta wrote:
> Add sysfs attributes to read max_mtu and min_mtu value for
> network devices

Absolutely pointless. You posted v1, dumping this as a driver
specific value, even tho it's already reported by the core...
And you can't even produce a meaningful commit message longer
than one sentence.

This is not meeting the bar. Please get your patches reviewed
internally at Microsoft by someone with good understanding of
Linux networking before you post.

Re: [PATCH net-next v2 1/2] net: Add sysfs atttributes for max_mtu min_mtu

[Shradha Gupta]

On Wed, Apr 24, 2024 at 08:27:03PM -0700, Jakub Kicinski wrote:
> On Wed, 24 Apr 2024 03:33:37 -0700 Shradha Gupta wrote:
> > Add sysfs attributes to read max_mtu and min_mtu value for
> > network devices
> 
> Absolutely pointless. You posted v1, dumping this as a driver
> specific value, even tho it's already reported by the core...
> And you can't even produce a meaningful commit message longer
> than one sentence.
> 
> This is not meeting the bar. Please get your patches reviewed
> internally at Microsoft by someone with good understanding of
> Linux networking before you post.
Noted, I'll do the needful going forward. Apologies.

[PATCH net-next v2 2/2] net: mana: Add new device attributes for mana

[Shradha Gupta]

Add new device attributes to read num_ports and max_num_msix setting for
MANA device.

Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
---
 Changes in v2
 * Used the suggested method(v1 dicsussion) to implement sysfs device parameters
   for MANA device
 * Implemented attributes max_mtu and min_mtu generically for all device
   drivers
---
 .../net/ethernet/microsoft/mana/gdma_main.c   | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 1332db9a08eb..e35f984e34ce 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -1471,6 +1471,37 @@ static bool mana_is_pf(unsigned short dev_id)
 	return dev_id == MANA_PF_DEVICE_ID;
 }
 
+static ssize_t num_ports_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct gdma_context *gc = pci_get_drvdata(pdev);
+	struct mana_context *ac = gc->mana.driver_data;
+
+	return sysfs_emit(buf, "%d\n", ac->num_ports);
+}
+
+static DEVICE_ATTR_RO(num_ports);
+
+static ssize_t max_num_msix_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct gdma_context *gc = pci_get_drvdata(pdev);
+
+	return sysfs_emit(buf, "%d\n", gc->max_num_msix);
+}
+
+static DEVICE_ATTR_RO(max_num_msix);
+
+static struct attribute *mana_gd_device_attrs[] = {
+	&dev_attr_num_ports.attr,
+	&dev_attr_max_num_msix.attr,
+	NULL,
+};
+
+ATTRIBUTE_GROUPS(mana_gd_device);
+
 static int mana_gd_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct gdma_context *gc;
@@ -1613,6 +1644,7 @@ static const struct pci_device_id mana_id_table[] = {
 };
 
 static struct pci_driver mana_driver = {
+	.dev_groups	= mana_gd_device_groups,
 	.name		= "mana",
 	.id_table	= mana_id_table,
 	.probe		= mana_gd_probe,
-- 
2.34.1

Re: [PATCH net-next v2 0/2] Add sysfs attributes for MANA

[Jiri Pirko]

Wed, Apr 24, 2024 at 12:32:54PM CEST, shradhagupta@linux.microsoft.com wrote:
>These patches include adding sysfs attributes for improving
>debuggability on MANA devices.
>
>The first patch consists on max_mtu, min_mtu attributes that are
>implemented generically for all devices
>
>The second patch has mana specific attributes max_num_msix and num_ports

1) you implement only max, min is never implemented, no point
introducing it.
2) having driver implement sysfs entry feels *very wrong*, don't do that
3) why DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX
   and DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN
   Are not what you want?

>
>Shradha Gupta (2):
>  net: Add sysfs atttributes for max_mtu min_mtu
>  net: mana: Add new device attributes for mana
>
> Documentation/ABI/testing/sysfs-class-net     | 16 ++++++++++
> .../net/ethernet/microsoft/mana/gdma_main.c   | 32 +++++++++++++++++++
> net/core/net-sysfs.c                          |  4 +++
> 3 files changed, 52 insertions(+)
>
>-- 
>2.34.1
>
>

Re: [PATCH net-next v2 0/2] Add sysfs attributes for MANA

[Shradha Gupta]

On Wed, Apr 24, 2024 at 04:48:06PM +0200, Jiri Pirko wrote:
> Wed, Apr 24, 2024 at 12:32:54PM CEST, shradhagupta@linux.microsoft.com wrote:
> >These patches include adding sysfs attributes for improving
> >debuggability on MANA devices.
> >
> >The first patch consists on max_mtu, min_mtu attributes that are
> >implemented generically for all devices
> >
> >The second patch has mana specific attributes max_num_msix and num_ports
> 
> 1) you implement only max, min is never implemented, no point
> introducing it.
Sure. I had added it for the sake of completeness.
> 2) having driver implement sysfs entry feels *very wrong*, don't do that
> 3) why DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX
>    and DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN
>    Are not what you want?
Thanks for pointing this out. We are still evaluating if this devlink param
could be used for our usecase where we only need a read-only msix value for VF.
We keep the thread updated.
> 
> >
> >Shradha Gupta (2):
> >  net: Add sysfs atttributes for max_mtu min_mtu
> >  net: mana: Add new device attributes for mana
> >
> > Documentation/ABI/testing/sysfs-class-net     | 16 ++++++++++
> > .../net/ethernet/microsoft/mana/gdma_main.c   | 32 +++++++++++++++++++
> > net/core/net-sysfs.c                          |  4 +++
> > 3 files changed, 52 insertions(+)
> >
> >-- 
> >2.34.1
> >
> >

Re: [PATCH net-next v2 0/2] Add sysfs attributes for MANA

[Shradha Gupta]

On Mon, Apr 29, 2024 at 10:31:38PM -0700, Shradha Gupta wrote:
> On Wed, Apr 24, 2024 at 04:48:06PM +0200, Jiri Pirko wrote:
> > Wed, Apr 24, 2024 at 12:32:54PM CEST, shradhagupta@linux.microsoft.com wrote:
> > >These patches include adding sysfs attributes for improving
> > >debuggability on MANA devices.
> > >
> > >The first patch consists on max_mtu, min_mtu attributes that are
> > >implemented generically for all devices
> > >
> > >The second patch has mana specific attributes max_num_msix and num_ports
> > 
> > 1) you implement only max, min is never implemented, no point
> > introducing it.
> Sure. I had added it for the sake of completeness.
> > 2) having driver implement sysfs entry feels *very wrong*, don't do that
> > 3) why DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX
> >    and DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN
> >    Are not what you want?
> Thanks for pointing this out. We are still evaluating if this devlink param
> could be used for our usecase where we only need a read-only msix value for VF.
> We keep the thread updated.
The attribute that we want is per VF msix max. This is per PF and would not be
the right one for our use case.
Do you have any other recommendations/suggestions around this?

Regards,
Shradha.
> > 
> > >
> > >Shradha Gupta (2):
> > >  net: Add sysfs atttributes for max_mtu min_mtu
> > >  net: mana: Add new device attributes for mana
> > >
> > > Documentation/ABI/testing/sysfs-class-net     | 16 ++++++++++
> > > .../net/ethernet/microsoft/mana/gdma_main.c   | 32 +++++++++++++++++++
> > > net/core/net-sysfs.c                          |  4 +++
> > > 3 files changed, 52 insertions(+)
> > >
> > >-- 
> > >2.34.1
> > >
> > >