Wed, Jan 30, 2019 at 08:05:07PM CET, jakub.kicin...@netronome.com wrote:
>ethtool -i has served us well for a long time, but its showing
>its limitations more and more. The device information should
Double space here -------------^^
>also be reported per device not per-netdev.
>
>Lay foundation for a simple devlink-based way of reading device
>info. Add driver name and device serial number as initial pieces
^^---------------+
Double space here -----+
>of information exposed via this new API.
>
>RFC v2:
> - wrap the skb into an opaque structure (Jiri);
> - allow the serial number of be any length (Jiri & Andrew);
> - add driver name (Jonathan).
>
>Signed-off-by: Jakub Kicinski <jakub.kicin...@netronome.com>
>---
> include/net/devlink.h | 18 ++++++
> include/uapi/linux/devlink.h | 5 ++
> net/core/devlink.c | 114 +++++++++++++++++++++++++++++++++++
> 3 files changed, 137 insertions(+)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 85c9eabaf056..5ef3570a3859 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -429,6 +429,7 @@ enum devlink_param_wol_types {
> }
>
> struct devlink_region;
>+struct devlink_info_req;
>
> typedef void devlink_snapshot_data_dest_t(const void *data);
>
>@@ -484,6 +485,8 @@ struct devlink_ops {
> int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
> *p_encap_mode);
> int (*eswitch_encap_mode_set)(struct devlink *devlink, u8 encap_mode,
> struct netlink_ext_ack *extack);
>+ int (*info_get)(struct devlink *devlink, struct devlink_info_req *req,
>+ struct netlink_ext_ack *extack);
> };
>
> static inline void *devlink_priv(struct devlink *devlink)
>@@ -607,6 +610,10 @@ u32 devlink_region_shapshot_id_get(struct devlink
>*devlink);
> int devlink_region_snapshot_create(struct devlink_region *region, u64
> data_len,
> u8 *data, u32 snapshot_id,
> devlink_snapshot_data_dest_t
> *data_destructor);
>+int devlink_info_report_serial_number(struct devlink_info_req *req,
>+ const char *sn);
I don't like the "report" part. The rest of the code uses "put".
Also. I think that verb should be at the
end of the function name, as it is common in the rest of the code.
So please rename to:
devlink_info_serial_number_put()
Same for the rest.
>+int devlink_info_report_driver_name(struct devlink_info_req *req,
>+ const char *name);
>
> #else
>
>@@ -905,6 +912,17 @@ devlink_region_snapshot_create(struct devlink_region
>*region, u64 data_len,
> return 0;
> }
>
>+static inline int
>+devlink_info_report_driver_name(struct devlink_info_req *req, const char
>*name)
>+{
>+ return 0;
>+}
>+
>+static inline int
>+devlink_info_report_serial_number(struct devlink_info_req *req, const char
>*sn)
>+{
>+ return 0;
>+}
> #endif
>
> #endif /* _NET_DEVLINK_H_ */
>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>index 61b4447a6c5b..fd089baa7c50 100644
>--- a/include/uapi/linux/devlink.h
>+++ b/include/uapi/linux/devlink.h
>@@ -94,6 +94,8 @@ enum devlink_command {
> DEVLINK_CMD_PORT_PARAM_NEW,
> DEVLINK_CMD_PORT_PARAM_DEL,
>
>+ DEVLINK_CMD_INFO_GET, /* can dump */
>+
> /* add new commands above here */
> __DEVLINK_CMD_MAX,
> DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
>@@ -290,6 +292,9 @@ enum devlink_attr {
> DEVLINK_ATTR_REGION_CHUNK_ADDR, /* u64 */
> DEVLINK_ATTR_REGION_CHUNK_LEN, /* u64 */
>
>+ DEVLINK_ATTR_INFO_DRV_NAME, /* string */
Please be consistent across the names of function, attr etc. So:
DEVLINK_ATTR_INFO_DRIVER_NAME,
Otherwise, this looks good.
Thanks!
[...]
