Commit f20a0a05 authored by Vincent Mailhol's avatar Vincent Mailhol Committed by Jakub Kicinski
Browse files

ethtool: doc: clarify what drivers can implement in their get_drvinfo() 

Many of the drivers which implement ethtool_ops::get_drvinfo() will
prints the .driver, .version or .bus_info of struct ethtool_drvinfo.
To have a glance of current state, do:

  $ git grep -W "get_drvinfo(struct"

Printing in those three fields is useless because:

  - since [1], the driver version should be the kernel version (at
    least for upstream drivers). Arguably, out of tree drivers might
    still want to set a custom version, but out of tree is not our
    focus.

  - since [2], the core is able to provide default values for .driver
    and .bus_info.

In summary, drivers may provide .fw_version and .erom_version, the
rest is expected to be done by the core.

In struct ethtool_ops doc from linux/ethtool: rephrase field
get_drvinfo() doc to discourage developers from implementing this
callback.

In struct ethtool_drvinfo doc from uapi/linux/ethtool.h: remove the
paragraph mentioning what drivers should do. Rationale: no need to
repeat what is already written in struct ethtool_ops doc. But add a
note that .fw_version and .erom_version are driver defined.

Also update the dummy driver and simply remove the callback in order
not to confuse the newcomers: most of the drivers will not need this
callback function any more.

[1] commit 6a7e25c7 ("net/core: Replace driver version to be
    kernel version")
Link: https://git.kernel.org/torvalds/linux/c/6a7e25c7fb48

[2] commit edaf5df2 ("ethtool: ethtool_get_drvinfo: populate
    drvinfo fields even if callback exits")
Link: https://git.kernel.org/netdev/net-next/c/edaf5df22cb8



Reviewed-by: default avatarLeon Romanovsky <leonro@nvidia.com>
Signed-off-by: default avatarVincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://lore.kernel.org/r/20221116171828.4093-1-mailhol.vincent@wanadoo.fr


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 224b744a

drivers/net/dummy.c

+0 −7
Original line number Diff line number Diff line
@@ -99,14 +99,7 @@ static const struct net_device_ops dummy_netdev_ops = { 
	.ndo_change_carrier	= dummy_change_carrier,

};



static void dummy_get_drvinfo(struct net_device *dev,

			      struct ethtool_drvinfo *info)

{

	strscpy(info->driver, DRV_NAME, sizeof(info->driver));

}



static const struct ethtool_ops dummy_ethtool_ops = {

	.get_drvinfo            = dummy_get_drvinfo,

	.get_ts_info		= ethtool_op_get_ts_info,

};

include/linux/ethtool.h

+4 −4
Original line number Diff line number Diff line
@@ -473,10 +473,10 @@ struct ethtool_module_power_mode_params { 
 *	parameter.

 * @supported_coalesce_params: supported types of interrupt coalescing.

 * @supported_ring_params: supported ring params.

 * @get_drvinfo: Report driver/device information.  Should only set the

 *	@driver, @version, @fw_version and @bus_info fields.  If not

 *	implemented, the @driver and @bus_info fields will be filled in

 *	according to the netdev's parent device.

 * @get_drvinfo: Report driver/device information. Modern drivers no

 *	longer have to implement this callback. Most fields are

 *	correctly filled in by the core using system information, or

 *	populated using other driver operations.

 * @get_regs_len: Get buffer length required for @get_regs

 * @get_regs: Get device registers

 * @get_wol: Report whether Wake-on-Lan is enabled

include/uapi/linux/ethtool.h

+4 −6
Original line number Diff line number Diff line
@@ -159,8 +159,10 @@ static inline __u32 ethtool_cmd_speed(const struct ethtool_cmd *ep) 
 *	in its bus driver structure (e.g. pci_driver::name).  Must

 *	not be an empty string.

 * @version: Driver version string; may be an empty string

 * @fw_version: Firmware version string; may be an empty string

 * @erom_version: Expansion ROM version string; may be an empty string

 * @fw_version: Firmware version string; driver defined; may be an

 *	empty string

 * @erom_version: Expansion ROM version string; driver defined; may be

 *	an empty string

 * @bus_info: Device bus address.  This should match the dev_name()

 *	string for the underlying bus device, if there is one.  May be

 *	an empty string.
@@ -179,10 +181,6 @@ static inline __u32 ethtool_cmd_speed(const struct ethtool_cmd *ep) 
 *

 * Users can use the %ETHTOOL_GSSET_INFO command to get the number of

 * strings in any string set (from Linux 2.6.34).

 *

 * Drivers should set at most @driver, @version, @fw_version and

 * @bus_info in their get_drvinfo() implementation.  The ethtool

 * core fills in the other fields using other driver operations.

 */

struct ethtool_drvinfo {

	__u32	cmd;