Re: [PATCH iproute2 V3 2/4] rdma: Add dev object

On Mon, Jul 10, 2017 at 10:01:02AM +0200, Jiri Pirko wrote:

Tue, Jul 04, 2017 at 09:55:39AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:

quoted

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Device (dev) object represents struct ib_device to the user space.

Device properties:
* Device capabilities
* FW version to the device output
* node_guid and sys_image_guid
* node_type

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
rdma/Makefile |   2 +-
rdma/dev.c    | 235 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
rdma/rdma.c   |   3 +-
rdma/rdma.h   |  12 ++-
rdma/utils.c  |  46 +++++++++++-
5 files changed, 293 insertions(+), 5 deletions(-)
create mode 100644 rdma/dev.c

diff --git a/rdma/Makefile b/rdma/Makefile
index 64da2142..123d7ac5 100644
--- a/rdma/Makefile
+++ b/rdma/Makefile

@@ -2,7 +2,7 @@ include ../Config

ifeq ($(HAVE_MNL),y)

-RDMA_OBJ = rdma.o utils.o
+RDMA_OBJ = rdma.o utils.o dev.o

TARGETS=rdma
CFLAGS += $(shell $(PKG_CONFIG) libmnl --cflags)

diff --git a/rdma/dev.c b/rdma/dev.c
new file mode 100644
index 00000000..b80e5288
--- /dev/null
+++ b/rdma/dev.c

@@ -0,0 +1,235 @@
+/*
+ * dev.c	RDMA tool
+ *
+ *              This program is free software; you can redistribute it and/or
+ *              modify it under the terms of the GNU General Public License
+ *              as published by the Free Software Foundation; either version
+ *              2 of the License, or (at your option) any later version.
+ *
+ * Authors:     Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
+ */
+
+#include "rdma.h"
+
+static int dev_help(struct rdma *rd)
+{
+	pr_out("Usage: %s dev show [DEV]\n", rd->filename);
+	return 0;
+}
+
+static void dev_print_caps(struct nlattr **tb)
+{
+	uint64_t caps;
+	uint32_t idx;
+
+	/*
+	 * FIXME: move to indexes when kernel will start exporting them.
+	 */
+	static const char *dev_caps[64] = {

	static const char *dev_caps[] = {

The same as for the "link" object.

quoted

+		"RESIZE_MAX_WR",
+		"BAD_PKEY_CNTR",
+		"BAD_QKEY_CNTR",
+		"RAW_MULTI",
+		"AUTO_PATH_MIG",
+		"CHANGE_PHY_PORT",
+		"UD_AV_PORT_ENFORCE",
+		"CURR_QP_STATE_MOD",
+		"SHUTDOWN_PORT",
+		"INIT_TYPE",
+		"PORT_ACTIVE_EVENT",
+		"SYS_IMAGE_GUID",
+		"RC_RNR_NAK_GEN",
+		"SRQ_RESIZE",
+		"N_NOTIFY_CQ",
+		"LOCAL_DMA_LKEY",
+		"RESERVED",
+		"MEM_WINDOW",
+		"UD_IP_CSUM",
+		"UD_TSO",
+		"XRC",
+		"MEM_MGT_EXTENSIONS",
+		"BLOCK_MULTICAST_LOOPBACK",
+		"MEM_WINDOW_TYPE_2A",
+		"MEM_WINDOW_TYPE_2B",
+		"RC_IP_CSUM",
+		"RAW_IP_CSUM",
+		"CROSS_CHANNEL",
+		"MANAGED_FLOW_STEERING",
+		"SIGNATURE_HANDOVER",
+		"ON_DEMAND_PAGING",
+		"SG_GAPS_REG",
+		"VIRTUAL_FUNCTION",
+		"RAW_SCATTER_FCS",
+		"RDMA_NETDEV_OPA_VNIC",
+	};
+

<...>

quoted

+	if (!tb[RDMA_NLDEV_ATTR_FW_VERSION])
+		return;
+
+	pr_out("fw %s ",
+	       mnl_attr_get_str(tb[RDMA_NLDEV_ATTR_FW_VERSION]));
+}
+
+static void _dev_print_be64(char *name, uint64_t val)

Why the initial "_"? I think it could be avoided.

Wait, I'm confused. Does kernel actually exposes BE64 in U64 attribute?
That is wrong. Please fix it while there is still a time (if it is).

It was, I fixed it in the last version of my RDMA netlink patches as a
response to Jason's comment.

The "_be64" suffix in the name is a leftover from dark ages. I'll fix it.

quoted

+{
+	uint16_t vp[4];
+
+	memcpy(vp, &val, sizeof(uint64_t));
+	pr_out("%s %04x:%04x:%04x:%04x ", name, vp[3], vp[2], vp[1], vp[0]);
+}
+

<...>

quoted

+	}
+	else {

"} else {"

Checkpatch should warn you. Did you run it?

Partially.

Thanks