[net-next,v2] ethtool: doc: clarify what drivers can implement in their get_drvinfo()
Commit Message
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. Update the doc to reflect the
facts.
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 6a7e25c7fb48 ("net/core: Replace driver version to be
kernel version")
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a7e25c7fb482dba3e80fec953f1907bcb24d52c
[2] commit edaf5df22cb8 ("ethtool: ethtool_get_drvinfo: populate
drvinfo fields even if callback exits")
Link: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=edaf5df22cb8e7e849773ce69fcc9bc20ca92160
CC: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
Arguably, dummy.c is code and not documentation, but for me, it makes
sense to treat it as documentation, thus I am putting everything in
one single patch.
* Changelog *
v1 -> v2:
* Forgot the net-next prefix in the patch subject... Sorry for the
noise.
---
drivers/net/dummy.c | 7 -------
include/uapi/linux/ethtool.h | 6 +++---
2 files changed, 3 insertions(+), 10 deletions(-)
Comments
On Fri, Nov 11, 2022 at 03:40:54PM +0900, Vincent Mailhol wrote:
> 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. Update the doc to reflect the
> facts.
>
> 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 6a7e25c7fb48 ("net/core: Replace driver version to be
> kernel version")
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a7e25c7fb482dba3e80fec953f1907bcb24d52c
>
> [2] commit edaf5df22cb8 ("ethtool: ethtool_get_drvinfo: populate
> drvinfo fields even if callback exits")
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=edaf5df22cb8e7e849773ce69fcc9bc20ca92160
>
> CC: Leon Romanovsky <leonro@mellanox.com>
> Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> ---
> Arguably, dummy.c is code and not documentation, but for me, it makes
> sense to treat it as documentation, thus I am putting everything in
> one single patch.
If to judge by newcomers submissions, many of them don't read documentation.
Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
@@ -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,
};
@@ -180,9 +180,9 @@ 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.
+ * Drivers should set at most @fw_version and @erom_version in their
+ * get_drvinfo() implementation. The ethtool core fills in the other
+ * fields using other driver operations.
*/
struct ethtool_drvinfo {
__u32 cmd;