RDMA Netlink Device Client

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

Leon Romanovsky

author

2017-06-21 06:05 UTC #5 lore raw

PATCH 4/19 RDMA/netlink: Rename and remove redundant parameter from ibnl_multicast

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

The pointer to netlink header was not used in the ibnl_multicast function, so let's remove it and simplify the function signature.

Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

---
 drivers/infiniband/core/addr.c     | 2 +-
 drivers/infiniband/core/iwpm_msg.c | 2 +-
 drivers/infiniband/core/netlink.c  | 5 ++---
 drivers/infiniband/core/sa_query.c | 2 +-
 include/rdma/rdma_netlink.h        | 4 +---
 5 files changed, 6 insertions(+), 9 deletions(-)

drivers/infiniband/core/addr.c +1 -1

index 134d8394fca5..ebd0242bab3d 100644
@@ -185,7 +185,7 @@ static int ib_nl_ip_send_msg(struct rdma_dev_addr *dev_addr,
 
 	/* Repair the nlmsg header length */
 	nlmsg_end(skb, nlh);
-	ibnl_multicast(skb, nlh, RDMA_NL_GROUP_LS, GFP_KERNEL);
+	rdma_nl_multicast(skb, RDMA_NL_GROUP_LS, GFP_KERNEL);
 
 	/* Make the request retry, so when we get the response from userspace
 	 * we will have something.

drivers/infiniband/core/iwpm_msg.c +1 -1

index 8f84557d04e3..561f312ea35a 100644
@@ -103,7 +103,7 @@ int iwpm_register_pid(struct iwpm_dev_data *pm_msg, u8 nl_client)
 	pr_debug("%s: Multicasting a nlmsg (dev = %s ifname = %s iwpm = %s)\n",
 		__func__, pm_msg->dev_name, pm_msg->if_name, iwpm_ulib_name);
 
-	ret = ibnl_multicast(skb, nlh, RDMA_NL_GROUP_IWPM, GFP_KERNEL);
+	ret = rdma_nl_multicast(skb, RDMA_NL_GROUP_IWPM, GFP_KERNEL);
 	if (ret) {
 		skb = NULL; /* skb is freed in the netlink send-op handling */
 		iwpm_user_pid = IWPM_PID_UNAVAILABLE;

drivers/infiniband/core/netlink.c +2 -3

index cd29311078b5..89a6219b6ec8 100644
@@ -247,12 +247,11 @@ int rdma_nl_unicast(struct sk_buff *skb, u32 pid)
 }
 EXPORT_SYMBOL(rdma_nl_unicast);
 
-int ibnl_multicast(struct sk_buff *skb, struct nlmsghdr *nlh,
-			unsigned int group, gfp_t flags)
+int rdma_nl_multicast(struct sk_buff *skb, unsigned int group, gfp_t flags)
 {
 	return nlmsg_multicast(nls, skb, 0, group, flags);
 }
-EXPORT_SYMBOL(ibnl_multicast);
+EXPORT_SYMBOL(rdma_nl_multicast);
 
 int __init rdma_nl_init(void)
 {

drivers/infiniband/core/sa_query.c +1 -1

index 6e39a763b220..d890600f1e2d 100644
@@ -862,7 +862,7 @@ static int ib_nl_send_msg(struct ib_sa_query *query, gfp_t gfp_mask)
 	/* Repair the nlmsg header length */
 	nlmsg_end(skb, nlh);
 
-	ret = ibnl_multicast(skb, nlh, RDMA_NL_GROUP_LS, gfp_mask);
+	ret = rdma_nl_multicast(skb, RDMA_NL_GROUP_LS, gfp_mask);
 	if (!ret)
 		ret = len;
 	else

include/rdma/rdma_netlink.h +1 -3

index 9779cd5520d7..145283896417 100644
@@ -67,13 +67,11 @@ int rdma_nl_unicast(struct sk_buff *skb, u32 pid);
 /**
  * Send the supplied skb to a netlink group.
  * @skb: The netlink skb
- * @nlh: Header of the netlink message to send
  * @group: Netlink group ID
  * @flags: allocation flags
  * Returns 0 on success or a negative error code.
  */
-int ibnl_multicast(struct sk_buff *skb, struct nlmsghdr *nlh,
-			unsigned int group, gfp_t flags);
+int rdma_nl_multicast(struct sk_buff *skb, unsigned int group, gfp_t flags);
 
 /**
  * Check if there are any listeners to the netlink group

Signature · 6 lines

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

A message that is not in the archive (sent off-list or not yet received): 20170621060528.3752-5-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Steve Wise

Reviewed-by

2017-06-21 14:03 UTC #7 lore raw

RE: [PATCH rdma-next 04/19] RDMA/netlink: Rename and remove redundant parameter from ibnl_multicast

Quoted 7 lines

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The pointer to netlink header was not used in the ibnl_multicast function, so let's remove it and simplify the function signature.
Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

Looks good.

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

Leon Romanovsky

author

2017-06-21 06:05 UTC #8 lore raw

PATCH 6/19 RDMA/netlink: Rename netlink callback struct

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

The RDMA netlink client infrastructure was removed and made obsolete the old name (ibnl_client_cbs). This patch renames to the more appropriate name (rdma_nl_cbs).

Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

---
 drivers/infiniband/core/cma.c     | 2 +-
 drivers/infiniband/core/device.c  | 2 +-
 drivers/infiniband/core/iwcm.c    | 2 +-
 drivers/infiniband/core/netlink.c | 4 ++--
 include/rdma/rdma_netlink.h       | 4 ++--
 5 files changed, 7 insertions(+), 7 deletions(-)

drivers/infiniband/core/cma.c +1 -1

index a4013b0908e2..2af30a19b926 100644
@@ -4477,7 +4477,7 @@ static int cma_get_id_stats(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
-static const struct ibnl_client_cbs cma_cb_table[] = {
+static const struct rdma_nl_cbs cma_cb_table[] = {
 	[RDMA_NL_RDMA_CM_ID_STATS] = { .dump = cma_get_id_stats},
 };

drivers/infiniband/core/device.c +1 -1

index 2001dabd1444..5326dcc6ede7 100644
@@ -1008,7 +1008,7 @@ struct net_device *ib_get_net_dev_by_params(struct ib_device *dev,
 }
 EXPORT_SYMBOL(ib_get_net_dev_by_params);
 
-static const struct ibnl_client_cbs ibnl_ls_cb_table[] = {
+static const struct rdma_nl_cbs ibnl_ls_cb_table[] = {
 	[RDMA_NL_LS_OP_RESOLVE] = {
 		.dump = ib_nl_handle_resolve_resp,
 		.flags = RDMA_NL_ADMIN_PERM,

drivers/infiniband/core/iwcm.c +1 -1

index 8599271d8be6..452a3115e3e6 100644
@@ -80,7 +80,7 @@ const char *__attribute_const__ iwcm_reject_msg(int reason)
 }
 EXPORT_SYMBOL(iwcm_reject_msg);
 
-static struct ibnl_client_cbs iwcm_nl_cb_table[] = {
+static struct rdma_nl_cbs iwcm_nl_cb_table[] = {
 	[RDMA_NL_IWPM_REG_PID] = {.dump = iwpm_register_pid_cb},
 	[RDMA_NL_IWPM_ADD_MAPPING] = {.dump = iwpm_add_mapping_cb},
 	[RDMA_NL_IWPM_QUERY_MAPPING] = {.dump = iwpm_add_and_query_mapping_cb},

drivers/infiniband/core/netlink.c +2 -2

index 1022ce6628ae..14b64e4d1e06 100644
@@ -43,7 +43,7 @@
 static DEFINE_MUTEX(rdma_nl_mutex);
 static struct sock *nls;
 static struct {
-	const struct ibnl_client_cbs   *cb_table;
+	const struct rdma_nl_cbs   *cb_table;
 } rdma_nl_types[RDMA_NL_NUM_CLIENTS];
 
 int rdma_nl_chk_listeners(unsigned int group)

@@ -83,7 +83,7 @@ static bool is_nl_valid(unsigned int type, unsigned int op)
 }
 
 void rdma_nl_register(unsigned int index,
-		      const struct ibnl_client_cbs cb_table[])
+		      const struct rdma_nl_cbs cb_table[])
 {
 	mutex_lock(&rdma_nl_mutex);
 	if (!is_nl_msg_valid(index, 0)) {

include/rdma/rdma_netlink.h +2 -2

index b39e030b3a64..8feeb899e2b2 100644
@@ -5,7 +5,7 @@
 #include <linux/netlink.h>
 #include <uapi/rdma/rdma_netlink.h>
 
-struct ibnl_client_cbs {
+struct rdma_nl_cbs {
 	int (*dump)(struct sk_buff *skb, struct netlink_callback *nlcb);
 	u8 flags;
 };

@@ -24,7 +24,7 @@ void rdma_nl_exit(void);
  * @cb_table: A table for op->callback
  */
 void rdma_nl_register(unsigned int index,
-		      const struct ibnl_client_cbs cb_table[]);
+		      const struct rdma_nl_cbs cb_table[]);
 
 /**
  * Remove a client from IB netlink.

Signature · 6 lines

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

Steve Wise

Reviewed-by on v1 6/19

2017-06-21 14:04 UTC #9 lore raw

RE: [PATCH rdma-next 06/19] RDMA/netlink: Rename netlink callback struct

Quoted 7 lines

From: Leon Romanovsky <[email protected]>
The RDMA netlink client infrastructure was removed and made obsolete the old name (ibnl_client_cbs). This patch renames to the more appropriate name (rdma_nl_cbs).
Signed-off-by Leon Romanovsky <[email protected]>

Looks good.

Leon Romanovsky

author

2017-06-21 06:05 UTC #10 lore raw

PATCH 9/19 RDMA/netlink: Add and implement doit netlink callback

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

The .doit callback is used by netlink core to differentiate between get and set operations. Common convention is to use that call for command operations like (SET, ADD, e.t.c.) and/or access without NLF_M_DUMP flag.

This commit adds proper declaration and implementation to RDMA netlink.

Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

---
 drivers/infiniband/core/netlink.c | 19 ++++++++++++++-----
 include/rdma/rdma_netlink.h       |  2 ++
 2 files changed, 16 insertions(+), 5 deletions(-)

drivers/infiniband/core/netlink.c +14 -5

index 14b64e4d1e06..34f529cc9776 100644
@@ -75,9 +75,13 @@ static bool is_nl_msg_valid(unsigned int type, unsigned int op)
 
 static bool is_nl_valid(unsigned int type, unsigned int op)
 {
-	if (!is_nl_msg_valid(type, op) ||
-	    !rdma_nl_types[type].cb_table ||
-	    !rdma_nl_types[type].cb_table[op].dump)
+	const struct rdma_nl_cbs *cb_table;
+
+	if (!is_nl_msg_valid(type, op))
+		return false;
+
+	cb_table = rdma_nl_types[type].cb_table;
+	if (!cb_table || (!cb_table[op].dump && !cb_table[op].doit))
 		return false;
 	return true;
 }

@@ -152,6 +156,7 @@ static int rdma_nl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 	unsigned int op = RDMA_NL_GET_OP(type);
 	struct netlink_callback cb = {};
 	struct netlink_dump_control c = {};
+	int ret;
 
 	if (!is_nl_valid(index, op))
 		return -EINVAL;

@@ -170,10 +175,14 @@ static int rdma_nl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 		cb.nlh = nlh;
 		cb.dump = rdma_nl_types[index].cb_table[op].dump;
 		return cb.dump(skb, &cb);
+	} else {
+		c.dump = rdma_nl_types[index].cb_table[op].dump;
+		return netlink_dump_start(nls, skb, nlh, &c);
 	}
+	if (rdma_nl_types[index].cb_table[op].doit)
+		ret = rdma_nl_types[index].cb_table[op].doit(skb, nlh, extack);
+	return ret;
 
-	c.dump = rdma_nl_types[index].cb_table[op].dump;
-	return netlink_dump_start(nls, skb, nlh, &c);
 }
 
 /*

include/rdma/rdma_netlink.h +2 -0

index 8feeb899e2b2..d6a481880f41 100644
@@ -6,6 +6,8 @@
 #include <uapi/rdma/rdma_netlink.h>
 
 struct rdma_nl_cbs {
+	int (*doit)(struct sk_buff *skb, struct nlmsghdr *nlh,
+		    struct netlink_ext_ack *extack);
 	int (*dump)(struct sk_buff *skb, struct netlink_callback *nlcb);
 	u8 flags;
 };

Signature · 6 lines

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

Steve Wise

Reviewed-by on v1 9/19

2017-06-21 14:11 UTC #11 lore raw

RE: [PATCH rdma-next 09/19] RDMA/netlink: Add and implement doit netlink callback

Quoted 11 lines

From: Leon Romanovsky <[email protected]>
The .doit callback is used by netlink core to differentiate between get and set operations. Common convention is to use that call for command operations like (SET, ADD, e.t.c.) and/or access without NLF_M_DUMP flag.
This commit adds proper declaration and implementation to RDMA netlink.
Signed-off-by Leon Romanovsky <[email protected]>

Looks good.

Leon Romanovsky

author

2017-06-21 06:05 UTC #12 lore raw

PATCH 13/19 RDMA/netlink: Add netlink device definitions to UAPI

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

Introduce new defines to rdma_netlink.h, so the RDMA configuration tool will be able to communicate with RDMA subsystem by using the shared defines.

The addition of new client (NLDEV) revealed the fact that we exposed by mistake the RDMA_NL_I40IW define which is not backed by any RDMA netlink by now and it won't be exposed in the future too. So this patch reuses the value and leaves the comment together with old definition to whose who are using RDMA_NL_I40IW as a replacement for digit "5".

The NLDEV operates with objects. The struct ib_device has two straightforward
objects: device itself and ports of that device.

This brings us to propose the following commands to work on those objects:
* RDMA_NLDEV_CMD_{GET,SET,NEW,DEL} - works on ib_device itself
* RDMA_NLDEV_CMD_PORT_{GET,SET,NEW,DEL} - works on ports of specific ib_device

Those commands receive/return the device name (RDMA_NLDEV_ATTR_DEV_NAME) and port index (RDMA_NLDEV_ATTR_PORT_INDEX). For device object accesses, the RDMA_NLDEV_ATTR_PORT_INDEX will return the maximum number of ports for specific ib_device and for port access the actual port index.

The port index starts from 1 to follow RDMA/core internal semantics and the sysfs exposed knobs..

Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

---
 drivers/infiniband/core/netlink.c |  2 +-
 include/uapi/rdma/rdma_netlink.h  | 42 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

drivers/infiniband/core/netlink.c +1 -1

index 9731c313e9b9..cce9b7af4a3b 100644
@@ -60,7 +60,7 @@ static bool is_nl_msg_valid(unsigned int type, unsigned int op)
 				  RDMA_NL_IWPM_NUM_OPS,
 				  0,
 				  RDMA_NL_LS_NUM_OPS,
-				  0 };
+				  RDMA_NLDEV_NUM_OPS };
 
 	/*
 	 * This BUILD_BUG_ON is intended to catch addition of new

include/uapi/rdma/rdma_netlink.h +42 -0

53 lines not on this page · raw message

Signature · 6 lines

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

Steve Wise

Reviewed-by on v1 13/19

2017-06-21 14:21 UTC #13 lore raw

RE: [PATCH rdma-next 13/19] RDMA/netlink: Add netlink device definitions to UAPI

Quoted 25 lines

From: Leon Romanovsky <[email protected]>
Introduce new defines to rdma_netlink.h, so the RDMA configuration tool will be able to communicate with RDMA subsystem by using the shared defines.
The addition of new client (NLDEV) revealed the fact that we exposed by mistake the RDMA_NL_I40IW define which is not backed by any RDMA netlink by now and it won't be exposed in the future too. So this patch reuses the value and leaves the comment together with old definition to whose who are using RDMA_NL_I40IW as a replacement for digit "5".
The NLDEV operates with objects. The struct ib_device has two straightforward
objects: device itself and ports of that device.
This brings us to propose the following commands to work on those objects:
* RDMA_NLDEV_CMD_{GET,SET,NEW,DEL} - works on ib_device itself
* RDMA_NLDEV_CMD_PORT_{GET,SET,NEW,DEL} - works on ports of specific
ib_device
Those commands receive/return the device name (RDMA_NLDEV_ATTR_DEV_NAME)
and port index (RDMA_NLDEV_ATTR_PORT_INDEX). For device object accesses, the RDMA_NLDEV_ATTR_PORT_INDEX will return the maximum number of ports for specific ib_device and for port access the actual port index.

Quoted 4 lines

The port index starts from 1 to follow RDMA/core internal semantics and the sysfs exposed knobs..
Signed-off-by Leon Romanovsky <[email protected]>

Jason Gunthorpe

2017-06-21 16:11 UTC #14 lore raw

Re: [PATCH rdma-next 13/19] RDMA/netlink: Add netlink device definitions to UAPI

include/uapi/rdma/rdma_netlink.h enum { · line 17

Quoted 10 lines · Leon Romanovsky

include/uapi/rdma/rdma_netlink.h +6 -0

 	RDMA_NL_IWCM,
 	RDMA_NL_RSVD,
 	RDMA_NL_LS,	/* RDMA Local Services */
+	/*
+	 * RDMA_NL_I40IW not in use and it was added here by mistake,
+	 * but we need to keep it anyway, because it is UAPI and for
+	 * unknown reasons, someone in the field decided to replace "5"
+	 * with this define.
+	 */
 	RDMA_NL_I40IW,

Since this doesn't do anything, never did anything, I think you should just delete it. Anything in userspace that refers to this name is unconditionally broken and a compile failure is the better choice.

Jason

A message that is not in the archive (sent off-list or not yet received): 20170621161149.GB10200-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org

Tung, Chien Tin

2017-06-21 16:18 UTC #16 lore raw

RE: [PATCH rdma-next 13/19] RDMA/netlink: Add netlink device definitions to UAPI

Quoted 28 lines

From: Jason Gunthorpe [mailto:jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org]
Sent: Wednesday, June 21, 2017 11:12 AM
To: Leon Romanovsky <[email protected]>
Cc: Doug Ledford <[email protected]>; [email protected]; Tung, Chien Tin <[email protected]>; Steve Wise <[email protected]>; Stephen Hemminger <[email protected]>; Jiri Pirko <jiri-VPRAkNaXOzVWk0Htik3J/[email protected]>; Ariel Almog <ariela-VPRAkNaXOzVWk0Htik3J/[email protected]>; Linux Netdev <[email protected]>; Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Subject: Re: [PATCH rdma-next 13/19] RDMA/netlink: Add netlink device
definitions to UAPI
include/uapi/rdma/rdma_netlink.h enum { · line 17
Quoted 10 lines · Leon Romanovsky
include/uapi/rdma/rdma_netlink.h +6 -0
 	RDMA_NL_IWCM,
 	RDMA_NL_RSVD,
 	RDMA_NL_LS,	/* RDMA Local Services */
+	/*
+	 * RDMA_NL_I40IW not in use and it was added here by mistake,
+	 * but we need to keep it anyway, because it is UAPI and for
+	 * unknown reasons, someone in the field decided to replace "5"
+	 * with this define.
+	 */
 	RDMA_NL_I40IW,
Since this doesn't do anything, never did anything, I think you should just delete it. Anything in userspace that refers to this name is unconditionally broken and a compile failure is the better choice.

Quoted 1 line

Jason

I agree. Please delete RDMA_NL_I40IW.

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

A message that is not in the archive (sent off-list or not yet received): 748B799B6A00724488C603FD7E5E7EB976FA5C5C-XfjTATA9Em864kNsxIetb7fspsVTdybXVpNB7YpNyf8@public.gmane.org

Leon Romanovsky

author

2017-06-22 04:28 UTC #18 lore raw

Re: [PATCH rdma-next 13/19] RDMA/netlink: Add netlink device definitions to UAPI

Quoted 28 lines · Chien Tin

Quoted 28 lines
From: Jason Gunthorpe [mailto:jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org]
Sent: Wednesday, June 21, 2017 11:12 AM
To: Leon Romanovsky <[email protected]>
Cc: Doug Ledford <[email protected]>; [email protected]; Tung, Chien Tin <[email protected]>; Steve Wise <[email protected]>; Stephen Hemminger <[email protected]>; Jiri Pirko <jiri-VPRAkNaXOzVWk0Htik3J/[email protected]>; Ariel Almog <ariela-VPRAkNaXOzVWk0Htik3J/[email protected]>; Linux Netdev <[email protected]>; Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Subject: Re: [PATCH rdma-next 13/19] RDMA/netlink: Add netlink device
definitions to UAPI
include/uapi/rdma/rdma_netlink.h enum { · line 17
Quoted 10 lines · Leon Romanovsky
include/uapi/rdma/rdma_netlink.h +6 -0
 	RDMA_NL_IWCM,
 	RDMA_NL_RSVD,
 	RDMA_NL_LS,	/* RDMA Local Services */
+	/*
+	 * RDMA_NL_I40IW not in use and it was added here by mistake,
+	 * but we need to keep it anyway, because it is UAPI and for
+	 * unknown reasons, someone in the field decided to replace "5"
+	 * with this define.
+	 */
 	RDMA_NL_I40IW,
Since this doesn't do anything, never did anything, I think you should just delete it. Anything in userspace that refers to this name is unconditionally broken and a compile failure is the better choice.

Quoted 3 lines

Quoted 1 line
Jason
I agree. Please delete RDMA_NL_I40IW.

Fixed and Steve's comments too.

Thanks

Quoted 2 lines

Chien

Leon Romanovsky

2017-06-21 06:05 UTC #19 lore raw

PATCH 2/19 RDMA/netlink: Simplify the put_msg and put_attr

From: Leon Romanovsky <[email protected]>

Reuse standard macros to cancel the netlink message in case of error.

---
 drivers/infiniband/core/netlink.c | 31 +++++++++----------------------
 1 file changed, 9 insertions(+), 22 deletions(-)

drivers/infiniband/core/netlink.c +9 ~~-22~~

index f0d482009c69..96057e722123 100644
@@ -126,36 +126,23 @@ EXPORT_SYMBOL(rdma_nl_unregister);
 void *ibnl_put_msg(struct sk_buff *skb, struct nlmsghdr **nlh, int seq,
 		   int len, int client, int op, int flags)
 {
-	unsigned char *prev_tail;
-
-	prev_tail = skb_tail_pointer(skb);
-	*nlh = nlmsg_put(skb, 0, seq, RDMA_NL_GET_TYPE(client, op),
-			 len, flags);
-	if (!*nlh)
-		goto out_nlmsg_trim;
-	(*nlh)->nlmsg_len = skb_tail_pointer(skb) - prev_tail;
+	*nlh = nlmsg_put(skb, 0, seq, RDMA_NL_GET_TYPE(client, op), len, flags);
+	if (!*nlh) {
+		nlmsg_cancel(skb, *nlh);
+		return NULL;
+	}
 	return nlmsg_data(*nlh);
-
-out_nlmsg_trim:
-	nlmsg_trim(skb, prev_tail);
-	return NULL;
 }
 EXPORT_SYMBOL(ibnl_put_msg);
 
 int ibnl_put_attr(struct sk_buff *skb, struct nlmsghdr *nlh,
 		  int len, void *data, int type)
 {
-	unsigned char *prev_tail;
-
-	prev_tail = skb_tail_pointer(skb);
-	if (nla_put(skb, type, len, data))
-		goto nla_put_failure;
-	nlh->nlmsg_len += skb_tail_pointer(skb) - prev_tail;
+	if (nla_put(skb, type, len, data)) {
+		nlmsg_cancel(skb, nlh);
+		return -EMSGSIZE;
+	}
 	return 0;
-
-nla_put_failure:
-	nlmsg_trim(skb, prev_tail - nlh->nlmsg_len);
-	return -EMSGSIZE;
 }
 EXPORT_SYMBOL(ibnl_put_attr);

Signature · 1 line

2.13.1

Steve Wise

Reviewed-by on v1 2/19

2017-06-21 14:01 UTC #20 lore raw

RE: [PATCH rdma-next 02/19] RDMA/netlink: Simplify the put_msg and put_attr

Quoted 6 lines

From: Leon Romanovsky <[email protected]>
Reuse standard macros to cancel the netlink message in case of error.
Signed-off-by Leon Romanovsky <[email protected]>

Looks good.

Leon Romanovsky

2017-06-21 06:05 UTC #21 lore raw

PATCH 3/19 RDMA/netlink: Rename and remove redundant parameter from ibnl_unicast

From: Leon Romanovsky <[email protected]>

Netlink message header is not needed for unicast reply, hence remove it.

---
 drivers/infiniband/core/iwpm_msg.c  | 6 +++---
 drivers/infiniband/core/iwpm_util.c | 4 ++--
 drivers/infiniband/core/netlink.c   | 5 ++---
 include/rdma/rdma_netlink.h         | 4 +---
 4 files changed, 8 insertions(+), 11 deletions(-)

drivers/infiniband/core/iwpm_msg.c +3 -3

index 1fab707b1f68..8f84557d04e3 100644
@@ -172,7 +172,7 @@ int iwpm_add_mapping(struct iwpm_sa_data *pm_msg, u8 nl_client)
 		goto add_mapping_error;
 	nlmsg_request->req_buffer = pm_msg;
 
-	ret = ibnl_unicast(skb, nlh, iwpm_user_pid);
+	ret = rdma_nl_unicast(skb, iwpm_user_pid);
 	if (ret) {
 		skb = NULL; /* skb is freed in the netlink send-op handling */
 		iwpm_user_pid = IWPM_PID_UNDEFINED;

@@ -248,7 +248,7 @@ int iwpm_add_and_query_mapping(struct iwpm_sa_data *pm_msg, u8 nl_client)
 		goto query_mapping_error;
 	nlmsg_request->req_buffer = pm_msg;
 
-	ret = ibnl_unicast(skb, nlh, iwpm_user_pid);
+	ret = rdma_nl_unicast(skb, iwpm_user_pid);
 	if (ret) {
 		skb = NULL; /* skb is freed in the netlink send-op handling */
 		err_str = "Unable to send a nlmsg";

@@ -308,7 +308,7 @@ int iwpm_remove_mapping(struct sockaddr_storage *local_addr, u8 nl_client)
 	if (ret)
 		goto remove_mapping_error;
 
-	ret = ibnl_unicast(skb, nlh, iwpm_user_pid);
+	ret = rdma_nl_unicast(skb, iwpm_user_pid);
 	if (ret) {
 		skb = NULL; /* skb is freed in the netlink send-op handling */
 		iwpm_user_pid = IWPM_PID_UNDEFINED;

drivers/infiniband/core/iwpm_util.c +2 -2

index c46442ac71a2..c81c55942626 100644
@@ -597,7 +597,7 @@ static int send_mapinfo_num(u32 mapping_num, u8 nl_client, int iwpm_pid)
 				&mapping_num, IWPM_NLA_MAPINFO_SEND_NUM);
 	if (ret)
 		goto mapinfo_num_error;
-	ret = ibnl_unicast(skb, nlh, iwpm_pid);
+	ret = rdma_nl_unicast(skb, iwpm_pid);
 	if (ret) {
 		skb = NULL;
 		err_str = "Unable to send a nlmsg";

@@ -626,7 +626,7 @@ static int send_nlmsg_done(struct sk_buff *skb, u8 nl_client, int iwpm_pid)
 		return -ENOMEM;
 	}
 	nlh->nlmsg_type = NLMSG_DONE;
-	ret = ibnl_unicast(skb, (struct nlmsghdr *)skb->data, iwpm_pid);
+	ret = rdma_nl_unicast(skb, iwpm_pid);
 	if (ret)
 		pr_warn("%s Unable to send a nlmsg\n", __func__);
 	return ret;

drivers/infiniband/core/netlink.c +2 -3

index 96057e722123..cd29311078b5 100644
@@ -241,12 +241,11 @@ static void rdma_nl_rcv(struct sk_buff *skb)
 	mutex_unlock(&rdma_nl_mutex);
 }
 
-int ibnl_unicast(struct sk_buff *skb, struct nlmsghdr *nlh,
-			__u32 pid)
+int rdma_nl_unicast(struct sk_buff *skb, u32 pid)
 {
 	return nlmsg_unicast(nls, skb, pid);
 }
-EXPORT_SYMBOL(ibnl_unicast);
+EXPORT_SYMBOL(rdma_nl_unicast);
 
 int ibnl_multicast(struct sk_buff *skb, struct nlmsghdr *nlh,
 			unsigned int group, gfp_t flags)

include/rdma/rdma_netlink.h +1 -3

index 6932b7acd3a6..9779cd5520d7 100644
@@ -59,12 +59,10 @@ int ibnl_put_attr(struct sk_buff *skb, struct nlmsghdr *nlh,
 /**
  * Send the supplied skb to a specific userspace PID.
  * @skb: The netlink skb
- * @nlh: Header of the netlink message to send
  * @pid: Userspace netlink process ID
  * Returns 0 on success or a negative error code.
  */
-int ibnl_unicast(struct sk_buff *skb, struct nlmsghdr *nlh,
-			__u32 pid);
+int rdma_nl_unicast(struct sk_buff *skb, u32 pid);
 
 /**
  * Send the supplied skb to a netlink group.

Signature · 1 line

2.13.1

Steve Wise

Reviewed-by on v1 3/19

2017-06-21 14:01 UTC #22 lore raw

RE: [PATCH rdma-next 03/19] RDMA/netlink: Rename and remove redundant parameter from ibnl_unicast

Quoted 5 lines

From: Leon Romanovsky <[email protected]>
Netlink message header is not needed for unicast reply, hence remove it.
Signed-off-by Leon Romanovsky <[email protected]>

Looks good.

Leon Romanovsky

2017-06-21 06:05 UTC #23 lore raw

PATCH 5/19 RDMA/netlink: Simplify and rename ibnl_chk_listeners

From: Leon Romanovsky <[email protected]>

Make ibnl_chk_listeners function to be one line by removing unneeded comparison.

Rename that function to be complaint to other functions in RDMA netlink.

---
 drivers/infiniband/core/addr.c     | 2 +-
 drivers/infiniband/core/netlink.c  | 8 +++-----
 drivers/infiniband/core/sa_query.c | 2 +-
 include/rdma/rdma_netlink.h        | 2 +-
 4 files changed, 6 insertions(+), 8 deletions(-)

drivers/infiniband/core/addr.c +1 -1

index ebd0242bab3d..2cc23a26ce4f 100644
@@ -324,7 +324,7 @@ static void queue_req(struct addr_req *req)
 static int ib_nl_fetch_ha(struct dst_entry *dst, struct rdma_dev_addr *dev_addr,
 			  const void *daddr, u32 seq, u16 family)
 {
-	if (ibnl_chk_listeners(RDMA_NL_GROUP_LS))
+	if (rdma_nl_chk_listeners(RDMA_NL_GROUP_LS))
 		return -EADDRNOTAVAIL;
 
 	/* We fill in what we can, the response will fill the rest */

drivers/infiniband/core/netlink.c +3 -5

index 89a6219b6ec8..1022ce6628ae 100644
@@ -46,13 +46,11 @@ static struct {
 	const struct ibnl_client_cbs   *cb_table;
 } rdma_nl_types[RDMA_NL_NUM_CLIENTS];
 
-int ibnl_chk_listeners(unsigned int group)
+int rdma_nl_chk_listeners(unsigned int group)
 {
-	if (netlink_has_listeners(nls, group) == 0)
-		return -1;
-	return 0;
+	return (netlink_has_listeners(nls, group)) ? 0 : -1;
 }
-EXPORT_SYMBOL(ibnl_chk_listeners);
+EXPORT_SYMBOL(rdma_nl_chk_listeners);
 
 static bool is_nl_msg_valid(unsigned int type, unsigned int op)
 {

drivers/infiniband/core/sa_query.c +1 -1

index d890600f1e2d..c06b7deea4d2 100644
@@ -1419,7 +1419,7 @@ static int send_mad(struct ib_sa_query *query, int timeout_ms, gfp_t gfp_mask)
 
 	if ((query->flags & IB_SA_ENABLE_LOCAL_SERVICE) &&
 	    (!(query->flags & IB_SA_QUERY_OPA))) {
-		if (!ibnl_chk_listeners(RDMA_NL_GROUP_LS)) {
+		if (!rdma_nl_chk_listeners(RDMA_NL_GROUP_LS)) {
 			if (!ib_nl_make_request(query, gfp_mask))
 				return id;
 		}

include/rdma/rdma_netlink.h +1 -1

index 145283896417..b39e030b3a64 100644
@@ -78,6 +78,6 @@ int rdma_nl_multicast(struct sk_buff *skb, unsigned int group, gfp_t flags);
  * @group: the netlink group ID
  * Returns 0 on success or a negative for no listeners.
  */
-int ibnl_chk_listeners(unsigned int group);
+int rdma_nl_chk_listeners(unsigned int group);
 
 #endif /* _RDMA_NETLINK_H */

Signature · 1 line

2.13.1

A message that is not in the archive (sent off-list or not yet received): 20170621060528.3752-6-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Steve Wise

Reviewed-by

2017-06-21 14:04 UTC #25 lore raw

RE: [PATCH rdma-next 05/19] RDMA/netlink: Simplify and rename ibnl_chk_listeners

Quoted 8 lines

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Make ibnl_chk_listeners function to be one line by removing unneeded comparison.
Rename that function to be complaint to other functions in RDMA netlink.
Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

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

Leon Romanovsky

2017-06-21 06:05 UTC #26 lore raw

PATCH 7/19 RDMA/core: Add iterator over ib_devices

From: Leon Romanovsky <[email protected]>

The coming nldev needs iterate over all IB devices in the system and in order do not expose the ib_devices list outside the devices.c, it is necessary to provide function iterator.

Current version is written explicitly for nldev callback to avoid over-engineering at this stage, but it can be easily extended for other types.

---
 drivers/infiniband/core/core_priv.h |  8 ++++++++
 drivers/infiniband/core/device.c    | 25 +++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

drivers/infiniband/core/core_priv.h +8 -0

index cb7d372e4bdf..4a150c4be175 100644
@@ -92,6 +92,14 @@ void ib_enum_all_roce_netdevs(roce_netdev_filter filter,
 			      roce_netdev_callback cb,
 			      void *cookie);
 
+typedef int (*nldev_callback)(struct ib_device *device,
+			      struct sk_buff *skb,
+			      struct netlink_callback *cb,
+			      unsigned int idx);
+
+int ib_enum_all_devs(nldev_callback nldev_cb, struct sk_buff *skb,
+		     struct netlink_callback *cb);
+
 enum ib_cache_gid_default_mode {
 	IB_CACHE_GID_DEFAULT_MODE_SET,
 	IB_CACHE_GID_DEFAULT_MODE_DELETE

drivers/infiniband/core/device.c +25 -0

index 5326dcc6ede7..7a799fc90348 100644
@@ -815,6 +815,31 @@ void ib_enum_all_roce_netdevs(roce_netdev_filter filter,
 }
 
 /**
+ * ib_enum_all_devs - enumerate all ib_devices
+ * @cb: Callback to call for each found ib_device
+ *
+ * Enumerates all ib_devices and calls callback() on each device.
+ */
+int ib_enum_all_devs(nldev_callback nldev_cb, struct sk_buff *skb,
+		     struct netlink_callback *cb)
+{
+	struct ib_device *dev;
+	unsigned int idx = 0;
+	int ret = 0;
+
+	down_read(&lists_rwsem);
+	list_for_each_entry(dev, &device_list, core_list) {
+		ret = nldev_cb(dev, skb, cb, idx);
+		if (ret)
+			break;
+		idx++;
+	}
+
+	up_read(&lists_rwsem);
+	return ret;
+}
+
+/**
  * ib_query_pkey - Get P_Key table entry
  * @device:Device to query
  * @port_num:Port number to query

Signature · 1 line

2.13.1

A message that is not in the archive (sent off-list or not yet received): 20170621060528.3752-8-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Steve Wise

Reviewed-by

2017-06-21 14:07 UTC #28 lore raw

RE: [PATCH rdma-next 07/19] RDMA/core: Add iterator over ib_devices

Quoted 5 lines

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The coming nldev needs iterate over all IB devices in the system and in order do not expose the ib_devices list outside the devices.c, it is necessary to provide function iterator.

nit: "needs iterate over" -> "needs to iterate over..."
nit: "order do" -> "order to"

Quoted 6 lines

Current version is written explicitly for nldev callback to avoid over-engineering at this stage, but it can be easily extended for other types.
Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

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

Leon Romanovsky

2017-06-21 06:05 UTC #29 lore raw

PATCH 8/19 RDMA/core: Expose translation from device name to ib_device

From: Leon Romanovsky <[email protected]>

Provide ability to convert from device name to ib_device for the IB/core users.

---
 drivers/infiniband/core/core_priv.h | 1 +
 drivers/infiniband/core/device.c    | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)

drivers/infiniband/core/core_priv.h +1 -0

index 4a150c4be175..049ccbfca988 100644
@@ -184,4 +184,5 @@ int ib_nl_handle_set_timeout(struct sk_buff *skb,
 int ib_nl_handle_ip_res_resp(struct sk_buff *skb,
 			     struct netlink_callback *cb);
 
+struct ib_device *__ib_device_get_by_name(const char *name);
 #endif /* _CORE_PRIV_H */

drivers/infiniband/core/device.c +1 -2

index 7a799fc90348..4ec1b24258de 100644
@@ -124,7 +124,7 @@ static int ib_device_check_mandatory(struct ib_device *device)
 	return 0;
 }
 
-static struct ib_device *__ib_device_get_by_name(const char *name)
+struct ib_device *__ib_device_get_by_name(const char *name)
 {
 	struct ib_device *device;

@@ -135,7 +135,6 @@ static struct ib_device *__ib_device_get_by_name(const char *name)
 	return NULL;
 }
 
-
 static int alloc_name(char *name)
 {
 	unsigned long *inuse;

Signature · 1 line

2.13.1

A message that is not in the archive (sent off-list or not yet received): 20170621060528.3752-9-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Steve Wise

Reviewed-by

2017-06-21 14:09 UTC #31 lore raw

RE: [PATCH rdma-next 08/19] RDMA/core: Expose translation from device name to ib_device

Review

Quoted 23 lines

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Provide ability to convert from device name to ib_device for the IB/core users.
Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
---
 drivers/infiniband/core/core_priv.h | 1 +
 drivers/infiniband/core/device.c    | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/infiniband/core/core_priv.h +1 -0
index 4a150c4be175..049ccbfca988 100644
@@ -184,4 +184,5 @@ int ib_nl_handle_set_timeout(struct sk_buff *skb,
 int ib_nl_handle_ip_res_resp(struct sk_buff *skb,
 			     struct netlink_callback *cb);
 
+struct ib_device *__ib_device_get_by_name(const char *name);
 #endif /* _CORE_PRIV_H */
(quoted diff) +0 -0

b/drivers/infiniband/core/device.c

Quoted 6 lines

index 7a799fc90348..4ec1b24258de 100644

drivers/infiniband/core/device.c static int ib_device_check_mandatory(struct ib_device · line 130

Quoted 8 lines

drivers/infiniband/core/device.c +1 -1

@@ -124,7 +124,7 @@ static int ib_device_check_mandatory(struct ib_device
 *device)
 	return 0;
 }
 
-static struct ib_device *__ib_device_get_by_name(const char *name)
+struct ib_device *__ib_device_get_by_name(const char *name)
 {
 	struct ib_device *device;

Nit: Why is this function prefixed with __? Perhaps now it should just be
ib_device_get_by_name()?

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

Leon Romanovsky

author

2017-06-21 15:00 UTC #32 lore raw

Re: [PATCH rdma-next 08/19] RDMA/core: Expose translation from device name to ib_device

Quoted 32 lines · Steve Wise

Review
Quoted 23 lines
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Provide ability to convert from device name to ib_device for the IB/core users.
Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
---
 drivers/infiniband/core/core_priv.h | 1 +
 drivers/infiniband/core/device.c    | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/infiniband/core/core_priv.h +1 -0
index 4a150c4be175..049ccbfca988 100644
@@ -184,4 +184,5 @@ int ib_nl_handle_set_timeout(struct sk_buff *skb,
 int ib_nl_handle_ip_res_resp(struct sk_buff *skb,
 			     struct netlink_callback *cb);
 
+struct ib_device *__ib_device_get_by_name(const char *name);
 #endif /* _CORE_PRIV_H */
(quoted diff) +0 -0
b/drivers/infiniband/core/device.c
Quoted 8 lines
index 7a799fc90348..4ec1b24258de 100644
drivers/infiniband/core/device.c +0 -0
@@ -124,7 +124,7 @@ static int ib_device_check_mandatory(struct ib_device
 *device)
 	return 0;
 }
 

Quoted 9 lines

drivers/infiniband/core/device.c static int ib_device_check_mandatory(struct ib_device *device) · line 129
Quoted 6 lines
drivers/infiniband/core/device.c +1 -1
-static struct ib_device *__ib_device_get_by_name(const char *name)
+struct ib_device *__ib_device_get_by_name(const char *name)
 {
 	struct ib_device *device;
Nit: Why is this function prefixed with __? Perhaps now it should just be
ib_device_get_by_name()?

The access to this function should be protected by device_mutex and the convention is to prefix functions with "_" to mark that it needs to handle lock.

Quoted 5 lines

Reviewed-by Steve Wise <[email protected]>

Jason Gunthorpe

2017-06-21 16:07 UTC #33 lore raw

Re: [PATCH rdma-next 08/19] RDMA/core: Expose translation from device name to ib_device

Quoted 4 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>
Provide ability to convert from device name to ib_device for the IB/core users.

FWIW, as a general comment, we really need to start using some kind of device index like netdev does instead of the name. I'd like the see the new netlink interface be index based.

As we saw in a past discussion there is a desire to have stable device naming, which means renaming...

Jason

A message that is not in the archive (sent off-list or not yet received): 20170621160748.GA10200-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org

Leon Romanovsky

author

2017-06-22 05:31 UTC #35 lore raw

Re: [PATCH rdma-next 08/19] RDMA/core: Expose translation from device name to ib_device

Quoted 9 lines · Jason Gunthorpe

Quoted 4 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Provide ability to convert from device name to ib_device for the IB/core users.
FWIW, as a general comment, we really need to start using some kind of device index like netdev does instead of the name. I'd like the see the new netlink interface be index based.

I can do it, but it won't be full solution as I would like to have.

At this stage, IB doesn't support namespaces yet and to implement them is a little bit overkill for rdmatool.

Quoted 3 lines

As we saw in a past discussion there is a desire to have stable device naming, which means renaming...

If I recall correctly, after your proposal the participants disappeared :).

Quoted 2 lines

Jason

Leon Romanovsky

2017-06-21 06:05 UTC #36 lore raw

PATCH 10/19 RDMA/netlink: Reduce indirection access to cb_table

From: Leon Romanovsky <[email protected]>

Introduce intermediate variable to store access to fields of cb_table.

---
 drivers/infiniband/core/netlink.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

drivers/infiniband/core/netlink.c +8 -5

index 34f529cc9776..73c74d1cd2a3 100644
@@ -156,12 +156,15 @@ static int rdma_nl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 	unsigned int op = RDMA_NL_GET_OP(type);
 	struct netlink_callback cb = {};
 	struct netlink_dump_control c = {};
+	const struct rdma_nl_cbs *cb_table;
 	int ret;
 
 	if (!is_nl_valid(index, op))
 		return -EINVAL;
 
-	if ((rdma_nl_types[index].cb_table[op].flags & RDMA_NL_ADMIN_PERM) &&
+	cb_table = rdma_nl_types[type].cb_table;
+
+	if ((cb_table[op].flags & RDMA_NL_ADMIN_PERM) &&
 	    !netlink_capable(skb, CAP_NET_ADMIN))
 		return -EPERM;

@@ -173,14 +176,14 @@ static int rdma_nl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 	    (index == RDMA_NL_LS && op == RDMA_NL_LS_OP_SET_TIMEOUT)) {
 		cb.skb = skb;
 		cb.nlh = nlh;
-		cb.dump = rdma_nl_types[index].cb_table[op].dump;
+		cb.dump = cb_table[op].dump;
 		return cb.dump(skb, &cb);
 	} else {
-		c.dump = rdma_nl_types[index].cb_table[op].dump;
+		c.dump = cb_table[op].dump;
 		return netlink_dump_start(nls, skb, nlh, &c);
 	}
-	if (rdma_nl_types[index].cb_table[op].doit)
-		ret = rdma_nl_types[index].cb_table[op].doit(skb, nlh, extack);
+	if (cb_table[op].doit)
+		ret = cb_table[op].doit(skb, nlh, extack);
 	return ret;
 
 }

Signature · 1 line

2.13.1

A message that is not in the archive (sent off-list or not yet received): 20170621060528.3752-11-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Steve Wise

Reviewed-by

2017-06-21 14:13 UTC #38 lore raw

RE: [PATCH rdma-next 10/19] RDMA/netlink: Reduce indirection access to cb_table

Quoted 6 lines

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Introduce intermediate variable to store access to fields of cb_table.
Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

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

Leon Romanovsky

2017-06-21 06:05 UTC #39 lore raw

PATCH 11/19 RDMA/netlink: Convert LS to doit callback

From: Leon Romanovsky <[email protected]>

RDMA_NL_LS protocol is actually is not dump anything, but sets data and it should be handled by doit callback.

This patch actually converts RDMA_NL_LS to doit callback, while preserving IWCM and RDMA_CM flows through netlink_dump_start().

---
 drivers/infiniband/core/addr.c      |  5 ++---
 drivers/infiniband/core/core_priv.h |  9 ++++++---
 drivers/infiniband/core/device.c    |  6 +++---
 drivers/infiniband/core/netlink.c   | 28 ++++++++++------------------
 drivers/infiniband/core/sa_query.c  |  8 ++++----
 5 files changed, 25 insertions(+), 31 deletions(-)

drivers/infiniband/core/addr.c +2 -3

index 2cc23a26ce4f..0b283029bc61 100644
@@ -129,10 +129,9 @@ static void ib_nl_process_good_ip_rsep(const struct nlmsghdr *nlh)
 }
 
 int ib_nl_handle_ip_res_resp(struct sk_buff *skb,
-			     struct netlink_callback *cb)
+			     struct nlmsghdr *nlh,
+			     struct netlink_ext_ack *extack)
 {
-	const struct nlmsghdr *nlh = (struct nlmsghdr *)cb->nlh;
-
 	if ((nlh->nlmsg_flags & NLM_F_REQUEST) ||
 	    !(NETLINK_CB(skb).sk))
 		return -EPERM;

drivers/infiniband/core/core_priv.h +6 -3

index 049ccbfca988..0137aa1ec471 100644
@@ -178,11 +178,14 @@ int ib_sa_init(void);
 void ib_sa_cleanup(void);
 
 int ib_nl_handle_resolve_resp(struct sk_buff *skb,
-			      struct netlink_callback *cb);
+			      struct nlmsghdr *nlh,
+			      struct netlink_ext_ack *extack);
 int ib_nl_handle_set_timeout(struct sk_buff *skb,
-			     struct netlink_callback *cb);
+			     struct nlmsghdr *nlh,
+			     struct netlink_ext_ack *extack);
 int ib_nl_handle_ip_res_resp(struct sk_buff *skb,
-			     struct netlink_callback *cb);
+			     struct nlmsghdr *nlh,
+			     struct netlink_ext_ack *extack);
 
 struct ib_device *__ib_device_get_by_name(const char *name);
 #endif /* _CORE_PRIV_H */

drivers/infiniband/core/device.c +3 -3

index 4ec1b24258de..7317f203a315 100644
@@ -1034,15 +1034,15 @@ EXPORT_SYMBOL(ib_get_net_dev_by_params);
 
 static const struct rdma_nl_cbs ibnl_ls_cb_table[] = {
 	[RDMA_NL_LS_OP_RESOLVE] = {
-		.dump = ib_nl_handle_resolve_resp,
+		.doit = ib_nl_handle_resolve_resp,
 		.flags = RDMA_NL_ADMIN_PERM,
 	},
 	[RDMA_NL_LS_OP_SET_TIMEOUT] = {
-		.dump = ib_nl_handle_set_timeout,
+		.doit = ib_nl_handle_set_timeout,
 		.flags = RDMA_NL_ADMIN_PERM,
 	},
 	[RDMA_NL_LS_OP_IP_RESOLVE] = {
-		.dump = ib_nl_handle_ip_res_resp,
+		.doit = ib_nl_handle_ip_res_resp,
 		.flags = RDMA_NL_ADMIN_PERM,
 	},
 };

drivers/infiniband/core/netlink.c +10 ~~-18~~

index 73c74d1cd2a3..52fce73be9c1 100644
@@ -154,38 +154,30 @@ static int rdma_nl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 	int type = nlh->nlmsg_type;
 	unsigned int index = RDMA_NL_GET_CLIENT(type);
 	unsigned int op = RDMA_NL_GET_OP(type);
-	struct netlink_callback cb = {};
-	struct netlink_dump_control c = {};
 	const struct rdma_nl_cbs *cb_table;
-	int ret;
 
 	if (!is_nl_valid(index, op))
 		return -EINVAL;
 
-	cb_table = rdma_nl_types[type].cb_table;
+	cb_table = rdma_nl_types[index].cb_table;
 
 	if ((cb_table[op].flags & RDMA_NL_ADMIN_PERM) &&
 	    !netlink_capable(skb, CAP_NET_ADMIN))
 		return -EPERM;
 
-	/*
-	 * For response or local service set_timeout request,
-	 * there is no need to use netlink_dump_start.
-	 */
-	if (!(nlh->nlmsg_flags & NLM_F_REQUEST) ||
-	    (index == RDMA_NL_LS && op == RDMA_NL_LS_OP_SET_TIMEOUT)) {
-		cb.skb = skb;
-		cb.nlh = nlh;
-		cb.dump = cb_table[op].dump;
-		return cb.dump(skb, &cb);
-	} else {
-		c.dump = cb_table[op].dump;
+	/* TODO: Convert IWCM to properly handle doit callbacks */
+	if ((nlh->nlmsg_flags & NLM_F_DUMP) || index == RDMA_NL_RDMA_CM ||
+	    index == RDMA_NL_IWCM) {
+		struct netlink_dump_control c = {
+			.dump = cb_table[op].dump,
+		};
 		return netlink_dump_start(nls, skb, nlh, &c);
 	}
+
 	if (cb_table[op].doit)
-		ret = cb_table[op].doit(skb, nlh, extack);
-	return ret;
+		return cb_table[op].doit(skb, nlh, extack);
 
+	return 0;
 }
 
 /*

drivers/infiniband/core/sa_query.c +4 -4

index c06b7deea4d2..42869d11e81e 100644
@@ -1022,9 +1022,9 @@ static void ib_nl_request_timeout(struct work_struct *work)
 }
 
 int ib_nl_handle_set_timeout(struct sk_buff *skb,
-			     struct netlink_callback *cb)
+			     struct nlmsghdr *nlh,
+			     struct netlink_ext_ack *extack)
 {
-	const struct nlmsghdr *nlh = (struct nlmsghdr *)cb->nlh;
 	int timeout, delta, abs_delta;
 	const struct nlattr *attr;
 	unsigned long flags;

@@ -1098,9 +1098,9 @@ static inline int ib_nl_is_good_resolve_resp(const struct nlmsghdr *nlh)
 }
 
 int ib_nl_handle_resolve_resp(struct sk_buff *skb,
-			      struct netlink_callback *cb)
+			      struct nlmsghdr *nlh,
+			      struct netlink_ext_ack *extack)
 {
-	const struct nlmsghdr *nlh = (struct nlmsghdr *)cb->nlh;
 	unsigned long flags;
 	struct ib_sa_query *query;
 	struct ib_mad_send_buf *send_buf;

Signature · 1 line

2.13.1

A message that is not in the archive (sent off-list or not yet received): 20170621060528.3752-12-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Steve Wise

2017-06-21 14:18 UTC #41 lore raw

RE: [PATCH rdma-next 11/19] RDMA/netlink: Convert LS to doit callback

Quoted 3 lines

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
RDMA_NL_LS protocol is actually is not dump anything,

nit: "is not" -> "does not"

Review

Quoted 59 lines

but sets data and it should be handled by doit callback.

This patch actually converts RDMA_NL_LS to doit callback, while preserving IWCM and RDMA_CM flows through netlink_dump_start().

Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

---
 drivers/infiniband/core/addr.c      |  5 ++---
 drivers/infiniband/core/core_priv.h |  9 ++++++---
 drivers/infiniband/core/device.c    |  6 +++---
 drivers/infiniband/core/netlink.c   | 28 ++++++++++------------------
 drivers/infiniband/core/sa_query.c  |  8 ++++----
 5 files changed, 25 insertions(+), 31 deletions(-)

drivers/infiniband/core/addr.c +2 -3

index 2cc23a26ce4f..0b283029bc61 100644
@@ -129,10 +129,9 @@ static void ib_nl_process_good_ip_rsep(const struct
 nlmsghdr *nlh)
 }
 
 int ib_nl_handle_ip_res_resp(struct sk_buff *skb,
-			     struct netlink_callback *cb)
+			     struct nlmsghdr *nlh,
+			     struct netlink_ext_ack *extack)
 {
-	const struct nlmsghdr *nlh = (struct nlmsghdr *)cb->nlh;
-
 	if ((nlh->nlmsg_flags & NLM_F_REQUEST) ||
 	    !(NETLINK_CB(skb).sk))

drivers/infiniband/core/addr.c +0 -0

 		return -EPERM;

drivers/infiniband/core/core_priv.h +6 -3

index 049ccbfca988..0137aa1ec471 100644
@@ -178,11 +178,14 @@ int ib_sa_init(void);
 void ib_sa_cleanup(void);
 
 int ib_nl_handle_resolve_resp(struct sk_buff *skb,
-			      struct netlink_callback *cb);
+			      struct nlmsghdr *nlh,
+			      struct netlink_ext_ack *extack);
 int ib_nl_handle_set_timeout(struct sk_buff *skb,
-			     struct netlink_callback *cb);
+			     struct nlmsghdr *nlh,
+			     struct netlink_ext_ack *extack);
 int ib_nl_handle_ip_res_resp(struct sk_buff *skb,
-			     struct netlink_callback *cb);
+			     struct nlmsghdr *nlh,
+			     struct netlink_ext_ack *extack);
 
 struct ib_device *__ib_device_get_by_name(const char *name);
 #endif /* _CORE_PRIV_H */

(quoted diff) +0 -0

b/drivers/infiniband/core/device.c

Review

Quoted 23 lines

index 4ec1b24258de..7317f203a315 100644

drivers/infiniband/core/device.c +3 -3

@@ -1034,15 +1034,15 @@ EXPORT_SYMBOL(ib_get_net_dev_by_params);
 
 static const struct rdma_nl_cbs ibnl_ls_cb_table[] = {
 	[RDMA_NL_LS_OP_RESOLVE] = {
-		.dump = ib_nl_handle_resolve_resp,
+		.doit = ib_nl_handle_resolve_resp,
 		.flags = RDMA_NL_ADMIN_PERM,
 	},
 	[RDMA_NL_LS_OP_SET_TIMEOUT] = {
-		.dump = ib_nl_handle_set_timeout,
+		.doit = ib_nl_handle_set_timeout,
 		.flags = RDMA_NL_ADMIN_PERM,
 	},
 	[RDMA_NL_LS_OP_IP_RESOLVE] = {
-		.dump = ib_nl_handle_ip_res_resp,
+		.doit = ib_nl_handle_ip_res_resp,
 		.flags = RDMA_NL_ADMIN_PERM,
 	},
 };

(quoted diff) +0 -0

b/drivers/infiniband/core/netlink.c

Quoted 38 lines

index 73c74d1cd2a3..52fce73be9c1 100644

drivers/infiniband/core/netlink.c +3 ~~-16~~

@@ -154,38 +154,30 @@ static int rdma_nl_rcv_msg(struct sk_buff *skb, struct
 nlmsghdr *nlh,
 	int type = nlh->nlmsg_type;
 	unsigned int index = RDMA_NL_GET_CLIENT(type);
 	unsigned int op = RDMA_NL_GET_OP(type);
-	struct netlink_callback cb = {};
-	struct netlink_dump_control c = {};
 	const struct rdma_nl_cbs *cb_table;
-	int ret;
 
 	if (!is_nl_valid(index, op))
 		return -EINVAL;
 
-	cb_table = rdma_nl_types[type].cb_table;
+	cb_table = rdma_nl_types[index].cb_table;
 
 	if ((cb_table[op].flags & RDMA_NL_ADMIN_PERM) &&
 	    !netlink_capable(skb, CAP_NET_ADMIN))
 		return -EPERM;
 
-	/*
-	 * For response or local service set_timeout request,
-	 * there is no need to use netlink_dump_start.
-	 */
-	if (!(nlh->nlmsg_flags & NLM_F_REQUEST) ||
-	    (index == RDMA_NL_LS && op == RDMA_NL_LS_OP_SET_TIMEOUT)) {
-		cb.skb = skb;
-		cb.nlh = nlh;
-		cb.dump = cb_table[op].dump;
-		return cb.dump(skb, &cb);
-	} else {
-		c.dump = cb_table[op].dump;
+	/* TODO: Convert IWCM to properly handle doit callbacks */
+	if ((nlh->nlmsg_flags & NLM_F_DUMP) || index == RDMA_NL_RDMA_CM

drivers/infiniband/core/netlink.c static int rdma_nl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, · line 173

Quoted 4 lines

drivers/infiniband/core/netlink.c +4 -0

+	    index == RDMA_NL_IWCM) {
+		struct netlink_dump_control c = {
+			.dump = cb_table[op].dump,
+		};

Any reason you didn't fix IWCM as part of this series? Or will you fix it in an upcoming series? Also, isn't FIXME: the norm for these sorts of "I don't want to fix this now" comments?

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

Leon Romanovsky

author

2017-06-21 14:53 UTC #42 lore raw

Re: [PATCH rdma-next 11/19] RDMA/netlink: Convert LS to doit callback

Quoted 128 lines · Steve Wise

Quoted 3 lines

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
RDMA_NL_LS protocol is actually is not dump anything,

nit: "is not" -> "does not"

Review

Quoted 59 lines

but sets data and it should be handled by doit callback.

This patch actually converts RDMA_NL_LS to doit callback, while preserving IWCM and RDMA_CM flows through netlink_dump_start().

Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

---
 drivers/infiniband/core/addr.c      |  5 ++---
 drivers/infiniband/core/core_priv.h |  9 ++++++---
 drivers/infiniband/core/device.c    |  6 +++---
 drivers/infiniband/core/netlink.c   | 28 ++++++++++------------------
 drivers/infiniband/core/sa_query.c  |  8 ++++----
 5 files changed, 25 insertions(+), 31 deletions(-)

drivers/infiniband/core/addr.c +2 -3

index 2cc23a26ce4f..0b283029bc61 100644
@@ -129,10 +129,9 @@ static void ib_nl_process_good_ip_rsep(const struct
 nlmsghdr *nlh)
 }
 
 int ib_nl_handle_ip_res_resp(struct sk_buff *skb,
-			     struct netlink_callback *cb)
+			     struct nlmsghdr *nlh,
+			     struct netlink_ext_ack *extack)
 {
-	const struct nlmsghdr *nlh = (struct nlmsghdr *)cb->nlh;
-
 	if ((nlh->nlmsg_flags & NLM_F_REQUEST) ||
 	    !(NETLINK_CB(skb).sk))

drivers/infiniband/core/addr.c +0 -0

 		return -EPERM;

drivers/infiniband/core/core_priv.h +6 -3

index 049ccbfca988..0137aa1ec471 100644
@@ -178,11 +178,14 @@ int ib_sa_init(void);
 void ib_sa_cleanup(void);
 
 int ib_nl_handle_resolve_resp(struct sk_buff *skb,
-			      struct netlink_callback *cb);
+			      struct nlmsghdr *nlh,
+			      struct netlink_ext_ack *extack);
 int ib_nl_handle_set_timeout(struct sk_buff *skb,
-			     struct netlink_callback *cb);
+			     struct nlmsghdr *nlh,
+			     struct netlink_ext_ack *extack);
 int ib_nl_handle_ip_res_resp(struct sk_buff *skb,
-			     struct netlink_callback *cb);
+			     struct nlmsghdr *nlh,
+			     struct netlink_ext_ack *extack);
 
 struct ib_device *__ib_device_get_by_name(const char *name);
 #endif /* _CORE_PRIV_H */

(quoted diff) +0 -0

b/drivers/infiniband/core/device.c

Review

Quoted 23 lines

index 4ec1b24258de..7317f203a315 100644

drivers/infiniband/core/device.c +3 -3

@@ -1034,15 +1034,15 @@ EXPORT_SYMBOL(ib_get_net_dev_by_params);
 
 static const struct rdma_nl_cbs ibnl_ls_cb_table[] = {
 	[RDMA_NL_LS_OP_RESOLVE] = {
-		.dump = ib_nl_handle_resolve_resp,
+		.doit = ib_nl_handle_resolve_resp,
 		.flags = RDMA_NL_ADMIN_PERM,
 	},
 	[RDMA_NL_LS_OP_SET_TIMEOUT] = {
-		.dump = ib_nl_handle_set_timeout,
+		.doit = ib_nl_handle_set_timeout,
 		.flags = RDMA_NL_ADMIN_PERM,
 	},
 	[RDMA_NL_LS_OP_IP_RESOLVE] = {
-		.dump = ib_nl_handle_ip_res_resp,
+		.doit = ib_nl_handle_ip_res_resp,
 		.flags = RDMA_NL_ADMIN_PERM,
 	},
 };

(quoted diff) +0 -0

b/drivers/infiniband/core/netlink.c

Quoted 38 lines

index 73c74d1cd2a3..52fce73be9c1 100644

drivers/infiniband/core/netlink.c +3 ~~-16~~

@@ -154,38 +154,30 @@ static int rdma_nl_rcv_msg(struct sk_buff *skb, struct
 nlmsghdr *nlh,
 	int type = nlh->nlmsg_type;
 	unsigned int index = RDMA_NL_GET_CLIENT(type);
 	unsigned int op = RDMA_NL_GET_OP(type);
-	struct netlink_callback cb = {};
-	struct netlink_dump_control c = {};
 	const struct rdma_nl_cbs *cb_table;
-	int ret;
 
 	if (!is_nl_valid(index, op))
 		return -EINVAL;
 
-	cb_table = rdma_nl_types[type].cb_table;
+	cb_table = rdma_nl_types[index].cb_table;
 
 	if ((cb_table[op].flags & RDMA_NL_ADMIN_PERM) &&
 	    !netlink_capable(skb, CAP_NET_ADMIN))
 		return -EPERM;
 
-	/*
-	 * For response or local service set_timeout request,
-	 * there is no need to use netlink_dump_start.
-	 */
-	if (!(nlh->nlmsg_flags & NLM_F_REQUEST) ||
-	    (index == RDMA_NL_LS && op == RDMA_NL_LS_OP_SET_TIMEOUT)) {
-		cb.skb = skb;
-		cb.nlh = nlh;
-		cb.dump = cb_table[op].dump;
-		return cb.dump(skb, &cb);
-	} else {
-		c.dump = cb_table[op].dump;
+	/* TODO: Convert IWCM to properly handle doit callbacks */
+	if ((nlh->nlmsg_flags & NLM_F_DUMP) || index == RDMA_NL_RDMA_CM

Quoted 8 lines

drivers/infiniband/core/netlink.c static int rdma_nl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, · line 173
Quoted 4 lines
drivers/infiniband/core/netlink.c +4 -0
+	    index == RDMA_NL_IWCM) {
+		struct netlink_dump_control c = {
+			.dump = cb_table[op].dump,
+		};
Any reason you didn't fix IWCM as part of this series? Or will you fix it in an upcoming series? Also, isn't FIXME: the norm for these sorts of "I don't want to fix this now" comments?

I wanted to stop before it is growing into enormous series. There are number of things which I wanted to discuss and fix before moving forward.

1. What should we do with exported RDMA-CM statistics and structures? I have a very strong feeling that it is broken and anyway, I'm not going to use it, because it doesn't follow netlink's TLV style.

2. How to handle IWCM code which sets manually MSG_DONE and isn't using MULTI flag? It has very similar pattern to my "workarounds", when I didn't handle properly end of message.

3. Need to remove nl_client from IWCM code.

Thanks

Quoted 2 lines

Steve Wise

2017-06-21 15:25 UTC #43 lore raw

RE: [PATCH rdma-next 11/19] RDMA/netlink: Convert LS to doit callback

Quoted 13 lines

drivers/infiniband/core/netlink.c +1 ~~-12~~

-	/*
-	 * For response or local service set_timeout request,
-	 * there is no need to use netlink_dump_start.
-	 */
-	if (!(nlh->nlmsg_flags & NLM_F_REQUEST) ||
-	    (index == RDMA_NL_LS && op == RDMA_NL_LS_OP_SET_TIMEOUT)) {
-		cb.skb = skb;
-		cb.nlh = nlh;
-		cb.dump = cb_table[op].dump;
-		return cb.dump(skb, &cb);
-	} else {
-		c.dump = cb_table[op].dump;
+	/* TODO: Convert IWCM to properly handle doit callbacks */

Quoted 8 lines

Quoted 8 lines
drivers/infiniband/core/netlink.c static int rdma_nl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, · line 173
Quoted 6 lines
(quoted diff) +1 -0
+	if ((nlh->nlmsg_flags & NLM_F_DUMP) || index == RDMA_NL_RDMA_CM
||
drivers/infiniband/core/netlink.c +4 -0
+	    index == RDMA_NL_IWCM) {
+		struct netlink_dump_control c = {
+			.dump = cb_table[op].dump,
+		};
Any reason you didn't fix IWCM as part of this series? Or will you fix it

in an

Quoted 1 line

Quoted 1 line
upcoming series? Also, isn't FIXME: the norm for these sorts of "I don't

want

Quoted 9 lines

Quoted 1 line
to fix this now" comments?
I wanted to stop before it is growing into enormous series. There are number of things which I wanted to discuss and fix before moving forward.
1. What should we do with exported RDMA-CM statistics and structures? I have a very strong feeling that it is broken and anyway, I'm not going to use it, because it doesn't follow netlink's TLV style.

Quoted 6 lines

2. How to handle IWCM code which sets manually MSG_DONE and isn't using MULTI flag? It has very similar pattern to my "workarounds", when I didn't handle properly end of message.
3. Need to remove nl_client from IWCM code.

Chien, can you please comment on this?

Thanks!

Steve.

Leon Romanovsky

2017-06-21 06:05 UTC #44 lore raw

PATCH 12/19 RDMA/netlink: Update copyright

From: Leon Romanovsky <[email protected]>

Add Mellanox to the copyright header.

---
 drivers/infiniband/core/netlink.c | 1 +
 1 file changed, 1 insertion(+)

drivers/infiniband/core/netlink.c +1 -0

index 52fce73be9c1..9731c313e9b9 100644
@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2017 Mellanox Technologies Inc.  All rights reserved.
  * Copyright (c) 2010 Voltaire Inc.  All rights reserved.
  *
  * This software is available to you under a choice of one of two

Signature · 1 line

2.13.1

Steve Wise

Reviewed-by on v1 12/19

2017-06-21 14:18 UTC #45 lore raw

RE: [PATCH rdma-next 12/19] RDMA/netlink: Update copyright

Quoted 5 lines

From: Leon Romanovsky <[email protected]>
Add Mellanox to the copyright header.
Signed-off-by Leon Romanovsky <[email protected]>

Leon Romanovsky

2017-06-21 06:05 UTC #46 lore raw

PATCH 14/19 RDMA/netlink: Add nldev initialization flows

From: Leon Romanovsky <[email protected]>

Add nldev init and exit flows to the RDMA/core.

---
 drivers/infiniband/core/Makefile    |  4 +++-
 drivers/infiniband/core/core_priv.h |  4 ++++
 drivers/infiniband/core/device.c    |  2 ++
 drivers/infiniband/core/nldev.c     | 45 +++++++++++++++++++++++++++++++++++++
 4 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644 drivers/infiniband/core/nldev.c

drivers/infiniband/core/Makefile +3 -1

index 6ebd9ad95010..d260c5ecf656 100644
@@ -10,7 +10,9 @@ obj-$(CONFIG_INFINIBAND_USER_ACCESS) +=	ib_uverbs.o ib_ucm.o \
 ib_core-y :=			packer.o ud_header.o verbs.o cq.o rw.o sysfs.o \
 				device.o fmr_pool.o cache.o netlink.o \
 				roce_gid_mgmt.o mr_pool.o addr.o sa_query.o \
-				multicast.o mad.o smi.o agent.o mad_rmpp.o
+				multicast.o mad.o smi.o agent.o mad_rmpp.o \
+				nldev.o
+
 ib_core-$(CONFIG_INFINIBAND_USER_MEM) += umem.o
 ib_core-$(CONFIG_INFINIBAND_ON_DEMAND_PAGING) += umem_odp.o umem_rbtree.o
 ib_core-$(CONFIG_CGROUP_RDMA) += cgroup.o

drivers/infiniband/core/core_priv.h +4 -0

index 0137aa1ec471..492c856393c9 100644
@@ -188,4 +188,8 @@ int ib_nl_handle_ip_res_resp(struct sk_buff *skb,
 			     struct netlink_ext_ack *extack);
 
 struct ib_device *__ib_device_get_by_name(const char *name);
+
+/* RDMA device netlink */
+void nldev_init(void);
+void nldev_exit(void);
 #endif /* _CORE_PRIV_H */

drivers/infiniband/core/device.c +2 -0

index 7317f203a315..bbcc210731e2 100644
@@ -1092,6 +1092,7 @@ static int __init ib_core_init(void)
 		goto err_mad;
 	}
 
+	nldev_init();
 	rdma_nl_register(RDMA_NL_LS, ibnl_ls_cb_table);
 	ib_cache_setup();

@@ -1115,6 +1116,7 @@ static int __init ib_core_init(void)
 static void __exit ib_core_cleanup(void)
 {
 	ib_cache_cleanup();
+	nldev_exit();
 	rdma_nl_unregister(RDMA_NL_LS);
 	ib_sa_cleanup();
 	ib_mad_cleanup();

drivers/infiniband/core/nldev.c new file +45 -0

45 lines not on this page · raw message

Signature · 1 line

2.13.1

A message that is not in the archive (sent off-list or not yet received): 20170621060528.3752-15-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Steve Wise

Reviewed-by

2017-06-21 14:22 UTC #48 lore raw

RE: [PATCH rdma-next 14/19] RDMA/netlink: Add nldev initialization flows

Quoted 5 lines

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Add nldev init and exit flows to the RDMA/core.
Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

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

Leon Romanovsky

2017-06-21 06:05 UTC #49 lore raw

PATCH 15/19 RDMA/netlink: Implement nldev device dumpit calback

From: Leon Romanovsky <[email protected]>

This patch adds the ability to return all available devices together with their properties.

---
 drivers/infiniband/core/nldev.c | 61 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 60 insertions(+), 1 deletion(-)

drivers/infiniband/core/nldev.c +60 -1

index 1d1e4f214874..219b52166c46 100644
@@ -30,13 +30,72 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
+#include <net/netlink.h>
 #include <rdma/rdma_netlink.h>
 
 #include "core_priv.h"
 
+static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
+	[RDMA_NLDEV_ATTR_DEV_NAME]	= { .type = NLA_NUL_STRING,
+					    .len = IB_DEVICE_NAME_MAX - 1},
+	[RDMA_NLDEV_ATTR_PORT_INDEX]	= { .type = NLA_U32 },
+};
+
+static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
+{
+	if (nla_put_string(msg, RDMA_NLDEV_ATTR_DEV_NAME, device->name))
+		return -EMSGSIZE;
+	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_PORT_INDEX, rdma_end_port(device)))
+		return -EMSGSIZE;
+	return 0;
+}
+
+static int _nldev_get_dumpit(struct ib_device *device,
+			     struct sk_buff *skb,
+			     struct netlink_callback *cb,
+			     unsigned int idx)
+{
+	int start = cb->args[0];
+	struct nlmsghdr *nlh;
+
+	if (idx < start)
+		return 0;
+
+	nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
+			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_GET),
+			0, NLM_F_MULTI);
+
+	if (fill_dev_info(skb, device)) {
+		nlmsg_cancel(skb, nlh);
+		goto out;
+	}
+
+	nlmsg_end(skb, nlh);
+
+	idx++;
+
+out:	cb->args[0] = idx;
+	return skb->len;
+}
+
+static int nldev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	/*
+	 * There is no need to take lock, because
+	 * we are relying on ib_core's lists_rwsem
+	 */
+	return ib_enum_all_devs(_nldev_get_dumpit, skb, cb);
+}
+
+static const struct rdma_nl_cbs nldev_cb_table[] = {
+	[RDMA_NLDEV_CMD_GET] = {
+		.dump = nldev_get_dumpit,
+	},
+};
+
 void __init nldev_init(void)
 {
-	rdma_nl_register(RDMA_NL_NLDEV, NULL);
+	rdma_nl_register(RDMA_NL_NLDEV, nldev_cb_table);
 }
 
 void __exit nldev_exit(void)

Signature · 1 line

2.13.1

Steve Wise

Reviewed-by on v1 15/19

2017-06-21 14:25 UTC #50 lore raw

RE: [PATCH rdma-next 15/19] RDMA/netlink: Implement nldev device dumpit calback

Quoted 6 lines

From: Leon Romanovsky <[email protected]>
This patch adds the ability to return all available devices together with their properties.
Signed-off-by Leon Romanovsky <[email protected]>

Leon Romanovsky

2017-06-21 06:05 UTC #51 lore raw

PATCH 16/19 RDMa/netlink: Add nldev device doit implementation

From: Leon Romanovsky <[email protected]>

Provide ability to query specific device.

---
 drivers/infiniband/core/nldev.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

drivers/infiniband/core/nldev.c +40 -0

index 219b52166c46..8d3fd9b58cc1 100644
@@ -50,6 +50,45 @@ static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
 	return 0;
 }
 
+static int nldev_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
+			  struct netlink_ext_ack *extack)
+{
+	struct nlattr *tb[RDMA_NLDEV_ATTR_MAX];
+	char name[IB_DEVICE_NAME_MAX];
+	struct ib_device *device;
+	struct sk_buff *msg;
+	int err;
+
+	err = nlmsg_parse(nlh, 0, tb, RDMA_NLDEV_ATTR_MAX,
+			  nldev_policy, extack);
+	if (err || !tb[RDMA_NLDEV_ATTR_DEV_NAME])
+		return -EINVAL;
+
+	nla_strlcpy(name, tb[RDMA_NLDEV_ATTR_DEV_NAME], IB_DEVICE_NAME_MAX);
+
+	device = __ib_device_get_by_name(name);
+	if (!device)
+		return -EINVAL;
+
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	nlh = nlmsg_put(msg, NETLINK_CB(skb).portid, nlh->nlmsg_seq,
+			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_GET),
+			0, 0);
+
+	err = fill_dev_info(msg, device);
+	if (err) {
+		nlmsg_free(msg);
+		return err;
+	}
+
+	nlmsg_end(msg, nlh);
+
+	return rdma_nl_unicast(msg, NETLINK_CB(skb).portid);
+}
+
 static int _nldev_get_dumpit(struct ib_device *device,
 			     struct sk_buff *skb,
 			     struct netlink_callback *cb,

@@ -89,6 +128,7 @@ static int nldev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 
 static const struct rdma_nl_cbs nldev_cb_table[] = {
 	[RDMA_NLDEV_CMD_GET] = {
+		.doit = nldev_get_doit,
 		.dump = nldev_get_dumpit,
 	},
 };

Signature · 1 line

2.13.1

A message that is not in the archive (sent off-list or not yet received): 20170621060528.3752-17-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Steve Wise

Reviewed-by

2017-06-21 14:26 UTC #53 lore raw

RE: [PATCH rdma-next 16/19] RDMa/netlink: Add nldev device doit implementation

Quoted 6 lines

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Provide ability to query specific device.
Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

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

Leon Romanovsky

2017-06-21 06:05 UTC #54 lore raw

PATCH 17/19 RDMA/netlink: Add nldev port dumpit implementation

From: Leon Romanovsky <[email protected]>

This patch implements the query interface to get all ports data for the specific device.

---
 drivers/infiniband/core/nldev.c | 60 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

drivers/infiniband/core/nldev.c +60 -0

index 8d3fd9b58cc1..ded06fa69ccb 100644
@@ -50,6 +50,16 @@ static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
 	return 0;
 }
 
+static int fill_port_info(struct sk_buff *msg,
+			  struct ib_device *device, u32 port)
+{
+	if (nla_put_string(msg, RDMA_NLDEV_ATTR_DEV_NAME, device->name))
+		return -EMSGSIZE;
+	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_PORT_INDEX, port))
+		return -EMSGSIZE;
+	return 0;
+}
+
 static int nldev_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
 			  struct netlink_ext_ack *extack)
 {

@@ -126,11 +136,61 @@ static int nldev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 	return ib_enum_all_devs(_nldev_get_dumpit, skb, cb);
 }
 
+static int nldev_port_get_dumpit(struct sk_buff *skb,
+				 struct netlink_callback *cb)
+{
+	struct nlattr *tb[RDMA_NLDEV_ATTR_MAX];
+	char name[IB_DEVICE_NAME_MAX];
+	struct ib_device *device;
+	int start = cb->args[0];
+	struct nlmsghdr *nlh;
+	u32 idx = 0;
+	int err;
+	u32 p;
+
+	err = nlmsg_parse(cb->nlh, 0, tb, RDMA_NLDEV_ATTR_MAX,
+			  nldev_policy, NULL);
+	if (err || !tb[RDMA_NLDEV_ATTR_DEV_NAME])
+		return -EINVAL;
+
+	nla_strlcpy(name, tb[RDMA_NLDEV_ATTR_DEV_NAME], IB_DEVICE_NAME_MAX);
+
+	device = __ib_device_get_by_name(name);
+	if (!device)
+		return -EINVAL;
+
+	for (p = rdma_start_port(device); p <= rdma_end_port(device); ++p) {
+		if (idx < start) {
+			idx++;
+			continue;
+		}
+
+		nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).portid,
+				cb->nlh->nlmsg_seq,
+				RDMA_NL_GET_TYPE(RDMA_NL_NLDEV,
+						 RDMA_NLDEV_CMD_PORT_GET),
+				0, NLM_F_MULTI);
+
+		if (fill_port_info(skb, device, p)) {
+			nlmsg_cancel(skb, nlh);
+			goto out;
+		}
+		idx++;
+		nlmsg_end(skb, nlh);
+	}
+
+out:	cb->args[0] = idx;
+	return skb->len;
+}
+
 static const struct rdma_nl_cbs nldev_cb_table[] = {
 	[RDMA_NLDEV_CMD_GET] = {
 		.doit = nldev_get_doit,
 		.dump = nldev_get_dumpit,
 	},
+	[RDMA_NLDEV_CMD_PORT_GET] = {
+		.dump = nldev_port_get_dumpit,
+	},
 };
 
 void __init nldev_init(void)

Signature · 1 line

2.13.1

A message that is not in the archive (sent off-list or not yet received): 20170621060528.3752-18-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Steve Wise

Reviewed-by

2017-06-21 14:28 UTC #56 lore raw

RE: [PATCH rdma-next 17/19] RDMA/netlink: Add nldev port dumpit implementation

Quoted 6 lines

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
This patch implements the query interface to get all ports data for the specific device.
Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

The fill_port/dev_info is just filling in names and indexes. Shouldn't it have all kinds of goodies about the port/dev? Will this be enhanced going forward as needed?

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

Leon Romanovsky

author

2017-06-21 14:42 UTC #57 lore raw

Re: [PATCH rdma-next 17/19] RDMA/netlink: Add nldev port dumpit implementation

Quoted 10 lines · Steve Wise

Quoted 6 lines
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
This patch implements the query interface to get all ports data for the specific device.
Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The fill_port/dev_info is just filling in names and indexes. Shouldn't it have all kinds of goodies about the port/dev? Will this be enhanced going forward as needed?

Yes, exactly.

Quoted 2 lines

Reviewed-by Steve Wise <[email protected]>

Thank you a lot for your review.

Quoted 2 lines

A message that is not in the archive (sent off-list or not yet received): 20170621144211.GE1248-U/[email protected]

Leon Romanovsky

author

2017-06-21 15:21 UTC #59 lore raw

Re: [PATCH rdma-next 17/19] RDMA/netlink: Add nldev port dumpit implementation

Quoted 6 lines · Leon Romanovsky

Quoted 5 lines · Steve Wise
Quoted 5 lines
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
This patch implements the query interface to get all ports data for the specific device.

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The fill_port/dev_info is just filling in names and indexes. Shouldn't it have all kinds of goodies about the port/dev? Will this be enhanced going forward as needed?
Yes, exactly.

You can see, it is happening in patch #19.

Thanks

Quoted 8 lines

Quoted 3 lines
Reviewed-by Steve Wise <[email protected]>
Thank you a lot for your review.
Quoted 2 lines

Leon Romanovsky

2017-06-21 06:05 UTC #60 lore raw

PATCH 18/19 RDMA/netlink: Implement nldev port doit callback

From: Leon Romanovsky <[email protected]>

Provide ability to get specific to device and port information.

---
 drivers/infiniband/core/nldev.c | 45 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

drivers/infiniband/core/nldev.c +45 -0

index ded06fa69ccb..250965329592 100644
@@ -136,6 +136,50 @@ static int nldev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 	return ib_enum_all_devs(_nldev_get_dumpit, skb, cb);
 }
 
+static int nldev_port_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
+			       struct netlink_ext_ack *extack)
+{
+	struct nlattr *tb[RDMA_NLDEV_ATTR_MAX];
+	char name[IB_DEVICE_NAME_MAX];
+	struct ib_device *device;
+	struct sk_buff *msg;
+	u32 port;
+	int err;
+
+	err = nlmsg_parse(nlh, 0, tb, RDMA_NLDEV_ATTR_MAX,
+			  nldev_policy, extack);
+	if (err || !tb[RDMA_NLDEV_ATTR_DEV_NAME] ||
+	    !tb[RDMA_NLDEV_ATTR_PORT_INDEX])
+		return -EINVAL;
+
+	nla_strlcpy(name, tb[RDMA_NLDEV_ATTR_DEV_NAME], IB_DEVICE_NAME_MAX);
+	device = __ib_device_get_by_name(name);
+	if (!device)
+		return -EINVAL;
+
+	port = nla_get_u32(tb[RDMA_NLDEV_ATTR_PORT_INDEX]);
+	if (rdma_is_port_valid(device, port))
+		return -EINVAL;
+
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	nlh = nlmsg_put(msg, NETLINK_CB(skb).portid, nlh->nlmsg_seq,
+			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_GET),
+			0, 0);
+
+	err = fill_port_info(msg, device, port);
+	if (err) {
+		nlmsg_free(msg);
+		return err;
+	}
+
+	nlmsg_end(msg, nlh);
+
+	return rdma_nl_unicast(msg, NETLINK_CB(skb).portid);
+}
+
 static int nldev_port_get_dumpit(struct sk_buff *skb,
 				 struct netlink_callback *cb)
 {

@@ -189,6 +233,7 @@ static const struct rdma_nl_cbs nldev_cb_table[] = {
 		.dump = nldev_get_dumpit,
 	},
 	[RDMA_NLDEV_CMD_PORT_GET] = {
+		.doit = nldev_port_get_doit,
 		.dump = nldev_port_get_dumpit,
 	},
 };

Signature · 1 line

2.13.1

A message that is not in the archive (sent off-list or not yet received): 20170621060528.3752-19-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Steve Wise

Reviewed-by

2017-06-21 14:29 UTC #62 lore raw

RE: [PATCH rdma-next 18/19] RDMA/netlink: Implement nldev port doit callback

Quoted 5 lines

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Provide ability to get specific to device and port information.
Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>

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

Leon Romanovsky

2017-06-21 06:05 UTC #63 lore raw

PATCH 19/19 RDMA/netlink: Expose device and port capability masks

From: Leon Romanovsky <[email protected]>

The port capability mask is exposed to user space via sysfs interface, while device capabilities are available for verbs only.

This patch provides those capabilities through netlink interface.

---
 drivers/infiniband/core/nldev.c  | 19 +++++++++++++++++++
 include/uapi/rdma/rdma_netlink.h |  5 +++++
 2 files changed, 24 insertions(+)

drivers/infiniband/core/nldev.c +19 -0

index 250965329592..77135fb29179 100644
@@ -47,16 +47,35 @@ static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
 		return -EMSGSIZE;
 	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_PORT_INDEX, rdma_end_port(device)))
 		return -EMSGSIZE;
+
+	BUILD_BUG_ON(sizeof(device->attrs.device_cap_flags) != sizeof(u64));
+	if (nla_put_u64_64bit(msg, RDMA_NLDEV_ATTR_CAP_FLAGS,
+			      device->attrs.device_cap_flags, 0))
+		return -EMSGSIZE;
+
 	return 0;
 }
 
 static int fill_port_info(struct sk_buff *msg,
 			  struct ib_device *device, u32 port)
 {
+	struct ib_port_attr attr;
+	int ret;
+
 	if (nla_put_string(msg, RDMA_NLDEV_ATTR_DEV_NAME, device->name))
 		return -EMSGSIZE;
 	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_PORT_INDEX, port))
 		return -EMSGSIZE;
+
+	ret = ib_query_port(device, port, &attr);
+	if (ret)
+		return ret;
+
+	BUILD_BUG_ON(sizeof(attr.port_cap_flags) > sizeof(u64));
+	if (nla_put_u64_64bit(msg, RDMA_NLDEV_ATTR_CAP_FLAGS,
+			      (u64)attr.port_cap_flags, 0))
+		return -EMSGSIZE;
+
 	return 0;
 }

include/uapi/rdma/rdma_netlink.h +5 -0

index bfafd5996d52..1fff898632f4 100644
@@ -262,6 +262,11 @@ enum rdma_nldev_attr {
 	 */
 	RDMA_NLDEV_ATTR_PORT_INDEX,		/* u32 */
 
+	/*
+	 * Device and port capabilities
+	 */
+	RDMA_NLDEV_ATTR_CAP_FLAGS,		/* u64 */
+
 	RDMA_NLDEV_ATTR_MAX
 };
 #endif /* _UAPI_RDMA_NETLINK_H */

Signature · 1 line

2.13.1

A message that is not in the archive (sent off-list or not yet received): 20170621060528.3752-20-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Steve Wise

Reviewed-by

2017-06-21 14:31 UTC #65 lore raw

RE: [PATCH rdma-next 19/19] RDMA/netlink: Expose device and port capability masks

Quoted 8 lines

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The port capability mask is exposed to user space via sysfs interface, while device capabilities are available for verbs only.
This patch provides those capabilities through netlink interface.
Signed-off-by Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>