[RFC PATCH] net: devlink: devlink_nl_info_fill: populate default information

Some piece of information are common to the vast majority of the
devices. Examples are:

  * the driver name.
  * the serial number of a USB device.

Modify devlink_nl_info_fill() to retrieve those information so that
the drivers do not have to. Rationale: factorize code.

Signed-off-by: Vincent Mailhol <redacted>
---
I am sending this as an RFC because I just started to study devlink.

I can see a parallel with ethtool for which the core will fill
whatever it can. c.f.:
commit f20a0a0519f3 ("ethtool: doc: clarify what drivers can implement in their get_drvinfo()")
Link: https://git.kernel.org/netdev/net-next/c/f20a0a0519f3

I think that devlink should do the same.

Right now, I identified two fields. If this RFC receive positive
feedback, I will iron it up and try to see if there is more that can
be filled by default.

Thank you for your comments.
---
 net/core/devlink.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 7f789bbcbbd7..1908b360caf7 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c

@@ -18,6 +18,7 @@
 #include <linux/netdevice.h>
 #include <linux/spinlock.h>
 #include <linux/refcount.h>
+#include <linux/usb.h>
 #include <linux/workqueue.h>
 #include <linux/u64_stats_sync.h>
 #include <linux/timekeeping.h>

@@ -6685,12 +6686,37 @@ int devlink_info_version_running_put_ext(struct devlink_info_req *req,
 }
 EXPORT_SYMBOL_GPL(devlink_info_version_running_put_ext);
 
+static int devlink_nl_driver_info_get(struct device_driver *drv,
+				      struct devlink_info_req *req)
+{
+	if (!drv)
+		return 0;
+
+	if (drv->name[0])
+		return devlink_info_driver_name_put(req, drv->name);
+
+	return 0;
+}
+
+static int devlink_nl_usb_info_get(struct usb_device *udev,
+				   struct devlink_info_req *req)
+{
+	if (!udev)
+		return 0;
+
+	if (udev->serial[0])
+		return devlink_info_serial_number_put(req, udev->serial);
+
+	return 0;
+}
+
 static int
 devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
 		     enum devlink_command cmd, u32 portid,
 		     u32 seq, int flags, struct netlink_ext_ack *extack)
 {
 	struct devlink_info_req req = {};
+	struct device *dev = devlink_to_dev(devlink);
 	void *hdr;
 	int err;
 

@@ -6707,6 +6733,16 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
 	if (err)
 		goto err_cancel_msg;
 
+	err = devlink_nl_driver_info_get(dev->driver, &req);
+	if (err)
+		goto err_cancel_msg;
+
+	if (!strcmp(dev->parent->type->name, "usb_device")) {
+		err = devlink_nl_usb_info_get(to_usb_device(dev->parent), &req);
+		if (err)
+			goto err_cancel_msg;
+	}
+
 	genlmsg_end(msg, hdr);
 	return 0;
 

-- 
2.37.4

On Wed, 23 Nov 2022 00:49:34 +0900 Vincent Mailhol wrote:

 static int
 devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
 		     enum devlink_command cmd, u32 portid,
 		     u32 seq, int flags, struct netlink_ext_ack *extack)
 {
 	struct devlink_info_req req = {};
+	struct device *dev = devlink_to_dev(devlink);

nit: longest to shortest lines

won't this result in repeated attributes, potentially?
Unlike ethtool which copies data into a struct devlink
adds an attribute each time you request. It does not override.
So we need to extend req with some tracking of whether driver
already put in the info in question

+	if (!strcmp(dev->parent->type->name, "usb_device")) {
+		err = devlink_nl_usb_info_get(to_usb_device(dev->parent), &req);
+		if (err)
+			goto err_cancel_msg;
+	}

On Wed. 23 Nov. 2022 at 13:12, Jakub Kicinski [off-list ref] wrote:

On Wed, 23 Nov 2022 00:49:34 +0900 Vincent Mailhol wrote:

quoted

 static int
 devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
                   enum devlink_command cmd, u32 portid,
                   u32 seq, int flags, struct netlink_ext_ack *extack)
 {
      struct devlink_info_req req = {};
+     struct device *dev = devlink_to_dev(devlink);

nit: longest to shortest lines

I was not aware of this convention. Thanks for the hint.

quoted

      void *hdr;
      int err;

@@ -6707,6 +6733,16 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
      if (err)
              goto err_cancel_msg;

+     err = devlink_nl_driver_info_get(dev->driver, &req);
+     if (err)
+             goto err_cancel_msg;

won't this result in repeated attributes, potentially?

You are right. It will because nla_put() doesn't check if an attribute
already exists and unconditionally reserves new space:
https://elixir.bootlin.com/linux/v6.0/source/lib/nlattr.c#L993

Unlike ethtool which copies data into a struct devlink
adds an attribute each time you request. It does not override.
So we need to extend req with some tracking of whether driver
already put in the info in question

I see three solutions:

1/ Do it in the core, clean up all drivers using
devlink_info_driver_name_put() and make the function static (i.e.
forbid the drivers to set the driver name themselves).
N.B. This first solution does not work for
devlink_info_serial_number_put() because the core will not always be
able to provide a default value (e.g. my code only covers USB
devices).

2/ Keep track of which attribute is already set (as you suggested).

3/ Do a function devlink_nl_info_fill_default() and let the drivers
choose to either call that function or set the attributes themselves.

I would tend to go with a mix of 1/ and 2/.

What do you think?

quoted

+     if (!strcmp(dev->parent->type->name, "usb_device")) {
+             err = devlink_nl_usb_info_get(to_usb_device(dev->parent), &req);
+             if (err)
+                     goto err_cancel_msg;
+     }

Tue, Nov 22, 2022 at 04:49:34PM CET, mailhol.vincent@wanadoo.fr wrote:

Make sure that this provides the same value for all existing drivers
using devlink.

Comparing to string does not seem correct here.

+		err = devlink_nl_usb_info_get(to_usb_device(dev->parent), &req);

As Jakub pointed out, you have to make sure that driver does not put the
same attrs again. You have to introduce this functionality with removing
the fill-ups in drivers atomically, in a single patch.

+		if (err)
+			goto err_cancel_msg;
+	}
+
	genlmsg_end(msg, hdr);
	return 0;

-- 
2.37.4

On Wed. 23 nov. 2022 à 18:46, Jiri Pirko [off-list ref] wrote:

Tue, Nov 22, 2022 at 04:49:34PM CET, mailhol.vincent@wanadoo.fr wrote:

quoted

Some piece of information are common to the vast majority of the
devices. Examples are:

 * the driver name.
 * the serial number of a USB device.

Modify devlink_nl_info_fill() to retrieve those information so that
the drivers do not have to. Rationale: factorize code.

Signed-off-by: Vincent Mailhol <redacted>
---
I am sending this as an RFC because I just started to study devlink.

I can see a parallel with ethtool for which the core will fill
whatever it can. c.f.:
commit f20a0a0519f3 ("ethtool: doc: clarify what drivers can implement in their get_drvinfo()")
Link: https://git.kernel.org/netdev/net-next/c/f20a0a0519f3

I think that devlink should do the same.

Right now, I identified two fields. If this RFC receive positive
feedback, I will iron it up and try to see if there is more that can
be filled by default.

Thank you for your comments.
---
net/core/devlink.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 7f789bbcbbd7..1908b360caf7 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c

@@ -18,6 +18,7 @@

#include <linux/netdevice.h>
#include <linux/spinlock.h>
#include <linux/refcount.h>
+#include <linux/usb.h>
#include <linux/workqueue.h>
#include <linux/u64_stats_sync.h>
#include <linux/timekeeping.h>

@@ -6685,12 +6686,37 @@ int devlink_info_version_running_put_ext(struct devlink_info_req *req,

}
EXPORT_SYMBOL_GPL(devlink_info_version_running_put_ext);

+static int devlink_nl_driver_info_get(struct device_driver *drv,
+                                    struct devlink_info_req *req)
+{
+      if (!drv)
+              return 0;
+
+      if (drv->name[0])
+              return devlink_info_driver_name_put(req, drv->name);

Make sure that this provides the same value for all existing drivers
using devlink.

There are 21 drivers so far which reports the driver name through devlink. c.f.:
  $ git grep "devlink_info_driver_name_put(" drivers | wc -l

Out of those 21, there is only one: the mlxsw which seems to report
something different than device_driver::name. Instead it reports some
bus_info:
  https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/net/ethernet/mellanox/mlxsw/core.c#L1462
  https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/net/ethernet/mellanox/mlxsw/core.h#L504

I am not sure what the bus_info is here, but it looks like a misuse of
the field here.

quoted

+
+      return 0;
+}
+
+static int devlink_nl_usb_info_get(struct usb_device *udev,
+                                 struct devlink_info_req *req)
+{
+      if (!udev)
+              return 0;
+
+      if (udev->serial[0])
+              return devlink_info_serial_number_put(req, udev->serial);
+
+      return 0;
+}
+
static int
devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
                   enum devlink_command cmd, u32 portid,
                   u32 seq, int flags, struct netlink_ext_ack *extack)
{
      struct devlink_info_req req = {};
+      struct device *dev = devlink_to_dev(devlink);
      void *hdr;
      int err;

@@ -6707,6 +6733,16 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
      if (err)
              goto err_cancel_msg;

+      err = devlink_nl_driver_info_get(dev->driver, &req);
+      if (err)
+              goto err_cancel_msg;
+
+      if (!strcmp(dev->parent->type->name, "usb_device")) {

Comparing to string does not seem correct here.

There is a is_usb_device() which does the check:
  https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/usb/core/usb.h#L152

but this macro is not exposed outside of the usb core. The string
comparison was the only solution I found.

Do you have any other ideas? If not and if this goes further than the
RFC stage, I will ask the USB folks if there is a better way.

quoted

+              err = devlink_nl_usb_info_get(to_usb_device(dev->parent), &req);

As Jakub pointed out, you have to make sure that driver does not put the
same attrs again. You have to introduce this functionality with removing
the fill-ups in drivers atomically, in a single patch.

Either this, either track if the attribute is already set. I would
prefer to remove all drivers fill-ups but this is not feasible for the
serial number. c.f. my reply to Jacub in this thread:
  https://lore.kernel.org/netdev/CAMZ6RqJ8_=h1SS7WmBeEB=75wsvVUZrb-8ELCDtpZb0gSs=2+A@mail.gmail.com/

quoted

+              if (err)
+                      goto err_cancel_msg;
+      }
+
      genlmsg_end(msg, hdr);
      return 0;

--
2.37.4

Wed, Nov 23, 2022 at 12:00:44PM CET, mailhol.vincent@wanadoo.fr wrote:

On Wed. 23 nov. 2022 à 18:46, Jiri Pirko [off-list ref] wrote:

quoted

Tue, Nov 22, 2022 at 04:49:34PM CET, mailhol.vincent@wanadoo.fr wrote:

quoted

Some piece of information are common to the vast majority of the
devices. Examples are:

 * the driver name.
 * the serial number of a USB device.

Modify devlink_nl_info_fill() to retrieve those information so that
the drivers do not have to. Rationale: factorize code.

Signed-off-by: Vincent Mailhol <redacted>
---
I am sending this as an RFC because I just started to study devlink.

I can see a parallel with ethtool for which the core will fill
whatever it can. c.f.:
commit f20a0a0519f3 ("ethtool: doc: clarify what drivers can implement in their get_drvinfo()")
Link: https://git.kernel.org/netdev/net-next/c/f20a0a0519f3

I think that devlink should do the same.

Right now, I identified two fields. If this RFC receive positive
feedback, I will iron it up and try to see if there is more that can
be filled by default.

Thank you for your comments.
---
net/core/devlink.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 7f789bbcbbd7..1908b360caf7 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c

@@ -18,6 +18,7 @@

#include <linux/netdevice.h>
#include <linux/spinlock.h>
#include <linux/refcount.h>
+#include <linux/usb.h>
#include <linux/workqueue.h>
#include <linux/u64_stats_sync.h>
#include <linux/timekeeping.h>

@@ -6685,12 +6686,37 @@ int devlink_info_version_running_put_ext(struct devlink_info_req *req,

}
EXPORT_SYMBOL_GPL(devlink_info_version_running_put_ext);

+static int devlink_nl_driver_info_get(struct device_driver *drv,
+                                    struct devlink_info_req *req)
+{
+      if (!drv)
+              return 0;
+
+      if (drv->name[0])
+              return devlink_info_driver_name_put(req, drv->name);

Make sure that this provides the same value for all existing drivers
using devlink.

There are 21 drivers so far which reports the driver name through devlink. c.f.:
 $ git grep "devlink_info_driver_name_put(" drivers | wc -l

Out of those 21, there is only one: the mlxsw which seems to report
something different than device_driver::name. Instead it reports some
bus_info:
 https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/net/ethernet/mellanox/mlxsw/core.c#L1462
 https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/net/ethernet/mellanox/mlxsw/core.h#L504

I am not sure what the bus_info is here, but it looks like a misuse of
the field here.

When you are not sure, look into the code to find out :) I see no misue.
What exactly do you mean by that?

quoted

quoted

+
+      return 0;
+}
+
+static int devlink_nl_usb_info_get(struct usb_device *udev,
+                                 struct devlink_info_req *req)
+{
+      if (!udev)
+              return 0;
+
+      if (udev->serial[0])
+              return devlink_info_serial_number_put(req, udev->serial);
+
+      return 0;
+}
+
static int
devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
                   enum devlink_command cmd, u32 portid,
                   u32 seq, int flags, struct netlink_ext_ack *extack)
{
      struct devlink_info_req req = {};
+      struct device *dev = devlink_to_dev(devlink);
      void *hdr;
      int err;

@@ -6707,6 +6733,16 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
      if (err)
              goto err_cancel_msg;

+      err = devlink_nl_driver_info_get(dev->driver, &req);
+      if (err)
+              goto err_cancel_msg;
+
+      if (!strcmp(dev->parent->type->name, "usb_device")) {

Comparing to string does not seem correct here.

There is a is_usb_device() which does the check:
 https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/usb/core/usb.h#L152

but this macro is not exposed outside of the usb core. The string
comparison was the only solution I found.

Find a different one. String check here is wrong.

Do you have any other ideas? If not and if this goes further than the
RFC stage, I will ask the USB folks if there is a better way.

quoted

quoted

+              err = devlink_nl_usb_info_get(to_usb_device(dev->parent), &req);

As Jakub pointed out, you have to make sure that driver does not put the
same attrs again. You have to introduce this functionality with removing
the fill-ups in drivers atomically, in a single patch.

Either this, either track if the attribute is already set. I would
prefer to remove all drivers fill-ups but this is not feasible for the
serial number. c.f. my reply to Jacub in this thread:
 https://lore.kernel.org/netdev/CAMZ6RqJ8_=h1SS7WmBeEB=75wsvVUZrb-8ELCDtpZb0gSs=2+A@mail.gmail.com/

Sure, but for the driver name it is. Let's start there.

quoted

quoted

+              if (err)
+                      goto err_cancel_msg;
+      }
+
      genlmsg_end(msg, hdr);
      return 0;

--
2.37.4

On Wed. 23 Nov. 2022 at 21:10, Jiri Pirko [off-list ref] wrote:

Wed, Nov 23, 2022 at 12:00:44PM CET, mailhol.vincent@wanadoo.fr wrote:

quoted

On Wed. 23 nov. 2022 à 18:46, Jiri Pirko [off-list ref] wrote:

quoted

Tue, Nov 22, 2022 at 04:49:34PM CET, mailhol.vincent@wanadoo.fr wrote:

quoted

Some piece of information are common to the vast majority of the
devices. Examples are:

 * the driver name.
 * the serial number of a USB device.

Modify devlink_nl_info_fill() to retrieve those information so that
the drivers do not have to. Rationale: factorize code.

Signed-off-by: Vincent Mailhol <redacted>
---
I am sending this as an RFC because I just started to study devlink.

I can see a parallel with ethtool for which the core will fill
whatever it can. c.f.:
commit f20a0a0519f3 ("ethtool: doc: clarify what drivers can implement in their get_drvinfo()")
Link: https://git.kernel.org/netdev/net-next/c/f20a0a0519f3

I think that devlink should do the same.

Right now, I identified two fields. If this RFC receive positive
feedback, I will iron it up and try to see if there is more that can
be filled by default.

Thank you for your comments.
---
net/core/devlink.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 7f789bbcbbd7..1908b360caf7 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c

@@ -18,6 +18,7 @@

#include <linux/netdevice.h>
#include <linux/spinlock.h>
#include <linux/refcount.h>
+#include <linux/usb.h>
#include <linux/workqueue.h>
#include <linux/u64_stats_sync.h>
#include <linux/timekeeping.h>

@@ -6685,12 +6686,37 @@ int devlink_info_version_running_put_ext(struct devlink_info_req *req,

}
EXPORT_SYMBOL_GPL(devlink_info_version_running_put_ext);

+static int devlink_nl_driver_info_get(struct device_driver *drv,
+                                    struct devlink_info_req *req)
+{
+      if (!drv)
+              return 0;
+
+      if (drv->name[0])
+              return devlink_info_driver_name_put(req, drv->name);

Make sure that this provides the same value for all existing drivers
using devlink.

There are 21 drivers so far which reports the driver name through devlink. c.f.:
 $ git grep "devlink_info_driver_name_put(" drivers | wc -l

Out of those 21, there is only one: the mlxsw which seems to report
something different than device_driver::name. Instead it reports some
bus_info:
 https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/net/ethernet/mellanox/mlxsw/core.c#L1462
 https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/net/ethernet/mellanox/mlxsw/core.h#L504

I am not sure what the bus_info is here, but it looks like a misuse of
the field here.

When you are not sure, look into the code to find out :) I see no misue.
What exactly do you mean by that?

I mean that device_kind, it does not sound like a field that would
hold the driver name.

Looking deeper in the code, I got the confirmation.
bus_info::device_kind is initialized here (among other):
https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/net/ethernet/mellanox/mlxsw/i2c.c#L714

and it uses ic2_client::name which indicate the type of the device
(e.g. chip name):
https://elixir.bootlin.com/linux/v6.1-rc1/source/include/linux/i2c.h#L317

So I confirm that this is a misuse. This driver does not report the
driver's name.

quoted

quoted

quoted

+
+      return 0;
+}
+
+static int devlink_nl_usb_info_get(struct usb_device *udev,
+                                 struct devlink_info_req *req)
+{
+      if (!udev)
+              return 0;
+
+      if (udev->serial[0])
+              return devlink_info_serial_number_put(req, udev->serial);
+
+      return 0;
+}
+
static int
devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
                   enum devlink_command cmd, u32 portid,
                   u32 seq, int flags, struct netlink_ext_ack *extack)
{
      struct devlink_info_req req = {};
+      struct device *dev = devlink_to_dev(devlink);
      void *hdr;
      int err;

@@ -6707,6 +6733,16 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
      if (err)
              goto err_cancel_msg;

+      err = devlink_nl_driver_info_get(dev->driver, &req);
+      if (err)
+              goto err_cancel_msg;
+
+      if (!strcmp(dev->parent->type->name, "usb_device")) {

Comparing to string does not seem correct here.

There is a is_usb_device() which does the check:
 https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/usb/core/usb.h#L152

but this macro is not exposed outside of the usb core. The string
comparison was the only solution I found.

Find a different one. String check here is wrong.

quoted

Do you have any other ideas? If not and if this goes further than the
RFC stage, I will ask the USB folks if there is a better way.

quoted

quoted

+              err = devlink_nl_usb_info_get(to_usb_device(dev->parent), &req);

As Jakub pointed out, you have to make sure that driver does not put the
same attrs again. You have to introduce this functionality with removing
the fill-ups in drivers atomically, in a single patch.

Either this, either track if the attribute is already set. I would
prefer to remove all drivers fill-ups but this is not feasible for the
serial number. c.f. my reply to Jacub in this thread:
 https://lore.kernel.org/netdev/CAMZ6RqJ8_=h1SS7WmBeEB=75wsvVUZrb-8ELCDtpZb0gSs=2+A@mail.gmail.com/

Sure, but for the driver name it is. Let's start there.

I will do a first patch only for the driver name and think again of
the USB serial later on.


Yours sincerely,
Vincent Mailhol

Wed, Nov 23, 2022 at 05:08:10PM CET, mailhol.vincent@wanadoo.fr wrote:

On Wed. 23 Nov. 2022 at 21:10, Jiri Pirko [off-list ref] wrote:

quoted

Wed, Nov 23, 2022 at 12:00:44PM CET, mailhol.vincent@wanadoo.fr wrote:

quoted

On Wed. 23 nov. 2022 à 18:46, Jiri Pirko [off-list ref] wrote:

quoted

Tue, Nov 22, 2022 at 04:49:34PM CET, mailhol.vincent@wanadoo.fr wrote:

quoted

Some piece of information are common to the vast majority of the
devices. Examples are:

 * the driver name.
 * the serial number of a USB device.

Modify devlink_nl_info_fill() to retrieve those information so that
the drivers do not have to. Rationale: factorize code.

Signed-off-by: Vincent Mailhol <redacted>
---
I am sending this as an RFC because I just started to study devlink.

I can see a parallel with ethtool for which the core will fill
whatever it can. c.f.:
commit f20a0a0519f3 ("ethtool: doc: clarify what drivers can implement in their get_drvinfo()")
Link: https://git.kernel.org/netdev/net-next/c/f20a0a0519f3

I think that devlink should do the same.

Right now, I identified two fields. If this RFC receive positive
feedback, I will iron it up and try to see if there is more that can
be filled by default.

Thank you for your comments.
---
net/core/devlink.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 7f789bbcbbd7..1908b360caf7 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c

@@ -18,6 +18,7 @@

#include <linux/netdevice.h>
#include <linux/spinlock.h>
#include <linux/refcount.h>
+#include <linux/usb.h>
#include <linux/workqueue.h>
#include <linux/u64_stats_sync.h>
#include <linux/timekeeping.h>

@@ -6685,12 +6686,37 @@ int devlink_info_version_running_put_ext(struct devlink_info_req *req,

}
EXPORT_SYMBOL_GPL(devlink_info_version_running_put_ext);

+static int devlink_nl_driver_info_get(struct device_driver *drv,
+                                    struct devlink_info_req *req)
+{
+      if (!drv)
+              return 0;
+
+      if (drv->name[0])
+              return devlink_info_driver_name_put(req, drv->name);

Make sure that this provides the same value for all existing drivers
using devlink.

There are 21 drivers so far which reports the driver name through devlink. c.f.:
 $ git grep "devlink_info_driver_name_put(" drivers | wc -l

Out of those 21, there is only one: the mlxsw which seems to report
something different than device_driver::name. Instead it reports some
bus_info:
 https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/net/ethernet/mellanox/mlxsw/core.c#L1462
 https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/net/ethernet/mellanox/mlxsw/core.h#L504

I am not sure what the bus_info is here, but it looks like a misuse of
the field here.

When you are not sure, look into the code to find out :) I see no misue.
What exactly do you mean by that?

I mean that device_kind, it does not sound like a field that would
hold the driver name.

Looking deeper in the code, I got the confirmation.
bus_info::device_kind is initialized here (among other):
https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/net/ethernet/mellanox/mlxsw/i2c.c#L714

and it uses ic2_client::name which indicate the type of the device
(e.g. chip name):
https://elixir.bootlin.com/linux/v6.1-rc1/source/include/linux/i2c.h#L317

So I confirm that this is a misuse. This driver does not report the
driver's name.

Okay, I think that is a bug of mlxsw_i2c implementation. You can fix it
along the way.

quoted

quoted

quoted

quoted

+
+      return 0;
+}
+
+static int devlink_nl_usb_info_get(struct usb_device *udev,
+                                 struct devlink_info_req *req)
+{
+      if (!udev)
+              return 0;
+
+      if (udev->serial[0])
+              return devlink_info_serial_number_put(req, udev->serial);
+
+      return 0;
+}
+
static int
devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
                   enum devlink_command cmd, u32 portid,
                   u32 seq, int flags, struct netlink_ext_ack *extack)
{
      struct devlink_info_req req = {};
+      struct device *dev = devlink_to_dev(devlink);
      void *hdr;
      int err;

@@ -6707,6 +6733,16 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
      if (err)
              goto err_cancel_msg;

+      err = devlink_nl_driver_info_get(dev->driver, &req);
+      if (err)
+              goto err_cancel_msg;
+
+      if (!strcmp(dev->parent->type->name, "usb_device")) {

Comparing to string does not seem correct here.

There is a is_usb_device() which does the check:
 https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/usb/core/usb.h#L152

but this macro is not exposed outside of the usb core. The string
comparison was the only solution I found.

Find a different one. String check here is wrong.

quoted

Do you have any other ideas? If not and if this goes further than the
RFC stage, I will ask the USB folks if there is a better way.

quoted

quoted

+              err = devlink_nl_usb_info_get(to_usb_device(dev->parent), &req);

As Jakub pointed out, you have to make sure that driver does not put the
same attrs again. You have to introduce this functionality with removing
the fill-ups in drivers atomically, in a single patch.

Either this, either track if the attribute is already set. I would
prefer to remove all drivers fill-ups but this is not feasible for the
serial number. c.f. my reply to Jacub in this thread:
 https://lore.kernel.org/netdev/CAMZ6RqJ8_=h1SS7WmBeEB=75wsvVUZrb-8ELCDtpZb0gSs=2+A@mail.gmail.com/

Sure, but for the driver name it is. Let's start there.

I will do a first patch only for the driver name and think again of
the USB serial later on.

Yep, thanks!

Yours sincerely,
Vincent Mailhol

On Wed, 23 Nov 2022 18:42:41 +0900 Vincent MAILHOL wrote:

I see three solutions:

1/ Do it in the core, clean up all drivers using
devlink_info_driver_name_put() and make the function static (i.e.
forbid the drivers to set the driver name themselves).
N.B. This first solution does not work for
devlink_info_serial_number_put() because the core will not always be
able to provide a default value (e.g. my code only covers USB
devices).

2/ Keep track of which attribute is already set (as you suggested).

3/ Do a function devlink_nl_info_fill_default() and let the drivers
choose to either call that function or set the attributes themselves.

I would tend to go with a mix of 1/ and 2/.

I think 2/ is best because it will generalize to serial numbers while
1/ will likely not. 3/ is a smaller gain.

Jiri already plumbed thru the struct devlink_info_req which is on the
stack of the caller, per request, so we can add the bool / bitmap for
already reported items there quite easily.

On Thu. 24 Nov. 2022 at 12:06, Jakub Kicinski [off-list ref] wrote:

On Wed, 23 Nov 2022 18:42:41 +0900 Vincent MAILHOL wrote:

quoted

I see three solutions:

1/ Do it in the core, clean up all drivers using
devlink_info_driver_name_put() and make the function static (i.e.
forbid the drivers to set the driver name themselves).
N.B. This first solution does not work for
devlink_info_serial_number_put() because the core will not always be
able to provide a default value (e.g. my code only covers USB
devices).

2/ Keep track of which attribute is already set (as you suggested).

3/ Do a function devlink_nl_info_fill_default() and let the drivers
choose to either call that function or set the attributes themselves.

I would tend to go with a mix of 1/ and 2/.

I think 2/ is best because it will generalize to serial numbers while
1/ will likely not. 3/ is a smaller gain.

Jiri already plumbed thru the struct devlink_info_req which is on the
stack of the caller, per request, so we can add the bool / bitmap for
already reported items there quite easily.

Sorry, let me clarify the next actions. Are you meaning that Jiri is
already working on the bitmap implementation and should I wait for his
patches first? Or do you expect me to do it?

Thu, Nov 24, 2022 at 06:33:58AM CET, mailhol.vincent@wanadoo.fr wrote:

On Thu. 24 Nov. 2022 at 12:06, Jakub Kicinski [off-list ref] wrote:

quoted

On Wed, 23 Nov 2022 18:42:41 +0900 Vincent MAILHOL wrote:

quoted

I see three solutions:

1/ Do it in the core, clean up all drivers using
devlink_info_driver_name_put() and make the function static (i.e.
forbid the drivers to set the driver name themselves).
N.B. This first solution does not work for
devlink_info_serial_number_put() because the core will not always be
able to provide a default value (e.g. my code only covers USB
devices).

2/ Keep track of which attribute is already set (as you suggested).

3/ Do a function devlink_nl_info_fill_default() and let the drivers
choose to either call that function or set the attributes themselves.

I would tend to go with a mix of 1/ and 2/.

I think 2/ is best because it will generalize to serial numbers while
1/ will likely not. 3/ is a smaller gain.

Jiri already plumbed thru the struct devlink_info_req which is on the
stack of the caller, per request, so we can add the bool / bitmap for
already reported items there quite easily.

Sorry, let me clarify the next actions. Are you meaning that Jiri is
already working on the bitmap implementation and should I wait for his
patches first? Or do you expect me to do it?

I'm not.

On Wed. 23 Nov. 2022 at 21:10, Jiri Pirko [off-list ref] wrote:

Wed, Nov 23, 2022 at 12:00:44PM CET, mailhol.vincent@wanadoo.fr wrote:

quoted

On Wed. 23 nov. 2022 à 18:46, Jiri Pirko [off-list ref] wrote:

quoted

Tue, Nov 22, 2022 at 04:49:34PM CET, mailhol.vincent@wanadoo.fr wrote:

(...)

quoted

quoted

quoted

+      if (!strcmp(dev->parent->type->name, "usb_device")) {

Comparing to string does not seem correct here.

There is a is_usb_device() which does the check:
 https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/usb/core/usb.h#L152

but this macro is not exposed outside of the usb core. The string
comparison was the only solution I found.

Find a different one. String check here is wrong.

After looking again, I found no alternative and so I asked the USB
mailing list and got an answer from Greg. There is no ways to do so
and the class code is not supposed to do this:
https://lore.kernel.org/linux-usb/Y3+VfNdt%2FK7UtRcw@kroah.com/

So I guess that there will be no code factorization for device specific values.

@Jakub, with this in mind, does it still make sense to add the bitmap?
Aside from the driver name, it seems that there will be no code
factorization for other types dependent values. If we only set the
driver name at the core level, I would rather just clean up the
existing. (Side comment, I finished implementing the bitmap just
before receiving Greg's answer; I guess my code will go to the trash
can...)

The driver name is available in device_driver::name. Right now,
drivers still have to report this piece of information themselves in
their devlink_ops::info_get callback function.

The goal of this series is to have the devlink core to report this
information instead of the drivers.

The first two patches clean up the mlxsw driver for both the ethtool
and the devlink (both are supposed to return the same information so
the ethtool got included as well). This is split in two patches
because of the different Fixes tag.

The third patch fulfills the actual goal of this series: modify
devlink core to report the driver name and clean-up all drivers. Both
as to be done in an atomic change to avoid attribute duplication.

The fourth patch removes the devlink_info_driver_name_put() function
to prevent future drivers from reporting the driver name themselves.

Finally, the fifth and last patch allows the core to call
devlink_nl_info_fill() even if the devlink_ops::info_get() callback is
NULL. This allows to do further more clean up in the drivers.
---
* Changelog *

RFC v1 -> v2

  * drop the RFC tag

  * big rework following the discussion on RFC:
    https://lore.kernel.org/netdev/20221122154934.13937-1-mailhol.vincent@wanadoo.fr/
    Went from one patch to a series of five patches:

  * drop the idea to report the USB serial number following Greg's
    comment:
    https://lore.kernel.org/linux-usb/Y3+VfNdt%2FK7UtRcw@kroah.com/

Vincent Mailhol (5):
  mlxsw: minimal: fix mlxsw_m_module_get_drvinfo() to correctly report
    driver name
  mlxsw: core: fix mlxsw_devlink_info_get() to correctly report driver
    name
  net: devlink: let the core report the driver name instead of the
    drivers
  net: devlink: remove devlink_info_driver_name_put()
  net: devlink: make the devlink_ops::info_get() callback optional

 .../marvell/octeontx2/otx2_cpt_devlink.c      |  4 ---
 drivers/net/dsa/hirschmann/hellcreek.c        |  5 ---
 drivers/net/dsa/mv88e6xxx/devlink.c           |  5 ---
 drivers/net/dsa/sja1105/sja1105_devlink.c     | 12 ++-----
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c |  4 ---
 .../freescale/dpaa2/dpaa2-eth-devlink.c       | 11 +-----
 .../ethernet/fungible/funeth/funeth_devlink.c |  7 ----
 .../hisilicon/hns3/hns3pf/hclge_devlink.c     |  5 ---
 .../hisilicon/hns3/hns3vf/hclgevf_devlink.c   |  5 ---
 drivers/net/ethernet/intel/ice/ice_devlink.c  |  6 ----
 .../marvell/octeontx2/af/rvu_devlink.c        |  7 ----
 .../marvell/octeontx2/nic/otx2_devlink.c      | 15 --------
 .../marvell/prestera/prestera_devlink.c       |  5 ---
 .../net/ethernet/mellanox/mlx5/core/devlink.c |  4 ---
 drivers/net/ethernet/mellanox/mlxsw/core.c    |  3 +-
 drivers/net/ethernet/mellanox/mlxsw/minimal.c |  2 +-
 .../net/ethernet/netronome/nfp/nfp_devlink.c  |  4 ---
 .../ethernet/pensando/ionic/ionic_devlink.c   |  4 ---
 drivers/net/ethernet/qlogic/qed/qed_devlink.c |  4 ---
 drivers/net/netdevsim/dev.c                   |  3 --
 drivers/ptp/ptp_ocp.c                         |  4 ---
 include/net/devlink.h                         |  2 --
 net/core/devlink.c                            | 35 ++++++++++++-------
 23 files changed, 29 insertions(+), 127 deletions(-)

-- 
2.37.4

Currently, mlxsw_m_module_get_drvinfo() reports the device_kind. The
device_kind is not necessarily the same as the device_name. For
example, the mlxsw_i2c implementation sets up the device_kind as
ic2_client::name in [1] which indicates the type of the device
(e.g. chip name), not the actual driver name.

Fix it so that it correctly reports the driver name.

[1] https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/net/ethernet/mellanox/mlxsw/i2c.c#L714

Fixes: 9bbd7efbc055 ("mlxsw: i2c: Extend initialization with querying firmware info")
CC: Shalom Toledo <redacted>
CC: Ido Schimmel <redacted>
CC: Vadim Pasternak <redacted>
Signed-off-by: Vincent Mailhol <redacted>
---
 drivers/net/ethernet/mellanox/mlxsw/minimal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/minimal.c b/drivers/net/ethernet/mellanox/mlxsw/minimal.c
index 6b56eadd736e..9b37ddbe0cba 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/minimal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/minimal.c

@@ -92,7 +92,7 @@ static void mlxsw_m_module_get_drvinfo(struct net_device *dev,
 	struct mlxsw_m_port *mlxsw_m_port = netdev_priv(dev);
 	struct mlxsw_m *mlxsw_m = mlxsw_m_port->mlxsw_m;
 
-	strscpy(drvinfo->driver, mlxsw_m->bus_info->device_kind,
+	strscpy(drvinfo->driver, dev_driver_string(dev->dev.parent),
 		sizeof(drvinfo->driver));
 	snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
 		 "%d.%d.%d",

-- 
2.37.4

Currently, mlxsw_devlink_info_get() reports the device_kind. The
device_kind is not necessarily the same as the device_name. For
example, the mlxsw_i2c implementation sets up the device_kind as
ic2_client::name in [1] which indicates the type of the device
(e.g. chip name).

Fix it so that it correctly reports the driver name.

[1] https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/net/ethernet/mellanox/mlxsw/i2c.c#L714

Fixes: a9c8336f6544 ("mlxsw: core: Add support for devlink info command")
CC: Shalom Toledo <redacted>
CC: Ido Schimmel <redacted>
CC: Vadim Pasternak <redacted>
Signed-off-by: Vincent Mailhol <redacted>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index a83f6bc30072..d8b1bb03cdb0 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c

@@ -1453,6 +1453,7 @@ mlxsw_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 		       struct netlink_ext_ack *extack)
 {
 	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
+	struct device *dev = mlxsw_core->bus_info->dev;
 	char fw_info_psid[MLXSW_REG_MGIR_FW_INFO_PSID_SIZE];
 	u32 hw_rev, fw_major, fw_minor, fw_sub_minor;
 	char mgir_pl[MLXSW_REG_MGIR_LEN];

@@ -1460,7 +1461,7 @@ mlxsw_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 	int err;
 
 	err = devlink_info_driver_name_put(req,
-					   mlxsw_core->bus_info->device_kind);
+					   dev_driver_string(dev->parent));
 	if (err)
 		return err;
 

-- 
2.37.4

The driver name is available in device_driver::name. Right now,
drivers still have to report this piece of information themselves in
their devlink_ops::info_get callback function.

In order to factorize code, make devlink_nl_info_fill() add the driver
name attribute.

nla_put() does not check if an attribute already exists and
unconditionally reserves new space [1]. To avoid attribute
duplication, clean-up all the drivers which are currently reporting
the driver name in their callback.

[1] https://elixir.bootlin.com/linux/v6.0/source/lib/nlattr.c#L993

Signed-off-by: Vincent Mailhol <redacted>
---
 .../crypto/marvell/octeontx2/otx2_cpt_devlink.c |  4 ----
 drivers/net/dsa/hirschmann/hellcreek.c          |  5 -----
 drivers/net/dsa/mv88e6xxx/devlink.c             |  5 -----
 drivers/net/dsa/sja1105/sja1105_devlink.c       | 12 +++---------
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c   |  4 ----
 .../freescale/dpaa2/dpaa2-eth-devlink.c         | 11 +----------
 .../ethernet/fungible/funeth/funeth_devlink.c   |  2 +-
 .../hisilicon/hns3/hns3pf/hclge_devlink.c       |  5 -----
 .../hisilicon/hns3/hns3vf/hclgevf_devlink.c     |  5 -----
 drivers/net/ethernet/intel/ice/ice_devlink.c    |  6 ------
 .../ethernet/marvell/octeontx2/af/rvu_devlink.c |  2 +-
 .../marvell/octeontx2/nic/otx2_devlink.c        |  9 +--------
 .../marvell/prestera/prestera_devlink.c         |  5 -----
 .../net/ethernet/mellanox/mlx5/core/devlink.c   |  4 ----
 .../net/ethernet/netronome/nfp/nfp_devlink.c    |  4 ----
 .../net/ethernet/pensando/ionic/ionic_devlink.c |  4 ----
 drivers/net/ethernet/qlogic/qed/qed_devlink.c   |  4 ----
 drivers/net/netdevsim/dev.c                     |  3 ---
 drivers/ptp/ptp_ocp.c                           |  4 ----
 net/core/devlink.c                              | 17 +++++++++++++++++
 20 files changed, 24 insertions(+), 91 deletions(-)

diff --git a/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c b/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c
index 7503f6b18ac5..a2aba0b0d68a 100644
--- a/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c
+++ b/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c

@@ -76,10 +76,6 @@ static int otx2_cpt_devlink_info_get(struct devlink *dl,
 	struct otx2_cptpf_dev *cptpf = cpt_dl->cptpf;
 	int err;
 
-	err = devlink_info_driver_name_put(req, "rvu_cptpf");
-	if (err)
-		return err;
-
 	err = otx2_cpt_dl_info_firmware_version_put(req, cptpf->eng_grps.grp,
 						    "fw.ae", OTX2_CPT_AE_TYPES);
 	if (err)

diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 951f7935c872..595a548bb0a8 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c

@@ -1176,11 +1176,6 @@ static int hellcreek_devlink_info_get(struct dsa_switch *ds,
 				      struct netlink_ext_ack *extack)
 {
 	struct hellcreek *hellcreek = ds->priv;
-	int ret;
-
-	ret = devlink_info_driver_name_put(req, "hellcreek");
-	if (ret)
-		return ret;
 
 	return devlink_info_version_fixed_put(req,
 					      DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,

diff --git a/drivers/net/dsa/mv88e6xxx/devlink.c b/drivers/net/dsa/mv88e6xxx/devlink.c
index 1266eabee086..a08dab75e0c0 100644
--- a/drivers/net/dsa/mv88e6xxx/devlink.c
+++ b/drivers/net/dsa/mv88e6xxx/devlink.c

@@ -821,11 +821,6 @@ int mv88e6xxx_devlink_info_get(struct dsa_switch *ds,
 			       struct netlink_ext_ack *extack)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
-	int err;
-
-	err = devlink_info_driver_name_put(req, "mv88e6xxx");
-	if (err)
-		return err;
 
 	return devlink_info_version_fixed_put(req,
 					      DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,

diff --git a/drivers/net/dsa/sja1105/sja1105_devlink.c b/drivers/net/dsa/sja1105/sja1105_devlink.c
index 10c6fea1227f..da532614f34a 100644
--- a/drivers/net/dsa/sja1105/sja1105_devlink.c
+++ b/drivers/net/dsa/sja1105/sja1105_devlink.c

@@ -120,16 +120,10 @@ int sja1105_devlink_info_get(struct dsa_switch *ds,
 			     struct netlink_ext_ack *extack)
 {
 	struct sja1105_private *priv = ds->priv;
-	int rc;
-
-	rc = devlink_info_driver_name_put(req, "sja1105");
-	if (rc)
-		return rc;
 
-	rc = devlink_info_version_fixed_put(req,
-					    DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,
-					    priv->info->name);
-	return rc;
+	return devlink_info_version_fixed_put(req,
+					      DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,
+					      priv->info->name);
 }
 
 int sja1105_devlink_setup(struct dsa_switch *ds)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 8a6f788f6294..26913dc816d3 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c

@@ -892,10 +892,6 @@ static int bnxt_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 	u32 ver = 0;
 	int rc;
 
-	rc = devlink_info_driver_name_put(req, DRV_MODULE_NAME);
-	if (rc)
-		return rc;
-
 	if (BNXT_PF(bp) && (bp->flags & BNXT_FLAG_DSN_VALID)) {
 		sprintf(buf, "%02X-%02X-%02X-%02X-%02X-%02X-%02X-%02X",
 			bp->dsn[7], bp->dsn[6], bp->dsn[5], bp->dsn[4],

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c
index 5c6dd3029e2f..76f808d38066 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c

@@ -37,18 +37,9 @@ static int dpaa2_eth_dl_info_get(struct devlink *devlink,
 	struct dpaa2_eth_devlink_priv *dl_priv = devlink_priv(devlink);
 	struct dpaa2_eth_priv *priv = dl_priv->dpaa2_priv;
 	char buf[10];
-	int err;
-
-	err = devlink_info_driver_name_put(req, KBUILD_MODNAME);
-	if (err)
-		return err;
 
 	scnprintf(buf, 10, "%d.%d", priv->dpni_ver_major, priv->dpni_ver_minor);
-	err = devlink_info_version_running_put(req, "dpni", buf);
-	if (err)
-		return err;
-
-	return 0;
+	return devlink_info_version_running_put(req, "dpni", buf);
 }
 
 static struct dpaa2_eth_trap_item *

diff --git a/drivers/net/ethernet/fungible/funeth/funeth_devlink.c b/drivers/net/ethernet/fungible/funeth/funeth_devlink.c
index d50c222948b4..6668375edff6 100644
--- a/drivers/net/ethernet/fungible/funeth/funeth_devlink.c
+++ b/drivers/net/ethernet/fungible/funeth/funeth_devlink.c

@@ -6,7 +6,7 @@
 static int fun_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 			   struct netlink_ext_ack *extack)
 {
-	return devlink_info_driver_name_put(req, KBUILD_MODNAME);
+	return 0;
 }
 
 static const struct devlink_ops fun_dl_ops = {

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
index 4c441e6a5082..3d3b69605423 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c

@@ -13,11 +13,6 @@ static int hclge_devlink_info_get(struct devlink *devlink,
 	struct hclge_devlink_priv *priv = devlink_priv(devlink);
 	char version_str[HCLGE_DEVLINK_FW_STRING_LEN];
 	struct hclge_dev *hdev = priv->hdev;
-	int ret;
-
-	ret = devlink_info_driver_name_put(req, KBUILD_MODNAME);
-	if (ret)
-		return ret;
 
 	snprintf(version_str, sizeof(version_str), "%lu.%lu.%lu.%lu",
 		 hnae3_get_field(hdev->fw_version, HNAE3_FW_VERSION_BYTE3_MASK,

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c
index fdc19868b818..a6c3c5e8f0ab 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c

@@ -13,11 +13,6 @@ static int hclgevf_devlink_info_get(struct devlink *devlink,
 	struct hclgevf_devlink_priv *priv = devlink_priv(devlink);
 	char version_str[HCLGEVF_DEVLINK_FW_STRING_LEN];
 	struct hclgevf_dev *hdev = priv->hdev;
-	int ret;
-
-	ret = devlink_info_driver_name_put(req, KBUILD_MODNAME);
-	if (ret)
-		return ret;
 
 	snprintf(version_str, sizeof(version_str), "%lu.%lu.%lu.%lu",
 		 hnae3_get_field(hdev->fw_version, HNAE3_FW_VERSION_BYTE3_MASK,

diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
index 1d638216484d..ba74977e75dc 100644
--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c

@@ -311,12 +311,6 @@ static int ice_devlink_info_get(struct devlink *devlink,
 		}
 	}
 
-	err = devlink_info_driver_name_put(req, KBUILD_MODNAME);
-	if (err) {
-		NL_SET_ERR_MSG_MOD(extack, "Unable to set driver name");
-		goto out_free_ctx;
-	}
-
 	ice_info_get_dsn(pf, ctx);
 
 	err = devlink_info_serial_number_put(req, ctx->buf);

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
index 88dee589cb21..f15439d26d21 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c

@@ -1550,7 +1550,7 @@ static int rvu_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
 static int rvu_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 				struct netlink_ext_ack *extack)
 {
-	return devlink_info_driver_name_put(req, DRV_NAME);
+	return 0;
 }
 
 static const struct devlink_ops rvu_devlink_ops = {

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
index 777a27047c8e..5cc6416cf1a6 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c

@@ -77,18 +77,11 @@ static const struct devlink_param otx2_dl_params[] = {
 			     otx2_dl_mcam_count_validate),
 };
 
-/* Devlink OPs */
 static int otx2_devlink_info_get(struct devlink *devlink,
 				 struct devlink_info_req *req,
 				 struct netlink_ext_ack *extack)
 {
-	struct otx2_devlink *otx2_dl = devlink_priv(devlink);
-	struct otx2_nic *pfvf = otx2_dl->pfvf;
-
-	if (is_otx2_vf(pfvf->pcifunc))
-		return devlink_info_driver_name_put(req, "rvu_nicvf");
-
-	return devlink_info_driver_name_put(req, "rvu_nicpf");
+	return 0;
 }
 
 static const struct devlink_ops otx2_devlink_ops = {

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_devlink.c b/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
index 84ad05c9f12d..2a4c9df4eb79 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_devlink.c

@@ -355,11 +355,6 @@ static int prestera_dl_info_get(struct devlink *dl,
 {
 	struct prestera_switch *sw = devlink_priv(dl);
 	char buf[16];
-	int err;
-
-	err = devlink_info_driver_name_put(req, PRESTERA_DRV_NAME);
-	if (err)
-		return err;
 
 	snprintf(buf, sizeof(buf), "%d.%d.%d",
 		 sw->dev->fw_rev.maj,

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index cc2ae427dcb0..751bc4a9edcf 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c

@@ -46,10 +46,6 @@ mlx5_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 	u32 running_fw, stored_fw;
 	int err;
 
-	err = devlink_info_driver_name_put(req, KBUILD_MODNAME);
-	if (err)
-		return err;
-
 	err = devlink_info_version_fixed_put(req, "fw.psid", dev->board_id);
 	if (err)
 		return err;

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index 8bfd48d50ef0..4c601ff09cd3 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c

@@ -239,10 +239,6 @@ nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 	char *buf = NULL;
 	int err;
 
-	err = devlink_info_driver_name_put(req, "nfp");
-	if (err)
-		return err;
-
 	vendor = nfp_hwinfo_lookup(pf->hwinfo, "assembly.vendor");
 	part = nfp_hwinfo_lookup(pf->hwinfo, "assembly.partno");
 	sn = nfp_hwinfo_lookup(pf->hwinfo, "assembly.serial");

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
index 567f778433e2..e6ff757895ab 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c

@@ -26,10 +26,6 @@ static int ionic_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 	char buf[16];
 	int err = 0;
 
-	err = devlink_info_driver_name_put(req, IONIC_DRV_NAME);
-	if (err)
-		return err;
-
 	err = devlink_info_version_running_put(req,
 					       DEVLINK_INFO_VERSION_GENERIC_FW,
 					       idev->dev_info.fw_version);

diff --git a/drivers/net/ethernet/qlogic/qed/qed_devlink.c b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
index 6bb4e165b592..922c47797af6 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_devlink.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_devlink.c

@@ -162,10 +162,6 @@ static int qed_devlink_info_get(struct devlink *devlink,
 
 	dev_info = &cdev->common_dev_info;
 
-	err = devlink_info_driver_name_put(req, KBUILD_MODNAME);
-	if (err)
-		return err;
-
 	memcpy(buf, cdev->hwfns[0].hw_info.part_num, sizeof(cdev->hwfns[0].hw_info.part_num));
 	buf[sizeof(cdev->hwfns[0].hw_info.part_num)] = 0;
 

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index e14686594a71..b962fc8e1397 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c

@@ -994,9 +994,6 @@ static int nsim_dev_info_get(struct devlink *devlink,
 {
 	int err;
 
-	err = devlink_info_driver_name_put(req, DRV_NAME);
-	if (err)
-		return err;
 	err = devlink_info_version_stored_put_ext(req, "fw.mgmt", "10.20.30",
 						  DEVLINK_INFO_VERSION_TYPE_COMPONENT);
 	if (err)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 154d58cbd9ce..4bbaccd543ad 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c

@@ -1647,10 +1647,6 @@ ptp_ocp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 	char buf[32];
 	int err;
 
-	err = devlink_info_driver_name_put(req, KBUILD_MODNAME);
-	if (err)
-		return err;
-
 	fw_image = bp->fw_loader ? "loader" : "fw";
 	sprintf(buf, "%d.%d", bp->fw_tag, bp->fw_version);
 	err = devlink_info_version_running_put(req, fw_image, buf);

diff --git a/net/core/devlink.c b/net/core/devlink.c
index cea154ddce7a..6478135d9ba1 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c

@@ -6749,11 +6749,24 @@ int devlink_info_version_running_put_ext(struct devlink_info_req *req,
 }
 EXPORT_SYMBOL_GPL(devlink_info_version_running_put_ext);
 
+static int devlink_nl_driver_info_get(struct device_driver *drv,
+				      struct devlink_info_req *req)
+{
+	if (!drv)
+		return 0;
+
+	if (drv->name[0])
+		return devlink_info_driver_name_put(req, drv->name);
+
+	return 0;
+}
+
 static int
 devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
 		     enum devlink_command cmd, u32 portid,
 		     u32 seq, int flags, struct netlink_ext_ack *extack)
 {
+	struct device *dev = devlink_to_dev(devlink);
 	struct devlink_info_req req = {};
 	void *hdr;
 	int err;

@@ -6771,6 +6784,10 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
 	if (err)
 		goto err_cancel_msg;
 
+	err = devlink_nl_driver_info_get(dev->driver, &req);
+	if (err)
+		goto err_cancel_msg;
+
 	genlmsg_end(msg, hdr);
 	return 0;
 

-- 
2.37.4

Now that the core sets the driver name attribute, drivers are not
supposed to call devlink_info_driver_name_put() anymore. Remove it.

Signed-off-by: Vincent Mailhol <redacted>
---
 include/net/devlink.h |  2 --
 net/core/devlink.c    | 11 ++---------
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 074a79b8933f..52d5fb67e9b8 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h

@@ -1746,8 +1746,6 @@ int devlink_region_snapshot_create(struct devlink_region *region,
 				   u8 *data, u32 snapshot_id);
 int devlink_info_serial_number_put(struct devlink_info_req *req,
 				   const char *sn);
-int devlink_info_driver_name_put(struct devlink_info_req *req,
-				 const char *name);
 int devlink_info_board_serial_number_put(struct devlink_info_req *req,
 					 const char *bsn);
 

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 6478135d9ba1..3babc16eeb6b 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c

@@ -6633,14 +6633,6 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
 	return err;
 }
 
-int devlink_info_driver_name_put(struct devlink_info_req *req, const char *name)
-{
-	if (!req->msg)
-		return 0;
-	return nla_put_string(req->msg, DEVLINK_ATTR_INFO_DRIVER_NAME, name);
-}
-EXPORT_SYMBOL_GPL(devlink_info_driver_name_put);
-
 int devlink_info_serial_number_put(struct devlink_info_req *req, const char *sn)
 {
 	if (!req->msg)

@@ -6756,7 +6748,8 @@ static int devlink_nl_driver_info_get(struct device_driver *drv,
 		return 0;
 
 	if (drv->name[0])
-		return devlink_info_driver_name_put(req, drv->name);
+		return nla_put_string(req->msg, DEVLINK_ATTR_INFO_DRIVER_NAME,
+				      drv->name);
 
 	return 0;
 }

-- 
2.37.4

Some drivers only reported the driver name in their
devlink_ops::info_get() callback. Now that the core provides this
information, the callback became empty. For such drivers, just
removing the callback would prevent the core from executing
devlink_nl_info_fill() meaning that "devlink dev info" would not
return anything.

Make the callback function optional by executing
devlink_nl_info_fill() even if devlink_ops::info_get() is NULL.

Remove all the empty devlink_ops::info_get() functions from the
drivers.

N.B.: the drivers with devlink support which previously did not
implement devlink_ops::info_get() will now also be able to report
the driver name.

Signed-off-by: Vincent Mailhol <redacted>
---
 .../net/ethernet/fungible/funeth/funeth_devlink.c   |  7 -------
 .../net/ethernet/marvell/octeontx2/af/rvu_devlink.c |  7 -------
 .../ethernet/marvell/octeontx2/nic/otx2_devlink.c   |  8 --------
 net/core/devlink.c                                  | 13 ++++++-------
 4 files changed, 6 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/fungible/funeth/funeth_devlink.c b/drivers/net/ethernet/fungible/funeth/funeth_devlink.c
index 6668375edff6..4fbeb3fd71a8 100644
--- a/drivers/net/ethernet/fungible/funeth/funeth_devlink.c
+++ b/drivers/net/ethernet/fungible/funeth/funeth_devlink.c

@@ -3,14 +3,7 @@
 #include "funeth.h"
 #include "funeth_devlink.h"
 
-static int fun_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
-			   struct netlink_ext_ack *extack)
-{
-	return 0;
-}
-
 static const struct devlink_ops fun_dl_ops = {
-	.info_get = fun_dl_info_get,
 };
 
 struct devlink *fun_devlink_alloc(struct device *dev)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
index f15439d26d21..bda1a6fa2ec4 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c

@@ -1547,14 +1547,7 @@ static int rvu_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
 	return 0;
 }
 
-static int rvu_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
-				struct netlink_ext_ack *extack)
-{
-	return 0;
-}
-
 static const struct devlink_ops rvu_devlink_ops = {
-	.info_get = rvu_devlink_info_get,
 	.eswitch_mode_get = rvu_devlink_eswitch_mode_get,
 	.eswitch_mode_set = rvu_devlink_eswitch_mode_set,
 };

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
index 5cc6416cf1a6..63ef7c41d18d 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c

@@ -77,15 +77,7 @@ static const struct devlink_param otx2_dl_params[] = {
 			     otx2_dl_mcam_count_validate),
 };
 
-static int otx2_devlink_info_get(struct devlink *devlink,
-				 struct devlink_info_req *req,
-				 struct netlink_ext_ack *extack)
-{
-	return 0;
-}
-
 static const struct devlink_ops otx2_devlink_ops = {
-	.info_get = otx2_devlink_info_get,
 };
 
 int otx2_register_dl(struct otx2_nic *pfvf)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 3babc16eeb6b..817d978bb729 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c

@@ -6773,9 +6773,11 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
 		goto err_cancel_msg;
 
 	req.msg = msg;
-	err = devlink->ops->info_get(devlink, &req, extack);
-	if (err)
-		goto err_cancel_msg;
+	if (devlink->ops->info_get) {
+		err = devlink->ops->info_get(devlink, &req, extack);
+		if (err)
+			goto err_cancel_msg;
+	}
 
 	err = devlink_nl_driver_info_get(dev->driver, &req);
 	if (err)

@@ -6796,9 +6798,6 @@ static int devlink_nl_cmd_info_get_doit(struct sk_buff *skb,
 	struct sk_buff *msg;
 	int err;
 
-	if (!devlink->ops->info_get)
-		return -EOPNOTSUPP;
-
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
 		return -ENOMEM;

@@ -6824,7 +6823,7 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
 	int err = 0;
 
 	devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
-		if (idx < start || !devlink->ops->info_get)
+		if (idx < start)
 			goto inc;
 
 		devl_lock(devlink);

-- 
2.37.4

Hi Vincent,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Vincent-Mailhol/mlxsw-minimal-fix-mlxsw_m_module_get_drvinfo-to-correctly-report-driver-name/20221127-171657
patch link:    https://lore.kernel.org/r/20221127081604.5242-5-mailhol.vincent%40wanadoo.fr
patch subject: [PATCH net-next v2 4/5] net: devlink: remove devlink_info_driver_name_put()
config: x86_64-randconfig-a016
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/32018f68afc2b6dca866e1cdf0e5417ef4413d75
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Vincent-Mailhol/mlxsw-minimal-fix-mlxsw_m_module_get_drvinfo-to-correctly-report-driver-name/20221127-171657
        git checkout 32018f68afc2b6dca866e1cdf0e5417ef4413d75
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot [off-list ref]

All errors (new ones prefixed by >>):

quoted

drivers/net/ethernet/mellanox/mlxsw/core.c:1463:8: error: implicit declaration of function 'devlink_info_driver_name_put' is invalid in C99 [-Werror,-Wimplicit-function-declaration]

           err = devlink_info_driver_name_put(req,
                 ^
   drivers/net/ethernet/mellanox/mlxsw/core.c:1463:8: note: did you mean 'devlink_info_serial_number_put'?
   include/net/devlink.h:1747:5: note: 'devlink_info_serial_number_put' declared here
   int devlink_info_serial_number_put(struct devlink_info_req *req,
       ^
   1 error generated.


vim +/devlink_info_driver_name_put +1463 drivers/net/ethernet/mellanox/mlxsw/core.c

1ceecc88d29bbb Jiri Pirko      2016-04-14  1450  
a9c8336f65446b Shalom Toledo   2019-04-08  1451  static int
a9c8336f65446b Shalom Toledo   2019-04-08  1452  mlxsw_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
a9c8336f65446b Shalom Toledo   2019-04-08  1453  		       struct netlink_ext_ack *extack)
a9c8336f65446b Shalom Toledo   2019-04-08  1454  {
a9c8336f65446b Shalom Toledo   2019-04-08  1455  	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
92f165ec5b1e76 Vincent Mailhol 2022-11-27  1456  	struct device *dev = mlxsw_core->bus_info->dev;
a9c8336f65446b Shalom Toledo   2019-04-08  1457  	char fw_info_psid[MLXSW_REG_MGIR_FW_INFO_PSID_SIZE];
a9c8336f65446b Shalom Toledo   2019-04-08  1458  	u32 hw_rev, fw_major, fw_minor, fw_sub_minor;
a9c8336f65446b Shalom Toledo   2019-04-08  1459  	char mgir_pl[MLXSW_REG_MGIR_LEN];
a9c8336f65446b Shalom Toledo   2019-04-08  1460  	char buf[32];
a9c8336f65446b Shalom Toledo   2019-04-08  1461  	int err;
a9c8336f65446b Shalom Toledo   2019-04-08  1462  
a9c8336f65446b Shalom Toledo   2019-04-08 @1463  	err = devlink_info_driver_name_put(req,
92f165ec5b1e76 Vincent Mailhol 2022-11-27  1464  					   dev_driver_string(dev->parent));
a9c8336f65446b Shalom Toledo   2019-04-08  1465  	if (err)
a9c8336f65446b Shalom Toledo   2019-04-08  1466  		return err;
a9c8336f65446b Shalom Toledo   2019-04-08  1467  
a9c8336f65446b Shalom Toledo   2019-04-08  1468  	mlxsw_reg_mgir_pack(mgir_pl);
a9c8336f65446b Shalom Toledo   2019-04-08  1469  	err = mlxsw_reg_query(mlxsw_core, MLXSW_REG(mgir), mgir_pl);
a9c8336f65446b Shalom Toledo   2019-04-08  1470  	if (err)
a9c8336f65446b Shalom Toledo   2019-04-08  1471  		return err;
a9c8336f65446b Shalom Toledo   2019-04-08  1472  	mlxsw_reg_mgir_unpack(mgir_pl, &hw_rev, fw_info_psid, &fw_major,
a9c8336f65446b Shalom Toledo   2019-04-08  1473  			      &fw_minor, &fw_sub_minor);
a9c8336f65446b Shalom Toledo   2019-04-08  1474  
a9c8336f65446b Shalom Toledo   2019-04-08  1475  	sprintf(buf, "%X", hw_rev);
a9c8336f65446b Shalom Toledo   2019-04-08  1476  	err = devlink_info_version_fixed_put(req, "hw.revision", buf);
a9c8336f65446b Shalom Toledo   2019-04-08  1477  	if (err)
a9c8336f65446b Shalom Toledo   2019-04-08  1478  		return err;
a9c8336f65446b Shalom Toledo   2019-04-08  1479  
7dafcc4c9dfb41 Jiri Pirko      2021-05-26  1480  	err = devlink_info_version_fixed_put(req,
7dafcc4c9dfb41 Jiri Pirko      2021-05-26  1481  					     DEVLINK_INFO_VERSION_GENERIC_FW_PSID,
7dafcc4c9dfb41 Jiri Pirko      2021-05-26  1482  					     fw_info_psid);
a9c8336f65446b Shalom Toledo   2019-04-08  1483  	if (err)
a9c8336f65446b Shalom Toledo   2019-04-08  1484  		return err;
a9c8336f65446b Shalom Toledo   2019-04-08  1485  
a9c8336f65446b Shalom Toledo   2019-04-08  1486  	sprintf(buf, "%d.%d.%d", fw_major, fw_minor, fw_sub_minor);
a9c8336f65446b Shalom Toledo   2019-04-08  1487  	err = devlink_info_version_running_put(req, "fw.version", buf);
a9c8336f65446b Shalom Toledo   2019-04-08  1488  	if (err)
a9c8336f65446b Shalom Toledo   2019-04-08  1489  		return err;
a9c8336f65446b Shalom Toledo   2019-04-08  1490  
f55c998c274e3b Jiri Pirko      2021-05-26  1491  	return devlink_info_version_running_put(req,
f55c998c274e3b Jiri Pirko      2021-05-26  1492  						DEVLINK_INFO_VERSION_GENERIC_FW,
f55c998c274e3b Jiri Pirko      2021-05-26  1493  						buf);
a9c8336f65446b Shalom Toledo   2019-04-08  1494  }
a9c8336f65446b Shalom Toledo   2019-04-08  1495  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Hi Vincent,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Vincent-Mailhol/mlxsw-minimal-fix-mlxsw_m_module_get_drvinfo-to-correctly-report-driver-name/20221127-171657
patch link:    https://lore.kernel.org/r/20221127081604.5242-5-mailhol.vincent%40wanadoo.fr
patch subject: [PATCH net-next v2 4/5] net: devlink: remove devlink_info_driver_name_put()
config: x86_64-allyesconfig
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/32018f68afc2b6dca866e1cdf0e5417ef4413d75
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Vincent-Mailhol/mlxsw-minimal-fix-mlxsw_m_module_get_drvinfo-to-correctly-report-driver-name/20221127-171657
        git checkout 32018f68afc2b6dca866e1cdf0e5417ef4413d75
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot [off-list ref]

All errors (new ones prefixed by >>):

   drivers/net/ethernet/mellanox/mlxsw/core.c: In function 'mlxsw_devlink_info_get':

quoted

drivers/net/ethernet/mellanox/mlxsw/core.c:1463:15: error: implicit declaration of function 'devlink_info_driver_name_put'; did you mean 'devlink_info_serial_number_put'? [-Werror=implicit-function-declaration]

    1463 |         err = devlink_info_driver_name_put(req,
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |               devlink_info_serial_number_put
   cc1: some warnings being treated as errors


vim +1463 drivers/net/ethernet/mellanox/mlxsw/core.c

1ceecc88d29bbb Jiri Pirko      2016-04-14  1450  
a9c8336f65446b Shalom Toledo   2019-04-08  1451  static int
a9c8336f65446b Shalom Toledo   2019-04-08  1452  mlxsw_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
a9c8336f65446b Shalom Toledo   2019-04-08  1453  		       struct netlink_ext_ack *extack)
a9c8336f65446b Shalom Toledo   2019-04-08  1454  {
a9c8336f65446b Shalom Toledo   2019-04-08  1455  	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
92f165ec5b1e76 Vincent Mailhol 2022-11-27  1456  	struct device *dev = mlxsw_core->bus_info->dev;
a9c8336f65446b Shalom Toledo   2019-04-08  1457  	char fw_info_psid[MLXSW_REG_MGIR_FW_INFO_PSID_SIZE];
a9c8336f65446b Shalom Toledo   2019-04-08  1458  	u32 hw_rev, fw_major, fw_minor, fw_sub_minor;
a9c8336f65446b Shalom Toledo   2019-04-08  1459  	char mgir_pl[MLXSW_REG_MGIR_LEN];
a9c8336f65446b Shalom Toledo   2019-04-08  1460  	char buf[32];
a9c8336f65446b Shalom Toledo   2019-04-08  1461  	int err;
a9c8336f65446b Shalom Toledo   2019-04-08  1462  
a9c8336f65446b Shalom Toledo   2019-04-08 @1463  	err = devlink_info_driver_name_put(req,
92f165ec5b1e76 Vincent Mailhol 2022-11-27  1464  					   dev_driver_string(dev->parent));
a9c8336f65446b Shalom Toledo   2019-04-08  1465  	if (err)
a9c8336f65446b Shalom Toledo   2019-04-08  1466  		return err;
a9c8336f65446b Shalom Toledo   2019-04-08  1467  
a9c8336f65446b Shalom Toledo   2019-04-08  1468  	mlxsw_reg_mgir_pack(mgir_pl);
a9c8336f65446b Shalom Toledo   2019-04-08  1469  	err = mlxsw_reg_query(mlxsw_core, MLXSW_REG(mgir), mgir_pl);
a9c8336f65446b Shalom Toledo   2019-04-08  1470  	if (err)
a9c8336f65446b Shalom Toledo   2019-04-08  1471  		return err;
a9c8336f65446b Shalom Toledo   2019-04-08  1472  	mlxsw_reg_mgir_unpack(mgir_pl, &hw_rev, fw_info_psid, &fw_major,
a9c8336f65446b Shalom Toledo   2019-04-08  1473  			      &fw_minor, &fw_sub_minor);
a9c8336f65446b Shalom Toledo   2019-04-08  1474  
a9c8336f65446b Shalom Toledo   2019-04-08  1475  	sprintf(buf, "%X", hw_rev);
a9c8336f65446b Shalom Toledo   2019-04-08  1476  	err = devlink_info_version_fixed_put(req, "hw.revision", buf);
a9c8336f65446b Shalom Toledo   2019-04-08  1477  	if (err)
a9c8336f65446b Shalom Toledo   2019-04-08  1478  		return err;
a9c8336f65446b Shalom Toledo   2019-04-08  1479  
7dafcc4c9dfb41 Jiri Pirko      2021-05-26  1480  	err = devlink_info_version_fixed_put(req,
7dafcc4c9dfb41 Jiri Pirko      2021-05-26  1481  					     DEVLINK_INFO_VERSION_GENERIC_FW_PSID,
7dafcc4c9dfb41 Jiri Pirko      2021-05-26  1482  					     fw_info_psid);
a9c8336f65446b Shalom Toledo   2019-04-08  1483  	if (err)
a9c8336f65446b Shalom Toledo   2019-04-08  1484  		return err;
a9c8336f65446b Shalom Toledo   2019-04-08  1485  
a9c8336f65446b Shalom Toledo   2019-04-08  1486  	sprintf(buf, "%d.%d.%d", fw_major, fw_minor, fw_sub_minor);
a9c8336f65446b Shalom Toledo   2019-04-08  1487  	err = devlink_info_version_running_put(req, "fw.version", buf);
a9c8336f65446b Shalom Toledo   2019-04-08  1488  	if (err)
a9c8336f65446b Shalom Toledo   2019-04-08  1489  		return err;
a9c8336f65446b Shalom Toledo   2019-04-08  1490  
f55c998c274e3b Jiri Pirko      2021-05-26  1491  	return devlink_info_version_running_put(req,
f55c998c274e3b Jiri Pirko      2021-05-26  1492  						DEVLINK_INFO_VERSION_GENERIC_FW,
f55c998c274e3b Jiri Pirko      2021-05-26  1493  						buf);
a9c8336f65446b Shalom Toledo   2019-04-08  1494  }
a9c8336f65446b Shalom Toledo   2019-04-08  1495  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

The driver name is available in device_driver::name. Right now,
drivers still have to report this piece of information themselves in
their devlink_ops::info_get callback function.

The goal of this series is to have the devlink core to report this
information instead of the drivers.

The first two patches clean up the mlxsw driver for both the ethtool
and the devlink (both are supposed to return the same information so
the ethtool got included as well). This is split in two patches
because of the different Fixes tag.

The third patch fulfills the actual goal of this series: modify
devlink core to report the driver name and clean-up all drivers. Both
as to be done in an atomic change to avoid attribute duplication.

The fourth patch removes the devlink_info_driver_name_put() function
to prevent future drivers from reporting the driver name themselves.

Finally, the fifth and last patch allows the core to call
devlink_nl_info_fill() even if the devlink_ops::info_get() callback is
NULL. This allows to do further more clean up in the drivers.
---
* Changelog *

v2 -> v3

  * [PATCH 3/5] remove the call to devlink_info_driver_name_put() in
    mlxsw driver as well (this was missing in v2, making the build
    fail... sorry for the noise).

  * add additional people in CC as pointed by netdev patchwork CI:
    https://patchwork.kernel.org/project/netdevbpf/list/?series=699451

  * Use the "Link:" prefix before URL to silence checkpatch's line
    length warning.

RFC v1 -> v2

  * drop the RFC tag

  * big rework following the discussion on RFC:
    https://lore.kernel.org/netdev/20221122154934.13937-1-mailhol.vincent@wanadoo.fr/
    Went from one patch to a series of five patches:

  * drop the idea to report the USB serial number following Greg's
    comment:
    https://lore.kernel.org/linux-usb/Y3+VfNdt%2FK7UtRcw@kroah.com/

Vincent Mailhol (5):
  mlxsw: minimal: fix mlxsw_m_module_get_drvinfo() to correctly report
    driver name
  mlxsw: core: fix mlxsw_devlink_info_get() to correctly report driver
    name
  net: devlink: let the core report the driver name instead of the
    drivers
  net: devlink: remove devlink_info_driver_name_put()
  net: devlink: make the devlink_ops::info_get() callback optional

 .../marvell/octeontx2/otx2_cpt_devlink.c      |  4 ---
 drivers/net/dsa/hirschmann/hellcreek.c        |  5 ---
 drivers/net/dsa/mv88e6xxx/devlink.c           |  5 ---
 drivers/net/dsa/sja1105/sja1105_devlink.c     | 12 ++-----
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c |  4 ---
 .../freescale/dpaa2/dpaa2-eth-devlink.c       | 11 +-----
 .../ethernet/fungible/funeth/funeth_devlink.c |  7 ----
 .../hisilicon/hns3/hns3pf/hclge_devlink.c     |  5 ---
 .../hisilicon/hns3/hns3vf/hclgevf_devlink.c   |  5 ---
 drivers/net/ethernet/intel/ice/ice_devlink.c  |  6 ----
 .../marvell/octeontx2/af/rvu_devlink.c        |  7 ----
 .../marvell/octeontx2/nic/otx2_devlink.c      | 15 --------
 .../marvell/prestera/prestera_devlink.c       |  5 ---
 .../net/ethernet/mellanox/mlx5/core/devlink.c |  4 ---
 drivers/net/ethernet/mellanox/mlxsw/core.c    |  5 ---
 drivers/net/ethernet/mellanox/mlxsw/minimal.c |  2 +-
 .../net/ethernet/netronome/nfp/nfp_devlink.c  |  4 ---
 .../ethernet/pensando/ionic/ionic_devlink.c   |  4 ---
 drivers/net/ethernet/qlogic/qed/qed_devlink.c |  4 ---
 drivers/net/netdevsim/dev.c                   |  3 --
 drivers/ptp/ptp_ocp.c                         |  4 ---
 include/net/devlink.h                         |  2 --
 net/core/devlink.c                            | 35 ++++++++++++-------
 23 files changed, 27 insertions(+), 131 deletions(-)

-- 
2.37.4

Currently, mlxsw_m_module_get_drvinfo() reports the device_kind. The
device_kind is not necessarily the same as the device_name. For
example, the mlxsw_i2c implementation sets up the device_kind as
ic2_client::name in [1] which indicates the type of the device
(e.g. chip name), not the actual driver name.

Fix it so that it correctly reports the driver name.

[1] mlxsw_i2c_probe() from drivers/net/ethernet/mellanox/mlxsw/i2c.c
Link: https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/net/ethernet/mellanox/mlxsw/i2c.c#L714

Fixes: 9bbd7efbc055 ("mlxsw: i2c: Extend initialization with querying firmware info")
CC: Shalom Toledo <redacted>
CC: Ido Schimmel <redacted>
CC: Vadim Pasternak <redacted>
Signed-off-by: Vincent Mailhol <redacted>
---
 drivers/net/ethernet/mellanox/mlxsw/minimal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/minimal.c b/drivers/net/ethernet/mellanox/mlxsw/minimal.c
index 6b56eadd736e..9b37ddbe0cba 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/minimal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/minimal.c

@@ -92,7 +92,7 @@ static void mlxsw_m_module_get_drvinfo(struct net_device *dev,
 	struct mlxsw_m_port *mlxsw_m_port = netdev_priv(dev);
 	struct mlxsw_m *mlxsw_m = mlxsw_m_port->mlxsw_m;
 
-	strscpy(drvinfo->driver, mlxsw_m->bus_info->device_kind,
+	strscpy(drvinfo->driver, dev_driver_string(dev->dev.parent),
 		sizeof(drvinfo->driver));
 	snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
 		 "%d.%d.%d",

-- 
2.37.4

Currently, mlxsw_devlink_info_get() reports the device_kind. The
device_kind is not necessarily the same as the device_name. For
example, the mlxsw_i2c implementation sets up the device_kind as
ic2_client::name in [1] which indicates the type of the device
(e.g. chip name).

Fix it so that it correctly reports the driver name.

[1] mlxsw_i2c_probe() from drivers/net/ethernet/mellanox/mlxsw/i2c.c
Link: https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/net/ethernet/mellanox/mlxsw/i2c.c#L714

Fixes: a9c8336f6544 ("mlxsw: core: Add support for devlink info command")
CC: Shalom Toledo <redacted>
CC: Ido Schimmel <redacted>
CC: Vadim Pasternak <redacted>
Signed-off-by: Vincent Mailhol <redacted>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index a83f6bc30072..d8b1bb03cdb0 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c

@@ -1453,6 +1453,7 @@ mlxsw_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 		       struct netlink_ext_ack *extack)
 {
 	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
+	struct device *dev = mlxsw_core->bus_info->dev;
 	char fw_info_psid[MLXSW_REG_MGIR_FW_INFO_PSID_SIZE];
 	u32 hw_rev, fw_major, fw_minor, fw_sub_minor;
 	char mgir_pl[MLXSW_REG_MGIR_LEN];

@@ -1460,7 +1461,7 @@ mlxsw_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 	int err;
 
 	err = devlink_info_driver_name_put(req,
-					   mlxsw_core->bus_info->device_kind);
+					   dev_driver_string(dev->parent));
 	if (err)
 		return err;
 

-- 
2.37.4

The driver name is available in device_driver::name. Right now,
drivers still have to report this piece of information themselves in
their devlink_ops::info_get callback function.

In order to factorize code, make devlink_nl_info_fill() add the driver
name attribute.

nla_put() does not check if an attribute already exists and
unconditionally reserves new space [1]. To avoid attribute
duplication, clean-up all the drivers which are currently reporting
the driver name in their callback.

[1] https://elixir.bootlin.com/linux/v6.0/source/lib/nlattr.c#L993

Signed-off-by: Vincent Mailhol <redacted>
---
 .../crypto/marvell/octeontx2/otx2_cpt_devlink.c |  4 ----
 drivers/net/dsa/hirschmann/hellcreek.c          |  5 -----
 drivers/net/dsa/mv88e6xxx/devlink.c             |  5 -----
 drivers/net/dsa/sja1105/sja1105_devlink.c       | 12 +++---------
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c   |  4 ----
 .../freescale/dpaa2/dpaa2-eth-devlink.c         | 11 +----------
 .../ethernet/fungible/funeth/funeth_devlink.c   |  2 +-
 .../hisilicon/hns3/hns3pf/hclge_devlink.c       |  5 -----
 .../hisilicon/hns3/hns3vf/hclgevf_devlink.c     |  5 -----
 drivers/net/ethernet/intel/ice/ice_devlink.c    |  6 ------
 .../ethernet/marvell/octeontx2/af/rvu_devlink.c |  2 +-
 .../marvell/octeontx2/nic/otx2_devlink.c        |  9 +--------
 .../marvell/prestera/prestera_devlink.c         |  5 -----
 .../net/ethernet/mellanox/mlx5/core/devlink.c   |  4 ----
 drivers/net/ethernet/mellanox/mlxsw/core.c      |  6 ------
 .../net/ethernet/netronome/nfp/nfp_devlink.c    |  4 ----
 .../net/ethernet/pensando/ionic/ionic_devlink.c |  4 ----
 drivers/net/ethernet/qlogic/qed/qed_devlink.c   |  4 ----
 drivers/net/netdevsim/dev.c                     |  3 ---
 drivers/ptp/ptp_ocp.c                           |  4 ----
 net/core/devlink.c                              | 17 +++++++++++++++++
 21 files changed, 24 insertions(+), 97 deletions(-)

diff --git a/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c b/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c
index 7503f6b18ac5..a2aba0b0d68a 100644
--- a/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c
+++ b/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c

@@ -76,10 +76,6 @@ static int otx2_cpt_devlink_info_get(struct devlink *dl,
 	struct otx2_cptpf_dev *cptpf = cpt_dl->cptpf;
 	int err;
 
-	err = devlink_info_driver_name_put(req, "rvu_cptpf");
-	if (err)
-		return err;
-
 	err = otx2_cpt_dl_info_firmware_version_put(req, cptpf->eng_grps.grp,
 						    "fw.ae", OTX2_CPT_AE_TYPES);
 	if (err)

diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 951f7935c872..595a548bb0a8 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c

@@ -1176,11 +1176,6 @@ static int hellcreek_devlink_info_get(struct dsa_switch *ds,
 				      struct netlink_ext_ack *extack)
 {
 	struct hellcreek *hellcreek = ds->priv;
-	int ret;
-
-	ret = devlink_info_driver_name_put(req, "hellcreek");
-	if (ret)
-		return ret;
 
 	return devlink_info_version_fixed_put(req,
 					      DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,

diff --git a/drivers/net/dsa/mv88e6xxx/devlink.c b/drivers/net/dsa/mv88e6xxx/devlink.c
index 1266eabee086..a08dab75e0c0 100644
--- a/drivers/net/dsa/mv88e6xxx/devlink.c
+++ b/drivers/net/dsa/mv88e6xxx/devlink.c

@@ -821,11 +821,6 @@ int mv88e6xxx_devlink_info_get(struct dsa_switch *ds,
 			       struct netlink_ext_ack *extack)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
-	int err;
-
-	err = devlink_info_driver_name_put(req, "mv88e6xxx");
-	if (err)
-		return err;
 
 	return devlink_info_version_fixed_put(req,
 					      DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,

diff --git a/drivers/net/dsa/sja1105/sja1105_devlink.c b/drivers/net/dsa/sja1105/sja1105_devlink.c
index 10c6fea1227f..da532614f34a 100644
--- a/drivers/net/dsa/sja1105/sja1105_devlink.c
+++ b/drivers/net/dsa/sja1105/sja1105_devlink.c

@@ -120,16 +120,10 @@ int sja1105_devlink_info_get(struct dsa_switch *ds,
 			     struct netlink_ext_ack *extack)
 {
 	struct sja1105_private *priv = ds->priv;
-	int rc;
-
-	rc = devlink_info_driver_name_put(req, "sja1105");
-	if (rc)
-		return rc;
 
-	rc = devlink_info_version_fixed_put(req,
-					    DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,
-					    priv->info->name);
-	return rc;
+	return devlink_info_version_fixed_put(req,
+					      DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,
+					      priv->info->name);
 }
 
 int sja1105_devlink_setup(struct dsa_switch *ds)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 8a6f788f6294..26913dc816d3 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c

@@ -892,10 +892,6 @@ static int bnxt_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 	u32 ver = 0;
 	int rc;
 
-	rc = devlink_info_driver_name_put(req, DRV_MODULE_NAME);
-	if (rc)
-		return rc;
-
 	if (BNXT_PF(bp) && (bp->flags & BNXT_FLAG_DSN_VALID)) {
 		sprintf(buf, "%02X-%02X-%02X-%02X-%02X-%02X-%02X-%02X",
 			bp->dsn[7], bp->dsn[6], bp->dsn[5], bp->dsn[4],

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c
index 5c6dd3029e2f..76f808d38066 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c

@@ -37,18 +37,9 @@ static int dpaa2_eth_dl_info_get(struct devlink *devlink,
 	struct dpaa2_eth_devlink_priv *dl_priv = devlink_priv(devlink);
 	struct dpaa2_eth_priv *priv = dl_priv->dpaa2_priv;
 	char buf[10];
-	int err;
-
-	err = devlink_info_driver_name_put(req, KBUILD_MODNAME);
-	if (err)
-		return err;
 
 	scnprintf(buf, 10, "%d.%d", priv->dpni_ver_major, priv->dpni_ver_minor);
-	err = devlink_info_version_running_put(req, "dpni", buf);
-	if (err)
-		return err;
-
-	return 0;
+	return devlink_info_version_running_put(req, "dpni", buf);
 }
 
 static struct dpaa2_eth_trap_item *

diff --git a/drivers/net/ethernet/fungible/funeth/funeth_devlink.c b/drivers/net/ethernet/fungible/funeth/funeth_devlink.c
index d50c222948b4..6668375edff6 100644
--- a/drivers/net/ethernet/fungible/funeth/funeth_devlink.c
+++ b/drivers/net/ethernet/fungible/funeth/funeth_devlink.c

@@ -6,7 +6,7 @@
 static int fun_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 			   struct netlink_ext_ack *extack)
 {
-	return devlink_info_driver_name_put(req, KBUILD_MODNAME);
+	return 0;
 }
 
 static const struct devlink_ops fun_dl_ops = {

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
index 4c441e6a5082..3d3b69605423 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c

@@ -13,11 +13,6 @@ static int hclge_devlink_info_get(struct devlink *devlink,
 	struct hclge_devlink_priv *priv = devlink_priv(devlink);
 	char version_str[HCLGE_DEVLINK_FW_STRING_LEN];
 	struct hclge_dev *hdev = priv->hdev;
-	int ret;
-
-	ret = devlink_info_driver_name_put(req, KBUILD_MODNAME);
-	if (ret)
-		return ret;
 
 	snprintf(version_str, sizeof(version_str), "%lu.%lu.%lu.%lu",
 		 hnae3_get_field(hdev->fw_version, HNAE3_FW_VERSION_BYTE3_MASK,

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c
index fdc19868b818..a6c3c5e8f0ab 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c

@@ -13,11 +13,6 @@ static int hclgevf_devlink_info_get(struct devlink *devlink,
 	struct hclgevf_devlink_priv *priv = devlink_priv(devlink);
 	char version_str[HCLGEVF_DEVLINK_FW_STRING_LEN];
 	struct hclgevf_dev *hdev = priv->hdev;
-	int ret;
-
-	ret = devlink_info_driver_name_put(req, KBUILD_MODNAME);
-	if (ret)
-		return ret;
 
 	snprintf(version_str, sizeof(version_str), "%lu.%lu.%lu.%lu",
 		 hnae3_get_field(hdev->fw_version, HNAE3_FW_VERSION_BYTE3_MASK,

diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
index 1d638216484d..ba74977e75dc 100644
--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c

@@ -311,12 +311,6 @@ static int ice_devlink_info_get(struct devlink *devlink,
 		}
 	}
 
-	err = devlink_info_driver_name_put(req, KBUILD_MODNAME);
-	if (err) {
-		NL_SET_ERR_MSG_MOD(extack, "Unable to set driver name");
-		goto out_free_ctx;
-	}
-
 	ice_info_get_dsn(pf, ctx);
 
 	err = devlink_info_serial_number_put(req, ctx->buf);

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
index 88dee589cb21..f15439d26d21 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c

@@ -1550,7 +1550,7 @@ static int rvu_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
 static int rvu_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 				struct netlink_ext_ack *extack)
 {
-	return devlink_info_driver_name_put(req, DRV_NAME);
+	return 0;
 }
 
 static const struct devlink_ops rvu_devlink_ops = {

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
index 777a27047c8e..5cc6416cf1a6 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c

@@ -77,18 +77,11 @@ static const struct devlink_param otx2_dl_params[] = {
 			     otx2_dl_mcam_count_validate),
 };
 
-/* Devlink OPs */
 static int otx2_devlink_info_get(struct devlink *devlink,
 				 struct devlink_info_req *req,
 				 struct netlink_ext_ack *extack)
 {
-	struct otx2_devlink *otx2_dl = devlink_priv(devlink);
-	struct otx2_nic *pfvf = otx2_dl->pfvf;
-
-	if (is_otx2_vf(pfvf->pcifunc))
-		return devlink_info_driver_name_put(req, "rvu_nicvf");
-
-	return devlink_info_driver_name_put(req, "rvu_nicpf");
+	return 0;
 }
 
 static const struct devlink_ops otx2_devlink_ops = {

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_devlink.c b/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
index 84ad05c9f12d..2a4c9df4eb79 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_devlink.c

@@ -355,11 +355,6 @@ static int prestera_dl_info_get(struct devlink *dl,
 {
 	struct prestera_switch *sw = devlink_priv(dl);
 	char buf[16];
-	int err;
-
-	err = devlink_info_driver_name_put(req, PRESTERA_DRV_NAME);
-	if (err)
-		return err;
 
 	snprintf(buf, sizeof(buf), "%d.%d.%d",
 		 sw->dev->fw_rev.maj,

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index cc2ae427dcb0..751bc4a9edcf 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c

@@ -46,10 +46,6 @@ mlx5_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 	u32 running_fw, stored_fw;
 	int err;
 
-	err = devlink_info_driver_name_put(req, KBUILD_MODNAME);
-	if (err)
-		return err;
-
 	err = devlink_info_version_fixed_put(req, "fw.psid", dev->board_id);
 	if (err)
 		return err;

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index d8b1bb03cdb0..a0a06e2eff82 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c

@@ -1453,18 +1453,12 @@ mlxsw_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 		       struct netlink_ext_ack *extack)
 {
 	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
-	struct device *dev = mlxsw_core->bus_info->dev;
 	char fw_info_psid[MLXSW_REG_MGIR_FW_INFO_PSID_SIZE];
 	u32 hw_rev, fw_major, fw_minor, fw_sub_minor;
 	char mgir_pl[MLXSW_REG_MGIR_LEN];
 	char buf[32];
 	int err;
 
-	err = devlink_info_driver_name_put(req,
-					   dev_driver_string(dev->parent));
-	if (err)
-		return err;
-
 	mlxsw_reg_mgir_pack(mgir_pl);
 	err = mlxsw_reg_query(mlxsw_core, MLXSW_REG(mgir), mgir_pl);
 	if (err)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index 8bfd48d50ef0..4c601ff09cd3 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c

@@ -239,10 +239,6 @@ nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 	char *buf = NULL;
 	int err;
 
-	err = devlink_info_driver_name_put(req, "nfp");
-	if (err)
-		return err;
-
 	vendor = nfp_hwinfo_lookup(pf->hwinfo, "assembly.vendor");
 	part = nfp_hwinfo_lookup(pf->hwinfo, "assembly.partno");
 	sn = nfp_hwinfo_lookup(pf->hwinfo, "assembly.serial");

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
index 567f778433e2..e6ff757895ab 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c

@@ -26,10 +26,6 @@ static int ionic_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 	char buf[16];
 	int err = 0;
 
-	err = devlink_info_driver_name_put(req, IONIC_DRV_NAME);
-	if (err)
-		return err;
-
 	err = devlink_info_version_running_put(req,
 					       DEVLINK_INFO_VERSION_GENERIC_FW,
 					       idev->dev_info.fw_version);

diff --git a/drivers/net/ethernet/qlogic/qed/qed_devlink.c b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
index 6bb4e165b592..922c47797af6 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_devlink.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_devlink.c

@@ -162,10 +162,6 @@ static int qed_devlink_info_get(struct devlink *devlink,
 
 	dev_info = &cdev->common_dev_info;
 
-	err = devlink_info_driver_name_put(req, KBUILD_MODNAME);
-	if (err)
-		return err;
-
 	memcpy(buf, cdev->hwfns[0].hw_info.part_num, sizeof(cdev->hwfns[0].hw_info.part_num));
 	buf[sizeof(cdev->hwfns[0].hw_info.part_num)] = 0;
 

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index e14686594a71..b962fc8e1397 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c

@@ -994,9 +994,6 @@ static int nsim_dev_info_get(struct devlink *devlink,
 {
 	int err;
 
-	err = devlink_info_driver_name_put(req, DRV_NAME);
-	if (err)
-		return err;
 	err = devlink_info_version_stored_put_ext(req, "fw.mgmt", "10.20.30",
 						  DEVLINK_INFO_VERSION_TYPE_COMPONENT);
 	if (err)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 154d58cbd9ce..4bbaccd543ad 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c

@@ -1647,10 +1647,6 @@ ptp_ocp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 	char buf[32];
 	int err;
 
-	err = devlink_info_driver_name_put(req, KBUILD_MODNAME);
-	if (err)
-		return err;
-
 	fw_image = bp->fw_loader ? "loader" : "fw";
 	sprintf(buf, "%d.%d", bp->fw_tag, bp->fw_version);
 	err = devlink_info_version_running_put(req, fw_image, buf);

diff --git a/net/core/devlink.c b/net/core/devlink.c
index cea154ddce7a..6478135d9ba1 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c

@@ -6749,11 +6749,24 @@ int devlink_info_version_running_put_ext(struct devlink_info_req *req,
 }
 EXPORT_SYMBOL_GPL(devlink_info_version_running_put_ext);
 
+static int devlink_nl_driver_info_get(struct device_driver *drv,
+				      struct devlink_info_req *req)
+{
+	if (!drv)
+		return 0;
+
+	if (drv->name[0])
+		return devlink_info_driver_name_put(req, drv->name);
+
+	return 0;
+}
+
 static int
 devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
 		     enum devlink_command cmd, u32 portid,
 		     u32 seq, int flags, struct netlink_ext_ack *extack)
 {
+	struct device *dev = devlink_to_dev(devlink);
 	struct devlink_info_req req = {};
 	void *hdr;
 	int err;

@@ -6771,6 +6784,10 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
 	if (err)
 		goto err_cancel_msg;
 
+	err = devlink_nl_driver_info_get(dev->driver, &req);
+	if (err)
+		goto err_cancel_msg;
+
 	genlmsg_end(msg, hdr);
 	return 0;
 

-- 
2.37.4

Now that the core sets the driver name attribute, drivers are not
supposed to call devlink_info_driver_name_put() anymore. Remove it.

Signed-off-by: Vincent Mailhol <redacted>
---
 include/net/devlink.h |  2 --
 net/core/devlink.c    | 11 ++---------
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 074a79b8933f..52d5fb67e9b8 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h

@@ -1746,8 +1746,6 @@ int devlink_region_snapshot_create(struct devlink_region *region,
 				   u8 *data, u32 snapshot_id);
 int devlink_info_serial_number_put(struct devlink_info_req *req,
 				   const char *sn);
-int devlink_info_driver_name_put(struct devlink_info_req *req,
-				 const char *name);
 int devlink_info_board_serial_number_put(struct devlink_info_req *req,
 					 const char *bsn);
 

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 6478135d9ba1..3babc16eeb6b 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c

@@ -6633,14 +6633,6 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
 	return err;
 }
 
-int devlink_info_driver_name_put(struct devlink_info_req *req, const char *name)
-{
-	if (!req->msg)
-		return 0;
-	return nla_put_string(req->msg, DEVLINK_ATTR_INFO_DRIVER_NAME, name);
-}
-EXPORT_SYMBOL_GPL(devlink_info_driver_name_put);
-
 int devlink_info_serial_number_put(struct devlink_info_req *req, const char *sn)
 {
 	if (!req->msg)

@@ -6756,7 +6748,8 @@ static int devlink_nl_driver_info_get(struct device_driver *drv,
 		return 0;
 
 	if (drv->name[0])
-		return devlink_info_driver_name_put(req, drv->name);
+		return nla_put_string(req->msg, DEVLINK_ATTR_INFO_DRIVER_NAME,
+				      drv->name);
 
 	return 0;
 }

-- 
2.37.4

Some drivers only reported the driver name in their
devlink_ops::info_get() callback. Now that the core provides this
information, the callback became empty. For such drivers, just
removing the callback would prevent the core from executing
devlink_nl_info_fill() meaning that "devlink dev info" would not
return anything.

Make the callback function optional by executing
devlink_nl_info_fill() even if devlink_ops::info_get() is NULL.

Remove all the empty devlink_ops::info_get() functions from the
drivers.

N.B.: the drivers with devlink support which previously did not
implement devlink_ops::info_get() will now also be able to report
the driver name.

Signed-off-by: Vincent Mailhol <redacted>
---
 .../net/ethernet/fungible/funeth/funeth_devlink.c   |  7 -------
 .../net/ethernet/marvell/octeontx2/af/rvu_devlink.c |  7 -------
 .../ethernet/marvell/octeontx2/nic/otx2_devlink.c   |  8 --------
 net/core/devlink.c                                  | 13 ++++++-------
 4 files changed, 6 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/fungible/funeth/funeth_devlink.c b/drivers/net/ethernet/fungible/funeth/funeth_devlink.c
index 6668375edff6..4fbeb3fd71a8 100644
--- a/drivers/net/ethernet/fungible/funeth/funeth_devlink.c
+++ b/drivers/net/ethernet/fungible/funeth/funeth_devlink.c

@@ -3,14 +3,7 @@
 #include "funeth.h"
 #include "funeth_devlink.h"
 
-static int fun_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
-			   struct netlink_ext_ack *extack)
-{
-	return 0;
-}
-
 static const struct devlink_ops fun_dl_ops = {
-	.info_get = fun_dl_info_get,
 };
 
 struct devlink *fun_devlink_alloc(struct device *dev)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
index f15439d26d21..bda1a6fa2ec4 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c

@@ -1547,14 +1547,7 @@ static int rvu_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
 	return 0;
 }
 
-static int rvu_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
-				struct netlink_ext_ack *extack)
-{
-	return 0;
-}
-
 static const struct devlink_ops rvu_devlink_ops = {
-	.info_get = rvu_devlink_info_get,
 	.eswitch_mode_get = rvu_devlink_eswitch_mode_get,
 	.eswitch_mode_set = rvu_devlink_eswitch_mode_set,
 };

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
index 5cc6416cf1a6..63ef7c41d18d 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c

@@ -77,15 +77,7 @@ static const struct devlink_param otx2_dl_params[] = {
 			     otx2_dl_mcam_count_validate),
 };
 
-static int otx2_devlink_info_get(struct devlink *devlink,
-				 struct devlink_info_req *req,
-				 struct netlink_ext_ack *extack)
-{
-	return 0;
-}
-
 static const struct devlink_ops otx2_devlink_ops = {
-	.info_get = otx2_devlink_info_get,
 };
 
 int otx2_register_dl(struct otx2_nic *pfvf)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 3babc16eeb6b..817d978bb729 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c

@@ -6773,9 +6773,11 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
 		goto err_cancel_msg;
 
 	req.msg = msg;
-	err = devlink->ops->info_get(devlink, &req, extack);
-	if (err)
-		goto err_cancel_msg;
+	if (devlink->ops->info_get) {
+		err = devlink->ops->info_get(devlink, &req, extack);
+		if (err)
+			goto err_cancel_msg;
+	}
 
 	err = devlink_nl_driver_info_get(dev->driver, &req);
 	if (err)