Wed, Jan 30, 2019 at 08:05:08PM CET, jakub.kicin...@netronome.com wrote:
>ethtool -i has a few fixed-size fields which can be used to report
>firmware version and expansion ROM version. Unfortunately, modern
>hardware has more firmware components. There is usually some
>datapath microcode, management controller, PXE drivers, and a
>CPLD load. Running ethtool -i on modern controllers reveals the
>fact that vendors cram multiple values into firmware version field.
>
>Here are some examples from systems I could lay my hands on quickly:
>
>tg3: "FFV20.2.17 bc 5720-v1.39"
>i40e: "6.01 0x800034a4 1.1747.0"
>nfp: "0.0.3.5 0.25 sriov-2.1.16 nic"
>
>Add a new devlink API to allow retrieving multiple versions, and
>provide user-readable name for those versions.
>
>While at it break down the versions into three categories:
> - fixed - this is the board/fixed component version, usually vendors
> report information like the board version in the PCI VPD,
> but it will benefit from naming and common API as well;
> - running - this is the running firmware version;
> - stored - this is firmware in the flash, after firmware update
> this value will reflect the flashed version, while the
> running version may only be updated after reboot.
>
>RFCv2:
> - remove the nesting in attr DEVLINK_ATTR_INFO_VERSIONS (now
> versions are mixed with other info attrs)l
> - have the driver report versions from the same callback as
> other info.
>
>Signed-off-by: Jakub Kicinski <jakub.kicin...@netronome.com>
>---
> include/net/devlink.h | 18 ++++++++++++++++
> include/uapi/linux/devlink.h | 5 +++++
> net/core/devlink.c | 40 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 63 insertions(+)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 5ef3570a3859..ec72638aa47f 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -428,6 +428,12 @@ enum devlink_param_wol_types {
> .validate = _validate, \
> }
>
>+enum devlink_version_type {
devlink_info_version_type
>+ DEVLINK_VERSION_FIXED,
DEVLINK_INFO_VERSION_*
>+ DEVLINK_VERSION_STORED,
>+ DEVLINK_VERSION_RUNNING,
>+};
>+
> struct devlink_region;
> struct devlink_info_req;
>
>@@ -614,6 +620,10 @@ int devlink_info_report_serial_number(struct
>devlink_info_req *req,
> const char *sn);
> int devlink_info_report_driver_name(struct devlink_info_req *req,
> const char *name);
>+int devlink_info_report_version(struct devlink_info_req *req,
>+ enum devlink_version_type type,
>+ const char *version_name,
>+ const char *version_value);
>
> #else
>
>@@ -923,6 +933,14 @@ devlink_info_report_serial_number(struct devlink_info_req
>*req, const char *sn)
> {
> return 0;
> }
>+
>+static inline int
>+devlink_info_report_version(struct devlink_info_req *req,
>+ enum devlink_version_type type,
>+ const char *version_name, const char *version_value)
>+{
>+ return 0;
>+}
> #endif
>
> #endif /* _NET_DEVLINK_H_ */
>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>index fd089baa7c50..a61b87c73c3f 100644
>--- a/include/uapi/linux/devlink.h
>+++ b/include/uapi/linux/devlink.h
>@@ -294,6 +294,11 @@ enum devlink_attr {
>
> DEVLINK_ATTR_INFO_DRV_NAME, /* string */
> DEVLINK_ATTR_INFO_SERIAL_NUMBER, /* string */
>+ DEVLINK_ATTR_INFO_VERSION_FIXED, /* nested */
>+ DEVLINK_ATTR_INFO_VERSION_RUNNING, /* nested */
>+ DEVLINK_ATTR_INFO_VERSION_STORED, /* nested */
>+ DEVLINK_ATTR_INFO_VERSION_NAME, /* string */
>+ DEVLINK_ATTR_INFO_VERSION_VALUE, /* string */
>
> /* add new attributes above here, update the policy in devlink.c */
>
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index 1b941493fdff..e2027d3a5e40 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -3732,6 +3732,46 @@ int devlink_info_report_serial_number(struct
>devlink_info_req *req,
> }
> EXPORT_SYMBOL_GPL(devlink_info_report_serial_number);
>
>+int devlink_info_report_version(struct devlink_info_req *req,
devlink_info_version_put()
>+ enum devlink_version_type type,
>+ const char *version_name,
>+ const char *version_value)
>+{
>+ static const enum devlink_attr type2attr[] = {
>+ [DEVLINK_VERSION_FIXED] = DEVLINK_ATTR_INFO_VERSION_FIXED,
>+ [DEVLINK_VERSION_STORED] = DEVLINK_ATTR_INFO_VERSION_STORED,
>+ [DEVLINK_VERSION_RUNNING] = DEVLINK_ATTR_INFO_VERSION_RUNNING,
I think it would be perhaps nice to have a set of wrappers:
devlink_info_version_fixed_put()
devlink_info_version_stored_put()
devlink_info_version_running_put()
And then have static devlink_info_version_put() which accepts ATTR value
directly. Then you can avoid the intermediate enum, array and checks.
>+ };
>+ struct nlattr *nest;
>+ int err;
>+
>+ if (type >= ARRAY_SIZE(type2attr) || !type2attr[type])
>+ return -EINVAL;
>+
>+ nest = nla_nest_start(req->msg, type2attr[type]);
>+ if (!nest)
>+ return -EMSGSIZE;
>+
>+ err = nla_put_string(req->msg, DEVLINK_ATTR_INFO_VERSION_NAME,
>+ version_name);
>+ if (err)
>+ goto nla_put_failure;
>+
>+ err = nla_put_string(req->msg, DEVLINK_ATTR_INFO_VERSION_VALUE,
>+ version_value);
>+ if (err)
>+ goto nla_put_failure;
>+
>+ nla_nest_end(req->msg, nest);
>+
>+ return 0;
>+
>+nla_put_failure:
>+ nla_nest_cancel(req->msg, nest);
>+ return err;
>+}
>+EXPORT_SYMBOL_GPL(devlink_info_report_version);
>+
> static int
> devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
> enum devlink_command cmd, u32 portid,
>--
>2.19.2
>
