cm_id, cq, mr, and pd resource tracking

PATCH Superseded Steve Wise 5 participants 52 messages Mar 2, 2018 – Mar 27, 2018

Subsystems (4)

Networking Network drivers Library code InfiniBand

lore

Share & tools

lore

Steve Wise

author

2018-03-02 20:10 UTC #0 lore raw

v2 cover cm_id, cq, mr, and pd resource tracking

Hello,

This series enhances the iproute2 rdma tool to include dumping of connection manager id (cm_id), completion queue (cq), memory region (mr), and protection domain (pd) rdma resources. It is the user-space part of the resource tracking series under review now on the linux-rdma list [1].

Changes since v1/RFC:
- removed RFC tag
- initialize rd properly to avoid passing a garbage port number
- revert accidental change to qp_valid_filters
- removed cm_id dev/network/transport types
- cm_id ip addrs now passed up as __kernel_sockaddr_storage
- cm_id ip address ports printed as "address:port" strings
- only parse/display memory keys and iova if available
- filter on "users" for cqs and pds
- fixed memory leaks
- removed PD_FLAGS attribute
- filter on "mrlen" for mrs
- filter on "poll-ctx" for cqs
- don't require addrs or qp_type for parsing cm_ids
- only filter optional attrs if they are present
- remove PGSIZE MR attr to match kernel

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

Thanks,

Steve.
---

Steve Wise (6):
rdma: update rdma_netlink.h
rdma: initialize the rd struct
rdma: Add CM_ID resource tracking information
rdma: Add CQ resource tracking information
rdma: Add MR resource tracking information
rdma: Add PD resource tracking information

 include/json_writer.h            |   2 +
 include/uapi/rdma/rdma_netlink.h |  44 ++-
 lib/json_writer.c                |  11 +
 rdma/rdma.c                      |   2 +-
 rdma/rdma.h                      |   2 +
 rdma/res.c                       | 611 ++++++++++++++++++++++++++++++++++++++-
 rdma/utils.c                     |  16 +
 7 files changed, 683 insertions(+), 5 deletions(-)

Signature · 1 line

1.8.3.1

Steve Wise

author

2018-03-02 20:10 UTC #1 lore raw

PATCH v2 1/6 rdma: update rdma_netlink.h

From: Steve Wise <[email protected]>

Pull in the latest rdma_netlink.h which has support for the rdma nldev resource tracking objects being added with this patch series.

Signed-off-by Steve Wise <[email protected]>

---
 include/uapi/rdma/rdma_netlink.h | 44 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 41 insertions(+), 3 deletions(-)

include/uapi/rdma/rdma_netlink.h +41 -3

index dbac3b8..351139c 100644
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _RDMA_NETLINK_H
-#define _RDMA_NETLINK_H
+#ifndef _UAPI_RDMA_NETLINK_H
+#define _UAPI_RDMA_NETLINK_H
 
 #include <linux/types.h>

@@ -238,6 +238,14 @@ enum rdma_nldev_command {
 
 	RDMA_NLDEV_CMD_RES_QP_GET, /* can dump */
 
+	RDMA_NLDEV_CMD_RES_CM_ID_GET, /* can dump */
+
+	RDMA_NLDEV_CMD_RES_CQ_GET, /* can dump */
+
+	RDMA_NLDEV_CMD_RES_MR_GET, /* can dump */
+
+	RDMA_NLDEV_CMD_RES_PD_GET, /* can dump */
+
 	RDMA_NLDEV_NUM_OPS
 };

@@ -350,6 +358,36 @@ enum rdma_nldev_attr {
 	 */
 	RDMA_NLDEV_ATTR_RES_KERN_NAME,		/* string */
 
+	RDMA_NLDEV_ATTR_RES_CM_ID,		/* nested table */
+	RDMA_NLDEV_ATTR_RES_CM_ID_ENTRY,	/* nested table */
+	/*
+	 * rdma_cm_id port space.
+	 */
+	RDMA_NLDEV_ATTR_RES_PS,			/* u32 */
+	/*
+	 * Source and destination socket addresses
+	 */
+	RDMA_NLDEV_ATTR_RES_SRC_ADDR,		/* __kernel_sockaddr_storage */
+	RDMA_NLDEV_ATTR_RES_DST_ADDR,		/* __kernel_sockaddr_storage */
+
+	RDMA_NLDEV_ATTR_RES_CQ,			/* nested table */
+	RDMA_NLDEV_ATTR_RES_CQ_ENTRY,		/* nested table */
+	RDMA_NLDEV_ATTR_RES_CQE,		/* u32 */
+	RDMA_NLDEV_ATTR_RES_USECNT,		/* u64 */
+	RDMA_NLDEV_ATTR_RES_POLL_CTX,		/* u8 */
+
+	RDMA_NLDEV_ATTR_RES_MR,			/* nested table */
+	RDMA_NLDEV_ATTR_RES_MR_ENTRY,		/* nested table */
+	RDMA_NLDEV_ATTR_RES_RKEY,		/* u32 */
+	RDMA_NLDEV_ATTR_RES_LKEY,		/* u32 */
+	RDMA_NLDEV_ATTR_RES_IOVA,		/* u64 */
+	RDMA_NLDEV_ATTR_RES_MRLEN,		/* u64 */
+
+	RDMA_NLDEV_ATTR_RES_PD,			/* nested table */
+	RDMA_NLDEV_ATTR_RES_PD_ENTRY,		/* nested table */
+	RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY,	/* u32 */
+	RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY,	/* u32 */
+
 	RDMA_NLDEV_ATTR_MAX
 };
-#endif /* _RDMA_NETLINK_H */
+#endif /* _UAPI_RDMA_NETLINK_H */

Signature · 1 line

1.8.3.1

Steve Wise

author

2018-03-02 20:10 UTC #2 lore raw

PATCH v2 2/6 rdma: initialize the rd struct

In reply to Steve Wise

Initialize the rd struct so port_idx is 0 unless set otherwise. Otherwise, strict_port queries end up passing an uninitialized PORT nlattr.

Signed-off-by Steve Wise <[email protected]>

---
 rdma/rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

rdma/rdma.c +1 -1

index 19608f4..c652550 100644
@@ -93,7 +93,7 @@ int main(int argc, char **argv)
 	bool show_details = false;
 	bool json_output = false;
 	char *filename;
-	struct rd rd;
+	struct rd rd = {};
 	int opt;
 	int err;

Signature · 1 line

1.8.3.1

Steve Wise

author

2018-03-02 20:10 UTC #4 lore raw

PATCH v2 3/6 rdma: Add CM_ID resource tracking information

In reply to Steve Wise

Sample output:

# rdma resource
2: cxgb4_0: pd 5 cq 2 qp 2 cm_id 3 mr 7
3: mlx4_0: pd 7 cq 3 qp 3 cm_id 3 mr 7

# rdma resource show cm_id
link cxgb4_0/- lqpn 0 qp-type RC state LISTEN ps TCP pid 30485 comm rping src-addr 0.0.0.0:7174
link cxgb4_0/2 lqpn 1048 qp-type RC state CONNECT ps TCP pid 30503 comm rping src-addr 172.16.2.1:7174 dst-addr 172.16.2.1:38246
link cxgb4_0/2 lqpn 1040 qp-type RC state CONNECT ps TCP pid 30498 comm rping src-addr 172.16.2.1:38246 dst-addr 172.16.2.1:7174
link mlx4_0/- lqpn 0 qp-type RC state LISTEN ps TCP pid 30485 comm rping src-addr 0.0.0.0:7174
link mlx4_0/1 lqpn 539 qp-type RC state CONNECT ps TCP pid 30494 comm rping src-addr 172.16.99.1:7174 dst-addr 172.16.99.1:43670
link mlx4_0/1 lqpn 538 qp-type RC state CONNECT ps TCP pid 30492 comm rping src-addr 172.16.99.1:43670 dst-addr 172.16.99.1:7174

# rdma resource show cm_id dst-port 7174
link cxgb4_0/2 lqpn 1040 qp-type RC state CONNECT ps TCP pid 30498 comm rping src-addr 172.16.2.1:38246 dst-addr 172.16.2.1:7174
link mlx4_0/1 lqpn 538 qp-type RC state CONNECT ps TCP pid 30492 comm rping src-addr 172.16.99.1:43670 dst-addr 172.16.99.1:7174

Signed-off-by Steve Wise <[email protected]>

---
 rdma/rdma.h  |   2 +
 rdma/res.c   | 258 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 rdma/utils.c |   5 ++
 3 files changed, 264 insertions(+), 1 deletion(-)

rdma/rdma.h +2 -0

index 5809f70..e55205b 100644
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
 #define pr_err(args...) fprintf(stderr, ##args)
 #define pr_out(args...) fprintf(stdout, ##args)

rdma/res.c +257 -1

283 lines not on this page · raw message

rdma/utils.c +5 -0

index f946016..ec81737 100644
@@ -375,6 +375,11 @@ static const enum mnl_attr_data_type nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
 	[RDMA_NLDEV_ATTR_RES_STATE]		= MNL_TYPE_U8,
 	[RDMA_NLDEV_ATTR_RES_PID]		= MNL_TYPE_U32,
 	[RDMA_NLDEV_ATTR_RES_KERN_NAME]	= MNL_TYPE_NUL_STRING,
+	[RDMA_NLDEV_ATTR_RES_CM_ID]		= MNL_TYPE_NESTED,
+	[RDMA_NLDEV_ATTR_RES_CM_ID_ENTRY]	= MNL_TYPE_NESTED,
+	[RDMA_NLDEV_ATTR_RES_PS]		= MNL_TYPE_U32,
+	[RDMA_NLDEV_ATTR_RES_SRC_ADDR]		= MNL_TYPE_UNSPEC,
+	[RDMA_NLDEV_ATTR_RES_DST_ADDR]		= MNL_TYPE_UNSPEC,
 };
 
 int rd_attr_cb(const struct nlattr *attr, void *data)

Signature · 1 line

1.8.3.1

Steve Wise

author

2018-03-02 20:10 UTC #30 lore raw

PATCH v2 4/6 rdma: Add CQ resource tracking information

In reply to Steve Wise

Sample output:

# rdma resource show cq
link cxgb4_0/- cqe 46 users 2 pid 30503 comm rping
link cxgb4_0/- cqe 46 users 2 pid 30498 comm rping
link mlx4_0/- cqe 63 users 2 pid 30494 comm rping
link mlx4_0/- cqe 63 users 2 pid 30489 comm rping
link mlx4_0/- cqe 1023 users 2 poll_ctx WORKQUEUE pid 0 comm [ib_core]

# rdma resource show cq pid 30489
link mlx4_0/- cqe 63 users 2 pid 30489 comm rping

Signed-off-by Steve Wise <[email protected]>

---
 rdma/res.c   | 136 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 rdma/utils.c |   5 +++
 2 files changed, 141 insertions(+)

rdma/res.c +136 -0

index 1ef4f20..595fbbb 100644
@@ -21,6 +21,8 @@ static int res_help(struct rd *rd)
 	pr_out("          resource show qp link [DEV/PORT] [FILTER-NAME FILTER-VALUE]\n");
 	pr_out("          resource show cm_id link [DEV/PORT]\n");
 	pr_out("          resource show cm_id link [DEV/PORT] [FILTER-NAME FILTER-VALUE]\n");
+	pr_out("          resource show cq link [DEV/PORT]\n");
+	pr_out("          resource show cq link [DEV/PORT] [FILTER-NAME FILTER-VALUE]\n");
 	return 0;
 }

@@ -659,6 +661,127 @@ static int res_cm_id_parse_cb(const struct nlmsghdr *nlh, void *data)
 	return MNL_CB_OK;
 }
 
+static void print_cqe(struct rd *rd, uint32_t val)
+{
+	if (rd->json_output)
+		jsonw_uint_field(rd->jw, "cqe", val);
+	else
+		pr_out("cqe %u ", val);
+}
+
+static void print_users(struct rd *rd, uint64_t val)
+{
+	if (rd->json_output)
+		jsonw_uint_field(rd->jw, "users", val);
+	else
+		pr_out("users %" PRIu64 " ", val);
+}
+
+static const char *poll_ctx_to_str(uint8_t idx)
+{
+	static const char * const cm_id_states_str[] = { "DIRECT", "SOFTIRQ",
+						      "WORKQUEUE"};
+
+	if (idx < ARRAY_SIZE(cm_id_states_str))
+		return cm_id_states_str[idx];
+	return "UNKNOWN";
+}
+
+static void print_poll_ctx(struct rd *rd, uint8_t poll_ctx)
+{
+	if (rd->json_output) {
+		jsonw_string_field(rd->jw, "poll-ctx", poll_ctx_to_str(poll_ctx));
+		return;
+	}
+	pr_out("poll-ctx %s ", poll_ctx_to_str(poll_ctx));
+}
+
+static int res_cq_parse_cb(const struct nlmsghdr *nlh, void *data)
+{
+	struct nlattr *tb[RDMA_NLDEV_ATTR_MAX] = {};
+	struct nlattr *nla_table, *nla_entry;
+	struct rd *rd = data;
+	const char *name;
+	uint32_t idx;
+
+	mnl_attr_parse(nlh, 0, rd_attr_cb, tb);
+	if (!tb[RDMA_NLDEV_ATTR_DEV_INDEX] ||
+	    !tb[RDMA_NLDEV_ATTR_DEV_NAME] ||
+	    !tb[RDMA_NLDEV_ATTR_RES_CQ])
+		return MNL_CB_ERROR;
+
+	name = mnl_attr_get_str(tb[RDMA_NLDEV_ATTR_DEV_NAME]);
+	idx =  mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]);
+	nla_table = tb[RDMA_NLDEV_ATTR_RES_CQ];
+
+	mnl_attr_for_each_nested(nla_entry, nla_table) {
+		struct nlattr *nla_line[RDMA_NLDEV_ATTR_MAX] = {};
+		char *comm = NULL;
+		uint32_t pid = 0;
+		uint8_t poll_ctx = 0;
+		uint64_t users;
+		uint32_t cqe;
+		int err;
+
+		err = mnl_attr_parse_nested(nla_entry, rd_attr_cb, nla_line);
+		if (err != MNL_CB_OK)
+			return MNL_CB_ERROR;
+
+		if (!nla_line[RDMA_NLDEV_ATTR_RES_CQE] ||
+		    !nla_line[RDMA_NLDEV_ATTR_RES_USECNT] ||
+		    (!nla_line[RDMA_NLDEV_ATTR_RES_PID] &&
+		     !nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])) {
+			return MNL_CB_ERROR;
+		}
+
+		cqe = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_CQE]);
+
+		users = mnl_attr_get_u64(nla_line[RDMA_NLDEV_ATTR_RES_USECNT]);
+		if (rd_check_is_filtered(rd, "users", users))
+			continue;
+
+		if (nla_line[RDMA_NLDEV_ATTR_RES_POLL_CTX]) {
+			poll_ctx = mnl_attr_get_u8(nla_line[RDMA_NLDEV_ATTR_RES_POLL_CTX]);
+			if (rd_check_is_string_filtered(rd, "poll-ctx", poll_ctx_to_str(poll_ctx)))
+				continue;
+		}
+
+		if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
+			pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
+			comm = get_task_name(pid);
+		}
+
+		if (rd_check_is_filtered(rd, "pid", pid)) {
+			free(comm);
+			continue;
+		}
+
+		if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
+			/* discard const from mnl_attr_get_str */
+			comm = (char *)mnl_attr_get_str(nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
+
+		if (rd->json_output)
+			jsonw_start_array(rd->jw);
+
+		print_link(rd, idx, name, 0, nla_line);
+		print_cqe(rd, cqe);
+		print_users(rd, users);
+		if (nla_line[RDMA_NLDEV_ATTR_RES_POLL_CTX])
+			print_poll_ctx(rd, poll_ctx);
+		print_pid(rd, pid);
+		print_comm(rd, comm, nla_line);
+
+		if (nla_line[RDMA_NLDEV_ATTR_RES_PID])
+			free(comm);
+
+		if (rd->json_output)
+			jsonw_end_array(rd->jw);
+		else
+			pr_out("\n");
+	}
+	return MNL_CB_OK;
+}
+
 RES_FUNC(res_no_args,	RDMA_NLDEV_CMD_RES_GET,	NULL, true);
 
 static const struct

@@ -712,12 +835,25 @@ filters cm_id_valid_filters[MAX_NUMBER_OF_FILTERS] = {{ .name = "link",
 RES_FUNC(res_cm_id,	RDMA_NLDEV_CMD_RES_CM_ID_GET, cm_id_valid_filters,
 	 false);
 
+static const struct
+filters cq_valid_filters[MAX_NUMBER_OF_FILTERS] = {{ .name = "link",
+						   .is_number = false },
+						   { .name = "users",
+						   .is_number = true },
+						   { .name = "poll-ctx",
+						   .is_number = false },
+						   { .name = "pid",
+						   .is_number = true }};
+
+RES_FUNC(res_cq,	RDMA_NLDEV_CMD_RES_CQ_GET, cq_valid_filters, true);
+
 static int res_show(struct rd *rd)
 {
 	const struct rd_cmd cmds[] = {
 		{ NULL,		res_no_args	},
 		{ "qp",		res_qp		},
 		{ "cm_id",	res_cm_id	},
+		{ "cq",		res_cq		},
 		{ 0 }
 	};

rdma/utils.c +5 -0

index ec81737..5e79b62 100644
@@ -380,6 +380,11 @@ static const enum mnl_attr_data_type nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
 	[RDMA_NLDEV_ATTR_RES_PS]		= MNL_TYPE_U32,
 	[RDMA_NLDEV_ATTR_RES_SRC_ADDR]		= MNL_TYPE_UNSPEC,
 	[RDMA_NLDEV_ATTR_RES_DST_ADDR]		= MNL_TYPE_UNSPEC,
+	[RDMA_NLDEV_ATTR_RES_CQ] = MNL_TYPE_NESTED,
+	[RDMA_NLDEV_ATTR_RES_CQ_ENTRY] = MNL_TYPE_NESTED,
+	[RDMA_NLDEV_ATTR_RES_CQE] = MNL_TYPE_U32,
+	[RDMA_NLDEV_ATTR_RES_USECNT] = MNL_TYPE_U64,
+	[RDMA_NLDEV_ATTR_RES_POLL_CTX] = MNL_TYPE_U8,
 };
 
 int rd_attr_cb(const struct nlattr *attr, void *data)

Signature · 1 line

1.8.3.1

Steve Wise

author

2018-03-02 20:10 UTC #32 lore raw

PATCH v2 5/6 rdma: Add MR resource tracking information

In reply to Steve Wise

Sample output:

Without CAP_NET_ADMIN:

$ rdma resource show mr mrlen 65536
link mlx4_0/- mrlen 65536 pid 0 comm [nvme_rdma]
link cxgb4_0/- mrlen 65536 pid 0 comm [nvme_rdma]

With CAP_NET_ADMIN:

# rdma resource show mr mrlen 65536
link mlx4_0/- rkey 0x12702 lkey 0x12702 iova 0x85724a000 mrlen 65536 pid 0 comm [nvme_rdma]
link cxgb4_0/- rkey 0x68fe4e9 lkey 0x68fe4e9 iova 0x835b91000 mrlen 65536 pid 0 comm [nvme_rdma]

Signed-off-by Steve Wise <[email protected]>

---
 include/json_writer.h |   2 +
 lib/json_writer.c     |  11 +++++
 rdma/res.c            | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++
 rdma/utils.c          |   6 +++
 4 files changed, 144 insertions(+)

include/json_writer.h +2 -0

index 1516aaf..34f2ccc 100644
@@ -39,6 +39,7 @@ void jsonw_bool(json_writer_t *self, bool value);
 void jsonw_float(json_writer_t *self, double number);
 void jsonw_float_fmt(json_writer_t *self, const char *fmt, double num);
 void jsonw_uint(json_writer_t *self, uint64_t number);
+void jsonw_xint(json_writer_t *self, uint64_t number);
 void jsonw_hu(json_writer_t *self, unsigned short number);
 void jsonw_int(json_writer_t *self, int64_t number);
 void jsonw_null(json_writer_t *self);

@@ -49,6 +50,7 @@ void jsonw_string_field(json_writer_t *self, const char *prop, const char *val);
 void jsonw_bool_field(json_writer_t *self, const char *prop, bool value);
 void jsonw_float_field(json_writer_t *self, const char *prop, double num);
 void jsonw_uint_field(json_writer_t *self, const char *prop, uint64_t num);
+void jsonw_xint_field(json_writer_t *self, const char *prop, uint64_t num);
 void jsonw_hu_field(json_writer_t *self, const char *prop, unsigned short num);
 void jsonw_int_field(json_writer_t *self, const char *prop, int64_t num);
 void jsonw_null_field(json_writer_t *self, const char *prop);

lib/json_writer.c +11 -0

index f3eeaf7..6d73a1b 100644
@@ -224,6 +224,11 @@ void jsonw_uint(json_writer_t *self, uint64_t num)
 	jsonw_printf(self, "%"PRIu64, num);
 }
 
+void jsonw_xint(json_writer_t *self, uint64_t num)
+{
+	jsonw_printf(self, "%"PRIx64, num);
+}
+
 void jsonw_lluint(json_writer_t *self, unsigned long long int num)
 {
 	jsonw_printf(self, "%llu", num);

@@ -268,6 +273,12 @@ void jsonw_uint_field(json_writer_t *self, const char *prop, uint64_t num)
 	jsonw_uint(self, num);
 }
 
+void jsonw_xint_field(json_writer_t *self, const char *prop, uint64_t num)
+{
+	jsonw_name(self, prop);
+	jsonw_xint(self, num);
+}
+
 void jsonw_hu_field(json_writer_t *self, const char *prop, unsigned short num)
 {
 	jsonw_name(self, prop);

rdma/res.c +125 -0

143 lines not on this page · raw message

rdma/utils.c +6 -0

index 5e79b62..a2e08e9 100644
@@ -385,6 +385,12 @@ static const enum mnl_attr_data_type nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
 	[RDMA_NLDEV_ATTR_RES_CQE] = MNL_TYPE_U32,
 	[RDMA_NLDEV_ATTR_RES_USECNT] = MNL_TYPE_U64,
 	[RDMA_NLDEV_ATTR_RES_POLL_CTX] = MNL_TYPE_U8,
+	[RDMA_NLDEV_ATTR_RES_MR] = MNL_TYPE_NESTED,
+	[RDMA_NLDEV_ATTR_RES_MR_ENTRY] = MNL_TYPE_NESTED,
+	[RDMA_NLDEV_ATTR_RES_RKEY] = MNL_TYPE_U32,
+	[RDMA_NLDEV_ATTR_RES_LKEY] = MNL_TYPE_U32,
+	[RDMA_NLDEV_ATTR_RES_IOVA] = MNL_TYPE_U64,
+	[RDMA_NLDEV_ATTR_RES_MRLEN] = MNL_TYPE_U64,
 };
 
 int rd_attr_cb(const struct nlattr *attr, void *data)

Signature · 1 line

1.8.3.1

Steve Wise

author

2018-03-02 20:10 UTC #34 lore raw

PATCH v2 6/6 rdma: Add PD resource tracking information

In reply to Steve Wise

Sample output:

Without CAP_NET_ADMIN capability:

link mlx4_0/- users 0 pid 0 comm [ib_srpt]
link mlx4_0/- users 0 pid 0 comm [ib_srp]
link mlx4_0/- users 1 pid 0 comm [ib_core]
link cxgb4_0/- users 0 pid 0 comm [ib_srp]

With CAP_NET_ADMIN capability:
link mlx4_0/- local_dma_lkey 0x8000 users 0 pid 0 comm [ib_srpt] link mlx4_0/- local_dma_lkey 0x8000 users 0 pid 0 comm [ib_srp] link mlx4_0/- local_dma_lkey 0x8000 users 1 pid 0 comm [ib_core] link cxgb4_0/- local_dma_lkey 0x0 users 0 pid 0 comm [ib_srp]

Signed-off-by Steve Wise <[email protected]>

---
 rdma/res.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

rdma/res.c +92 -0

index 4caf1d0..435e1ad 100644
@@ -892,6 +892,87 @@ static int res_mr_parse_cb(const struct nlmsghdr *nlh, void *data)
 	return MNL_CB_OK;
 }
 
+static int res_pd_parse_cb(const struct nlmsghdr *nlh, void *data)
+{
+	struct nlattr *tb[RDMA_NLDEV_ATTR_MAX] = {};
+	struct nlattr *nla_table, *nla_entry;
+	struct rd *rd = data;
+	const char *name;
+	uint32_t idx;
+
+	mnl_attr_parse(nlh, 0, rd_attr_cb, tb);
+	if (!tb[RDMA_NLDEV_ATTR_DEV_INDEX] ||
+	    !tb[RDMA_NLDEV_ATTR_DEV_NAME] ||
+	    !tb[RDMA_NLDEV_ATTR_RES_PD])
+		return MNL_CB_ERROR;
+
+	name = mnl_attr_get_str(tb[RDMA_NLDEV_ATTR_DEV_NAME]);
+	idx =  mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]);
+	nla_table = tb[RDMA_NLDEV_ATTR_RES_PD];
+
+	mnl_attr_for_each_nested(nla_entry, nla_table) {
+		uint32_t local_dma_lkey = 0, unsafe_global_rkey = 0;
+		struct nlattr *nla_line[RDMA_NLDEV_ATTR_MAX] = {};
+		char *comm = NULL;
+		uint32_t pid = 0;
+		uint64_t users;
+		int err;
+
+		err = mnl_attr_parse_nested(nla_entry, rd_attr_cb, nla_line);
+		if (err != MNL_CB_OK)
+			return MNL_CB_ERROR;
+
+		if (!nla_line[RDMA_NLDEV_ATTR_RES_USECNT] ||
+		    (!nla_line[RDMA_NLDEV_ATTR_RES_PID] &&
+		     !nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])) {
+			return MNL_CB_ERROR;
+		}
+
+		if (nla_line[RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY])
+			local_dma_lkey = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY]);
+
+		users = mnl_attr_get_u64(nla_line[RDMA_NLDEV_ATTR_RES_USECNT]);
+		if (rd_check_is_filtered(rd, "users", users))
+			continue;
+
+		if (nla_line[RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY])
+			unsafe_global_rkey = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY]);
+
+		if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
+			pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
+			comm = get_task_name(pid);
+		}
+
+		if (rd_check_is_filtered(rd, "pid", pid))
+			continue;
+
+		if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
+			/* discard const from mnl_attr_get_str */
+			comm = (char *)mnl_attr_get_str(nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
+
+		if (rd->json_output)
+			jsonw_start_array(rd->jw);
+
+		print_link(rd, idx, name, 0, nla_line);
+		if (nla_line[RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY])
+			print_key(rd, "local_dma_lkey", local_dma_lkey);
+		print_users(rd, users);
+		if (nla_line[RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY])
+			print_key(rd, "unsafe_global_rkey", unsafe_global_rkey);
+		print_pid(rd, pid);
+		print_comm(rd, comm, nla_line);
+
+		if (nla_line[RDMA_NLDEV_ATTR_RES_PID])
+			free(comm);
+
+		if (rd->json_output)
+			jsonw_end_array(rd->jw);
+		else
+			pr_out("\n");
+	}
+	return MNL_CB_OK;
+}
+
 RES_FUNC(res_no_args,	RDMA_NLDEV_CMD_RES_GET,	NULL, true);
 
 static const struct

@@ -971,6 +1052,16 @@ filters mr_valid_filters[MAX_NUMBER_OF_FILTERS] = {{ .name = "link",
 
 RES_FUNC(res_mr,	RDMA_NLDEV_CMD_RES_MR_GET, mr_valid_filters, true);
 
+static const struct
+filters pd_valid_filters[MAX_NUMBER_OF_FILTERS] = {{ .name = "link",
+						   .is_number = false },
+						   { .name = "users",
+						   .is_number = true },
+						   { .name = "pid",
+						   .is_number = true }};
+
+RES_FUNC(res_pd,	RDMA_NLDEV_CMD_RES_PD_GET, pd_valid_filters, true);
+
 static int res_show(struct rd *rd)
 {
 	const struct rd_cmd cmds[] = {

@@ -979,6 +1070,7 @@ static int res_show(struct rd *rd)
 		{ "cm_id",	res_cm_id	},
 		{ "cq",		res_cq		},
 		{ "mr",		res_mr		},
+		{ "pd",		res_pd		},
 		{ 0 }
 	};

Signature · 1 line

1.8.3.1

Steve Wise

author

2018-03-12 15:16 UTC #36 lore raw

In reply to Steve Wise

Hey all,

The kernel side of this series has been merged for rdma-next [1]. Let me know if this iproute2 series can be merged, of if it needs more changes.

Thanks,

Steve.

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

Quoted 8 lines

-----Original Message-----
From: [email protected] [mailto:linux-rdma- [email protected]] On Behalf Of Steve Wise
Sent: Friday, March 02, 2018 1:56 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]
Subject: [PATCH v2 iproute2-next 0/6] cm_id, cq, mr, and pd resource

tracking

Quoted 40 lines

Hello,
This series enhances the iproute2 rdma tool to include dumping of connection manager id (cm_id), completion queue (cq), memory region (mr), and protection domain (pd) rdma resources. It is the user-space part of the resource tracking series under review now on the linux-rdma list [1].
Changes since v1/RFC:
- removed RFC tag
- initialize rd properly to avoid passing a garbage port number
- revert accidental change to qp_valid_filters
- removed cm_id dev/network/transport types
- cm_id ip addrs now passed up as __kernel_sockaddr_storage
- cm_id ip address ports printed as "address:port" strings
- only parse/display memory keys and iova if available
- filter on "users" for cqs and pds
- fixed memory leaks
- removed PD_FLAGS attribute
- filter on "mrlen" for mrs
- filter on "poll-ctx" for cqs
- don't require addrs or qp_type for parsing cm_ids
- only filter optional attrs if they are present
- remove PGSIZE MR attr to match kernel
[1] https://www.spinics.net/lists/linux-rdma/msg61400.html
Thanks,
Steve.
---
Steve Wise (6):
rdma: update rdma_netlink.h
rdma: initialize the rd struct
rdma: Add CM_ID resource tracking information
rdma: Add CQ resource tracking information
rdma: Add MR resource tracking information
rdma: Add PD resource tracking information

Quoted 6 lines

 include/json_writer.h            |   2 +
 include/uapi/rdma/rdma_netlink.h |  44 ++-
 lib/json_writer.c                |  11 +
 rdma/rdma.c                      |   2 +-
 rdma/rdma.h                      |   2 +
 rdma/res.c                       | 611

++++++++++++++++++++++++++++++++++++++-

Quoted 10 lines

 rdma/utils.c                     |  16 +
 7 files changed, 683 insertions(+), 5 deletions(-)
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

David Ahern

2018-03-12 17:52 UTC #37 lore raw

Quoted 5 lines · Steve Wise

Hey all,
The kernel side of this series has been merged for rdma-next [1]. Let me know if this iproute2 series can be merged, of if it needs more changes.

The problem is that iproute2 headers are synced to kernel headers from DaveM's tree (net-next mainly). I take it this series will not appear in Dave's tree until after a merge through Linus' tree. Correct?

Steve Wise

author

2018-03-12 18:43 UTC #38 lore raw

Quoted 11 lines · Steve Wise

Quoted 5 lines
Hey all,
The kernel side of this series has been merged for rdma-next [1]. Let me know if this iproute2 series can be merged, of if it needs more changes.
The problem is that iproute2 headers are synced to kernel headers from DaveM's tree (net-next mainly). I take it this series will not appear in Dave's tree until after a merge through Linus' tree. Correct?

Yes, I think so.

Steve.

Leon Romanovsky

2018-03-13 08:32 UTC #39 lore raw

In reply to David Ahern

Quoted 10 lines · David Ahern

Quoted 5 lines · Steve Wise
Hey all,
The kernel side of this series has been merged for rdma-next [1]. Let me know if this iproute2 series can be merged, of if it needs more changes.
The problem is that iproute2 headers are synced to kernel headers from DaveM's tree (net-next mainly). I take it this series will not appear in Dave's tree until after a merge through Linus' tree. Correct?

David,

Technically, you are right, and we would like to ask you for an extra tweak to the flow for the RDMAtool, because current scheme causes delays at least cycle.

Every RDMAtool's patchset which requires changes to headers is always includes header patch, can you please accept those series and once you are bringing new net-next headers from Linus, simply overwrite all our headers?

Thanks

Quoted 1 line

Leon Romanovsky

Reviewed-by on v2 3/6

2018-03-13 18:07 UTC #5 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

In reply to Steve Wise

Quoted 18 lines · Steve Wise

Sample output:

# rdma resource
2: cxgb4_0: pd 5 cq 2 qp 2 cm_id 3 mr 7
3: mlx4_0: pd 7 cq 3 qp 3 cm_id 3 mr 7

# rdma resource show cm_id
link cxgb4_0/- lqpn 0 qp-type RC state LISTEN ps TCP pid 30485 comm rping src-addr 0.0.0.0:7174
link cxgb4_0/2 lqpn 1048 qp-type RC state CONNECT ps TCP pid 30503 comm rping src-addr 172.16.2.1:7174 dst-addr 172.16.2.1:38246
link cxgb4_0/2 lqpn 1040 qp-type RC state CONNECT ps TCP pid 30498 comm rping src-addr 172.16.2.1:38246 dst-addr 172.16.2.1:7174
link mlx4_0/- lqpn 0 qp-type RC state LISTEN ps TCP pid 30485 comm rping src-addr 0.0.0.0:7174
link mlx4_0/1 lqpn 539 qp-type RC state CONNECT ps TCP pid 30494 comm rping src-addr 172.16.99.1:7174 dst-addr 172.16.99.1:43670
link mlx4_0/1 lqpn 538 qp-type RC state CONNECT ps TCP pid 30492 comm rping src-addr 172.16.99.1:43670 dst-addr 172.16.99.1:7174

# rdma resource show cm_id dst-port 7174
link cxgb4_0/2 lqpn 1040 qp-type RC state CONNECT ps TCP pid 30498 comm rping src-addr 172.16.2.1:38246 dst-addr 172.16.2.1:7174
link mlx4_0/1 lqpn 538 qp-type RC state CONNECT ps TCP pid 30492 comm rping src-addr 172.16.99.1:43670 dst-addr 172.16.99.1:7174

Quoted 7 lines

Signed-off-by Steve Wise <[email protected]>

---
 rdma/rdma.h  |   2 +
 rdma/res.c   | 258 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 rdma/utils.c |   5 ++
 3 files changed, 264 insertions(+), 1 deletion(-)

Steve, can you please add man pages update as a followup to this series?

Especially the fact that you are providing dst-port filter option should be mentioned.

Thanks,

Reviewed-by Leon Romanovsky <[email protected]>

Leon Romanovsky

Reviewed-by on v2 2/6

2018-03-13 18:11 UTC #3 lore raw

Re: [PATCH v2 iproute2-next 2/6] rdma: initialize the rd struct

In reply to Steve Wise

Quoted 9 lines · Steve Wise

Initialize the rd struct so port_idx is 0 unless set otherwise. Otherwise, strict_port queries end up passing an uninitialized PORT nlattr.
Signed-off-by Steve Wise <[email protected]>
---
 rdma/rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Thanks,

Reviewed-by Leon Romanovsky <[email protected]>

Leon Romanovsky

Reviewed-by on v2 4/6

2018-03-13 19:05 UTC #31 lore raw

Re: [PATCH v2 iproute2-next 4/6] rdma: Add CQ resource tracking information

In reply to Steve Wise

Quoted 9 lines · Steve Wise

Sample output:

# rdma resource show cq
link cxgb4_0/- cqe 46 users 2 pid 30503 comm rping
link cxgb4_0/- cqe 46 users 2 pid 30498 comm rping
link mlx4_0/- cqe 63 users 2 pid 30494 comm rping
link mlx4_0/- cqe 63 users 2 pid 30489 comm rping
link mlx4_0/- cqe 1023 users 2 poll_ctx WORKQUEUE pid 0 comm [ib_core]

Quoted 9 lines

# rdma resource show cq pid 30489
link mlx4_0/- cqe 63 users 2 pid 30489 comm rping

Signed-off-by Steve Wise <[email protected]>

---
 rdma/res.c   | 136 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 rdma/utils.c |   5 +++
 2 files changed, 141 insertions(+)

Thanks,

Reviewed-by Leon Romanovsky <[email protected]>

Leon Romanovsky

Reviewed-by on v2 5/6

2018-03-13 19:06 UTC #33 lore raw

Re: [PATCH v2 iproute2-next 5/6] rdma: Add MR resource tracking information

In reply to Steve Wise

Quoted 14 lines · Steve Wise

Sample output:

Without CAP_NET_ADMIN:

$ rdma resource show mr mrlen 65536
link mlx4_0/- mrlen 65536 pid 0 comm [nvme_rdma]
link cxgb4_0/- mrlen 65536 pid 0 comm [nvme_rdma]

With CAP_NET_ADMIN:

# rdma resource show mr mrlen 65536
link mlx4_0/- rkey 0x12702 lkey 0x12702 iova 0x85724a000 mrlen 65536 pid 0 comm [nvme_rdma]
link cxgb4_0/- rkey 0x68fe4e9 lkey 0x68fe4e9 iova 0x835b91000 mrlen 65536 pid 0 comm [nvme_rdma]

Quoted 8 lines

Signed-off-by Steve Wise <[email protected]>

---
 include/json_writer.h |   2 +
 lib/json_writer.c     |  11 +++++
 rdma/res.c            | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++
 rdma/utils.c          |   6 +++
 4 files changed, 144 insertions(+)

Thanks,

Reviewed-by Leon Romanovsky <[email protected]>

Leon Romanovsky

Reviewed-by on v2 6/6

2018-03-13 19:07 UTC #35 lore raw

Re: [PATCH v2 iproute2-next 6/6] rdma: Add PD resource tracking information

In reply to Steve Wise

Quoted 15 lines · Steve Wise

Sample output:
Without CAP_NET_ADMIN capability:
link mlx4_0/- users 0 pid 0 comm [ib_srpt]
link mlx4_0/- users 0 pid 0 comm [ib_srp]
link mlx4_0/- users 1 pid 0 comm [ib_core]
link cxgb4_0/- users 0 pid 0 comm [ib_srp]
With CAP_NET_ADMIN capability:
link mlx4_0/- local_dma_lkey 0x8000 users 0 pid 0 comm [ib_srpt] link mlx4_0/- local_dma_lkey 0x8000 users 0 pid 0 comm [ib_srp] link mlx4_0/- local_dma_lkey 0x8000 users 1 pid 0 comm [ib_core] link cxgb4_0/- local_dma_lkey 0x0 users 0 pid 0 comm [ib_srp]

Quoted 5 lines

Signed-off-by Steve Wise <[email protected]>

---
 rdma/res.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

Thanks,

Reviewed-by Leon Romanovsky <[email protected]>

Steve Wise

author

2018-03-13 19:44 UTC #6 lore raw

RE: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

In reply to Leon Romanovsky

Quoted 36 lines · Steve Wise

Quoted 8 lines
Sample output:
# rdma resource
2: cxgb4_0: pd 5 cq 2 qp 2 cm_id 3 mr 7
3: mlx4_0: pd 7 cq 3 qp 3 cm_id 3 mr 7
# rdma resource show cm_id
link cxgb4_0/- lqpn 0 qp-type RC state LISTEN ps TCP pid 30485 comm
rping src-addr 0.0.0.0:7174
Quoted 1 line
link cxgb4_0/2 lqpn 1048 qp-type RC state CONNECT ps TCP pid 30503
comm rping src-addr 172.16.2.1:7174 dst-addr 172.16.2.1:38246
Quoted 1 line
link cxgb4_0/2 lqpn 1040 qp-type RC state CONNECT ps TCP pid 30498
comm rping src-addr 172.16.2.1:38246 dst-addr 172.16.2.1:7174
Quoted 1 line
link mlx4_0/- lqpn 0 qp-type RC state LISTEN ps TCP pid 30485 comm rping
src-addr 0.0.0.0:7174
Quoted 1 line
link mlx4_0/1 lqpn 539 qp-type RC state CONNECT ps TCP pid 30494
comm rping src-addr 172.16.99.1:7174 dst-addr 172.16.99.1:43670
Quoted 1 line
link mlx4_0/1 lqpn 538 qp-type RC state CONNECT ps TCP pid 30492
comm rping src-addr 172.16.99.1:43670 dst-addr 172.16.99.1:7174
Quoted 3 lines
# rdma resource show cm_id dst-port 7174
link cxgb4_0/2 lqpn 1040 qp-type RC state CONNECT ps TCP pid 30498
comm rping src-addr 172.16.2.1:38246 dst-addr 172.16.2.1:7174
Quoted 1 line
link mlx4_0/1 lqpn 538 qp-type RC state CONNECT ps TCP pid 30492
comm rping src-addr 172.16.99.1:43670 dst-addr 172.16.99.1:7174
Quoted 5 lines
Signed-off-by Steve Wise <[email protected]>
---
 rdma/rdma.h  |   2 +
 rdma/res.c   | 258
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
Quoted 3 lines
 rdma/utils.c |   5 ++
 3 files changed, 264 insertions(+), 1 deletion(-)

Quoted 7 lines

Steve, can you please add man pages update as a followup to this series?
Especially the fact that you are providing dst-port filter option should be mentioned.
Thanks,
Reviewed-by Leon Romanovsky <[email protected]>

Will do.

David Ahern

2018-03-13 20:45 UTC #40 lore raw

In reply to Leon Romanovsky

Quoted 14 lines · Leon Romanovsky

Quoted 10 lines · David Ahern
Quoted 5 lines · Steve Wise
Hey all,
The kernel side of this series has been merged for rdma-next [1]. Let me know if this iproute2 series can be merged, of if it needs more changes.
The problem is that iproute2 headers are synced to kernel headers from DaveM's tree (net-next mainly). I take it this series will not appear in Dave's tree until after a merge through Linus' tree. Correct?
David,

Quoted 8 lines

Technically, you are right, and we would like to ask you for an extra tweak to the flow for the RDMAtool, because current scheme causes delays at least cycle.
Every RDMAtool's patchset which requires changes to headers is always includes header patch, can you please accept those series and once you are bringing new net-next headers from Linus, simply overwrite all our headers?

I did not follow the discussion back when this decision was made, so how did rdma tool end up in iproute2? I do not need the overhead of sometimes I sync the rdma header file and sometimes I don't.

One option that comes to mind is to move the rdma header file under the rdma directory. It breaks the uapi model, but it seems that iproute2 is just a delivery vehicle for this command.

Doug Ledford

2018-03-13 20:58 UTC #41 lore raw

Quoted 19 lines · David Ahern

Quoted 18 lines · Leon Romanovsky
Quoted 10 lines · David Ahern
Quoted 5 lines · Steve Wise
Hey all,
The kernel side of this series has been merged for rdma-next [1]. Let me know if this iproute2 series can be merged, of if it needs more changes.
The problem is that iproute2 headers are synced to kernel headers from DaveM's tree (net-next mainly). I take it this series will not appear in Dave's tree until after a merge through Linus' tree. Correct?
David,
Technically, you are right, and we would like to ask you for an extra tweak to the flow for the RDMAtool, because current scheme causes delays at least cycle.

Quoted 7 lines

Quoted 4 lines
Every RDMAtool's patchset which requires changes to headers is always includes header patch, can you please accept those series and once you are bringing new net-next headers from Linus, simply overwrite all our headers?
I did not follow the discussion back when this decision was made, so how did rdma tool end up in iproute2?

It is modeled after the ip command, and for better or worse, the iproute2 package has become the standard drop box for low level kernel network configuring tools. The RDMA subsystem may not be IP networking, but it is still networking, so it seemed an appropriate fit.

Quoted 6 lines

I do not need the overhead of
sometimes I sync the rdma header file and sometimes I don't.
One option that comes to mind is to move the rdma header file under the rdma directory. It breaks the uapi model, but it seems that iproute2 is just a delivery vehicle for this command.

Signature · 3 lines

Doug Ledford <[email protected]>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Jason Gunthorpe

2018-03-13 21:13 UTC #47 lore raw

In reply to David Ahern

Quoted 19 lines · David Ahern

Quoted 18 lines · Leon Romanovsky
Quoted 10 lines · David Ahern
Quoted 5 lines · Steve Wise
Hey all,
The kernel side of this series has been merged for rdma-next [1]. Let me know if this iproute2 series can be merged, of if it needs more changes.
The problem is that iproute2 headers are synced to kernel headers from DaveM's tree (net-next mainly). I take it this series will not appear in Dave's tree until after a merge through Linus' tree. Correct?
David,
Technically, you are right, and we would like to ask you for an extra tweak to the flow for the RDMAtool, because current scheme causes delays at least cycle.

Quoted 8 lines

Quoted 4 lines
Every RDMAtool's patchset which requires changes to headers is always includes header patch, can you please accept those series and once you are bringing new net-next headers from Linus, simply overwrite all our headers?
I did not follow the discussion back when this decision was made, so how did rdma tool end up in iproute2? I do not need the overhead of sometimes I sync the rdma header file and sometimes I don't.

Could you pull the uapi headers from linux-next? That tree will have both netdev and rdma stuff merged together properly.

Jason

David Ahern

2018-03-15 03:15 UTC #48 lore raw

Quoted 2 lines · Jason Gunthorpe

Could you pull the uapi headers from linux-next? That tree will have both netdev and rdma stuff merged together properly.

What's the merge history between linux-next, Linus' tree, net-next + rdma-next?

Jason Gunthorpe

2018-03-15 03:35 UTC #49 lore raw

Quoted 6 lines · David Ahern

Quoted 2 lines · Jason Gunthorpe
Could you pull the uapi headers from linux-next? That tree will have both netdev and rdma stuff merged together properly.
What's the merge history between linux-next, Linus' tree, net-next + rdma-next?

Not sure I understand the question?

linux-next is thrown away every cycle, so for instance you can't say "took uapi headers from linux-next commit XYZ" in the iproute git commit..

Otherwise, linux-next is built, I think daily/weekly and includes the latest of both rdma and netdev next trees, so it certainly has the right content.

A script could figure out the stable netdev and rdma commit IDs from linux-next and record those, if that is interest..

Jason

David Ahern

2018-03-16 16:08 UTC #50 lore raw

Quoted 14 lines · Jason Gunthorpe

Quoted 6 lines · David Ahern
Quoted 2 lines · Jason Gunthorpe
Could you pull the uapi headers from linux-next? That tree will have both netdev and rdma stuff merged together properly.
What's the merge history between linux-next, Linus' tree, net-next + rdma-next?
Not sure I understand the question?
linux-next is thrown away every cycle, so for instance you can't say "took uapi headers from linux-next commit XYZ" in the iproute git commit..

Quoted 6 lines

Otherwise, linux-next is built, I think daily/weekly and includes the latest of both rdma and netdev next trees, so it certainly has the right content.
A script could figure out the stable netdev and rdma commit IDs from linux-next and record those, if that is interest..

I'd prefer not to sync headers to a tree that disappears; I like the traceability to specific points in time.

David Ahern

2018-03-16 16:18 UTC #42 lore raw

In reply to Doug Ledford

Quoted 25 lines · Doug Ledford

Quoted 24 lines · David Ahern
Quoted 22 lines · Leon Romanovsky
Quoted 10 lines · David Ahern
Quoted 5 lines · Steve Wise
Hey all,
The kernel side of this series has been merged for rdma-next [1]. Let me know if this iproute2 series can be merged, of if it needs more changes.
The problem is that iproute2 headers are synced to kernel headers from DaveM's tree (net-next mainly). I take it this series will not appear in Dave's tree until after a merge through Linus' tree. Correct?
David,
Technically, you are right, and we would like to ask you for an extra tweak to the flow for the RDMAtool, because current scheme causes delays at least cycle.
Every RDMAtool's patchset which requires changes to headers is always includes header patch, can you please accept those series and once you are bringing new net-next headers from Linus, simply overwrite all our headers?

Quoted 7 lines

Quoted 2 lines
I did not follow the discussion back when this decision was made, so how did rdma tool end up in iproute2?
It is modeled after the ip command, and for better or worse, the iproute2 package has become the standard drop box for low level kernel network configuring tools. The RDMA subsystem may not be IP networking, but it is still networking, so it seemed an appropriate fit.

why doesn't the rdma tree go through Dave then?

Leon Romanovsky

2018-03-16 16:23 UTC #51 lore raw

In reply to David Ahern

Quoted 19 lines · David Ahern

Quoted 18 lines · Jason Gunthorpe
Quoted 6 lines · David Ahern
Quoted 2 lines · Jason Gunthorpe
Could you pull the uapi headers from linux-next? That tree will have both netdev and rdma stuff merged together properly.
What's the merge history between linux-next, Linus' tree, net-next + rdma-next?
Not sure I understand the question?
linux-next is thrown away every cycle, so for instance you can't say "took uapi headers from linux-next commit XYZ" in the iproute git commit..
Otherwise, linux-next is built, I think daily/weekly and includes the latest of both rdma and netdev next trees, so it certainly has the right content.

Quoted 5 lines

Quoted 2 lines
A script could figure out the stable netdev and rdma commit IDs from linux-next and record those, if that is interest..
I'd prefer not to sync headers to a tree that disappears; I like the traceability to specific points in time.

The headers itself will have real SHAs, the same as netdev and RDMA trees have, so it will give needed traceability as you are looking.

Thanks

Doug Ledford

2018-03-20 17:21 UTC #43 lore raw

In reply to David Ahern

Quoted 29 lines · David Ahern

Quoted 28 lines · Doug Ledford
Quoted 26 lines · David Ahern
Quoted 14 lines · Leon Romanovsky
Quoted 10 lines · David Ahern
Quoted 5 lines · Steve Wise
Hey all,
The kernel side of this series has been merged for rdma-next [1]. Let me know if this iproute2 series can be merged, of if it needs more changes.
The problem is that iproute2 headers are synced to kernel headers from DaveM's tree (net-next mainly). I take it this series will not appear in Dave's tree until after a merge through Linus' tree. Correct?
David,
Quoted 8 lines
Technically, you are right, and we would like to ask you for an extra tweak to the flow for the RDMAtool, because current scheme causes delays at least cycle.
Every RDMAtool's patchset which requires changes to headers is always includes header patch, can you please accept those series and once you are bringing new net-next headers from Linus, simply overwrite all our headers?
I did not follow the discussion back when this decision was made, so how did rdma tool end up in iproute2?

Quoted 7 lines

Quoted 4 lines
It is modeled after the ip command, and for better or worse, the iproute2 package has become the standard drop box for low level kernel network configuring tools. The RDMA subsystem may not be IP networking, but it is still networking, so it seemed an appropriate fit.
why doesn't the rdma tree go through Dave then?

Because it doesn't use the core network stack hardly at all. It creates netdevs when it needs to bridge the two stacks, but otherwise the RDMA subsystem core is apart and unique from the network stack Dave manages. When I said it was networking, I meant it literally. The RDMA fabrics are networks. It wasn't meant to imply that they shared anything substantial in common with the typical Ethernet/IP networking that is the core of what Dave manages.

David Ahern

2018-03-21 16:59 UTC #44 lore raw

Quoted 38 lines · Doug Ledford

Quoted 29 lines · David Ahern
Quoted 28 lines · Doug Ledford
Quoted 26 lines · David Ahern
Quoted 14 lines · Leon Romanovsky
Quoted 10 lines · David Ahern
Quoted 5 lines · Steve Wise
Hey all,
The kernel side of this series has been merged for rdma-next [1]. Let me know if this iproute2 series can be merged, of if it needs more changes.
The problem is that iproute2 headers are synced to kernel headers from DaveM's tree (net-next mainly). I take it this series will not appear in Dave's tree until after a merge through Linus' tree. Correct?
David,
Quoted 8 lines
Technically, you are right, and we would like to ask you for an extra tweak to the flow for the RDMAtool, because current scheme causes delays at least cycle.
Every RDMAtool's patchset which requires changes to headers is always includes header patch, can you please accept those series and once you are bringing new net-next headers from Linus, simply overwrite all our headers?
I did not follow the discussion back when this decision was made, so how did rdma tool end up in iproute2?
Quoted 7 lines
Quoted 4 lines
It is modeled after the ip command, and for better or worse, the iproute2 package has become the standard drop box for low level kernel network configuring tools. The RDMA subsystem may not be IP networking, but it is still networking, so it seemed an appropriate fit.
why doesn't the rdma tree go through Dave then?

Quoted 8 lines

Because it doesn't use the core network stack hardly at all. It creates netdevs when it needs to bridge the two stacks, but otherwise the RDMA subsystem core is apart and unique from the network stack Dave manages. When I said it was networking, I meant it literally. The RDMA fabrics are networks. It wasn't meant to imply that they shared anything substantial in common with the typical Ethernet/IP networking that is the core of what Dave manages.

I think the simplest approach is to move the uapi header under the rdma directory and you folks take ownership of that header.

Steve Wise

author

2018-03-22 20:20 UTC #45 lore raw

Quoted 41 lines · David Ahern

Quoted 32 lines · Doug Ledford
Quoted 23 lines · David Ahern
Quoted 22 lines · Doug Ledford
Quoted 21 lines · David Ahern
Quoted 20 lines · Leon Romanovsky
Quoted 9 lines · David Ahern
Quoted 5 lines · Steve Wise
Hey all,
The kernel side of this series has been merged for rdma-next [1]. Let me know if this iproute2 series can be merged, of if it needs more changes.
The problem is that iproute2 headers are synced to kernel headers from DaveM's tree (net-next mainly). I take it this series will not appear in Dave's tree until after a merge through Linus' tree. Correct?
David,
Technically, you are right, and we would like to ask you for an extra tweak to the flow for the RDMAtool, because current scheme causes delays at least cycle.
Every RDMAtool's patchset which requires changes to headers is always includes header patch, can you please accept those series and once you are bringing new net-next headers from Linus, simply overwrite all our headers?
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
I did not follow the discussion back when this decision was made, so how did rdma tool end up in iproute2?
It is modeled after the ip command, and for better or worse, the iproute2 package has become the standard drop box for low level kernel network configuring tools. The RDMA subsystem may not be IP networking, but it is still networking, so it seemed an appropriate fit.
why doesn't the rdma tree go through Dave then?
Quoted 8 lines
Because it doesn't use the core network stack hardly at all. It creates netdevs when it needs to bridge the two stacks, but otherwise the RDMA subsystem core is apart and unique from the network stack Dave manages. When I said it was networking, I meant it literally. The RDMA fabrics are networks. It wasn't meant to imply that they shared anything substantial in common with the typical Ethernet/IP networking that is the core of what Dave manages.

Quoted 2 lines

I think the simplest approach is to move the uapi header under the rdma directory and you folks take ownership of that header.

Fine with me. Leon?

Steve.

David Ahern

2018-03-22 20:26 UTC #46 lore raw

Quoted 3 lines · Steve Wise

Quoted 2 lines
I think the simplest approach is to move the uapi header under the rdma directory and you folks take ownership of that header.
Fine with me. Leon?

Please make a patch moving the existing file and then make changes to it in follow on patches.

David Ahern

2018-03-26 14:17 UTC #7 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

In reply to Steve Wise

Quoted 5 lines · Steve Wise

rdma/rdma.h +0 -0
index 5809f70..e55205b 100644

rdma/rdma.h line 27

Quoted 10 lines

rdma/rdma.h +2 -0

@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>

did you forget to add rdma_cma.h? I don't see that file in my repo.

Steve Wise

author

2018-03-26 14:30 UTC #8 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 11 lines · David Ahern

Quoted 10 lines · Steve Wise

rdma/rdma.h +1 -0

index 5809f70..e55205b 100644
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>

Quoted 6 lines

rdma/rdma.h line 27
Quoted 5 lines
rdma/rdma.h +1 -0
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my repo.

It is provided by the rdma-core package, upon which rdma tool now depends for the rdma_port_space enum.

Steve.

David Ahern

2018-03-26 14:44 UTC #9 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 21 lines · Steve Wise

Quoted 11 lines · David Ahern
Quoted 10 lines · Steve Wise
rdma/rdma.h +1 -0
index 5809f70..e55205b 100644
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
Quoted 6 lines
rdma/rdma.h line 27
Quoted 5 lines
rdma/rdma.h +1 -0
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my repo.

Quoted 3 lines

It is provided by the rdma-core package, upon which rdma tool now depends for the rdma_port_space enum.

You need to add a check for the package, and only build rdma if that package is installed. See check_mnl in configure for an example.

Steve Wise

author

2018-03-26 14:55 UTC #10 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 20 lines · David Ahern

Quoted 19 lines · Steve Wise
Quoted 17 lines · David Ahern
Quoted 5 lines · Steve Wise
rdma/rdma.h +0 -0
index 5809f70..e55205b 100644
rdma/rdma.h line 27
Quoted 10 lines
rdma/rdma.h +2 -0
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my repo.

Quoted 5 lines

Quoted 3 lines
It is provided by the rdma-core package, upon which rdma tool now depends for the rdma_port_space enum.
You need to add a check for the package, and only build rdma if that package is installed. See check_mnl in configure for an example.

Ok, that makes sense.

Steve.

Leon Romanovsky

2018-03-26 15:08 UTC #11 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 23 lines · Steve Wise

Quoted 20 lines · David Ahern
Quoted 19 lines · Steve Wise
Quoted 17 lines · David Ahern
Quoted 5 lines · Steve Wise
rdma/rdma.h +0 -0
index 5809f70..e55205b 100644
rdma/rdma.h line 27
Quoted 10 lines
rdma/rdma.h +2 -0
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my repo.

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
It is provided by the rdma-core package, upon which rdma tool now depends for the rdma_port_space enum.
You need to add a check for the package, and only build rdma if that package is installed. See check_mnl in configure for an example.
Ok, that makes sense.

IMHO, better solution will be to copy those files to iproute2.

Thanks

Quoted 2 lines

Steve.

Steve Wise

author

2018-03-26 15:24 UTC #12 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 22 lines · Leon Romanovsky

Quoted 21 lines · Steve Wise
Quoted 19 lines · David Ahern
Quoted 18 lines · Steve Wise
Quoted 17 lines · David Ahern
Quoted 5 lines · Steve Wise
rdma/rdma.h +0 -0
index 5809f70..e55205b 100644
rdma/rdma.h line 27
Quoted 10 lines
rdma/rdma.h +2 -0
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my repo.

Quoted 7 lines

Quoted 6 lines
Quoted 5 lines
Quoted 3 lines
It is provided by the rdma-core package, upon which rdma tool now depends for the rdma_port_space enum.
You need to add a check for the package, and only build rdma if that package is installed. See check_mnl in configure for an example.
Ok, that makes sense.
IMHO, better solution will be to copy those files to iproute2.

Hey Leon,

Why is it better in your opinion? My gut tells me adding rdma_cma.h to iproute2 means more uabi type syncing.

David Ahern

2018-03-26 15:40 UTC #28 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

In reply to Steve Wise

Quoted 69 lines · Steve Wise

Sample output:

# rdma resource
2: cxgb4_0: pd 5 cq 2 qp 2 cm_id 3 mr 7
3: mlx4_0: pd 7 cq 3 qp 3 cm_id 3 mr 7

# rdma resource show cm_id
link cxgb4_0/- lqpn 0 qp-type RC state LISTEN ps TCP pid 30485 comm rping src-addr 0.0.0.0:7174
link cxgb4_0/2 lqpn 1048 qp-type RC state CONNECT ps TCP pid 30503 comm rping src-addr 172.16.2.1:7174 dst-addr 172.16.2.1:38246
link cxgb4_0/2 lqpn 1040 qp-type RC state CONNECT ps TCP pid 30498 comm rping src-addr 172.16.2.1:38246 dst-addr 172.16.2.1:7174
link mlx4_0/- lqpn 0 qp-type RC state LISTEN ps TCP pid 30485 comm rping src-addr 0.0.0.0:7174
link mlx4_0/1 lqpn 539 qp-type RC state CONNECT ps TCP pid 30494 comm rping src-addr 172.16.99.1:7174 dst-addr 172.16.99.1:43670
link mlx4_0/1 lqpn 538 qp-type RC state CONNECT ps TCP pid 30492 comm rping src-addr 172.16.99.1:43670 dst-addr 172.16.99.1:7174

# rdma resource show cm_id dst-port 7174
link cxgb4_0/2 lqpn 1040 qp-type RC state CONNECT ps TCP pid 30498 comm rping src-addr 172.16.2.1:38246 dst-addr 172.16.2.1:7174
link mlx4_0/1 lqpn 538 qp-type RC state CONNECT ps TCP pid 30492 comm rping src-addr 172.16.99.1:43670 dst-addr 172.16.99.1:7174

Signed-off-by Steve Wise <[email protected]>

---
 rdma/rdma.h  |   2 +
 rdma/res.c   | 258 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 rdma/utils.c |   5 ++
 3 files changed, 264 insertions(+), 1 deletion(-)

rdma/rdma.h +2 -0

index 5809f70..e55205b 100644
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
 #define pr_err(args...) fprintf(stderr, ##args)
 #define pr_out(args...) fprintf(stdout, ##args)

rdma/res.c +9 -1

index 62f5c54..1ef4f20 100644
@@ -16,9 +16,11 @@ static int res_help(struct rd *rd)
 {
 	pr_out("Usage: %s resource\n", rd->filename);
 	pr_out("          resource show [DEV]\n");
-	pr_out("          resource show [qp]\n");
+	pr_out("          resource show [qp|cm_id]\n");
 	pr_out("          resource show qp link [DEV/PORT]\n");
 	pr_out("          resource show qp link [DEV/PORT] [FILTER-NAME FILTER-VALUE]\n");
+	pr_out("          resource show cm_id link [DEV/PORT]\n");
+	pr_out("          resource show cm_id link [DEV/PORT] [FILTER-NAME FILTER-VALUE]\n");
 	return 0;
 }

@@ -433,6 +435,230 @@ static int res_qp_parse_cb(const struct nlmsghdr *nlh, void *data)
 	return MNL_CB_OK;
 }
 
+static void print_qp_type(struct rd *rd, uint32_t val)
+{
+	if (rd->json_output)
+		jsonw_string_field(rd->jw, "qp-type",
+				   qp_types_to_str(val));
+	else

rdma/res.c static int res_qp_parse_cb(const struct nlmsghdr *nlh, void *data) · line 453

Quoted 10 lines

rdma/res.c +10 -0

+		pr_out("qp-type %s ", qp_types_to_str(val));
+}
+
+static const char *cm_id_state_to_str(uint8_t idx)
+{
+	static const char * const cm_id_states_str[] = { "IDLE", "ADDR_QUERY",
+						      "ADDR_RESOLVED", "ROUTE_QUERY", "ROUTE_RESOLVED",
+						      "CONNECT", "DISCONNECT",
+						      "ADDR_BOUND", "LISTEN", "DEVICE_REMOVAL", "DESTROYING" };
+

In general lines should stay under 80 columns. There are a few exceptions to the rule (e.g., print strings), but most of the long lines you have in this patch need to conform.

Quoted 20 lines

rdma/res.c +16 -0

@@ -457,11 +683,41 @@ filters qp_valid_filters[MAX_NUMBER_OF_FILTERS] = {{ .name = "link",
 
 RES_FUNC(res_qp,	RDMA_NLDEV_CMD_RES_QP_GET, qp_valid_filters, false);
 
+static const struct
+filters cm_id_valid_filters[MAX_NUMBER_OF_FILTERS] = {{ .name = "link",
+						   .is_number = false },
+						   { .name = "lqpn",
+						   .is_number = true },
+						   { .name = "qp-type",
+						   .is_number = false },
+						   { .name = "state",
+						   .is_number = false },
+						   { .name = "ps",
+						   .is_number = false },
+						   { .name = "dev-type",
+						   .is_number = false },
+						   { .name = "transport-type",
+						   .is_number = false },
+						   { .name = "pid",

rdma/res.c filters qp_valid_filters[MAX_NUMBER_OF_FILTERS] = {{ .name = "link", · line 711

Quoted 10 lines

rdma/res.c +10 -0

+						   .is_number = true },
+						   { .name = "src-addr",
+						   .is_number = false },
+						   { .name = "src-port",
+						   .is_number = true },
+						   { .name = "dst-addr",
+						   .is_number = false },
+						   { .name = "dst-port",
+						   .is_number = true }};
+

The above would be more readable as

	static const
	struct filters cm_id_valid_filters[MAX_NUMBER_OF_FILTERS] = {
		{ .name = "link", .is_number = false },
		{ .name = "lqpn", .is_number = true },

...

Definitely do not split between struct and filters.

Leon Romanovsky

2018-03-26 17:06 UTC #13 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

In reply to Steve Wise

Quoted 30 lines · Steve Wise

Quoted 27 lines · Leon Romanovsky
Quoted 26 lines · Steve Wise
Quoted 24 lines · David Ahern
Quoted 12 lines · Steve Wise
Quoted 11 lines · David Ahern
Quoted 10 lines · Steve Wise
rdma/rdma.h +1 -0
index 5809f70..e55205b 100644
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
Quoted 9 lines
Quoted 6 lines
rdma/rdma.h line 27
Quoted 5 lines
rdma/rdma.h +1 -0
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my repo.
It is provided by the rdma-core package, upon which rdma tool now depends for the rdma_port_space enum.
You need to add a check for the package, and only build rdma if that package is installed. See check_mnl in configure for an example.

Quoted 7 lines

Quoted 2 lines
Quoted 1 line
Ok, that makes sense.
IMHO, better solution will be to copy those files to iproute2.
Hey Leon,
Why is it better in your opinion? My gut tells me adding rdma_cma.h to iproute2 means more uabi type syncing.

Making rdmatool be dependant on rdma-core will require that distributions will update their specs to install rdma-core as a dependency for every iprotue2 install.

The rdma-core dependency makes sense for RDMA users, but doesn't for most of the iproute2 users.

Thanks

Quoted 2 lines

Steve Wise

author

2018-03-26 17:13 UTC #14 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 33 lines · Leon Romanovsky

Quoted 32 lines · Steve Wise
Quoted 21 lines · Leon Romanovsky
Quoted 20 lines · Steve Wise
Quoted 19 lines · David Ahern
Quoted 18 lines · Steve Wise
Quoted 17 lines · David Ahern
Quoted 5 lines · Steve Wise
rdma/rdma.h +0 -0
index 5809f70..e55205b 100644
rdma/rdma.h line 27
Quoted 10 lines
rdma/rdma.h +2 -0
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my repo.
Quoted 7 lines
Quoted 6 lines
Quoted 5 lines
Quoted 3 lines
It is provided by the rdma-core package, upon which rdma tool now depends for the rdma_port_space enum.
You need to add a check for the package, and only build rdma if that package is installed. See check_mnl in configure for an example.
Ok, that makes sense.
IMHO, better solution will be to copy those files to iproute2.
Hey Leon,

Quoted 8 lines

Quoted 2 lines
Why is it better in your opinion? My gut tells me adding rdma_cma.h to iproute2 means more uabi type syncing.
Making rdmatool be dependant on rdma-core will require that distributions will update their specs to install rdma-core as a dependency for every iprotue2 install.
The rdma-core dependency makes sense for RDMA users, but doesn't for most of the iproute2 users.

I'm fuzzy on the details of distro packaging, but David's suggestion is that rdmatool wouldn't get built if rdma-core isn't present. But everything else would. Just like it does not get built if libmnl is not installed. For pre-built rpms, the rdma-core would have to be present.

I'm ok pulling it in, I'm just trying to understand. :)

Steve.

Leon Romanovsky

2018-03-26 17:27 UTC #15 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 45 lines · Steve Wise

Quoted 33 lines · Leon Romanovsky
Quoted 32 lines · Steve Wise
Quoted 21 lines · Leon Romanovsky
Quoted 20 lines · Steve Wise
Quoted 19 lines · David Ahern
Quoted 18 lines · Steve Wise
Quoted 17 lines · David Ahern
Quoted 5 lines · Steve Wise
rdma/rdma.h +0 -0
index 5809f70..e55205b 100644
rdma/rdma.h line 27
Quoted 10 lines
rdma/rdma.h +2 -0
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my repo.
Quoted 7 lines
Quoted 6 lines
Quoted 5 lines
Quoted 3 lines
It is provided by the rdma-core package, upon which rdma tool now depends for the rdma_port_space enum.
You need to add a check for the package, and only build rdma if that package is installed. See check_mnl in configure for an example.
Ok, that makes sense.
IMHO, better solution will be to copy those files to iproute2.
Hey Leon,
Quoted 8 lines
Quoted 2 lines
Why is it better in your opinion? My gut tells me adding rdma_cma.h to iproute2 means more uabi type syncing.
Making rdmatool be dependant on rdma-core will require that distributions will update their specs to install rdma-core as a dependency for every iprotue2 install.
The rdma-core dependency makes sense for RDMA users, but doesn't for most of the iproute2 users.

Quoted 7 lines

I'm fuzzy on the details of distro packaging, but David's suggestion is that rdmatool wouldn't get built if rdma-core isn't present. But everything else would. Just like it does not get built if libmnl is not installed. For pre-built rpms, the rdma-core would have to be present.
I'm ok pulling it in, I'm just trying to understand. :)

Distros supply pre-built packages, for example Fedora's RPM:
https://rpmfind.net/linux/RPM/fedora/27/x86_64/i/iproute-4.12.0-3.fc27.x86_64.html
It requires that libmnl will be installed. Once rdmatool will need rdma-core, it will pulled in too.

BTW, don't forget to change header's guards (ifdef/defne ..), see rdma_netlink.h as an example.

Quoted 5 lines

Steve.

Steve Wise

author

2018-03-26 19:55 UTC #29 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

In reply to David Ahern

Quoted 75 lines · David Ahern

Quoted 74 lines · Steve Wise

Sample output:

# rdma resource
2: cxgb4_0: pd 5 cq 2 qp 2 cm_id 3 mr 7
3: mlx4_0: pd 7 cq 3 qp 3 cm_id 3 mr 7

# rdma resource show cm_id
link cxgb4_0/- lqpn 0 qp-type RC state LISTEN ps TCP pid 30485 comm rping src-addr 0.0.0.0:7174
link cxgb4_0/2 lqpn 1048 qp-type RC state CONNECT ps TCP pid 30503 comm rping src-addr 172.16.2.1:7174 dst-addr 172.16.2.1:38246
link cxgb4_0/2 lqpn 1040 qp-type RC state CONNECT ps TCP pid 30498 comm rping src-addr 172.16.2.1:38246 dst-addr 172.16.2.1:7174
link mlx4_0/- lqpn 0 qp-type RC state LISTEN ps TCP pid 30485 comm rping src-addr 0.0.0.0:7174
link mlx4_0/1 lqpn 539 qp-type RC state CONNECT ps TCP pid 30494 comm rping src-addr 172.16.99.1:7174 dst-addr 172.16.99.1:43670
link mlx4_0/1 lqpn 538 qp-type RC state CONNECT ps TCP pid 30492 comm rping src-addr 172.16.99.1:43670 dst-addr 172.16.99.1:7174

# rdma resource show cm_id dst-port 7174
link cxgb4_0/2 lqpn 1040 qp-type RC state CONNECT ps TCP pid 30498 comm rping src-addr 172.16.2.1:38246 dst-addr 172.16.2.1:7174
link mlx4_0/1 lqpn 538 qp-type RC state CONNECT ps TCP pid 30492 comm rping src-addr 172.16.99.1:43670 dst-addr 172.16.99.1:7174

Signed-off-by Steve Wise <[email protected]>

---
 rdma/rdma.h  |   2 +
 rdma/res.c   | 258 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 rdma/utils.c |   5 ++
 3 files changed, 264 insertions(+), 1 deletion(-)

rdma/rdma.h +2 -0

index 5809f70..e55205b 100644
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
 #define pr_err(args...) fprintf(stderr, ##args)
 #define pr_out(args...) fprintf(stdout, ##args)

rdma/res.c +14 -1

index 62f5c54..1ef4f20 100644
@@ -16,9 +16,11 @@ static int res_help(struct rd *rd)
 {
 	pr_out("Usage: %s resource\n", rd->filename);
 	pr_out("          resource show [DEV]\n");
-	pr_out("          resource show [qp]\n");
+	pr_out("          resource show [qp|cm_id]\n");
 	pr_out("          resource show qp link [DEV/PORT]\n");
 	pr_out("          resource show qp link [DEV/PORT] [FILTER-NAME FILTER-VALUE]\n");
+	pr_out("          resource show cm_id link [DEV/PORT]\n");
+	pr_out("          resource show cm_id link [DEV/PORT] [FILTER-NAME FILTER-VALUE]\n");
 	return 0;
 }

@@ -433,6 +435,230 @@ static int res_qp_parse_cb(const struct nlmsghdr *nlh, void *data)
 	return MNL_CB_OK;
 }
 
+static void print_qp_type(struct rd *rd, uint32_t val)
+{
+	if (rd->json_output)
+		jsonw_string_field(rd->jw, "qp-type",
+				   qp_types_to_str(val));
+	else
+		pr_out("qp-type %s ", qp_types_to_str(val));
+}
+
+static const char *cm_id_state_to_str(uint8_t idx)
+{

Quoted 8 lines

rdma/res.c static int res_qp_parse_cb(const struct nlmsghdr *nlh, void *data) · line 453
Quoted 5 lines
rdma/res.c +5 -0
+	static const char * const cm_id_states_str[] = { "IDLE", "ADDR_QUERY",
+						      "ADDR_RESOLVED", "ROUTE_QUERY", "ROUTE_RESOLVED",
+						      "CONNECT", "DISCONNECT",
+						      "ADDR_BOUND", "LISTEN", "DEVICE_REMOVAL", "DESTROYING" };
+
In general lines should stay under 80 columns. There are a few exceptions to the rule (e.g., print strings), but most of the long lines you have in this patch need to conform.

Ok. Will do.

Quoted 31 lines

Quoted 20 lines

rdma/res.c +16 -0

@@ -457,11 +683,41 @@ filters qp_valid_filters[MAX_NUMBER_OF_FILTERS] = {{ .name = "link",
 
 RES_FUNC(res_qp,	RDMA_NLDEV_CMD_RES_QP_GET, qp_valid_filters, false);
 
+static const struct
+filters cm_id_valid_filters[MAX_NUMBER_OF_FILTERS] = {{ .name = "link",
+						   .is_number = false },
+						   { .name = "lqpn",
+						   .is_number = true },
+						   { .name = "qp-type",
+						   .is_number = false },
+						   { .name = "state",
+						   .is_number = false },
+						   { .name = "ps",
+						   .is_number = false },
+						   { .name = "dev-type",
+						   .is_number = false },
+						   { .name = "transport-type",
+						   .is_number = false },
+						   { .name = "pid",

rdma/res.c filters qp_valid_filters[MAX_NUMBER_OF_FILTERS] = {{ .name = "link", · line 711

Quoted 10 lines

rdma/res.c +10 -0

+						   .is_number = true },
+						   { .name = "src-addr",
+						   .is_number = false },
+						   { .name = "src-port",
+						   .is_number = true },
+						   { .name = "dst-addr",
+						   .is_number = false },
+						   { .name = "dst-port",
+						   .is_number = true }};
+

The above would be more readable as

Quoted 7 lines

	static const
	struct filters cm_id_valid_filters[MAX_NUMBER_OF_FILTERS] = {
		{ .name = "link", .is_number = false },
		{ .name = "lqpn", .is_number = true },

...

Definitely do not split between struct and filters.

Your formatting is easier to read/maintain.

Thanks for the comments!

Jason Gunthorpe

2018-03-26 21:15 UTC #16 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

In reply to Steve Wise

Quoted 20 lines · Steve Wise

Quoted 10 lines · David Ahern
Quoted 9 lines · Steve Wise
rdma/rdma.h +2 -0
index 5809f70..e55205b 100644
+++ b/rdma/rdma.h
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
Quoted 6 lines
rdma/rdma.h line 27
Quoted 5 lines
rdma/rdma.h +1 -0
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my repo.

Quoted 2 lines

It is provided by the rdma-core package, upon which rdma tool now depends for the rdma_port_space enum.

It is a kernel bug that enum is not in an include/uapi/rdma header

Fix it there and don't try to use rdma-core headers to get kernel ABI.

Jason

Steve Wise

author

2018-03-26 21:34 UTC #17 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 19 lines · Jason Gunthorpe

Quoted 18 lines · Steve Wise
Quoted 16 lines · David Ahern
Quoted 4 lines · Steve Wise
rdma/rdma.h +1 -0
index 5809f70..e55205b 100644
+++ b/rdma/rdma.h
rdma/rdma.h line 27
Quoted 10 lines
rdma/rdma.h +2 -0
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my repo.

Quoted 7 lines

Quoted 2 lines
It is provided by the rdma-core package, upon which rdma tool now depends for the rdma_port_space enum.
It is a kernel bug that enum is not in an include/uapi/rdma header
Fix it there and don't try to use rdma-core headers to get kernel ABI.
Jason

I wish you'd commented on this just a little sooner. I just resent v3 of this series... with rdma_cma.h included. :)

How about the restrack/nldev code just translates the port space from enum rdma_port_space to a new ABI enum, say nldev_rdma_port_space, that i add to rdma_netlink.h? I'd hate to open the can of worms of trying to split rdma_cma.h into uabi and no uabi headers. :(

Steve.

Jason Gunthorpe

2018-03-26 22:30 UTC #18 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 29 lines · Steve Wise

Quoted 19 lines · Jason Gunthorpe
Quoted 18 lines · Steve Wise
Quoted 16 lines · David Ahern
Quoted 4 lines · Steve Wise
rdma/rdma.h +1 -0
index 5809f70..e55205b 100644
+++ b/rdma/rdma.h
rdma/rdma.h line 27
Quoted 10 lines
rdma/rdma.h +2 -0
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my repo.
Quoted 7 lines
Quoted 2 lines
It is provided by the rdma-core package, upon which rdma tool now depends for the rdma_port_space enum.
It is a kernel bug that enum is not in an include/uapi/rdma header
Fix it there and don't try to use rdma-core headers to get kernel ABI.
Jason

Quoted 7 lines

I wish you'd commented on this just a little sooner. I just resent v3 of this series... with rdma_cma.h included. :)
How about the restrack/nldev code just translates the port space from enum rdma_port_space to a new ABI enum, say nldev_rdma_port_space, that i add to rdma_netlink.h? I'd hate to open the can of worms of trying to split rdma_cma.h into uabi and no uabi headers. :(

If port space is already part of the ABI there isn't much reason to translate it.

You just need to pick the right header to put it in, since it is a verbs define it doesn't belong in the netlink header.

Jason

Leon Romanovsky

2018-03-27 03:21 UTC #19 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 38 lines · Jason Gunthorpe

Quoted 29 lines · Steve Wise
Quoted 19 lines · Jason Gunthorpe
Quoted 18 lines · Steve Wise
Quoted 16 lines · David Ahern
Quoted 4 lines · Steve Wise
rdma/rdma.h +1 -0
index 5809f70..e55205b 100644
+++ b/rdma/rdma.h
rdma/rdma.h line 27
Quoted 10 lines
rdma/rdma.h +2 -0
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my repo.
Quoted 7 lines
Quoted 2 lines
It is provided by the rdma-core package, upon which rdma tool now depends for the rdma_port_space enum.
It is a kernel bug that enum is not in an include/uapi/rdma header
Fix it there and don't try to use rdma-core headers to get kernel ABI.
Jason
Quoted 7 lines
I wish you'd commented on this just a little sooner. I just resent v3 of this series... with rdma_cma.h included. :)
How about the restrack/nldev code just translates the port space from enum rdma_port_space to a new ABI enum, say nldev_rdma_port_space, that i add to rdma_netlink.h? I'd hate to open the can of worms of trying to split rdma_cma.h into uabi and no uabi headers. :(

Quoted 5 lines

If port space is already part of the ABI there isn't much reason to translate it.
You just need to pick the right header to put it in, since it is a verbs define it doesn't belong in the netlink header.

I completely understand Steve's concerns.

I tried to do such thing (expose kernel headers) in first incarnation of rdmatool with attempt to clean IB/core as well to ensure that we won't expose anything that is not implemented. It didn't go well.

Thanks

Quoted 2 lines

Jason

Jason Gunthorpe

2018-03-27 14:44 UTC #20 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 42 lines · Leon Romanovsky

Quoted 41 lines · Jason Gunthorpe
Quoted 29 lines · Steve Wise
Quoted 19 lines · Jason Gunthorpe
Quoted 18 lines · Steve Wise
Quoted 16 lines · David Ahern
Quoted 4 lines · Steve Wise
rdma/rdma.h +1 -0
index 5809f70..e55205b 100644
+++ b/rdma/rdma.h
rdma/rdma.h line 27
Quoted 10 lines
rdma/rdma.h +2 -0
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my repo.
Quoted 7 lines
Quoted 2 lines
It is provided by the rdma-core package, upon which rdma tool now depends for the rdma_port_space enum.
It is a kernel bug that enum is not in an include/uapi/rdma header
Fix it there and don't try to use rdma-core headers to get kernel ABI.
Jason
Quoted 7 lines
I wish you'd commented on this just a little sooner. I just resent v3 of this series... with rdma_cma.h included. :)
How about the restrack/nldev code just translates the port space from enum rdma_port_space to a new ABI enum, say nldev_rdma_port_space, that i add to rdma_netlink.h? I'd hate to open the can of worms of trying to split rdma_cma.h into uabi and no uabi headers. :(
If port space is already part of the ABI there isn't much reason to translate it.

Quoted 8 lines

Quoted 2 lines
You just need to pick the right header to put it in, since it is a verbs define it doesn't belong in the netlink header.
I completely understand Steve's concerns.
I tried to do such thing (expose kernel headers) in first incarnation of rdmatool with attempt to clean IB/core as well to ensure that we won't expose anything that is not implemented. It didn't go well.

rdma-core is now using the kernel uapi/ headers natively, seems to be going OK. What problem did you face?

Jason

Leon Romanovsky

2018-03-27 15:15 UTC #21 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 46 lines · Jason Gunthorpe

Quoted 45 lines · Leon Romanovsky
Quoted 43 lines · Jason Gunthorpe
Quoted 29 lines · Steve Wise
Quoted 19 lines · Jason Gunthorpe
Quoted 18 lines · Steve Wise
Quoted 16 lines · David Ahern
Quoted 4 lines · Steve Wise
rdma/rdma.h +1 -0
index 5809f70..e55205b 100644
+++ b/rdma/rdma.h
rdma/rdma.h line 27
Quoted 10 lines
rdma/rdma.h +2 -0
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my repo.
Quoted 7 lines
Quoted 2 lines
It is provided by the rdma-core package, upon which rdma tool now depends for the rdma_port_space enum.
It is a kernel bug that enum is not in an include/uapi/rdma header
Fix it there and don't try to use rdma-core headers to get kernel ABI.
Jason
Quoted 7 lines
I wish you'd commented on this just a little sooner. I just resent v3 of this series... with rdma_cma.h included. :)
How about the restrack/nldev code just translates the port space from enum rdma_port_space to a new ABI enum, say nldev_rdma_port_space, that i add to rdma_netlink.h? I'd hate to open the can of worms of trying to split rdma_cma.h into uabi and no uabi headers. :(
If port space is already part of the ABI there isn't much reason to translate it.
You just need to pick the right header to put it in, since it is a verbs define it doesn't belong in the netlink header.

Quoted 8 lines

Quoted 5 lines
I completely understand Steve's concerns.
I tried to do such thing (expose kernel headers) in first incarnation of rdmatool with attempt to clean IB/core as well to ensure that we won't expose anything that is not implemented. It didn't go well.
rdma-core is now using the kernel uapi/ headers natively, seems to be going OK. What problem did you face?

I didn't agree to move to UAPI defines which are not implemented and not used in the kernel, so I sent small number of patches similar to those [1, 2].

Those patches were rejected.

So please don't mix Steve's need to use 3 defines with very large and painful task to expose proper UAPIs.

Thanks

[1] https://patchwork.kernel.org/patch/9901319/
[2] https://patchwork.kernel.org/patch/9901317/

Quoted 2 lines

Jason

Jason Gunthorpe

2018-03-27 15:23 UTC #22 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 56 lines · Leon Romanovsky

Quoted 46 lines · Jason Gunthorpe
Quoted 45 lines · Leon Romanovsky
Quoted 43 lines · Jason Gunthorpe
Quoted 29 lines · Steve Wise
Quoted 19 lines · Jason Gunthorpe
Quoted 18 lines · Steve Wise
Quoted 16 lines · David Ahern
Quoted 4 lines · Steve Wise
rdma/rdma.h +1 -0
index 5809f70..e55205b 100644
+++ b/rdma/rdma.h
rdma/rdma.h line 27
Quoted 10 lines
rdma/rdma.h +2 -0
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my repo.
Quoted 7 lines
Quoted 2 lines
It is provided by the rdma-core package, upon which rdma tool now depends for the rdma_port_space enum.
It is a kernel bug that enum is not in an include/uapi/rdma header
Fix it there and don't try to use rdma-core headers to get kernel ABI.
Jason
Quoted 7 lines
I wish you'd commented on this just a little sooner. I just resent v3 of this series... with rdma_cma.h included. :)
How about the restrack/nldev code just translates the port space from enum rdma_port_space to a new ABI enum, say nldev_rdma_port_space, that i add to rdma_netlink.h? I'd hate to open the can of worms of trying to split rdma_cma.h into uabi and no uabi headers. :(
If port space is already part of the ABI there isn't much reason to translate it.
You just need to pick the right header to put it in, since it is a verbs define it doesn't belong in the netlink header.
Quoted 8 lines
Quoted 5 lines
I completely understand Steve's concerns.
I tried to do such thing (expose kernel headers) in first incarnation of rdmatool with attempt to clean IB/core as well to ensure that we won't expose anything that is not implemented. It didn't go well.
rdma-core is now using the kernel uapi/ headers natively, seems to be going OK. What problem did you face?

Quoted 7 lines

I didn't agree to move to UAPI defines which are not implemented and not used in the kernel, so I sent small number of patches similar to those [1, 2].
Those patches were rejected.
So please don't mix Steve's need to use 3 defines with very large and painful task to expose proper UAPIs.

Steve can just move the 3 defines he needs to the uapi, we are doing this incrementally..

rdma_core does not define kernel ABI and it is totally wrong to use random constants from rdma_cma.h as kernel ABI.

Jason

Steve Wise

author

2018-03-27 15:45 UTC #23 lore raw

RE: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 75 lines · Leon Romanovsky

Quoted 19 lines · Jason Gunthorpe
Quoted 18 lines · Leon Romanovsky
Quoted 17 lines · Jason Gunthorpe
Quoted 16 lines · Steve Wise
Quoted 15 lines · Jason Gunthorpe
Quoted 13 lines · Steve Wise
Quoted 12 lines · David Ahern
Quoted 10 lines · Steve Wise
Quoted 9 lines
rdma/rdma.h +2 -0
index 5809f70..e55205b 100644
+++ b/rdma/rdma.h
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
rdma/rdma.h line 27
Quoted 5 lines
rdma/rdma.h +1 -0
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my
repo.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
It is provided by the rdma-core package, upon which rdma tool
now
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
depends for the rdma_port_space enum.
It is a kernel bug that enum is not in an include/uapi/rdma
header
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Fix it there and don't try to use rdma-core headers to get kernel
ABI.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
Jason
I wish you'd commented on this just a little sooner. I just resent
v3
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
of this series... with rdma_cma.h included. :)
How about the restrack/nldev code just translates the port space
from
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
enum rdma_port_space to a new ABI enum, say
nldev_rdma_port_space, that
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
i add to rdma_netlink.h? I'd hate to open the can of worms of
trying to
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
split rdma_cma.h into uabi and no uabi headers. :(
If port space is already part of the ABI there isn't much reason to translate it.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 2 lines
You just need to pick the right header to put it in, since it is a verbs define it doesn't belong in the netlink header.
I completely understand Steve's concerns.
I tried to do such thing (expose kernel headers) in first incarnation of rdmatool with attempt to clean IB/core as well to ensure that we
won't expose
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
anything that is not implemented. It didn't go well.
rdma-core is now using the kernel uapi/ headers natively, seems to be going OK. What problem did you face?
I didn't agree to move to UAPI defines which are not implemented and
not
Quoted 1 line
used in the kernel, so I sent small number of patches similar to those [1,
2].
Quoted 3 lines
Those patches were rejected.

Quoted 9 lines

Quoted 2 lines
So please don't mix Steve's need to use 3 defines with very large and painful task to expose proper UAPIs.
Steve can just move the 3 defines he needs to the uapi, we are doing this incrementally..
rdma_core does not define kernel ABI and it is totally wrong to use random constants from rdma_cma.h as kernel ABI.

Proposal:

Since the cm_id port space is part of the rdma_ucm_create_id struct in include/uapi/rdma/rdma_user_cm.h, I'll move the rdma_port_space enum there. And then my iproute2 series will have to add a copy of rdma_user_cm.h locally into rdma/include/uapi/rdma, right?

Will that work for everyone?

Steve.

Leon Romanovsky

2018-03-27 16:01 UTC #24 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 85 lines · Steve Wise

Quoted 75 lines · Leon Romanovsky
Quoted 19 lines · Jason Gunthorpe
Quoted 18 lines · Leon Romanovsky
Quoted 17 lines · Jason Gunthorpe
Quoted 16 lines · Steve Wise
Quoted 15 lines · Jason Gunthorpe
Quoted 13 lines · Steve Wise
Quoted 12 lines · David Ahern
Quoted 10 lines · Steve Wise
Quoted 9 lines
rdma/rdma.h +2 -0
index 5809f70..e55205b 100644
+++ b/rdma/rdma.h
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
rdma/rdma.h line 27
Quoted 5 lines
rdma/rdma.h +1 -0
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file in my
repo.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
It is provided by the rdma-core package, upon which rdma tool
now
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
depends for the rdma_port_space enum.
It is a kernel bug that enum is not in an include/uapi/rdma
header
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Fix it there and don't try to use rdma-core headers to get kernel
ABI.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
Jason
I wish you'd commented on this just a little sooner. I just resent
v3
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
of this series... with rdma_cma.h included. :)
How about the restrack/nldev code just translates the port space
from
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
enum rdma_port_space to a new ABI enum, say
nldev_rdma_port_space, that
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
i add to rdma_netlink.h? I'd hate to open the can of worms of
trying to
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
split rdma_cma.h into uabi and no uabi headers. :(
If port space is already part of the ABI there isn't much reason to translate it.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 2 lines
You just need to pick the right header to put it in, since it is a verbs define it doesn't belong in the netlink header.
I completely understand Steve's concerns.
I tried to do such thing (expose kernel headers) in first incarnation of rdmatool with attempt to clean IB/core as well to ensure that we
won't expose
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
anything that is not implemented. It didn't go well.
rdma-core is now using the kernel uapi/ headers natively, seems to be going OK. What problem did you face?
I didn't agree to move to UAPI defines which are not implemented and
not
Quoted 1 line
used in the kernel, so I sent small number of patches similar to those [1,
2].
Quoted 3 lines
Those patches were rejected.
Quoted 9 lines
Quoted 2 lines
So please don't mix Steve's need to use 3 defines with very large and painful task to expose proper UAPIs.
Steve can just move the 3 defines he needs to the uapi, we are doing this incrementally..
rdma_core does not define kernel ABI and it is totally wrong to use random constants from rdma_cma.h as kernel ABI.

Quoted 5 lines

Proposal:
Since the cm_id port space is part of the rdma_ucm_create_id struct in include/uapi/rdma/rdma_user_cm.h, I'll move the rdma_port_space enum there. And then my iproute2 series will have to add a copy of rdma_user_cm.h locally into rdma/include/uapi/rdma, right?
Will that work for everyone?

You need to remove _PS from that structure and from the kernel with justification that it is safe to do.

Thanks

Quoted 3 lines

Steve.

Steve Wise

author

2018-03-27 16:20 UTC #25 lore raw

RE: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 26 lines · Steve Wise

Quoted 5 lines
Quoted 5 lines · Leon Romanovsky
Quoted 2 lines · Jason Gunthorpe
Quoted 1 line
On Tue, Mar 27, 2018 at 06:21:41AM +0300, Leon Romanovsky
Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
On Mon, Mar 26, 2018 at 04:30:33PM -0600, Jason Gunthorpe
Quoted 4 lines · wrote:
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines · Steve Wise
Quoted 3 lines · Jason Gunthorpe
Quoted 1 line
On Mon, Mar 26, 2018 at 09:30:41AM -0500, Steve Wise
Quoted 12 lines · wrote:
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines · David Ahern
Quoted 10 lines · Steve Wise
Quoted 9 lines
rdma/rdma.h +2 -0
index 5809f70..e55205b 100644
+++ b/rdma/rdma.h
@@ -18,10 +18,12 @@
 #include <libmnl/libmnl.h>
 #include <rdma/rdma_netlink.h>
 #include <time.h>
+#include <net/if_arp.h>
 

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
rdma/rdma.h line 27
Quoted 5 lines
rdma/rdma.h +1 -0
 #include "list.h"
 #include "utils.h"
 #include "json_writer.h"
+#include <rdma/rdma_cma.h>
 
did you forget to add rdma_cma.h? I don't see that file

Quoted 7 lines

my
Quoted 2 lines
Quoted 2 lines
repo.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
It is provided by the rdma-core package, upon which rdma
tool
Quoted 3 lines
Quoted 3 lines
now
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
depends for the rdma_port_space enum.
It is a kernel bug that enum is not in an

include/uapi/rdma

Quoted 8 lines

Quoted 3 lines
Quoted 3 lines
header
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Fix it there and don't try to use rdma-core headers to get
kernel
Quoted 4 lines
Quoted 4 lines
ABI.
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 2 lines
Jason

Quoted 11 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
I wish you'd commented on this just a little sooner. I just
resent
Quoted 4 lines
Quoted 4 lines
v3
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
of this series... with rdma_cma.h included. :)
How about the restrack/nldev code just translates the port
space
Quoted 4 lines
Quoted 4 lines
from
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
enum rdma_port_space to a new ABI enum, say
nldev_rdma_port_space, that
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
i add to rdma_netlink.h? I'd hate to open the can of worms

Quoted 4 lines

Quoted 4 lines
Quoted 4 lines
trying to
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
split rdma_cma.h into uabi and no uabi headers. :(
If port space is already part of the ABI there isn't much

reason to

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
translate it.
You just need to pick the right header to put it in, since it

is a

Quoted 6 lines

verbs
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
define it doesn't belong in the netlink header.
I completely understand Steve's concerns.
I tried to do such thing (expose kernel headers) in first

incarnation

Quoted 12 lines

of
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
rdmatool with attempt to clean IB/core as well to ensure that we
won't expose
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
anything that is not implemented. It didn't go well.
rdma-core is now using the kernel uapi/ headers natively, seems to
be
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
going OK. What problem did you face?
I didn't agree to move to UAPI defines which are not implemented and
not
Quoted 1 line
used in the kernel, so I sent small number of patches similar to

those

Quoted 6 lines

[1,
Quoted 5 lines
Quoted 5 lines
2].
Quoted 4 lines
Those patches were rejected.
So please don't mix Steve's need to use 3 defines with very large

and

Quoted 16 lines

Quoted 5 lines
Quoted 5 lines
Quoted 1 line
painful task to expose proper UAPIs.
Steve can just move the 3 defines he needs to the uapi, we are doing this incrementally..
Quoted 7 lines
Quoted 3 lines
rdma_core does not define kernel ABI and it is totally wrong to use random constants from rdma_cma.h as kernel ABI.
Proposal:
Since the cm_id port space is part of the rdma_ucm_create_id struct in
include/uapi/rdma/rdma_user_cm.h, I'll move the rdma_port_space enum there. And then my iproute2 series will have to add a copy of rdma_user_cm.h locally into rdma/include/uapi/rdma, right?
Quoted 1 line

Quoted 6 lines

Quoted 1 line
Will that work for everyone?
You need to remove _PS from that structure and from the kernel with justification that it is safe to do.
Thanks

I'm pretty sure port space is needed. That struct is used to create a user mode cm_id...

Leon Romanovsky

2018-03-27 16:30 UTC #26 lore raw

Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 115 lines · Steve Wise

Quoted 26 lines · Steve Wise
Quoted 5 lines
Quoted 5 lines · Leon Romanovsky
Quoted 2 lines · Jason Gunthorpe
Quoted 1 line
On Tue, Mar 27, 2018 at 06:21:41AM +0300, Leon Romanovsky
Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
On Mon, Mar 26, 2018 at 04:30:33PM -0600, Jason Gunthorpe
Quoted 4 lines · wrote:
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines · Steve Wise
Quoted 3 lines · Jason Gunthorpe
Quoted 1 line
On Mon, Mar 26, 2018 at 09:30:41AM -0500, Steve Wise
Quoted 12 lines · wrote:
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines · David Ahern
Quoted 10 lines · Steve Wise
Quoted 9 lines
diff --git a/rdma/rdma.h b/rdma/rdma.h
index 5809f70..e55205b 100644
+++ b/rdma/rdma.h
@@ -18,10 +18,12 @@
#include <libmnl/libmnl.h>
#include <rdma/rdma_netlink.h>
#include <time.h>
+#include <net/if_arp.h>
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
#include "list.h"
#include "utils.h"
#include "json_writer.h"
+#include <rdma/rdma_cma.h>
did you forget to add rdma_cma.h? I don't see that file
in
Quoted 7 lines
my
Quoted 2 lines
Quoted 2 lines
repo.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
It is provided by the rdma-core package, upon which rdma
tool
Quoted 3 lines
Quoted 3 lines
now
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
depends for the rdma_port_space enum.
It is a kernel bug that enum is not in an
include/uapi/rdma
Quoted 8 lines
Quoted 3 lines
Quoted 3 lines
header
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Fix it there and don't try to use rdma-core headers to get
kernel
Quoted 4 lines
Quoted 4 lines
ABI.
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 2 lines
Jason
Quoted 11 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
I wish you'd commented on this just a little sooner. I just
resent
Quoted 4 lines
Quoted 4 lines
v3
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
of this series... with rdma_cma.h included. :)
How about the restrack/nldev code just translates the port
space
Quoted 4 lines
Quoted 4 lines
from
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
enum rdma_port_space to a new ABI enum, say
nldev_rdma_port_space, that
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
i add to rdma_netlink.h? I'd hate to open the can of worms
of
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
trying to
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
split rdma_cma.h into uabi and no uabi headers. :(
If port space is already part of the ABI there isn't much
reason to
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
translate it.
You just need to pick the right header to put it in, since it
is a
Quoted 6 lines
verbs
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
define it doesn't belong in the netlink header.
I completely understand Steve's concerns.
I tried to do such thing (expose kernel headers) in first
incarnation
Quoted 12 lines
of
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
rdmatool with attempt to clean IB/core as well to ensure that we
won't expose
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
anything that is not implemented. It didn't go well.
rdma-core is now using the kernel uapi/ headers natively, seems to
be
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
going OK. What problem did you face?
I didn't agree to move to UAPI defines which are not implemented and
not
Quoted 1 line
used in the kernel, so I sent small number of patches similar to
those
Quoted 6 lines
[1,
Quoted 5 lines
Quoted 5 lines
2].
Quoted 4 lines
Those patches were rejected.
So please don't mix Steve's need to use 3 defines with very large
and
Quoted 18 lines
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
painful task to expose proper UAPIs.
Steve can just move the 3 defines he needs to the uapi, we are doing this incrementally..
Quoted 7 lines
Quoted 3 lines
rdma_core does not define kernel ABI and it is totally wrong to use random constants from rdma_cma.h as kernel ABI.
Proposal:
Since the cm_id port space is part of the rdma_ucm_create_id struct in
include/uapi/rdma/rdma_user_cm.h, I'll move the rdma_port_space enum there. And then my iproute2 series will have to add a copy of rdma_user_cm.h locally into rdma/include/uapi/rdma, right?
Quoted 2 lines
Will that work for everyone?

Quoted 7 lines

Quoted 4 lines
You need to remove _PS from that structure and from the kernel with justification that it is safe to do.
Thanks
I'm pretty sure port space is needed. That struct is used to create a user mode cm_id...

Sorry, it is RDMA_PS_SDP.

Thanks

Quoted 3 lines

Steve Wise

author

2018-03-27 16:38 UTC #27 lore raw

RE: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information

Quoted 30 lines · Steve Wise

Quoted 5 lines
Quoted 5 lines · Steve Wise
Quoted 4 lines
Quoted 4 lines · Leon Romanovsky
Quoted 1 line
On Tue, Mar 27, 2018 at 08:44:55AM -0600, Jason Gunthorpe
Quoted 5 lines · wrote:
Quoted 5 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
On Tue, Mar 27, 2018 at 06:21:41AM +0300, Leon Romanovsky
Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
On Mon, Mar 26, 2018 at 04:30:33PM -0600, Jason Gunthorpe
Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
On Mon, Mar 26, 2018 at 04:34:44PM -0500, Steve Wise
Quoted 16 lines · wrote:
Quoted 16 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines · Jason Gunthorpe
Quoted 1 line
On Mon, Mar 26, 2018 at 09:30:41AM -0500, Steve Wise
Quoted 12 lines · wrote:
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines · David Ahern
Quoted 10 lines · Steve Wise
diff --git a/rdma/rdma.h b/rdma/rdma.h
index 5809f70..e55205b 100644
+++ b/rdma/rdma.h
@@ -18,10 +18,12 @@
#include <libmnl/libmnl.h>
#include <rdma/rdma_netlink.h>
#include <time.h>
+#include <net/if_arp.h>

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
#include "list.h"
#include "utils.h"
#include "json_writer.h"
+#include <rdma/rdma_cma.h>
did you forget to add rdma_cma.h? I don't see that

file

Quoted 13 lines

Quoted 4 lines
in
Quoted 3 lines
my
Quoted 2 lines
Quoted 2 lines
repo.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
It is provided by the rdma-core package, upon which
rdma
Quoted 8 lines
Quoted 4 lines
tool
Quoted 3 lines
Quoted 3 lines
now
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
depends for the rdma_port_space enum.
It is a kernel bug that enum is not in an
include/uapi/rdma
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
header
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Fix it there and don't try to use rdma-core headers to

get

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
kernel
Quoted 5 lines
Quoted 5 lines
ABI.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
Jason
I wish you'd commented on this just a little sooner. I

just

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
resent
Quoted 4 lines
Quoted 4 lines
v3
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
of this series... with rdma_cma.h included. :)
How about the restrack/nldev code just translates the

port

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
space
Quoted 4 lines
Quoted 4 lines
from
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
enum rdma_port_space to a new ABI enum, say
nldev_rdma_port_space, that
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
i add to rdma_netlink.h? I'd hate to open the can of

worms

Quoted 9 lines

Quoted 9 lines
of
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
trying to
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
split rdma_cma.h into uabi and no uabi headers. :(
If port space is already part of the ABI there isn't much
reason to
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
translate it.
You just need to pick the right header to put it in, since

Quoted 10 lines

Quoted 10 lines
is a
Quoted 6 lines
verbs
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
define it doesn't belong in the netlink header.
I completely understand Steve's concerns.
I tried to do such thing (expose kernel headers) in first
incarnation
Quoted 2 lines
of
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
rdmatool with attempt to clean IB/core as well to ensure

that

Quoted 5 lines

we
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
won't expose
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
anything that is not implemented. It didn't go well.
rdma-core is now using the kernel uapi/ headers natively,

seems

Quoted 15 lines

to
Quoted 4 lines
Quoted 4 lines
be
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
going OK. What problem did you face?
I didn't agree to move to UAPI defines which are not implemented
and
Quoted 9 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
not
Quoted 1 line
used in the kernel, so I sent small number of patches similar to
those
Quoted 6 lines
[1,
Quoted 5 lines
Quoted 5 lines
2].
Quoted 4 lines
Those patches were rejected.
So please don't mix Steve's need to use 3 defines with very

large

Quoted 4 lines

Quoted 4 lines
and
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
painful task to expose proper UAPIs.
Steve can just move the 3 defines he needs to the uapi, we are

doing

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
this incrementally..
rdma_core does not define kernel ABI and it is totally wrong to

use

Quoted 20 lines

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 2 lines
random constants from rdma_cma.h as kernel ABI.
Proposal:
Since the cm_id port space is part of the rdma_ucm_create_id struct
in
Quoted 1 line
Quoted 1 line
include/uapi/rdma/rdma_user_cm.h, I'll move the rdma_port_space
enum
Quoted 11 lines
Quoted 9 lines
there. And then my iproute2 series will have to add a copy of rdma_user_cm.h locally into rdma/include/uapi/rdma, right?
Quoted 2 lines
Will that work for everyone?
You need to remove _PS from that structure and from the kernel with justification that it is safe to do.
Thanks
I'm pretty sure port space is needed. That struct is used to create a

user

Quoted 5 lines

Quoted 1 line
mode cm_id...
Sorry, it is RDMA_PS_SDP.
Thanks

Right. Will do.