[PATCH RFC iproute2-next 0/2] RDMA tool provider resource tracking

[PATCH RFC iproute2-next 0/2] RDMA tool provider resource tracking

[Steve Wise]

Hello,

This series enhances the iproute2 rdma tool to include displaying
provider-specific resource attributes.  It is the user-space part of
the kernel provider resource tracking series currently under
review [1].

This is an RFC and should not be merged yet.  Once [1] is in the
linux-rdma for-next branch (and all reviewing is complete), I'll post
a final version and request merging.

Thanks,

Steve.

[1] https://www.spinics.net/lists/linux-rdma/msg64013.html

Steve Wise (2):
  rdma: update rdma_netlink.h to get provider attrs
  rdma: print provider resource attributes

 rdma/include/uapi/rdma/rdma_netlink.h |  37 ++++++-
 rdma/rdma.c                           |   7 +-
 rdma/rdma.h                           |  11 ++
 rdma/res.c                            |  30 ++----
 rdma/utils.c                          | 194 ++++++++++++++++++++++++++++++++++
 5 files changed, 257 insertions(+), 22 deletions(-)

-- 
1.8.3.1

[PATCH RFC iproute2-next 1/2] rdma: update rdma_netlink.h to get provider attrs

[Steve Wise]

Signed-off-by: Steve Wise <swise@opengridcomputing.com>
---
 rdma/include/uapi/rdma/rdma_netlink.h | 37 ++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/rdma/include/uapi/rdma/rdma_netlink.h b/rdma/include/uapi/rdma/rdma_netlink.h
index 45474f1..faea9d5 100644
--- a/rdma/include/uapi/rdma/rdma_netlink.h
+++ b/rdma/include/uapi/rdma/rdma_netlink.h
@@ -249,10 +249,22 @@ enum rdma_nldev_command {
 	RDMA_NLDEV_NUM_OPS
 };
 
+enum {
+	RDMA_NLDEV_ATTR_ENTRY_STRLEN = 16,
+};
+
+enum rdma_nldev_print_type {
+	RDMA_NLDEV_PRINT_TYPE_UNSPEC,
+	RDMA_NLDEV_PRINT_TYPE_HEX,
+};
+
 enum rdma_nldev_attr {
 	/* don't change the order or add anything between, this is ABI! */
 	RDMA_NLDEV_ATTR_UNSPEC,
 
+	/* Pad attribute for 64b alignment */
+	RDMA_NLDEV_ATTR_PAD = RDMA_NLDEV_ATTR_UNSPEC,
+
 	/* Identifier for ib_device */
 	RDMA_NLDEV_ATTR_DEV_INDEX,		/* u32 */
 
@@ -387,8 +399,31 @@ enum rdma_nldev_attr {
 	RDMA_NLDEV_ATTR_RES_PD_ENTRY,		/* nested table */
 	RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY,	/* u32 */
 	RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY,	/* u32 */
+	/*
+	 * provider-specific attributes.
+	 */
+	RDMA_NLDEV_ATTR_PROVIDER,		/* nested table */
+	RDMA_NLDEV_ATTR_PROVIDER_ENTRY,		/* nested table */
+	RDMA_NLDEV_ATTR_PROVIDER_STRING,	/* string */
+	/*
+	 * u8 values from enum rdma_nldev_print_type
+	 */
+	RDMA_NLDEV_ATTR_PROVIDER_PRINT_TYPE,	/* u8 */
+	RDMA_NLDEV_ATTR_PROVIDER_S32,		/* s32 */
+	RDMA_NLDEV_ATTR_PROVIDER_U32,		/* u32 */
+	RDMA_NLDEV_ATTR_PROVIDER_S64,		/* s64 */
+	RDMA_NLDEV_ATTR_PROVIDER_U64,		/* u64 */
 
-	/* Netdev information for relevant protocols, like RoCE and iWARP */
+	/*
+	 * Provides logical name and index of netdevice which is
+	 * connected to physical port. This information is relevant
+	 * for RoCE and iWARP.
+	 *
+	 * The netdevices which are associated with containers are
+	 * supposed to be exported together with GID table once it
+	 * will be exposed through the netlink. Because the
+	 * associated netdevices are properties of GIDs.
+	 */
 	RDMA_NLDEV_ATTR_NDEV_INDEX,		/* u32 */
 	RDMA_NLDEV_ATTR_NDEV_NAME,		/* string */
 
-- 
1.8.3.1

Re: [PATCH RFC iproute2-next 1/2] rdma: update rdma_netlink.h to get provider attrs

[David Ahern]

On 4/30/18 8:36 AM, Steve Wise wrote:
> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> ---
>  rdma/include/uapi/rdma/rdma_netlink.h | 37 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/rdma/include/uapi/rdma/rdma_netlink.h b/rdma/include/uapi/rdma/rdma_netlink.h
> index 45474f1..faea9d5 100644
> --- a/rdma/include/uapi/rdma/rdma_netlink.h
> +++ b/rdma/include/uapi/rdma/rdma_netlink.h
> @@ -249,10 +249,22 @@ enum rdma_nldev_command {
>  	RDMA_NLDEV_NUM_OPS
>  };
>  
> +enum {
> +	RDMA_NLDEV_ATTR_ENTRY_STRLEN = 16,
> +};
> +
> +enum rdma_nldev_print_type {
> +	RDMA_NLDEV_PRINT_TYPE_UNSPEC,
> +	RDMA_NLDEV_PRINT_TYPE_HEX,
> +};
> +
>  enum rdma_nldev_attr {
>  	/* don't change the order or add anything between, this is ABI! */
>  	RDMA_NLDEV_ATTR_UNSPEC,
>  
> +	/* Pad attribute for 64b alignment */
> +	RDMA_NLDEV_ATTR_PAD = RDMA_NLDEV_ATTR_UNSPEC,

are you really inserting a new value here?

> +
>  	/* Identifier for ib_device */
>  	RDMA_NLDEV_ATTR_DEV_INDEX,		/* u32 */
>  
> @@ -387,8 +399,31 @@ enum rdma_nldev_attr {
>  	RDMA_NLDEV_ATTR_RES_PD_ENTRY,		/* nested table */
>  	RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY,	/* u32 */
>  	RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY,	/* u32 */
> +	/*
> +	 * provider-specific attributes.
> +	 */
> +	RDMA_NLDEV_ATTR_PROVIDER,		/* nested table */
> +	RDMA_NLDEV_ATTR_PROVIDER_ENTRY,		/* nested table */
> +	RDMA_NLDEV_ATTR_PROVIDER_STRING,	/* string */
> +	/*
> +	 * u8 values from enum rdma_nldev_print_type
> +	 */
> +	RDMA_NLDEV_ATTR_PROVIDER_PRINT_TYPE,	/* u8 */
> +	RDMA_NLDEV_ATTR_PROVIDER_S32,		/* s32 */
> +	RDMA_NLDEV_ATTR_PROVIDER_U32,		/* u32 */
> +	RDMA_NLDEV_ATTR_PROVIDER_S64,		/* s64 */
> +	RDMA_NLDEV_ATTR_PROVIDER_U64,		/* u64 */

and this sequence too.



>  
> -	/* Netdev information for relevant protocols, like RoCE and iWARP */
> +	/*
> +	 * Provides logical name and index of netdevice which is
> +	 * connected to physical port. This information is relevant
> +	 * for RoCE and iWARP.
> +	 *
> +	 * The netdevices which are associated with containers are
> +	 * supposed to be exported together with GID table once it
> +	 * will be exposed through the netlink. Because the
> +	 * associated netdevices are properties of GIDs.
> +	 */
>  	RDMA_NLDEV_ATTR_NDEV_INDEX,		/* u32 */
>  	RDMA_NLDEV_ATTR_NDEV_NAME,		/* string */
>  
> 

[PATCH RFC iproute2-next 2/2] rdma: print provider resource attributes

[Steve Wise]

This enhancement allows printing rdma device-specific state, if provided
by the kernel.  This is done in a generic manner, so rdma tool doesn't
need to know about the details of every type of rdma device.

Provider attributes for a rdma resource are in the form of <key,
[print_type], value> tuples, where the key is a string and the value can
be any supported provider attribute.  The print_type attribute, if present,
provides a print format to use vs the standard print format for the type.
For example, the default print type for a PROVIDER_S32 value is "%d ",
but "0x%x " if the print_type of PRINT_TYPE_HEX is included inthe tuple.

Provider resources are only printed when the -dd flag is present.
If -p is present, then the output is formatted to not exceed 80 columns,
otherwise it is printed as a single row to be grep/awk friendly.

Example output:

# rdma resource show qp lqpn 1028 -dd -p
link cxgb4_0/- lqpn 1028 rqpn 0 type RC state RTS rq-psn 0 sq-psn 0 path-mig-state MIGRATED pid 0 comm [nvme_rdma]
    sqid 1028 flushed 0 memsize 123968 cidx 85 pidx 85 wq_pidx 106 flush_cidx 85 in_use 0
    size 386 flags 0x0 rqid 1029 memsize 16768 cidx 43 pidx 41 wq_pidx 171 msn 44 rqt_hwaddr 0x2a8a5d00
    rqt_size 256 in_use 128 size 130 idx 43 wr_id 0xffff881057c03408 idx 40 wr_id 0xffff881057c033f0

Signed-off-by: Steve Wise <swise@opengridcomputing.com>
---
 rdma/rdma.c  |   7 ++-
 rdma/rdma.h  |  11 ++++
 rdma/res.c   |  30 +++------
 rdma/utils.c | 194 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 221 insertions(+), 21 deletions(-)

diff --git a/rdma/rdma.c b/rdma/rdma.c
index b43e538..c7c8b83 100644
--- a/rdma/rdma.c
+++ b/rdma/rdma.c
@@ -132,6 +132,7 @@ int main(int argc, char **argv)
 	const char *batch_file = NULL;
 	bool pretty_output = false;
 	bool show_details = false;
+	bool show_provider_details = false;
 	bool json_output = false;
 	bool force = false;
 	char *filename;
@@ -152,7 +153,10 @@ int main(int argc, char **argv)
 			pretty_output = true;
 			break;
 		case 'd':
-			show_details = true;
+			if (show_details)
+				show_provider_details = true;
+			else
+				show_details = true;
 			break;
 		case 'j':
 			json_output = true;
@@ -180,6 +184,7 @@ int main(int argc, char **argv)
 	argv += optind;
 
 	rd.show_details = show_details;
+	rd.show_provider_details = show_provider_details;
 	rd.json_output = json_output;
 	rd.pretty_output = pretty_output;
 
diff --git a/rdma/rdma.h b/rdma/rdma.h
index 1908fc4..e9581fe 100644
--- a/rdma/rdma.h
+++ b/rdma/rdma.h
@@ -55,6 +55,7 @@ struct rd {
 	char **argv;
 	char *filename;
 	bool show_details;
+	bool show_provider_details;
 	struct list_head dev_map_list;
 	uint32_t dev_idx;
 	uint32_t port_idx;
@@ -115,4 +116,14 @@ int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, uint32_t seq);
 void rd_prepare_msg(struct rd *rd, uint32_t cmd, uint32_t *seq, uint16_t flags);
 int rd_dev_init_cb(const struct nlmsghdr *nlh, void *data);
 int rd_attr_cb(const struct nlattr *attr, void *data);
+int rd_attr_check(const struct nlattr *attr, int *typep);
+
+/*
+ * Print helpers
+ */
+void print_provider_table(struct rd *rd, struct nlattr *tb);
+void newline(struct rd *rd);
+void newline_indent(struct rd *rd);
+#define MAX_LINE_LENGTH 80
+
 #endif /* _RDMA_TOOL_H_ */
diff --git a/rdma/res.c b/rdma/res.c
index 1a0aab6..bc0aef5 100644
--- a/rdma/res.c
+++ b/rdma/res.c
@@ -439,10 +439,8 @@ static int res_qp_parse_cb(const struct nlmsghdr *nlh, void *data)
 		if (nla_line[RDMA_NLDEV_ATTR_RES_PID])
 			free(comm);
 
-		if (rd->json_output)
-			jsonw_end_array(rd->jw);
-		else
-			pr_out("\n");
+		print_provider_table(rd, nla_line[RDMA_NLDEV_ATTR_PROVIDER]);
+		newline(rd);
 	}
 	return MNL_CB_OK;
 }
@@ -678,10 +676,8 @@ static int res_cm_id_parse_cb(const struct nlmsghdr *nlh, void *data)
 		if (nla_line[RDMA_NLDEV_ATTR_RES_PID])
 			free(comm);
 
-		if (rd->json_output)
-			jsonw_end_array(rd->jw);
-		else
-			pr_out("\n");
+		print_provider_table(rd, nla_line[RDMA_NLDEV_ATTR_PROVIDER]);
+		newline(rd);
 	}
 	return MNL_CB_OK;
 }
@@ -804,10 +800,8 @@ static int res_cq_parse_cb(const struct nlmsghdr *nlh, void *data)
 		if (nla_line[RDMA_NLDEV_ATTR_RES_PID])
 			free(comm);
 
-		if (rd->json_output)
-			jsonw_end_array(rd->jw);
-		else
-			pr_out("\n");
+		print_provider_table(rd, nla_line[RDMA_NLDEV_ATTR_PROVIDER]);
+		newline(rd);
 	}
 	return MNL_CB_OK;
 }
@@ -919,10 +913,8 @@ static int res_mr_parse_cb(const struct nlmsghdr *nlh, void *data)
 		if (nla_line[RDMA_NLDEV_ATTR_RES_PID])
 			free(comm);
 
-		if (rd->json_output)
-			jsonw_end_array(rd->jw);
-		else
-			pr_out("\n");
+		print_provider_table(rd, nla_line[RDMA_NLDEV_ATTR_PROVIDER]);
+		newline(rd);
 	}
 	return MNL_CB_OK;
 }
@@ -1004,10 +996,8 @@ static int res_pd_parse_cb(const struct nlmsghdr *nlh, void *data)
 		if (nla_line[RDMA_NLDEV_ATTR_RES_PID])
 			free(comm);
 
-		if (rd->json_output)
-			jsonw_end_array(rd->jw);
-		else
-			pr_out("\n");
+		print_provider_table(rd, nla_line[RDMA_NLDEV_ATTR_PROVIDER]);
+		newline(rd);
 	}
 	return MNL_CB_OK;
 }
diff --git a/rdma/utils.c b/rdma/utils.c
index 49c967f..3d3225c 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -11,6 +11,7 @@
 
 #include "rdma.h"
 #include <ctype.h>
+#include <inttypes.h>
 
 int rd_argc(struct rd *rd)
 {
@@ -393,8 +394,32 @@ static const enum mnl_attr_data_type nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
 	[RDMA_NLDEV_ATTR_RES_MRLEN] = MNL_TYPE_U64,
 	[RDMA_NLDEV_ATTR_NDEV_INDEX]		= MNL_TYPE_U32,
 	[RDMA_NLDEV_ATTR_NDEV_NAME]		= MNL_TYPE_NUL_STRING,
+	[RDMA_NLDEV_ATTR_PROVIDER] = MNL_TYPE_NESTED,
+	[RDMA_NLDEV_ATTR_PROVIDER_ENTRY] = MNL_TYPE_NESTED,
+	[RDMA_NLDEV_ATTR_PROVIDER_STRING] = MNL_TYPE_NUL_STRING,
+	[RDMA_NLDEV_ATTR_PROVIDER_PRINT_TYPE] = MNL_TYPE_U8,
+	[RDMA_NLDEV_ATTR_PROVIDER_S32] = MNL_TYPE_U32,
+	[RDMA_NLDEV_ATTR_PROVIDER_U32] = MNL_TYPE_U32,
+	[RDMA_NLDEV_ATTR_PROVIDER_S64] = MNL_TYPE_U64,
+	[RDMA_NLDEV_ATTR_PROVIDER_U64] = MNL_TYPE_U64,
 };
 
+int rd_attr_check(const struct nlattr *attr, int *typep)
+{
+	int type;
+
+	if (mnl_attr_type_valid(attr, RDMA_NLDEV_ATTR_MAX) < 0)
+		return MNL_CB_ERROR;
+
+	type = mnl_attr_get_type(attr);
+
+	if (mnl_attr_validate(attr, nldev_policy[type]) < 0)
+		return MNL_CB_ERROR;
+
+	*typep = nldev_policy[type];
+	return MNL_CB_OK;
+}
+
 int rd_attr_cb(const struct nlattr *attr, void *data)
 {
 	const struct nlattr **tb = data;
@@ -660,3 +685,172 @@ struct dev_map *dev_map_lookup(struct rd *rd, bool allow_port_index)
 	free(dev_name);
 	return dev_map;
 }
+
+#define nla_type(attr) ((attr)->nla_type & NLA_TYPE_MASK)
+
+void newline(struct rd *rd)
+{
+	if (rd->json_output)
+		jsonw_end_array(rd->jw);
+	else
+		pr_out("\n");
+}
+
+void newline_indent(struct rd *rd)
+{
+	newline(rd);
+	if (!rd->json_output)
+		pr_out("    ");
+}
+
+static int print_provider_string(struct rd *rd, const char *key_str,
+				 const char *val_str)
+{
+	if (rd->json_output) {
+		jsonw_string_field(rd->jw, key_str, val_str);
+		return 0;
+	} else {
+		return pr_out("%s %s ", key_str, val_str);
+	}
+}
+
+static int print_provider_s32(struct rd *rd, const char *key_str, int32_t val,
+			      enum rdma_nldev_print_type print_type)
+{
+	if (rd->json_output) {
+		jsonw_int_field(rd->jw, key_str, val);
+		return 0;
+	}
+	switch (print_type) {
+	case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
+		return pr_out("%s %d ", key_str, val);
+	case RDMA_NLDEV_PRINT_TYPE_HEX:
+		return pr_out("%s 0x%x ", key_str, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int print_provider_u32(struct rd *rd, const char *key_str, uint32_t val,
+			      enum rdma_nldev_print_type print_type)
+{
+	if (rd->json_output) {
+		jsonw_int_field(rd->jw, key_str, val);
+		return 0;
+	}
+	switch (print_type) {
+	case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
+		return pr_out("%s %u ", key_str, val);
+	case RDMA_NLDEV_PRINT_TYPE_HEX:
+		return pr_out("%s 0x%x ", key_str, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int print_provider_s64(struct rd *rd, const char *key_str, int64_t val,
+			      enum rdma_nldev_print_type print_type)
+{
+	if (rd->json_output) {
+		jsonw_int_field(rd->jw, key_str, val);
+		return 0;
+	}
+	switch (print_type) {
+	case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
+		return pr_out("%s %" PRId64 " ", key_str, val);
+	case RDMA_NLDEV_PRINT_TYPE_HEX:
+		return pr_out("%s 0x%" PRIx64 " ", key_str, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int print_provider_u64(struct rd *rd, const char *key_str, uint64_t val,
+			      enum rdma_nldev_print_type print_type)
+{
+	if (rd->json_output) {
+		jsonw_int_field(rd->jw, key_str, val);
+		return 0;
+	}
+	switch (print_type) {
+	case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
+		return pr_out("%s %" PRIu64 " ", key_str, val);
+	case RDMA_NLDEV_PRINT_TYPE_HEX:
+		return pr_out("%s 0x%" PRIx64 " ", key_str, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int print_provider_entry(struct rd *rd, struct nlattr *key_attr,
+				struct nlattr *val_attr,
+				enum rdma_nldev_print_type print_type)
+{
+	const char *key_str = mnl_attr_get_str(key_attr);
+	int attr_type = nla_type(val_attr);
+
+	switch (attr_type) {
+	case RDMA_NLDEV_ATTR_PROVIDER_STRING:
+		return print_provider_string(rd, key_str,
+				mnl_attr_get_str(val_attr));
+	case RDMA_NLDEV_ATTR_PROVIDER_S32:
+		return print_provider_s32(rd, key_str,
+				mnl_attr_get_u32(val_attr), print_type);
+	case RDMA_NLDEV_ATTR_PROVIDER_U32:
+		return print_provider_u32(rd, key_str,
+				mnl_attr_get_u32(val_attr), print_type);
+	case RDMA_NLDEV_ATTR_PROVIDER_S64:
+		return print_provider_s64(rd, key_str,
+				mnl_attr_get_u64(val_attr), print_type);
+	case RDMA_NLDEV_ATTR_PROVIDER_U64:
+		return print_provider_u64(rd, key_str,
+				mnl_attr_get_u64(val_attr), print_type);
+	}
+	return -EINVAL;
+}
+
+void print_provider_table(struct rd *rd, struct nlattr *tb)
+{
+	int print_type = RDMA_NLDEV_PRINT_TYPE_UNSPEC;
+	struct nlattr *tb_entry, *key = NULL, *val;
+	int type, cc = 0;
+
+	if (!rd->show_provider_details || !tb)
+		return;
+
+	if (rd->pretty_output)
+		newline_indent(rd);
+
+	/*
+	 * Provider attrs are tuples of {key, [print-type], value}.
+	 * The key must be a string.  If print-type is present, it 
+	 * defines an alternate printf format type vs the native format
+	 * for the attribute.  And the value can be any available
+	 * provider type.
+	 */
+	mnl_attr_for_each_nested(tb_entry, tb) {
+
+		if (cc > MAX_LINE_LENGTH) {
+			if (rd->pretty_output)
+				newline_indent(rd);
+			cc = 0;
+		}
+		if (rd_attr_check(tb_entry, &type) != MNL_CB_OK)
+			return;
+		if (!key) {
+			if (type != MNL_TYPE_NUL_STRING)
+				return;
+			key = tb_entry;
+		} else if (type == MNL_TYPE_U8) {
+			print_type = mnl_attr_get_u8(tb_entry);
+		} else {
+			val = tb_entry;
+			cc += print_provider_entry(rd, key, val, print_type);
+			if (cc < 0)
+				return;
+			print_type = RDMA_NLDEV_PRINT_TYPE_UNSPEC;
+			key = NULL;
+		}
+	}
+	return;
+}
-- 
1.8.3.1

Re: [PATCH RFC iproute2-next 2/2] rdma: print provider resource attributes

[Stephen Hemminger]

On Mon, 30 Apr 2018 07:36:18 -0700
Steve Wise <swise@opengridcomputing.com> wrote:

> +#define nla_type(attr) ((attr)->nla_type & NLA_TYPE_MASK)
> +
> +void newline(struct rd *rd)
> +{
> +	if (rd->json_output)
> +		jsonw_end_array(rd->jw);
> +	else
> +		pr_out("\n");
> +}
> +
> +void newline_indent(struct rd *rd)
> +{
> +	newline(rd);
> +	if (!rd->json_output)
> +		pr_out("    ");
> +}
> +
> +static int print_provider_string(struct rd *rd, const char *key_str,
> +				 const char *val_str)
> +{
> +	if (rd->json_output) {
> +		jsonw_string_field(rd->jw, key_str, val_str);
> +		return 0;
> +	} else {
> +		return pr_out("%s %s ", key_str, val_str);
> +	}
> +}
> +
> +static int print_provider_s32(struct rd *rd, const char *key_str, int32_t val,
> +			      enum rdma_nldev_print_type print_type)
> +{
> +	if (rd->json_output) {
> +		jsonw_int_field(rd->jw, key_str, val);
> +		return 0;
> +	}
> +	switch (print_type) {
> +	case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
> +		return pr_out("%s %d ", key_str, val);
> +	case RDMA_NLDEV_PRINT_TYPE_HEX:
> +		return pr_out("%s 0x%x ", key_str, val);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +

This code should get converted to json_print library that handles the
different output modes; rather than rolling it's own equivalent functionality.

Re: [PATCH RFC iproute2-next 2/2] rdma: print provider resource attributes

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1629 bytes --]

On Mon, Apr 30, 2018 at 08:25:24AM -0700, Stephen Hemminger wrote:
> On Mon, 30 Apr 2018 07:36:18 -0700
> Steve Wise <swise@opengridcomputing.com> wrote:
>
> > +#define nla_type(attr) ((attr)->nla_type & NLA_TYPE_MASK)
> > +
> > +void newline(struct rd *rd)
> > +{
> > +	if (rd->json_output)
> > +		jsonw_end_array(rd->jw);
> > +	else
> > +		pr_out("\n");
> > +}
> > +
> > +void newline_indent(struct rd *rd)
> > +{
> > +	newline(rd);
> > +	if (!rd->json_output)
> > +		pr_out("    ");
> > +}
> > +
> > +static int print_provider_string(struct rd *rd, const char *key_str,
> > +				 const char *val_str)
> > +{
> > +	if (rd->json_output) {
> > +		jsonw_string_field(rd->jw, key_str, val_str);
> > +		return 0;
> > +	} else {
> > +		return pr_out("%s %s ", key_str, val_str);
> > +	}
> > +}
> > +
> > +static int print_provider_s32(struct rd *rd, const char *key_str, int32_t val,
> > +			      enum rdma_nldev_print_type print_type)
> > +{
> > +	if (rd->json_output) {
> > +		jsonw_int_field(rd->jw, key_str, val);
> > +		return 0;
> > +	}
> > +	switch (print_type) {
> > +	case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
> > +		return pr_out("%s %d ", key_str, val);
> > +	case RDMA_NLDEV_PRINT_TYPE_HEX:
> > +		return pr_out("%s 0x%x ", key_str, val);
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
>
> This code should get converted to json_print library that handles the
> different output modes; rather than rolling it's own equivalent functionality.

Can it be done after this patch is merged? It will simplify review and
testing because current code is implemented to be in the same format as
the rest of the tool.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH RFC iproute2-next 2/2] rdma: print provider resource attributes

[Stephen Hemminger]

[-- Attachment #1: Type: text/plain, Size: 1938 bytes --]

On Wed, 2 May 2018 16:38:52 +0300
Leon Romanovsky <leon@kernel.org> wrote:

> On Mon, Apr 30, 2018 at 08:25:24AM -0700, Stephen Hemminger wrote:
> > On Mon, 30 Apr 2018 07:36:18 -0700
> > Steve Wise <swise@opengridcomputing.com> wrote:
> >  
> > > +#define nla_type(attr) ((attr)->nla_type & NLA_TYPE_MASK)
> > > +
> > > +void newline(struct rd *rd)
> > > +{
> > > +	if (rd->json_output)
> > > +		jsonw_end_array(rd->jw);
> > > +	else
> > > +		pr_out("\n");
> > > +}
> > > +
> > > +void newline_indent(struct rd *rd)
> > > +{
> > > +	newline(rd);
> > > +	if (!rd->json_output)
> > > +		pr_out("    ");
> > > +}
> > > +
> > > +static int print_provider_string(struct rd *rd, const char *key_str,
> > > +				 const char *val_str)
> > > +{
> > > +	if (rd->json_output) {
> > > +		jsonw_string_field(rd->jw, key_str, val_str);
> > > +		return 0;
> > > +	} else {
> > > +		return pr_out("%s %s ", key_str, val_str);
> > > +	}
> > > +}
> > > +
> > > +static int print_provider_s32(struct rd *rd, const char *key_str, int32_t val,
> > > +			      enum rdma_nldev_print_type print_type)
> > > +{
> > > +	if (rd->json_output) {
> > > +		jsonw_int_field(rd->jw, key_str, val);
> > > +		return 0;
> > > +	}
> > > +	switch (print_type) {
> > > +	case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
> > > +		return pr_out("%s %d ", key_str, val);
> > > +	case RDMA_NLDEV_PRINT_TYPE_HEX:
> > > +		return pr_out("%s 0x%x ", key_str, val);
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +}
> > > +  
> >
> > This code should get converted to json_print library that handles the
> > different output modes; rather than rolling it's own equivalent functionality.  
> 
> Can it be done after this patch is merged? It will simplify review and
> testing because current code is implemented to be in the same format as
> the rest of the tool.
> 
> Thanks

Sure, I was just encouraging future direction.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]