[PATCH net-next 0/4] devlink: minor tweaks to reported device info

[PATCH net-next 0/4] devlink: minor tweaks to reported device info

[Jakub Kicinski]

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".

Jakub Kicinski (4):
  devlink: fix condition for compat device info
  devlink: don't allocate attrs on the stack
  nfp: devlink: rename vendor to manufacture
  nfp: devlink: include vendor/product info in serial number

 .../net/ethernet/netronome/nfp/nfp_devlink.c  | 23 +++++++++++++++----
 net/core/devlink.c                            | 15 ++++++++----
 2 files changed, 29 insertions(+), 9 deletions(-)

-- 
2.19.2

[PATCH net-next 1/4] devlink: fix condition for compat device info

[Jakub Kicinski]

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>
---
 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

Re: [PATCH net-next 1/4] devlink: fix condition for compat device info

[Jiri Pirko]

Sat, Feb 09, 2019 at 04:16:08AM CET, jakub.kicinski@netronome.com wrote:
>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>

[PATCH net-next 2/4] devlink: don't allocate attrs on the stack

[Jakub Kicinski]

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>
---
 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

Re: [PATCH net-next 2/4] devlink: don't allocate attrs on the stack

[Jiri Pirko]

Sat, Feb 09, 2019 at 04:16:09AM CET, jakub.kicinski@netronome.com wrote:
>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>

[PATCH net-next 3/4] nfp: devlink: rename vendor to manufacture

[Jakub Kicinski]

Vendor may sound ambiguous, let's rename the fab string to
"board.manufacture".

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..6e15e216732a 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", },
+	{ "board.manufacture",				"assembly.vendor", },
 	{ "board.model", /* code name */		"assembly.model", },
 };
 
-- 
2.19.2

Re: [PATCH net-next 3/4] nfp: devlink: rename vendor to manufacture

[Jiri Pirko]

Sat, Feb 09, 2019 at 04:16:10AM CET, jakub.kicinski@netronome.com wrote:
>Vendor may sound ambiguous, let's rename the fab string to
>"board.manufacture".
>
>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..6e15e216732a 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", },
>+	{ "board.manufacture",				"assembly.vendor", },

I wonder, why this is not among generic?


> 	{ "board.model", /* code name */		"assembly.model", },
> };
> 
>-- 
>2.19.2
>

Re: [PATCH net-next 3/4] nfp: devlink: rename vendor to manufacture

[Jakub Kicinski]

On Sat, 9 Feb 2019 09:36:44 +0100, Jiri Pirko wrote:
> >+	{ "board.manufacture",				"assembly.vendor", },  
> 
> I wonder, why this is not among generic?

No real reason, I'll move it in v2.

[PATCH net-next 4/4] nfp: devlink: include vendor/product info in serial number

[Jakub Kicinski]

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 6e15e216732a..5fa11a27d640 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