Hi! This series contains two minor touch ups for devlink code. First || is corrected to && in the ethtool compat code. Next patch decreases the stack allocation size. On the nfp side after further discussions with the manufacturing team we decided to realign the serial number contents slightly and rename one of the other fields from "vendor" to "mfr", short for "manufacture". v2: - add patch 3 - move board maker as a generic attribute. Jakub Kicinski (5): devlink: fix condition for compat device info devlink: don't allocate attrs on the stack devlink: add a generic board.manufacture version name nfp: devlink: use the generic manufacture identifier instead of vendor nfp: devlink: include vendor/product info in serial number .../networking/devlink-info-versions.rst | 5 ++++ .../net/ethernet/netronome/nfp/nfp_devlink.c | 23 +++++++++++++++---- include/net/devlink.h | 2 ++ net/core/devlink.c | 16 +++++++++---- 4 files changed, 37 insertions(+), 9 deletions(-) -- 2.19.2
We need the port to be both ethernet and have the rigth netdev, not one or the other. Fixes: ddb6e99e2db1 ("ethtool: add compat for devlink info") Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> Acked-by: Jiri Pirko <jiri@mellanox.com> --- net/core/devlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/core/devlink.c b/net/core/devlink.c index e6a015b8ac9b..cf0f511bc56c 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -6385,7 +6385,7 @@ void devlink_compat_running_version(struct net_device *dev, 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 || + if (devlink_port->type == DEVLINK_PORT_TYPE_ETH && devlink_port->type_dev == dev) { __devlink_compat_running_version(devlink, buf, len); -- 2.19.2
Number of devlink attributes has grown over 128, causing the following warning: ../net/core/devlink.c: In function ‘devlink_nl_cmd_region_read_dumpit’: ../net/core/devlink.c:3740:1: warning: the frame size of 1064 bytes is larger than 1024 bytes [-Wframe-larger-than=] } ^ Since the number of attributes is only going to grow allocate the array dynamically. Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> Acked-by: Jiri Pirko <jiri@mellanox.com> --- net/core/devlink.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/net/core/devlink.c b/net/core/devlink.c index cf0f511bc56c..46c468a1f3dc 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -3629,26 +3629,30 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb, struct netlink_callback *cb) { u64 ret_offset, start_offset, end_offset = 0; - struct nlattr *attrs[DEVLINK_ATTR_MAX + 1]; const struct genl_ops *ops = cb->data; struct devlink_region *region; struct nlattr *chunks_attr; const char *region_name; struct devlink *devlink; + struct nlattr **attrs; bool dump = true; void *hdr; int err; start_offset = *((u64 *)&cb->args[0]); + attrs = kmalloc_array(DEVLINK_ATTR_MAX + 1, sizeof(*attrs), GFP_KERNEL); + if (!attrs) + return -ENOMEM; + err = nlmsg_parse(cb->nlh, GENL_HDRLEN + devlink_nl_family.hdrsize, attrs, DEVLINK_ATTR_MAX, ops->policy, cb->extack); if (err) - goto out; + goto out_free; devlink = devlink_get_from_attrs(sock_net(cb->skb->sk), attrs); if (IS_ERR(devlink)) - goto out; + goto out_free; mutex_lock(&devlink_mutex); mutex_lock(&devlink->lock); @@ -3710,6 +3714,7 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb, genlmsg_end(skb, hdr); mutex_unlock(&devlink->lock); mutex_unlock(&devlink_mutex); + kfree(attrs); return skb->len; @@ -3718,7 +3723,8 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb, out_unlock: mutex_unlock(&devlink->lock); mutex_unlock(&devlink_mutex); -out: +out_free: + kfree(attrs); return 0; } -- 2.19.2
At Jiri's suggestion add a generic "board.manufacture" version identifier. Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> --- Documentation/networking/devlink-info-versions.rst | 5 +++++ include/net/devlink.h | 2 ++ 2 files changed, 7 insertions(+) diff --git a/Documentation/networking/devlink-info-versions.rst b/Documentation/networking/devlink-info-versions.rst index 7d4ecf6b6f34..c79ad8593383 100644 --- a/Documentation/networking/devlink-info-versions.rst +++ b/Documentation/networking/devlink-info-versions.rst @@ -14,6 +14,11 @@ board.rev Board design revision. +board.manufacture +================= + +An identifier of the company or the facility which produced the part. + fw.mgmt ======= diff --git a/include/net/devlink.h b/include/net/devlink.h index 2b384a38911b..07660fe4c0e3 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -435,6 +435,8 @@ enum devlink_param_wol_types { #define DEVLINK_INFO_VERSION_GENERIC_BOARD_ID "board.id" /* Revision of board design */ #define DEVLINK_INFO_VERSION_GENERIC_BOARD_REV "board.rev" +/* Maker of the board */ +#define DEVLINK_INFO_VERSION_GENERIC_BOARD_MANUFACTURE "board.manufacture" /* Control processor FW version */ #define DEVLINK_INFO_VERSION_GENERIC_FW_MGMT "fw.mgmt" -- 2.19.2
Vendor may sound ambiguous, let's rename the fab string to "board.manufacture" (which was just added as a generic identifier). Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com> --- drivers/net/ethernet/netronome/nfp/nfp_devlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c index dddbb0575be9..bf4e124dbdd2 100644 --- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c +++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c @@ -178,7 +178,7 @@ static const struct nfp_devlink_versions_simple { } nfp_devlink_versions_hwinfo[] = { { DEVLINK_INFO_VERSION_GENERIC_BOARD_ID, "assembly.partno", }, { DEVLINK_INFO_VERSION_GENERIC_BOARD_REV, "assembly.revision", }, - { "board.vendor", /* fab */ "assembly.vendor", }, + { DEVLINK_INFO_VERSION_GENERIC_BOARD_MANUFACTURE, "assembly.vendor", }, { "board.model", /* code name */ "assembly.model", }, }; -- 2.19.2
The manufacturing team requests we include vendor and product in the serial number field, as the serial number itself is not unique across manufacturing facilities and products. Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com> --- .../net/ethernet/netronome/nfp/nfp_devlink.c | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c index bf4e124dbdd2..080a301f379e 100644 --- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c +++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c @@ -258,18 +258,33 @@ 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, *vendor, *part; struct nfp_nsp *nsp; char *buf = NULL; - const char *sn; int err; err = devlink_info_driver_name_put(req, "nfp"); if (err) return err; + vendor = nfp_hwinfo_lookup(pf->hwinfo, "assembly.vendor"); + part = nfp_hwinfo_lookup(pf->hwinfo, "assembly.partno"); sn = nfp_hwinfo_lookup(pf->hwinfo, "assembly.serial"); - if (sn) { - err = devlink_info_serial_number_put(req, sn); + if (vendor && part && sn) { + char *buf; + + buf = kmalloc(strlen(vendor) + strlen(part) + strlen(sn) + 1, + GFP_KERNEL); + if (!buf) + return -ENOMEM; + + buf[0] = '\0'; + strcat(buf, vendor); + strcat(buf, part); + strcat(buf, sn); + + err = devlink_info_serial_number_put(req, buf); + kfree(buf); if (err) return err; } -- 2.19.2
Mon, Feb 11, 2019 at 04:35:29AM CET, jakub.kicinski@netronome.com wrote:
>At Jiri's suggestion add a generic "board.manufacture"
>version identifier.
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Mon, Feb 11, 2019 at 04:35:31AM CET, jakub.kicinski@netronome.com wrote: >The manufacturing team requests we include vendor and product >in the serial number field, as the serial number itself is not >unique across manufacturing facilities and products. Oups. > >Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> >Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com> Acked-by: Jiri Pirko <jiri@mellanox.com>
Mon, Feb 11, 2019 at 04:35:30AM CET, jakub.kicinski@netronome.com wrote:
>Vendor may sound ambiguous, let's rename the fab string to
>"board.manufacture" (which was just added as a generic identifier).
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Sun, 10 Feb 2019 19:35:26 -0800
> This series contains two minor touch ups for devlink code. First
> || is corrected to && in the ethtool compat code. Next patch
> decreases the stack allocation size.
>
> On the nfp side after further discussions with the manufacturing
> team we decided to realign the serial number contents slightly and
> rename one of the other fields from "vendor" to "mfr", short for
> "manufacture".
>
> v2: - add patch 3 - move board maker as a generic attribute.
Series applied, thanks Jakub.