| Message ID | 20221108035754.2143-1-mailhol.vincent@wanadoo.fr (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | edaf5df22cb8e7e849773ce69fcc9bc20ca92160 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v1] ethtool: ethtool_get_drvinfo: populate drvinfo fields even if callback exits | expand |
On Tue, Nov 08, 2022 at 12:57:54PM +0900, Vincent Mailhol wrote: > If ethtool_ops::get_drvinfo() callback isn't set, > ethtool_get_drvinfo() will fill the ethtool_drvinfo::name and > ethtool_drvinfo::bus_info fields. > > However, if the driver provides the callback function, those two > fields are not touched. This means that the driver has to fill these > itself. Can you please point to such drivers? One can argue that they don't need to touch these fields in a first place and ethtool_drvinfo should always overwrite them. Thanks
On Wed, 9 Nov 2022 19:52:13 +0200 Leon Romanovsky wrote: > On Tue, Nov 08, 2022 at 12:57:54PM +0900, Vincent Mailhol wrote: > > If ethtool_ops::get_drvinfo() callback isn't set, > > ethtool_get_drvinfo() will fill the ethtool_drvinfo::name and > > ethtool_drvinfo::bus_info fields. > > > > However, if the driver provides the callback function, those two > > fields are not touched. This means that the driver has to fill these > > itself. > > Can you please point to such drivers? What you mean by "such drivers" is not clear from the quoted context, at least to me. > One can argue that they don't need to touch these fields in a first > place and ethtool_drvinfo should always overwrite them. Quite likely most driver prints to .driver and .bus_info can be dropped with this patch in place. Then again, I'm suspecting it's a bit of a chicken and an egg problem with people adding new drivers not having an incentive to add the print in the core and people who want to add the print in the core not having any driver that would benefit. Therefore I'd lean towards accepting Vincent's patch as is even if the submission can likely be more thorough and strict. While I'm typing - I've used dev_driver_string() to get the driver name in the past. Perhaps something to consider?
On Thu. 10 nov. 2022 at 05:26, Jakub Kicinski <kuba@kernel.org> wrote: > On Wed, 9 Nov 2022 19:52:13 +0200 Leon Romanovsky wrote: > > On Tue, Nov 08, 2022 at 12:57:54PM +0900, Vincent Mailhol wrote: > > > If ethtool_ops::get_drvinfo() callback isn't set, > > > ethtool_get_drvinfo() will fill the ethtool_drvinfo::name and > > > ethtool_drvinfo::bus_info fields. > > > > > > However, if the driver provides the callback function, those two > > > fields are not touched. This means that the driver has to fill these > > > itself. > > > > Can you please point to such drivers? > > What you mean by "such drivers" is not clear from the quoted context, > at least to me. An example: https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/broadcom/bnx2.c#L7041 This driver wants to set fw_version but needs to also fill the driver name and bus_info. My patch will enable *such drivers* to only fill the fw_version and delegate the rest to the core. > > One can argue that they don't need to touch these fields in a first > > place and ethtool_drvinfo should always overwrite them. > > Quite likely most driver prints to .driver and .bus_info can be dropped > with this patch in place. Then again, I'm suspecting it's a bit of a > chicken and an egg problem with people adding new drivers not having > an incentive to add the print in the core and people who want to add > the print in the core not having any driver that would benefit. > Therefore I'd lean towards accepting Vincent's patch as is even if > the submission can likely be more thorough and strict. If we can agree that no drivers should ever print .driver and .bus_info, then I am fine to send a clean-up patch to remove all this after this one gets accepted. However, I am not willing to invest time for nothing. So would one of you be ready to sign-off such a clean-up patch? > While I'm typing - I've used dev_driver_string() to get the driver > name in the past. Perhaps something to consider? I am not sure of that one. If dev->dev.parent->driver is not set, it defaults to dev_bus_name() which is .bus_info, isn't it? https://elixir.bootlin.com/linux/latest/source/drivers/base/core.c#L2181 For the end user, it might be better to display an empty driver name in 'ethtool -i' rather than reporting the bus_info twice? I mean, if you ask me for my opinion, then my answer is "I am not sure". If you have confidence that dev_driver_string() is better, then I will send a v2 right away. Yours sincerely, Vincent Mailhol
On Thu, Nov 10, 2022 at 05:34:55PM +0900, Vincent MAILHOL wrote: > On Thu. 10 nov. 2022 at 05:26, Jakub Kicinski <kuba@kernel.org> wrote: > > On Wed, 9 Nov 2022 19:52:13 +0200 Leon Romanovsky wrote: > > > On Tue, Nov 08, 2022 at 12:57:54PM +0900, Vincent Mailhol wrote: > > > > If ethtool_ops::get_drvinfo() callback isn't set, > > > > ethtool_get_drvinfo() will fill the ethtool_drvinfo::name and > > > > ethtool_drvinfo::bus_info fields. > > > > > > > > However, if the driver provides the callback function, those two > > > > fields are not touched. This means that the driver has to fill these > > > > itself. > > > > > > Can you please point to such drivers? > > > > What you mean by "such drivers" is not clear from the quoted context, > > at least to me. > > An example: > https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/broadcom/bnx2.c#L7041 > > This driver wants to set fw_version but needs to also fill the driver > name and bus_info. My patch will enable *such drivers* to only fill > the fw_version and delegate the rest to the core. Sorry for being misleading, It looks like I typed only part of the sentence which I had in my mind. I wanted to see if any driver exists which prints drv_name and bus_info different from default. > > > > One can argue that they don't need to touch these fields in a first > > > place and ethtool_drvinfo should always overwrite them. > > > > Quite likely most driver prints to .driver and .bus_info can be dropped > > with this patch in place. Then again, I'm suspecting it's a bit of a > > chicken and an egg problem with people adding new drivers not having > > an incentive to add the print in the core and people who want to add > > the print in the core not having any driver that would benefit. > > Therefore I'd lean towards accepting Vincent's patch as is even if > > the submission can likely be more thorough and strict. > > If we can agree that no drivers should ever print .driver and > .bus_info, then I am fine to send a clean-up patch to remove all this > after this one gets accepted. However, I am not willing to invest time > for nothing. So would one of you be ready to sign-off such a clean-up > patch? I will be happy to see such patch and will review it, but can't add sign-off as I'm not netdev maintainer. Thanks
On Thu. 10 Nov. 2022 at 18:11, Leon Romanovsky <leon@kernel.org> wrote: > On Thu, Nov 10, 2022 at 05:34:55PM +0900, Vincent MAILHOL wrote: > > On Thu. 10 nov. 2022 at 05:26, Jakub Kicinski <kuba@kernel.org> wrote: > > > On Wed, 9 Nov 2022 19:52:13 +0200 Leon Romanovsky wrote: > > > > On Tue, Nov 08, 2022 at 12:57:54PM +0900, Vincent Mailhol wrote: > > > > > If ethtool_ops::get_drvinfo() callback isn't set, > > > > > ethtool_get_drvinfo() will fill the ethtool_drvinfo::name and > > > > > ethtool_drvinfo::bus_info fields. > > > > > > > > > > However, if the driver provides the callback function, those two > > > > > fields are not touched. This means that the driver has to fill these > > > > > itself. > > > > > > > > Can you please point to such drivers? > > > > > > What you mean by "such drivers" is not clear from the quoted context, > > > at least to me. > > > > An example: > > https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/broadcom/bnx2.c#L7041 > > > > This driver wants to set fw_version but needs to also fill the driver > > name and bus_info. My patch will enable *such drivers* to only fill > > the fw_version and delegate the rest to the core. > > Sorry for being misleading, It looks like I typed only part of the sentence > which I had in my mind. I wanted to see if any driver exists which prints > drv_name and bus_info different from default. > > > > > > > One can argue that they don't need to touch these fields in a first > > > > place and ethtool_drvinfo should always overwrite them. > > > > > > Quite likely most driver prints to .driver and .bus_info can be dropped > > > with this patch in place. Then again, I'm suspecting it's a bit of a > > > chicken and an egg problem with people adding new drivers not having > > > an incentive to add the print in the core and people who want to add > > > the print in the core not having any driver that would benefit. > > > Therefore I'd lean towards accepting Vincent's patch as is even if > > > the submission can likely be more thorough and strict. > > > > If we can agree that no drivers should ever print .driver and > > .bus_info, then I am fine to send a clean-up patch to remove all this > > after this one gets accepted. However, I am not willing to invest time > > for nothing. So would one of you be ready to sign-off such a clean-up > > patch? > > I will be happy to see such patch and will review it, but can't add sign-off > as I'm not netdev maintainer. Well, if you want to review, just have a look at: $ git grep -W "get_drvinfo(struct" Unless some driver names their callback function in some non standard way, that should be pretty it. Regardless, this will give you a fair representation of the current situation. There are a few nuances. For example, some drivers use dev_name() for the .bus_info, while some others use pci_name(). I am not sure if this makes a difference... Also, there are many hardcoded names for .driver and I do not have the courage to check all of them. I am OK to blindly remove all that mess but no more. Yours sincerely, Vincent Mailhol
On Thu, Nov 10, 2022 at 08:43:25PM +0900, Vincent MAILHOL wrote: > On Thu. 10 Nov. 2022 at 18:11, Leon Romanovsky <leon@kernel.org> wrote: > > On Thu, Nov 10, 2022 at 05:34:55PM +0900, Vincent MAILHOL wrote: > > > On Thu. 10 nov. 2022 at 05:26, Jakub Kicinski <kuba@kernel.org> wrote: > > > > On Wed, 9 Nov 2022 19:52:13 +0200 Leon Romanovsky wrote: > > > > > On Tue, Nov 08, 2022 at 12:57:54PM +0900, Vincent Mailhol wrote: > > > > > > If ethtool_ops::get_drvinfo() callback isn't set, > > > > > > ethtool_get_drvinfo() will fill the ethtool_drvinfo::name and > > > > > > ethtool_drvinfo::bus_info fields. > > > > > > > > > > > > However, if the driver provides the callback function, those two > > > > > > fields are not touched. This means that the driver has to fill these > > > > > > itself. > > > > > > > > > > Can you please point to such drivers? > > > > > > > > What you mean by "such drivers" is not clear from the quoted context, > > > > at least to me. > > > > > > An example: > > > https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/broadcom/bnx2.c#L7041 > > > > > > This driver wants to set fw_version but needs to also fill the driver > > > name and bus_info. My patch will enable *such drivers* to only fill > > > the fw_version and delegate the rest to the core. > > > > Sorry for being misleading, It looks like I typed only part of the sentence > > which I had in my mind. I wanted to see if any driver exists which prints > > drv_name and bus_info different from default. > > > > > > > > > > One can argue that they don't need to touch these fields in a first > > > > > place and ethtool_drvinfo should always overwrite them. > > > > > > > > Quite likely most driver prints to .driver and .bus_info can be dropped > > > > with this patch in place. Then again, I'm suspecting it's a bit of a > > > > chicken and an egg problem with people adding new drivers not having > > > > an incentive to add the print in the core and people who want to add > > > > the print in the core not having any driver that would benefit. > > > > Therefore I'd lean towards accepting Vincent's patch as is even if > > > > the submission can likely be more thorough and strict. > > > > > > If we can agree that no drivers should ever print .driver and > > > .bus_info, then I am fine to send a clean-up patch to remove all this > > > after this one gets accepted. However, I am not willing to invest time > > > for nothing. So would one of you be ready to sign-off such a clean-up > > > patch? > > > > I will be happy to see such patch and will review it, but can't add sign-off > > as I'm not netdev maintainer. > > Well, if you want to review, just have a look at: > $ git grep -W "get_drvinfo(struct" BTW, in some of the callbacks, if driver doesn't exists, they print "N/A", while in your patch it will be empty string. Thanks
> I will be happy to see such patch and will review it, but can't add sign-off > as I'm not netdev maintainer. Anybody can review any patch, and give an Acked-by or Reviewed-by. It is up to the Maintainer doing the actual merge to decided if it has any actual weight. Andrew
On Thu. 10 Nov. 2022 at 21:00, Leon Romanovsky <leon@kernel.org> wrote: > On Thu, Nov 10, 2022 at 08:43:25PM +0900, Vincent MAILHOL wrote: > > On Thu. 10 Nov. 2022 at 18:11, Leon Romanovsky <leon@kernel.org> wrote: > > > On Thu, Nov 10, 2022 at 05:34:55PM +0900, Vincent MAILHOL wrote: > > > > On Thu. 10 nov. 2022 at 05:26, Jakub Kicinski <kuba@kernel.org> wrote: > > > > > On Wed, 9 Nov 2022 19:52:13 +0200 Leon Romanovsky wrote: > > > > > > On Tue, Nov 08, 2022 at 12:57:54PM +0900, Vincent Mailhol wrote: > > > > > > > If ethtool_ops::get_drvinfo() callback isn't set, > > > > > > > ethtool_get_drvinfo() will fill the ethtool_drvinfo::name and > > > > > > > ethtool_drvinfo::bus_info fields. > > > > > > > > > > > > > > However, if the driver provides the callback function, those two > > > > > > > fields are not touched. This means that the driver has to fill these > > > > > > > itself. > > > > > > > > > > > > Can you please point to such drivers? > > > > > > > > > > What you mean by "such drivers" is not clear from the quoted context, > > > > > at least to me. > > > > > > > > An example: > > > > https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/broadcom/bnx2.c#L7041 > > > > > > > > This driver wants to set fw_version but needs to also fill the driver > > > > name and bus_info. My patch will enable *such drivers* to only fill > > > > the fw_version and delegate the rest to the core. > > > > > > Sorry for being misleading, It looks like I typed only part of the sentence > > > which I had in my mind. I wanted to see if any driver exists which prints > > > drv_name and bus_info different from default. > > > > > > > > > > > > > One can argue that they don't need to touch these fields in a first > > > > > > place and ethtool_drvinfo should always overwrite them. > > > > > > > > > > Quite likely most driver prints to .driver and .bus_info can be dropped > > > > > with this patch in place. Then again, I'm suspecting it's a bit of a > > > > > chicken and an egg problem with people adding new drivers not having > > > > > an incentive to add the print in the core and people who want to add > > > > > the print in the core not having any driver that would benefit. > > > > > Therefore I'd lean towards accepting Vincent's patch as is even if > > > > > the submission can likely be more thorough and strict. > > > > > > > > If we can agree that no drivers should ever print .driver and > > > > .bus_info, then I am fine to send a clean-up patch to remove all this > > > > after this one gets accepted. However, I am not willing to invest time > > > > for nothing. So would one of you be ready to sign-off such a clean-up > > > > patch? > > > > > > I will be happy to see such patch and will review it, but can't add sign-off > > > as I'm not netdev maintainer. > > > > Well, if you want to review, just have a look at: > > $ git grep -W "get_drvinfo(struct" > > BTW, in some of the callbacks, if driver doesn't exists, they print "N/A", > while in your patch it will be empty string. Indeed and this is inconsistent. For example, no one sets .erom_version to "N/A". So you will have some of the fields set to "N/A" and some others set to an empty string. The "N/A" thing was a mistake to begin with. I will not change my patch.
On Thu, 10 Nov 2022 17:34:55 +0900 Vincent MAILHOL wrote: > > While I'm typing - I've used dev_driver_string() to get the driver > > name in the past. Perhaps something to consider? > > I am not sure of that one. If dev->dev.parent->driver is not set, it > defaults to dev_bus_name() which is .bus_info, isn't it? > https://elixir.bootlin.com/linux/latest/source/drivers/base/core.c#L2181 I don't think so? We put dev_name() into bus_info, which is usually the address of the device on the bus (e.g. (D)BDF for PCI, like 0000:00:14.3). The name of the bus is pci. dev_driver_string() will also fall back to printing class name. > For the end user, it might be better to display an empty driver name > in 'ethtool -i' rather than reporting the bus_info twice? > > I mean, if you ask me for my opinion, then my answer is "I am not > sure". If you have confidence that dev_driver_string() is better, then > I will send a v2 right away. Well, it doesn't matter. I asked because handful of popular drivers use dev_driver_string(). But.. these are drivers so parent->driver will be set and it ends up not making any difference.
On Thu, 10 Nov 2022 11:11:38 +0200 Leon Romanovsky wrote: > I will be happy to see such patch and will review it, but can't add sign-off > as I'm not netdev maintainer. Did we finish the version removal work? :S Personally I'd rather direct any effort towards writing a checkpatch / cocci / python check that catches new cases than cleaning up the pile of drivers we have. A lot of which are not actively used..
On Thu, Nov 10, 2022 at 09:01:27AM -0800, Jakub Kicinski wrote: > On Thu, 10 Nov 2022 11:11:38 +0200 Leon Romanovsky wrote: > > I will be happy to see such patch and will review it, but can't add sign-off > > as I'm not netdev maintainer. > > Did we finish the version removal work? :S Good point :) > > Personally I'd rather direct any effort towards writing a checkpatch / > cocci / python check that catches new cases than cleaning up the pile > of drivers we have. A lot of which are not actively used.. I thought about this too, but came to conclusion what first step is to remove "version" from in-kernel API. It will reduce drastically desire to add driver version to new drivers. "There is no shame in doing the right thing, even if its too late.". BTW, I hope that we all learned that allowing drivers to return free string to users will always lead to disaster. Thanks
On Fri, Nov 11, 2022 at 12:53:16AM +0900, Vincent MAILHOL wrote: > On Thu. 10 Nov. 2022 at 21:00, Leon Romanovsky <leon@kernel.org> wrote: > > On Thu, Nov 10, 2022 at 08:43:25PM +0900, Vincent MAILHOL wrote: > > > On Thu. 10 Nov. 2022 at 18:11, Leon Romanovsky <leon@kernel.org> wrote: > > > > On Thu, Nov 10, 2022 at 05:34:55PM +0900, Vincent MAILHOL wrote: > > > > > On Thu. 10 nov. 2022 at 05:26, Jakub Kicinski <kuba@kernel.org> wrote: > > > > > > On Wed, 9 Nov 2022 19:52:13 +0200 Leon Romanovsky wrote: > > > > > > > On Tue, Nov 08, 2022 at 12:57:54PM +0900, Vincent Mailhol wrote: > > > > > > > > If ethtool_ops::get_drvinfo() callback isn't set, > > > > > > > > ethtool_get_drvinfo() will fill the ethtool_drvinfo::name and > > > > > > > > ethtool_drvinfo::bus_info fields. > > > > > > > > > > > > > > > > However, if the driver provides the callback function, those two > > > > > > > > fields are not touched. This means that the driver has to fill these > > > > > > > > itself. > > > > > > > > > > > > > > Can you please point to such drivers? > > > > > > > > > > > > What you mean by "such drivers" is not clear from the quoted context, > > > > > > at least to me. > > > > > > > > > > An example: > > > > > https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/broadcom/bnx2.c#L7041 > > > > > > > > > > This driver wants to set fw_version but needs to also fill the driver > > > > > name and bus_info. My patch will enable *such drivers* to only fill > > > > > the fw_version and delegate the rest to the core. > > > > > > > > Sorry for being misleading, It looks like I typed only part of the sentence > > > > which I had in my mind. I wanted to see if any driver exists which prints > > > > drv_name and bus_info different from default. > > > > > > > > > > > > > > > > One can argue that they don't need to touch these fields in a first > > > > > > > place and ethtool_drvinfo should always overwrite them. > > > > > > > > > > > > Quite likely most driver prints to .driver and .bus_info can be dropped > > > > > > with this patch in place. Then again, I'm suspecting it's a bit of a > > > > > > chicken and an egg problem with people adding new drivers not having > > > > > > an incentive to add the print in the core and people who want to add > > > > > > the print in the core not having any driver that would benefit. > > > > > > Therefore I'd lean towards accepting Vincent's patch as is even if > > > > > > the submission can likely be more thorough and strict. > > > > > > > > > > If we can agree that no drivers should ever print .driver and > > > > > .bus_info, then I am fine to send a clean-up patch to remove all this > > > > > after this one gets accepted. However, I am not willing to invest time > > > > > for nothing. So would one of you be ready to sign-off such a clean-up > > > > > patch? > > > > > > > > I will be happy to see such patch and will review it, but can't add sign-off > > > > as I'm not netdev maintainer. > > > > > > Well, if you want to review, just have a look at: > > > $ git grep -W "get_drvinfo(struct" > > > > BTW, in some of the callbacks, if driver doesn't exists, they print "N/A", > > while in your patch it will be empty string. > > Indeed and this is inconsistent. For example, no one sets > .erom_version to "N/A". So you will have some of the fields set to > "N/A" and some others set to an empty string. The "N/A" thing was a > mistake to begin with. I will not change my patch. I don't see anyone asking you to change your patch drastically. This discussion is more about next steps. Thanks
Hello: This patch was applied to netdev/net-next.git (master) by Jakub Kicinski <kuba@kernel.org>: On Tue, 8 Nov 2022 12:57:54 +0900 you wrote: > If ethtool_ops::get_drvinfo() callback isn't set, > ethtool_get_drvinfo() will fill the ethtool_drvinfo::name and > ethtool_drvinfo::bus_info fields. > > However, if the driver provides the callback function, those two > fields are not touched. This means that the driver has to fill these > itself. > > [...] Here is the summary with links: - [net-next,v1] ethtool: ethtool_get_drvinfo: populate drvinfo fields even if callback exits https://git.kernel.org/netdev/net-next/c/edaf5df22cb8 You are awesome, thank you!
On Fri. 11 Nov. 2022 à 02:01, Jakub Kicinski <kuba@kernel.org> wrote: > On Thu, 10 Nov 2022 11:11:38 +0200 Leon Romanovsky wrote: > > I will be happy to see such patch and will review it, but can't add sign-off > > as I'm not netdev maintainer. > > Did we finish the version removal work? :S > > Personally I'd rather direct any effort towards writing a checkpatch / > cocci / python check that catches new cases than cleaning up the pile > of drivers we have. A lot of which are not actively used.. Agree, but I will not work on that (because of other personal priorities). If someone else wants to do it, go ahead :) What I can do is update the documentation: https://lore.kernel.org/netdev/20221111064054.371965-1-mailhol.vincent@wanadoo.fr/ Yours sincerely, Vincent Mailhol
On Fri, Nov 11, 2022 at 03:43:36PM +0900, Vincent MAILHOL wrote: > On Fri. 11 Nov. 2022 à 02:01, Jakub Kicinski <kuba@kernel.org> wrote: > > On Thu, 10 Nov 2022 11:11:38 +0200 Leon Romanovsky wrote: > > > I will be happy to see such patch and will review it, but can't add sign-off > > > as I'm not netdev maintainer. > > > > Did we finish the version removal work? :S > > > > Personally I'd rather direct any effort towards writing a checkpatch / > > cocci / python check that catches new cases than cleaning up the pile > > of drivers we have. A lot of which are not actively used.. > > Agree, but I will not work on that (because of other personal > priorities). If someone else wants to do it, go ahead :) It's ok, Jakub referred to me. I sent a lot of patches like this. https://lore.kernel.org/netdev/5d76f3116ee795071ec044eabb815d6c2bdc7dbd.1649922731.git.leonro@nvidia.com/ > > What I can do is update the documentation: > https://lore.kernel.org/netdev/20221111064054.371965-1-mailhol.vincent@wanadoo.fr/ > > > Yours sincerely, > Vincent Mailhol
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index 57e7238a4136..546f931c3b6c 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -713,15 +713,22 @@ static int ethtool_get_drvinfo(struct net_device *dev, struct ethtool_devlink_compat *rsp) { const struct ethtool_ops *ops = dev->ethtool_ops; + struct device *parent = dev->dev.parent; rsp->info.cmd = ETHTOOL_GDRVINFO; strscpy(rsp->info.version, UTS_RELEASE, sizeof(rsp->info.version)); if (ops->get_drvinfo) { ops->get_drvinfo(dev, &rsp->info); - } else if (dev->dev.parent && dev->dev.parent->driver) { - strscpy(rsp->info.bus_info, dev_name(dev->dev.parent), + if (!rsp->info.bus_info[0] && parent) + strscpy(rsp->info.bus_info, dev_name(parent), + sizeof(rsp->info.bus_info)); + if (!rsp->info.driver[0] && parent && parent->driver) + strscpy(rsp->info.driver, parent->driver->name, + sizeof(rsp->info.driver)); + } else if (parent && parent->driver) { + strscpy(rsp->info.bus_info, dev_name(parent), sizeof(rsp->info.bus_info)); - strscpy(rsp->info.driver, dev->dev.parent->driver->name, + strscpy(rsp->info.driver, parent->driver->name, sizeof(rsp->info.driver)); } else if (dev->rtnl_link_ops) { strscpy(rsp->info.driver, dev->rtnl_link_ops->kind,
If ethtool_ops::get_drvinfo() callback isn't set, ethtool_get_drvinfo() will fill the ethtool_drvinfo::name and ethtool_drvinfo::bus_info fields. However, if the driver provides the callback function, those two fields are not touched. This means that the driver has to fill these itself. Allow the driver to leave those two fields empty and populate them in such case. This way, the driver can rely on the default values for the name and the bus_info. If the driver provides values, do nothing. Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> --- net/ethtool/ioctl.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)