* [PATCH net-next v2 1/7] devlink: add device information API
2019-01-30 19:05 [PATCH net-next v2 0/7] devlink: add device (driver) information API Jakub Kicinski
@ 2019-01-30 19:05 ` Jakub Kicinski
2019-01-30 21:16 ` Jiri Pirko
2019-01-30 19:05 ` [PATCH net-next v2 2/7] devlink: add version reporting to devlink info API Jakub Kicinski
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2019-01-30 19:05 UTC (permalink / raw)
To: davem
Cc: netdev, oss-drivers, jiri, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon, Jakub Kicinski
ethtool -i has served us well for a long time, but its showing
its limitations more and more. The device information should
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
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.kicinski@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);
+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 */
+ DEVLINK_ATTR_INFO_SERIAL_NUMBER, /* string */
+
/* add new attributes above here, update the policy in devlink.c */
__DEVLINK_ATTR_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index e6f170caf449..1b941493fdff 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -3714,6 +3714,112 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
return 0;
}
+struct devlink_info_req {
+ struct sk_buff *msg;
+};
+
+int devlink_info_report_driver_name(struct devlink_info_req *req,
+ const char *name)
+{
+ return nla_put_string(req->msg, DEVLINK_ATTR_INFO_DRV_NAME, name);
+}
+EXPORT_SYMBOL_GPL(devlink_info_report_driver_name);
+
+int devlink_info_report_serial_number(struct devlink_info_req *req,
+ const char *sn)
+{
+ return nla_put_string(req->msg, DEVLINK_ATTR_INFO_SERIAL_NUMBER, sn);
+}
+EXPORT_SYMBOL_GPL(devlink_info_report_serial_number);
+
+static int
+devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
+ enum devlink_command cmd, u32 portid,
+ u32 seq, int flags, struct netlink_ext_ack *extack)
+{
+ struct devlink_info_req req;
+ void *hdr;
+ int err;
+
+ hdr = genlmsg_put(msg, portid, seq, &devlink_nl_family, flags, cmd);
+ if (!hdr)
+ return -EMSGSIZE;
+
+ err = -EMSGSIZE;
+ if (devlink_nl_put_handle(msg, devlink))
+ goto err_cancel_msg;
+
+ req.msg = msg;
+ err = devlink->ops->info_get(devlink, &req, extack);
+ if (err)
+ goto err_cancel_msg;
+
+ genlmsg_end(msg, hdr);
+ return 0;
+
+err_cancel_msg:
+ genlmsg_cancel(msg, hdr);
+ return err;
+}
+
+static int devlink_nl_cmd_info_get_doit(struct sk_buff *skb,
+ struct genl_info *info)
+{
+ struct devlink *devlink = info->user_ptr[0];
+ struct sk_buff *msg;
+ int err;
+
+ if (!devlink->ops || !devlink->ops->info_get)
+ return -EOPNOTSUPP;
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ err = devlink_nl_info_fill(msg, devlink, DEVLINK_CMD_INFO_GET,
+ info->snd_portid, info->snd_seq, 0,
+ info->extack);
+ if (err) {
+ nlmsg_free(msg);
+ return err;
+ }
+
+ return genlmsg_reply(msg, info);
+}
+
+static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
+ struct netlink_callback *cb)
+{
+ struct devlink *devlink;
+ int start = cb->args[0];
+ int idx = 0;
+ int err;
+
+ mutex_lock(&devlink_mutex);
+ list_for_each_entry(devlink, &devlink_list, list) {
+ if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
+ continue;
+ if (idx < start) {
+ idx++;
+ continue;
+ }
+
+ mutex_lock(&devlink->lock);
+ err = devlink_nl_info_fill(msg, devlink, DEVLINK_CMD_INFO_GET,
+ NETLINK_CB(cb->skb).portid,
+ cb->nlh->nlmsg_seq, NLM_F_MULTI,
+ cb->extack);
+ mutex_unlock(&devlink->lock);
+ if (err)
+ break;
+ idx++;
+ }
+ mutex_unlock(&devlink_mutex);
+
+ cb->args[0] = idx;
+ return msg->len;
+}
+
static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
[DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING },
[DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING },
@@ -3974,6 +4080,14 @@ static const struct genl_ops devlink_nl_ops[] = {
.flags = GENL_ADMIN_PERM,
.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
},
+ {
+ .cmd = DEVLINK_CMD_INFO_GET,
+ .doit = devlink_nl_cmd_info_get_doit,
+ .dumpit = devlink_nl_cmd_info_get_dumpit,
+ .policy = devlink_nl_policy,
+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
+ /* can be retrieved by unprivileged users */
+ },
};
static struct genl_family devlink_nl_family __ro_after_init = {
--
2.19.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v2 1/7] devlink: add device information API
2019-01-30 19:05 ` [PATCH net-next v2 1/7] devlink: add device " Jakub Kicinski
@ 2019-01-30 21:16 ` Jiri Pirko
0 siblings, 0 replies; 17+ messages in thread
From: Jiri Pirko @ 2019-01-30 21:16 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek,
eugenem, jonathan.lemon
Wed, Jan 30, 2019 at 08:05:07PM CET, jakub.kicinski@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.kicinski@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!
[...]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next v2 2/7] devlink: add version reporting to devlink info API
2019-01-30 19:05 [PATCH net-next v2 0/7] devlink: add device (driver) information API Jakub Kicinski
2019-01-30 19:05 ` [PATCH net-next v2 1/7] devlink: add device " Jakub Kicinski
@ 2019-01-30 19:05 ` Jakub Kicinski
2019-01-30 21:54 ` Jiri Pirko
2019-01-30 19:05 ` [PATCH net-next v2 3/7] nfp: devlink: report driver name and serial number Jakub Kicinski
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2019-01-30 19:05 UTC (permalink / raw)
To: davem
Cc: netdev, oss-drivers, jiri, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon, Jakub Kicinski
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.kicinski@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_VERSION_FIXED,
+ 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,
+ 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,
+ };
+ 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v2 2/7] devlink: add version reporting to devlink info API
2019-01-30 19:05 ` [PATCH net-next v2 2/7] devlink: add version reporting to devlink info API Jakub Kicinski
@ 2019-01-30 21:54 ` Jiri Pirko
0 siblings, 0 replies; 17+ messages in thread
From: Jiri Pirko @ 2019-01-30 21:54 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek,
eugenem, jonathan.lemon
Wed, Jan 30, 2019 at 08:05:08PM CET, jakub.kicinski@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.kicinski@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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next v2 3/7] nfp: devlink: report driver name and serial number
2019-01-30 19:05 [PATCH net-next v2 0/7] devlink: add device (driver) information API Jakub Kicinski
2019-01-30 19:05 ` [PATCH net-next v2 1/7] devlink: add device " Jakub Kicinski
2019-01-30 19:05 ` [PATCH net-next v2 2/7] devlink: add version reporting to devlink info API Jakub Kicinski
@ 2019-01-30 19:05 ` Jakub Kicinski
2019-01-30 19:05 ` [PATCH net-next v2 4/7] nfp: devlink: report fixed versions Jakub Kicinski
` (3 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2019-01-30 19:05 UTC (permalink / raw)
To: davem
Cc: netdev, oss-drivers, jiri, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon, Jakub Kicinski
Report the basic info through new devlink info API.
RFCv2:
- add driver name;
- align serial to core changes.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../net/ethernet/netronome/nfp/nfp_devlink.c | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index 808647ec3573..cb3ef7e46614 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -4,6 +4,7 @@
#include <linux/rtnetlink.h>
#include <net/devlink.h>
+#include "nfpcore/nfp.h"
#include "nfpcore/nfp_nsp.h"
#include "nfp_app.h"
#include "nfp_main.h"
@@ -171,6 +172,28 @@ static int nfp_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
return ret;
}
+static int
+nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
+ struct netlink_ext_ack *extack)
+{
+ struct nfp_pf *pf = devlink_priv(devlink);
+ const char *sn;
+ int err;
+
+ err = devlink_info_report_driver_name(req, "nfp");
+ if (err)
+ return err;
+
+ sn = nfp_hwinfo_lookup(pf->hwinfo, "assembly.serial");
+ if (sn) {
+ err = devlink_info_report_serial_number(req, sn);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
const struct devlink_ops nfp_devlink_ops = {
.port_split = nfp_devlink_port_split,
.port_unsplit = nfp_devlink_port_unsplit,
@@ -178,6 +201,7 @@ const struct devlink_ops nfp_devlink_ops = {
.sb_pool_set = nfp_devlink_sb_pool_set,
.eswitch_mode_get = nfp_devlink_eswitch_mode_get,
.eswitch_mode_set = nfp_devlink_eswitch_mode_set,
+ .info_get = nfp_devlink_info_get,
};
int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
--
2.19.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next v2 4/7] nfp: devlink: report fixed versions
2019-01-30 19:05 [PATCH net-next v2 0/7] devlink: add device (driver) information API Jakub Kicinski
` (2 preceding siblings ...)
2019-01-30 19:05 ` [PATCH net-next v2 3/7] nfp: devlink: report driver name and serial number Jakub Kicinski
@ 2019-01-30 19:05 ` Jakub Kicinski
2019-01-30 21:56 ` Jiri Pirko
2019-01-30 19:05 ` [PATCH net-next v2 5/7] nfp: nsp: add support for versions command Jakub Kicinski
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2019-01-30 19:05 UTC (permalink / raw)
To: davem
Cc: netdev, oss-drivers, jiri, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon, Jakub Kicinski
Report information about the hardware.
RFCv2:
- add defines for board IDs which are likely to be reusable for
other drivers (Jiri).
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../net/ethernet/netronome/nfp/nfp_devlink.c | 37 ++++++++++++++++++-
include/net/devlink.h | 5 +++
2 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index cb3ef7e46614..9857fa663adf 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -172,6 +172,41 @@ static int nfp_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
return ret;
}
+static const struct nfp_devlink_versions_simple {
+ const char *key;
+ const char *hwinfo;
+} nfp_devlink_versions_hwinfo[] = {
+ { DEVLINK_VERSION_GENERIC_BOARD_ID, "assembly.partno", },
+ { DEVLINK_VERSION_GENERIC_BOARD_REV, "assembly.revision", },
+ { "board.vendor", /* fab */ "assembly.vendor", },
+ { "board.model", /* code name */ "assembly.model", },
+};
+
+static int
+nfp_devlink_versions_get_hwinfo(struct nfp_pf *pf, struct devlink_info_req *req)
+{
+ unsigned int i;
+ int err;
+
+ for (i = 0; i < ARRAY_SIZE(nfp_devlink_versions_hwinfo); i++) {
+ const struct nfp_devlink_versions_simple *info;
+ const char *val;
+
+ info = &nfp_devlink_versions_hwinfo[i];
+
+ val = nfp_hwinfo_lookup(pf->hwinfo, info->hwinfo);
+ if (!val)
+ continue;
+
+ err = devlink_info_report_version(req, DEVLINK_VERSION_FIXED,
+ info->key, val);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
static int
nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
struct netlink_ext_ack *extack)
@@ -191,7 +226,7 @@ nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
return err;
}
- return 0;
+ return nfp_devlink_versions_get_hwinfo(pf, req);
}
const struct devlink_ops nfp_devlink_ops = {
diff --git a/include/net/devlink.h b/include/net/devlink.h
index ec72638aa47f..3d553cc6693d 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -428,6 +428,11 @@ enum devlink_param_wol_types {
.validate = _validate, \
}
+/* Part number, identifier of board design */
+#define DEVLINK_VERSION_GENERIC_BOARD_ID "board.id"
+/* Revision of board design */
+#define DEVLINK_VERSION_GENERIC_BOARD_REV "board.rev"
+
enum devlink_version_type {
DEVLINK_VERSION_FIXED,
DEVLINK_VERSION_STORED,
--
2.19.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v2 4/7] nfp: devlink: report fixed versions
2019-01-30 19:05 ` [PATCH net-next v2 4/7] nfp: devlink: report fixed versions Jakub Kicinski
@ 2019-01-30 21:56 ` Jiri Pirko
0 siblings, 0 replies; 17+ messages in thread
From: Jiri Pirko @ 2019-01-30 21:56 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek,
eugenem, jonathan.lemon
Wed, Jan 30, 2019 at 08:05:10PM CET, jakub.kicinski@netronome.com wrote:
>Report information about the hardware.
>
>RFCv2:
> - add defines for board IDs which are likely to be reusable for
> other drivers (Jiri).
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>---
> .../net/ethernet/netronome/nfp/nfp_devlink.c | 37 ++++++++++++++++++-
> include/net/devlink.h | 5 +++
> 2 files changed, 41 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>index cb3ef7e46614..9857fa663adf 100644
>--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>@@ -172,6 +172,41 @@ static int nfp_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
> return ret;
> }
>
>+static const struct nfp_devlink_versions_simple {
>+ const char *key;
>+ const char *hwinfo;
>+} nfp_devlink_versions_hwinfo[] = {
>+ { DEVLINK_VERSION_GENERIC_BOARD_ID, "assembly.partno", },
>+ { DEVLINK_VERSION_GENERIC_BOARD_REV, "assembly.revision", },
>+ { "board.vendor", /* fab */ "assembly.vendor", },
>+ { "board.model", /* code name */ "assembly.model", },
>+};
>+
>+static int
>+nfp_devlink_versions_get_hwinfo(struct nfp_pf *pf, struct devlink_info_req *req)
>+{
>+ unsigned int i;
>+ int err;
>+
>+ for (i = 0; i < ARRAY_SIZE(nfp_devlink_versions_hwinfo); i++) {
>+ const struct nfp_devlink_versions_simple *info;
>+ const char *val;
>+
>+ info = &nfp_devlink_versions_hwinfo[i];
>+
>+ val = nfp_hwinfo_lookup(pf->hwinfo, info->hwinfo);
>+ if (!val)
>+ continue;
>+
>+ err = devlink_info_report_version(req, DEVLINK_VERSION_FIXED,
>+ info->key, val);
>+ if (err)
>+ return err;
>+ }
>+
>+ return 0;
>+}
>+
> static int
> nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
> struct netlink_ext_ack *extack)
>@@ -191,7 +226,7 @@ nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
> return err;
> }
>
>- return 0;
>+ return nfp_devlink_versions_get_hwinfo(pf, req);
> }
>
> const struct devlink_ops nfp_devlink_ops = {
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index ec72638aa47f..3d553cc6693d 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -428,6 +428,11 @@ enum devlink_param_wol_types {
> .validate = _validate, \
> }
>
>+/* Part number, identifier of board design */
>+#define DEVLINK_VERSION_GENERIC_BOARD_ID "board.id"
>+/* Revision of board design */
>+#define DEVLINK_VERSION_GENERIC_BOARD_REV "board.rev"
Please have this as a separate patch, not part of "nfp".
>+
> enum devlink_version_type {
> DEVLINK_VERSION_FIXED,
> DEVLINK_VERSION_STORED,
>--
>2.19.2
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next v2 5/7] nfp: nsp: add support for versions command
2019-01-30 19:05 [PATCH net-next v2 0/7] devlink: add device (driver) information API Jakub Kicinski
` (3 preceding siblings ...)
2019-01-30 19:05 ` [PATCH net-next v2 4/7] nfp: devlink: report fixed versions Jakub Kicinski
@ 2019-01-30 19:05 ` Jakub Kicinski
2019-01-30 19:05 ` [PATCH net-next v2 6/7] nfp: devlink: report the running and flashed versions Jakub Kicinski
2019-01-30 19:05 ` [PATCH net-next v2 7/7] ethtool: add compat for devlink info Jakub Kicinski
6 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2019-01-30 19:05 UTC (permalink / raw)
To: davem
Cc: netdev, oss-drivers, jiri, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon, Jakub Kicinski
Retrieve the FW versions with the new command.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../ethernet/netronome/nfp/nfpcore/nfp_nsp.c | 61 +++++++++++++++++++
.../ethernet/netronome/nfp/nfpcore/nfp_nsp.h | 20 ++++++
2 files changed, 81 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
index ce1577bbbd2a..a9d53df0070c 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
@@ -7,6 +7,7 @@
* Jason McMullan <jason.mcmullan@netronome.com>
*/
+#include <asm/unaligned.h>
#include <linux/bitfield.h>
#include <linux/delay.h>
#include <linux/firmware.h>
@@ -62,6 +63,16 @@
#define NFP_HWINFO_LOOKUP_SIZE GENMASK(11, 0)
+#define NFP_VERSIONS_SIZE GENMASK(11, 0)
+#define NFP_VERSIONS_CNT_OFF 0
+#define NFP_VERSIONS_BSP_OFF 2
+#define NFP_VERSIONS_CPLD_OFF 6
+#define NFP_VERSIONS_APP_OFF 10
+#define NFP_VERSIONS_BUNDLE_OFF 14
+#define NFP_VERSIONS_UNDI_OFF 18
+#define NFP_VERSIONS_NCSI_OFF 22
+#define NFP_VERSIONS_CFGR_OFF 26
+
enum nfp_nsp_cmd {
SPCODE_NOOP = 0, /* No operation */
SPCODE_SOFT_RESET = 1, /* Soft reset the NFP */
@@ -77,6 +88,7 @@ enum nfp_nsp_cmd {
SPCODE_NSP_IDENTIFY = 13, /* Read NSP version */
SPCODE_FW_STORED = 16, /* If no FW loaded, load flash app FW */
SPCODE_HWINFO_LOOKUP = 17, /* Lookup HWinfo with overwrites etc. */
+ SPCODE_VERSIONS = 21, /* Report FW versions */
};
static const struct {
@@ -711,3 +723,52 @@ int nfp_nsp_hwinfo_lookup(struct nfp_nsp *state, void *buf, unsigned int size)
return 0;
}
+
+int nfp_nsp_versions(struct nfp_nsp *state, void *buf, unsigned int size)
+{
+ struct nfp_nsp_command_buf_arg versions = {
+ {
+ .code = SPCODE_VERSIONS,
+ .option = min_t(u32, size, NFP_VERSIONS_SIZE),
+ },
+ .out_buf = buf,
+ .out_size = min_t(u32, size, NFP_VERSIONS_SIZE),
+ };
+
+ return nfp_nsp_command_buf(state, &versions);
+}
+
+const char *nfp_nsp_versions_get(enum nfp_nsp_versions id, bool flash,
+ const u8 *buf, unsigned int size)
+{
+ static const u32 id2off[] = {
+ [NFP_VERSIONS_BSP] = NFP_VERSIONS_BSP_OFF,
+ [NFP_VERSIONS_CPLD] = NFP_VERSIONS_CPLD_OFF,
+ [NFP_VERSIONS_APP] = NFP_VERSIONS_APP_OFF,
+ [NFP_VERSIONS_BUNDLE] = NFP_VERSIONS_BUNDLE_OFF,
+ [NFP_VERSIONS_UNDI] = NFP_VERSIONS_UNDI_OFF,
+ [NFP_VERSIONS_NCSI] = NFP_VERSIONS_NCSI_OFF,
+ [NFP_VERSIONS_CFGR] = NFP_VERSIONS_CFGR_OFF,
+ };
+ unsigned int field, buf_field_cnt, buf_off;
+
+ if (id >= ARRAY_SIZE(id2off) || !id2off[id])
+ return ERR_PTR(-EINVAL);
+
+ field = id * 2 + flash;
+
+ buf_field_cnt = get_unaligned_le16(buf);
+ if (buf_field_cnt <= field)
+ return ERR_PTR(-ENOENT);
+
+ buf_off = get_unaligned_le16(buf + id2off[id] + flash * 2);
+ if (!buf_off)
+ return ERR_PTR(-ENOENT);
+
+ if (buf_off >= size)
+ return ERR_PTR(-EINVAL);
+ if (strnlen(&buf[buf_off], size - buf_off) == size - buf_off)
+ return ERR_PTR(-EINVAL);
+
+ return (const char *)&buf[buf_off];
+}
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h
index ff33ac54097a..246e213f1514 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h
@@ -38,6 +38,11 @@ static inline bool nfp_nsp_has_hwinfo_lookup(struct nfp_nsp *state)
return nfp_nsp_get_abi_ver_minor(state) > 24;
}
+static inline bool nfp_nsp_has_versions(struct nfp_nsp *state)
+{
+ return nfp_nsp_get_abi_ver_minor(state) > 27;
+}
+
enum nfp_eth_interface {
NFP_INTERFACE_NONE = 0,
NFP_INTERFACE_SFP = 1,
@@ -208,4 +213,19 @@ enum nfp_nsp_sensor_id {
int nfp_hwmon_read_sensor(struct nfp_cpp *cpp, enum nfp_nsp_sensor_id id,
long *val);
+#define NFP_NSP_VERSION_BUFSZ 1024 /* reasonable size, not in the ABI */
+
+enum nfp_nsp_versions {
+ NFP_VERSIONS_BSP,
+ NFP_VERSIONS_CPLD,
+ NFP_VERSIONS_APP,
+ NFP_VERSIONS_BUNDLE,
+ NFP_VERSIONS_UNDI,
+ NFP_VERSIONS_NCSI,
+ NFP_VERSIONS_CFGR,
+};
+
+int nfp_nsp_versions(struct nfp_nsp *state, void *buf, unsigned int size);
+const char *nfp_nsp_versions_get(enum nfp_nsp_versions id, bool flash,
+ const u8 *buf, unsigned int size);
#endif
--
2.19.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next v2 6/7] nfp: devlink: report the running and flashed versions
2019-01-30 19:05 [PATCH net-next v2 0/7] devlink: add device (driver) information API Jakub Kicinski
` (4 preceding siblings ...)
2019-01-30 19:05 ` [PATCH net-next v2 5/7] nfp: nsp: add support for versions command Jakub Kicinski
@ 2019-01-30 19:05 ` Jakub Kicinski
2019-01-30 21:57 ` Jiri Pirko
2019-01-30 19:05 ` [PATCH net-next v2 7/7] ethtool: add compat for devlink info Jakub Kicinski
6 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2019-01-30 19:05 UTC (permalink / raw)
To: davem
Cc: netdev, oss-drivers, jiri, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon, Jakub Kicinski
Report versions of firmware components using the new NSP command.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../net/ethernet/netronome/nfp/nfp_devlink.c | 86 +++++++++++++++++++
include/net/devlink.h | 11 +++
2 files changed, 97 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index 9857fa663adf..fade37d2b796 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -207,11 +207,59 @@ nfp_devlink_versions_get_hwinfo(struct nfp_pf *pf, struct devlink_info_req *req)
return 0;
}
+static const struct nfp_devlink_versions {
+ enum nfp_nsp_versions id;
+ const char *key;
+} nfp_devlink_versions_nsp[] = {
+ { NFP_VERSIONS_BUNDLE, "fw.bundle_id", },
+ { NFP_VERSIONS_BSP, DEVLINK_VERSION_GENERIC_FW_MGMT, },
+ { NFP_VERSIONS_CPLD, "fw.cpld", },
+ { NFP_VERSIONS_APP, DEVLINK_VERSION_GENERIC_FW_APP, },
+ { NFP_VERSIONS_UNDI, DEVLINK_VERSION_GENERIC_FW_UNDI, },
+ { NFP_VERSIONS_NCSI, DEVLINK_VERSION_GENERIC_FW_NCSI, },
+ { NFP_VERSIONS_CFGR, "chip.init", },
+};
+
+static int
+nfp_devlink_versions_get_nsp(struct devlink_info_req *req, bool flash,
+ const u8 *buf, unsigned int size)
+{
+ enum devlink_version_type type;
+ unsigned int i;
+ int err;
+
+ type = flash ? DEVLINK_VERSION_STORED : DEVLINK_VERSION_RUNNING;
+
+ for (i = 0; i < ARRAY_SIZE(nfp_devlink_versions_nsp); i++) {
+ const struct nfp_devlink_versions *info;
+ const char *version;
+
+ info = &nfp_devlink_versions_nsp[i];
+
+ version = nfp_nsp_versions_get(info->id, flash, buf, size);
+ if (IS_ERR(version)) {
+ if (PTR_ERR(version) == -ENOENT)
+ continue;
+ else
+ return PTR_ERR(version);
+ }
+
+ err = devlink_info_report_version(req, type,
+ info->key, version);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
static int
nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
struct netlink_ext_ack *extack)
{
struct nfp_pf *pf = devlink_priv(devlink);
+ struct nfp_nsp *nsp;
+ char *buf = NULL;
const char *sn;
int err;
@@ -226,7 +274,45 @@ nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
return err;
}
+ nsp = nfp_nsp_open(pf->cpp);
+ if (IS_ERR(nsp)) {
+ NL_SET_ERR_MSG_MOD(extack, "can't access NSP");
+ return PTR_ERR(nsp);
+ }
+
+ if (nfp_nsp_has_versions(nsp)) {
+ buf = kzalloc(NFP_NSP_VERSION_BUFSZ, GFP_KERNEL);
+ if (!buf) {
+ err = -ENOMEM;
+ goto err_close_nsp;
+ }
+
+ err = nfp_nsp_versions(nsp, buf, NFP_NSP_VERSION_BUFSZ);
+ if (err)
+ goto err_free_buf;
+
+ err = nfp_devlink_versions_get_nsp(req, false,
+ buf, NFP_NSP_VERSION_BUFSZ);
+ if (err)
+ goto err_free_buf;
+
+ err = nfp_devlink_versions_get_nsp(req, true,
+ buf, NFP_NSP_VERSION_BUFSZ);
+ if (err)
+ goto err_free_buf;
+
+ kfree(buf);
+ }
+
+ nfp_nsp_close(nsp);
+
return nfp_devlink_versions_get_hwinfo(pf, req);
+
+err_free_buf:
+ kfree(buf);
+err_close_nsp:
+ nfp_nsp_close(nsp);
+ return err;
}
const struct devlink_ops nfp_devlink_ops = {
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 3d553cc6693d..c678ed0cb099 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -433,6 +433,17 @@ enum devlink_param_wol_types {
/* Revision of board design */
#define DEVLINK_VERSION_GENERIC_BOARD_REV "board.rev"
+/* Control processor FW version, FW is responsible for house keeping tasks,
+ * PHY control etc.
+ */
+#define DEVLINK_VERSION_GENERIC_FW_MGMT "fw.mgmt"
+/* Data path microcode controlling high-speed packet processing */
+#define DEVLINK_VERSION_GENERIC_FW_APP "fw.app"
+/* UNDI software version */
+#define DEVLINK_VERSION_GENERIC_FW_UNDI "fw.undi"
+/* NCSI support/handler version */
+#define DEVLINK_VERSION_GENERIC_FW_NCSI "fw.ncsi"
+
enum devlink_version_type {
DEVLINK_VERSION_FIXED,
DEVLINK_VERSION_STORED,
--
2.19.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v2 6/7] nfp: devlink: report the running and flashed versions
2019-01-30 19:05 ` [PATCH net-next v2 6/7] nfp: devlink: report the running and flashed versions Jakub Kicinski
@ 2019-01-30 21:57 ` Jiri Pirko
2019-01-30 22:21 ` Jakub Kicinski
0 siblings, 1 reply; 17+ messages in thread
From: Jiri Pirko @ 2019-01-30 21:57 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek,
eugenem, jonathan.lemon
Wed, Jan 30, 2019 at 08:05:12PM CET, jakub.kicinski@netronome.com wrote:
>Report versions of firmware components using the new NSP command.
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>---
> .../net/ethernet/netronome/nfp/nfp_devlink.c | 86 +++++++++++++++++++
> include/net/devlink.h | 11 +++
> 2 files changed, 97 insertions(+)
>
>diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>index 9857fa663adf..fade37d2b796 100644
>--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>@@ -207,11 +207,59 @@ nfp_devlink_versions_get_hwinfo(struct nfp_pf *pf, struct devlink_info_req *req)
> return 0;
> }
>
>+static const struct nfp_devlink_versions {
>+ enum nfp_nsp_versions id;
>+ const char *key;
>+} nfp_devlink_versions_nsp[] = {
>+ { NFP_VERSIONS_BUNDLE, "fw.bundle_id", },
>+ { NFP_VERSIONS_BSP, DEVLINK_VERSION_GENERIC_FW_MGMT, },
>+ { NFP_VERSIONS_CPLD, "fw.cpld", },
>+ { NFP_VERSIONS_APP, DEVLINK_VERSION_GENERIC_FW_APP, },
>+ { NFP_VERSIONS_UNDI, DEVLINK_VERSION_GENERIC_FW_UNDI, },
>+ { NFP_VERSIONS_NCSI, DEVLINK_VERSION_GENERIC_FW_NCSI, },
>+ { NFP_VERSIONS_CFGR, "chip.init", },
>+};
>+
>+static int
>+nfp_devlink_versions_get_nsp(struct devlink_info_req *req, bool flash,
>+ const u8 *buf, unsigned int size)
>+{
>+ enum devlink_version_type type;
>+ unsigned int i;
>+ int err;
>+
>+ type = flash ? DEVLINK_VERSION_STORED : DEVLINK_VERSION_RUNNING;
>+
>+ for (i = 0; i < ARRAY_SIZE(nfp_devlink_versions_nsp); i++) {
>+ const struct nfp_devlink_versions *info;
>+ const char *version;
>+
>+ info = &nfp_devlink_versions_nsp[i];
>+
>+ version = nfp_nsp_versions_get(info->id, flash, buf, size);
>+ if (IS_ERR(version)) {
>+ if (PTR_ERR(version) == -ENOENT)
>+ continue;
>+ else
>+ return PTR_ERR(version);
>+ }
>+
>+ err = devlink_info_report_version(req, type,
>+ info->key, version);
>+ if (err)
>+ return err;
>+ }
>+
>+ return 0;
>+}
>+
> static int
> nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
> struct netlink_ext_ack *extack)
> {
> struct nfp_pf *pf = devlink_priv(devlink);
>+ struct nfp_nsp *nsp;
>+ char *buf = NULL;
> const char *sn;
> int err;
>
>@@ -226,7 +274,45 @@ nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
> return err;
> }
>
>+ nsp = nfp_nsp_open(pf->cpp);
>+ if (IS_ERR(nsp)) {
>+ NL_SET_ERR_MSG_MOD(extack, "can't access NSP");
>+ return PTR_ERR(nsp);
>+ }
>+
>+ if (nfp_nsp_has_versions(nsp)) {
>+ buf = kzalloc(NFP_NSP_VERSION_BUFSZ, GFP_KERNEL);
>+ if (!buf) {
>+ err = -ENOMEM;
>+ goto err_close_nsp;
>+ }
>+
>+ err = nfp_nsp_versions(nsp, buf, NFP_NSP_VERSION_BUFSZ);
>+ if (err)
>+ goto err_free_buf;
>+
>+ err = nfp_devlink_versions_get_nsp(req, false,
>+ buf, NFP_NSP_VERSION_BUFSZ);
>+ if (err)
>+ goto err_free_buf;
>+
>+ err = nfp_devlink_versions_get_nsp(req, true,
>+ buf, NFP_NSP_VERSION_BUFSZ);
>+ if (err)
>+ goto err_free_buf;
>+
>+ kfree(buf);
>+ }
>+
>+ nfp_nsp_close(nsp);
>+
> return nfp_devlink_versions_get_hwinfo(pf, req);
>+
>+err_free_buf:
>+ kfree(buf);
>+err_close_nsp:
>+ nfp_nsp_close(nsp);
>+ return err;
> }
>
> const struct devlink_ops nfp_devlink_ops = {
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 3d553cc6693d..c678ed0cb099 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -433,6 +433,17 @@ enum devlink_param_wol_types {
> /* Revision of board design */
> #define DEVLINK_VERSION_GENERIC_BOARD_REV "board.rev"
>
>+/* Control processor FW version, FW is responsible for house keeping tasks,
>+ * PHY control etc.
>+ */
>+#define DEVLINK_VERSION_GENERIC_FW_MGMT "fw.mgmt"
>+/* Data path microcode controlling high-speed packet processing */
>+#define DEVLINK_VERSION_GENERIC_FW_APP "fw.app"
>+/* UNDI software version */
>+#define DEVLINK_VERSION_GENERIC_FW_UNDI "fw.undi"
>+/* NCSI support/handler version */
>+#define DEVLINK_VERSION_GENERIC_FW_NCSI "fw.ncsi"
Same here. Also, please put "INFO" in the names to respect the namespacing
>+
> enum devlink_version_type {
> DEVLINK_VERSION_FIXED,
> DEVLINK_VERSION_STORED,
>--
>2.19.2
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v2 6/7] nfp: devlink: report the running and flashed versions
2019-01-30 21:57 ` Jiri Pirko
@ 2019-01-30 22:21 ` Jakub Kicinski
2019-01-30 22:19 ` Jiri Pirko
0 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2019-01-30 22:21 UTC (permalink / raw)
To: Jiri Pirko
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek,
eugenem, jonathan.lemon
On Wed, 30 Jan 2019 22:57:52 +0100, Jiri Pirko wrote:
> >+/* Control processor FW version, FW is responsible for house keeping tasks,
> >+ * PHY control etc.
> >+ */
> >+#define DEVLINK_VERSION_GENERIC_FW_MGMT "fw.mgmt"
> >+/* Data path microcode controlling high-speed packet processing */
> >+#define DEVLINK_VERSION_GENERIC_FW_APP "fw.app"
> >+/* UNDI software version */
> >+#define DEVLINK_VERSION_GENERIC_FW_UNDI "fw.undi"
> >+/* NCSI support/handler version */
> >+#define DEVLINK_VERSION_GENERIC_FW_NCSI "fw.ncsi"
>
> Same here. Also, please put "INFO" in the names to respect the namespacing
Ack on all, and thanks for the reviews! Do you also think I should add
a doc with them? I was going back and forth on that..
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v2 6/7] nfp: devlink: report the running and flashed versions
2019-01-30 22:21 ` Jakub Kicinski
@ 2019-01-30 22:19 ` Jiri Pirko
0 siblings, 0 replies; 17+ messages in thread
From: Jiri Pirko @ 2019-01-30 22:19 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek,
eugenem, jonathan.lemon
Wed, Jan 30, 2019 at 11:21:58PM CET, jakub.kicinski@netronome.com wrote:
>On Wed, 30 Jan 2019 22:57:52 +0100, Jiri Pirko wrote:
>> >+/* Control processor FW version, FW is responsible for house keeping tasks,
>> >+ * PHY control etc.
>> >+ */
>> >+#define DEVLINK_VERSION_GENERIC_FW_MGMT "fw.mgmt"
>> >+/* Data path microcode controlling high-speed packet processing */
>> >+#define DEVLINK_VERSION_GENERIC_FW_APP "fw.app"
>> >+/* UNDI software version */
>> >+#define DEVLINK_VERSION_GENERIC_FW_UNDI "fw.undi"
>> >+/* NCSI support/handler version */
>> >+#define DEVLINK_VERSION_GENERIC_FW_NCSI "fw.ncsi"
>>
>> Same here. Also, please put "INFO" in the names to respect the namespacing
>
>Ack on all, and thanks for the reviews! Do you also think I should add
>a doc with them? I was going back and forth on that..
Adding docs would not do harm I believe.
Thanks!
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next v2 7/7] ethtool: add compat for devlink info
2019-01-30 19:05 [PATCH net-next v2 0/7] devlink: add device (driver) information API Jakub Kicinski
` (5 preceding siblings ...)
2019-01-30 19:05 ` [PATCH net-next v2 6/7] nfp: devlink: report the running and flashed versions Jakub Kicinski
@ 2019-01-30 19:05 ` Jakub Kicinski
2019-01-30 22:12 ` Jiri Pirko
` (2 more replies)
6 siblings, 3 replies; 17+ messages in thread
From: Jakub Kicinski @ 2019-01-30 19:05 UTC (permalink / raw)
To: davem
Cc: netdev, oss-drivers, jiri, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon, Jakub Kicinski
If driver did not fill the fw_version field, try to call into
the new devlink get_info op and collect the versions that way.
We assume ethtool was always reporting running versions.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
include/net/devlink.h | 7 ++++++
net/core/devlink.c | 52 ++++++++++++++++++++++++++++++++++++++++++-
net/core/ethtool.c | 7 ++++++
3 files changed, 65 insertions(+), 1 deletion(-)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index c678ed0cb099..b4750e93303a 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -640,6 +640,8 @@ int devlink_info_report_version(struct devlink_info_req *req,
enum devlink_version_type type,
const char *version_name,
const char *version_value);
+void devlink_compat_running_versions(struct net_device *dev,
+ char *buf, size_t len);
#else
@@ -957,6 +959,11 @@ devlink_info_report_version(struct devlink_info_req *req,
{
return 0;
}
+
+static inline void
+devlink_compat_running_versions(struct net_device *dev, char *buf, size_t len)
+{
+}
#endif
#endif /* _NET_DEVLINK_H_ */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index e2027d3a5e40..5313e5918ee2 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -3715,12 +3715,18 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
}
struct devlink_info_req {
+ bool compat;
struct sk_buff *msg;
+ /* For compat call */
+ char *buf;
+ size_t len;
};
int devlink_info_report_driver_name(struct devlink_info_req *req,
const char *name)
{
+ if (req->compat)
+ return 0;
return nla_put_string(req->msg, DEVLINK_ATTR_INFO_DRV_NAME, name);
}
EXPORT_SYMBOL_GPL(devlink_info_report_driver_name);
@@ -3728,6 +3734,8 @@ EXPORT_SYMBOL_GPL(devlink_info_report_driver_name);
int devlink_info_report_serial_number(struct devlink_info_req *req,
const char *sn)
{
+ if (req->compat)
+ return 0;
return nla_put_string(req->msg, DEVLINK_ATTR_INFO_SERIAL_NUMBER, sn);
}
EXPORT_SYMBOL_GPL(devlink_info_report_serial_number);
@@ -3743,7 +3751,15 @@ int devlink_info_report_version(struct devlink_info_req *req,
[DEVLINK_VERSION_RUNNING] = DEVLINK_ATTR_INFO_VERSION_RUNNING,
};
struct nlattr *nest;
- int err;
+ int len, err;
+
+ if (req->compat) {
+ if (type == DEVLINK_VERSION_RUNNING) {
+ len = strlcpy(req->buf, version_value, req->len);
+ req->len = max_t(size_t, 0, req->len - len);
+ }
+ return 0;
+ }
if (type >= ARRAY_SIZE(type2attr) || !type2attr[type])
return -EINVAL;
@@ -3789,6 +3805,7 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
if (devlink_nl_put_handle(msg, devlink))
goto err_cancel_msg;
+ memset(&req, 0, sizeof(req));
req.msg = msg;
err = devlink->ops->info_get(devlink, &req, extack);
if (err)
@@ -5263,6 +5280,39 @@ int devlink_region_snapshot_create(struct devlink_region *region, u64 data_len,
}
EXPORT_SYMBOL_GPL(devlink_region_snapshot_create);
+void devlink_compat_running_versions(struct net_device *dev,
+ char *buf, size_t len)
+{
+ struct devlink_port *devlink_port;
+ struct devlink_info_req req;
+ struct devlink *devlink;
+ bool found = false;
+
+ mutex_lock(&devlink_mutex);
+ list_for_each_entry(devlink, &devlink_list, list) {
+ mutex_lock(&devlink->lock);
+ list_for_each_entry(devlink_port, &devlink->port_list, list) {
+ if (devlink_port->type == DEVLINK_PORT_TYPE_ETH ||
+ devlink_port->type_dev == dev) {
+ mutex_unlock(&devlink->lock);
+ found = true;
+ goto out;
+ }
+ }
+ mutex_unlock(&devlink->lock);
+ }
+out:
+ if (found && devlink->ops->info_get) {
+ memset(&req, 0, sizeof(req));
+ req.compat = true;
+ req.buf = buf;
+ req.len = len;
+
+ devlink->ops->info_get(devlink, &req, NULL);
+ }
+ mutex_unlock(&devlink_mutex);
+}
+
static int __init devlink_module_init(void)
{
return genl_register_family(&devlink_nl_family);
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 158264f7cfaf..176b17d11f08 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -27,6 +27,7 @@
#include <linux/rtnetlink.h>
#include <linux/sched/signal.h>
#include <linux/net.h>
+#include <net/devlink.h>
#include <net/xdp_sock.h>
/*
@@ -803,6 +804,12 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
if (ops->get_eeprom_len)
info.eedump_len = ops->get_eeprom_len(dev);
+ rtnl_unlock();
+ if (!info.fw_version[0])
+ devlink_compat_running_versions(dev, info.fw_version,
+ ARRAY_SIZE(info.fw_version));
+ rtnl_lock();
+
if (copy_to_user(useraddr, &info, sizeof(info)))
return -EFAULT;
return 0;
--
2.19.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v2 7/7] ethtool: add compat for devlink info
2019-01-30 19:05 ` [PATCH net-next v2 7/7] ethtool: add compat for devlink info Jakub Kicinski
@ 2019-01-30 22:12 ` Jiri Pirko
2019-01-31 19:05 ` kbuild test robot
2019-02-01 4:06 ` kbuild test robot
2 siblings, 0 replies; 17+ messages in thread
From: Jiri Pirko @ 2019-01-30 22:12 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek,
eugenem, jonathan.lemon
Wed, Jan 30, 2019 at 08:05:13PM CET, jakub.kicinski@netronome.com wrote:
>If driver did not fill the fw_version field, try to call into
>the new devlink get_info op and collect the versions that way.
>We assume ethtool was always reporting running versions.
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>---
> include/net/devlink.h | 7 ++++++
> net/core/devlink.c | 52 ++++++++++++++++++++++++++++++++++++++++++-
> net/core/ethtool.c | 7 ++++++
> 3 files changed, 65 insertions(+), 1 deletion(-)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index c678ed0cb099..b4750e93303a 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -640,6 +640,8 @@ int devlink_info_report_version(struct devlink_info_req *req,
> enum devlink_version_type type,
> const char *version_name,
> const char *version_value);
>+void devlink_compat_running_versions(struct net_device *dev,
>+ char *buf, size_t len);
>
> #else
>
>@@ -957,6 +959,11 @@ devlink_info_report_version(struct devlink_info_req *req,
> {
> return 0;
> }
>+
>+static inline void
>+devlink_compat_running_versions(struct net_device *dev, char *buf, size_t len)
>+{
>+}
> #endif
>
> #endif /* _NET_DEVLINK_H_ */
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index e2027d3a5e40..5313e5918ee2 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -3715,12 +3715,18 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
> }
>
> struct devlink_info_req {
>+ bool compat;
> struct sk_buff *msg;
>+ /* For compat call */
>+ char *buf;
union?
>+ size_t len;
> };
>
> int devlink_info_report_driver_name(struct devlink_info_req *req,
> const char *name)
> {
>+ if (req->compat)
>+ return 0;
> return nla_put_string(req->msg, DEVLINK_ATTR_INFO_DRV_NAME, name);
> }
> EXPORT_SYMBOL_GPL(devlink_info_report_driver_name);
>@@ -3728,6 +3734,8 @@ EXPORT_SYMBOL_GPL(devlink_info_report_driver_name);
> int devlink_info_report_serial_number(struct devlink_info_req *req,
> const char *sn)
> {
>+ if (req->compat)
>+ return 0;
> return nla_put_string(req->msg, DEVLINK_ATTR_INFO_SERIAL_NUMBER, sn);
> }
> EXPORT_SYMBOL_GPL(devlink_info_report_serial_number);
>@@ -3743,7 +3751,15 @@ int devlink_info_report_version(struct devlink_info_req *req,
> [DEVLINK_VERSION_RUNNING] = DEVLINK_ATTR_INFO_VERSION_RUNNING,
> };
> struct nlattr *nest;
>- int err;
>+ int len, err;
>+
>+ if (req->compat) {
>+ if (type == DEVLINK_VERSION_RUNNING) {
>+ len = strlcpy(req->buf, version_value, req->len);
If you have multiple running versions, shouldn't you concat them into a
single string for compat?
>+ req->len = max_t(size_t, 0, req->len - len);
>+ }
>+ return 0;
>+ }
>
> if (type >= ARRAY_SIZE(type2attr) || !type2attr[type])
> return -EINVAL;
>@@ -3789,6 +3805,7 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
> if (devlink_nl_put_handle(msg, devlink))
> goto err_cancel_msg;
>
>+ memset(&req, 0, sizeof(req));
> req.msg = msg;
> err = devlink->ops->info_get(devlink, &req, extack);
> if (err)
>@@ -5263,6 +5280,39 @@ int devlink_region_snapshot_create(struct devlink_region *region, u64 data_len,
> }
> EXPORT_SYMBOL_GPL(devlink_region_snapshot_create);
>
>+void devlink_compat_running_versions(struct net_device *dev,
Why "versionS?"
>+ char *buf, size_t len)
>+{
>+ struct devlink_port *devlink_port;
>+ struct devlink_info_req req;
>+ struct devlink *devlink;
>+ bool found = false;
>+
>+ mutex_lock(&devlink_mutex);
>+ list_for_each_entry(devlink, &devlink_list, list) {
>+ mutex_lock(&devlink->lock);
>+ list_for_each_entry(devlink_port, &devlink->port_list, list) {
>+ if (devlink_port->type == DEVLINK_PORT_TYPE_ETH ||
>+ devlink_port->type_dev == dev) {
>+ mutex_unlock(&devlink->lock);
>+ found = true;
>+ goto out;
>+ }
>+ }
>+ mutex_unlock(&devlink->lock);
>+ }
>+out:
>+ if (found && devlink->ops->info_get) {
>+ memset(&req, 0, sizeof(req));
>+ req.compat = true;
>+ req.buf = buf;
>+ req.len = len;
>+
>+ devlink->ops->info_get(devlink, &req, NULL);
I don't really like this compat bool and check in "put" functions.
I would rather like to run info_get() in case of both compat and
non-compat in the same way. Then for compat just pickup the info
which is needed and put to buf.
I don't see problem in using netlink skb for that direcly.
>+ }
>+ mutex_unlock(&devlink_mutex);
>+}
>+
> static int __init devlink_module_init(void)
> {
> return genl_register_family(&devlink_nl_family);
>diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>index 158264f7cfaf..176b17d11f08 100644
>--- a/net/core/ethtool.c
>+++ b/net/core/ethtool.c
>@@ -27,6 +27,7 @@
> #include <linux/rtnetlink.h>
> #include <linux/sched/signal.h>
> #include <linux/net.h>
>+#include <net/devlink.h>
> #include <net/xdp_sock.h>
>
> /*
>@@ -803,6 +804,12 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
> if (ops->get_eeprom_len)
> info.eedump_len = ops->get_eeprom_len(dev);
>
>+ rtnl_unlock();
>+ if (!info.fw_version[0])
>+ devlink_compat_running_versions(dev, info.fw_version,
>+ ARRAY_SIZE(info.fw_version));
ARRAY_SIZE looks odd here. "sizeof()" would be better I think.
>+ rtnl_lock();
>+
> if (copy_to_user(useraddr, &info, sizeof(info)))
> return -EFAULT;
> return 0;
>--
>2.19.2
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v2 7/7] ethtool: add compat for devlink info
2019-01-30 19:05 ` [PATCH net-next v2 7/7] ethtool: add compat for devlink info Jakub Kicinski
2019-01-30 22:12 ` Jiri Pirko
@ 2019-01-31 19:05 ` kbuild test robot
2019-02-01 4:06 ` kbuild test robot
2 siblings, 0 replies; 17+ messages in thread
From: kbuild test robot @ 2019-01-31 19:05 UTC (permalink / raw)
To: Jakub Kicinski
Cc: kbuild-all, davem, netdev, oss-drivers, jiri, andrew, f.fainelli,
mkubecek, eugenem, jonathan.lemon, Jakub Kicinski
[-- Attachment #1: Type: text/plain, Size: 2583 bytes --]
Hi Jakub,
I love your patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Jakub-Kicinski/devlink-add-device-driver-information-API/20190131-222221
config: i386-randconfig-n0-01300126 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-15) 8.2.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
ld: net/core/ethtool.o: in function `ethtool_get_drvinfo':
>> net/core/ethtool.c:809: undefined reference to `devlink_compat_running_versions'
vim +809 net/core/ethtool.c
760
761 static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
762 void __user *useraddr)
763 {
764 struct ethtool_drvinfo info;
765 const struct ethtool_ops *ops = dev->ethtool_ops;
766
767 memset(&info, 0, sizeof(info));
768 info.cmd = ETHTOOL_GDRVINFO;
769 if (ops->get_drvinfo) {
770 ops->get_drvinfo(dev, &info);
771 } else if (dev->dev.parent && dev->dev.parent->driver) {
772 strlcpy(info.bus_info, dev_name(dev->dev.parent),
773 sizeof(info.bus_info));
774 strlcpy(info.driver, dev->dev.parent->driver->name,
775 sizeof(info.driver));
776 } else {
777 return -EOPNOTSUPP;
778 }
779
780 /*
781 * this method of obtaining string set info is deprecated;
782 * Use ETHTOOL_GSSET_INFO instead.
783 */
784 if (ops->get_sset_count) {
785 int rc;
786
787 rc = ops->get_sset_count(dev, ETH_SS_TEST);
788 if (rc >= 0)
789 info.testinfo_len = rc;
790 rc = ops->get_sset_count(dev, ETH_SS_STATS);
791 if (rc >= 0)
792 info.n_stats = rc;
793 rc = ops->get_sset_count(dev, ETH_SS_PRIV_FLAGS);
794 if (rc >= 0)
795 info.n_priv_flags = rc;
796 }
797 if (ops->get_regs_len) {
798 int ret = ops->get_regs_len(dev);
799
800 if (ret > 0)
801 info.regdump_len = ret;
802 }
803
804 if (ops->get_eeprom_len)
805 info.eedump_len = ops->get_eeprom_len(dev);
806
807 rtnl_unlock();
808 if (!info.fw_version[0])
> 809 devlink_compat_running_versions(dev, info.fw_version,
810 ARRAY_SIZE(info.fw_version));
811 rtnl_lock();
812
813 if (copy_to_user(useraddr, &info, sizeof(info)))
814 return -EFAULT;
815 return 0;
816 }
817
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26890 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v2 7/7] ethtool: add compat for devlink info
2019-01-30 19:05 ` [PATCH net-next v2 7/7] ethtool: add compat for devlink info Jakub Kicinski
2019-01-30 22:12 ` Jiri Pirko
2019-01-31 19:05 ` kbuild test robot
@ 2019-02-01 4:06 ` kbuild test robot
2 siblings, 0 replies; 17+ messages in thread
From: kbuild test robot @ 2019-02-01 4:06 UTC (permalink / raw)
To: Jakub Kicinski
Cc: kbuild-all, davem, netdev, oss-drivers, jiri, andrew, f.fainelli,
mkubecek, eugenem, jonathan.lemon, Jakub Kicinski
[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]
Hi Jakub,
I love your patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Jakub-Kicinski/devlink-add-device-driver-information-API/20190131-222221
config: m68k-sun3_defconfig (attached as .config)
compiler: m68k-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=m68k
All errors (new ones prefixed by >>):
m68k-linux-gnu-ld: drivers/rtc/proc.o: in function `is_rtc_hctosys.isra.0':
proc.c:(.text+0x178): undefined reference to `strcmp'
m68k-linux-gnu-ld: net/core/ethtool.o: in function `ethtool_get_drvinfo':
>> ethtool.c:(.text+0xc08): undefined reference to `devlink_compat_running_versions'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 12123 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread