[pull request][rdma-next 00/33] RDMA netlink refactoring and RDMAtool code

[pull request][rdma-next 00/33] RDMA netlink refactoring and RDMAtool code

[Leon Romanovsky]

Latest code drop with fixed LMC name (LID mask control).

----------------------------------------------------------------
The following changes since commit 913cc67159bc85a96d94df301ca39c1b2c540dca:

  IB/hfi1: Always perform offline transition (2017-07-31 15:18:38 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git tags/rdma-next-2017-08-01

for you to fetch changes up to 76bed394b018e89cc0a7d69a2c724655a1506c2c:

  RDMA/netlink: Export node_type (2017-08-01 14:22:50 +0300)

----------------------------------------------------------------
RDMA netlink refactoring and RDMAtool code

----------------------------------------------------------------
Leon Romanovsky (33):
      Revert "IB/core: Add flow control to the portmapper netlink calls"
      RDMA/netlink: Remove netlink clients infrastructure
      RDMA/netlink: Remove redundant owner option for netlink callbacks
      RDMA/netlink: Avoid double pass for RDMA netlink messages
      RDMA/iwcm: Remove useless check of nelink client validity
      RDMA/iwcm: Remove extra EXPORT_SYMBOLS
      RDMA/netlink: Add flag to consolidate common handing
      RDMA/netlink: Simplify the put_msg and put_attr
      RDMA/netlink: Rename and remove redundant parameter from ibnl_unicast
      RDMA/netlink: Rename and remove redundant parameter from ibnl_multicast
      RDMA/netlink: Simplify and rename ibnl_chk_listeners
      RDMA/netlink: Rename netlink callback struct
      RDMA/core: Add iterator over ib_devices
      RDMA/core: Add and expose static device index
      RDMA/netlink: Add and implement doit netlink callback
      RDMA/netlink: Reduce indirection access to cb_table
      RDMA/netlink: Convert LS to doit callback
      RDMA/netlink: Update copyright
      RDMA/netlink: Add netlink device definitions to UAPI
      RDMA/netlink: Add nldev initialization flows
      RDMA/netlink: Implement nldev device dumpit calback
      RDMA/netlink: Add nldev device doit implementation
      RDMA/netlink: Add nldev port dumpit implementation
      RDMA/netlink: Implement nldev port doit callback
      RDMA/netlink: Expose device and port capability masks
      RDMA: Simplify get firmware interface
      RDMA/netlink: Export FW version
      RDMA/netlink: Export node_guid and sys_image_guid
      RDMA/netlink: Advertise IB subnet prefix
      RDMA/netink: Export lids and sm_lids
      RDMA/netlink: Export LID mask control (LMC)
      RDMA/netlink: Provide port state and physical link state
      RDMA/netlink: Export node_type

 drivers/infiniband/core/Makefile               |   3 +-
 drivers/infiniband/core/addr.c                 |  12 +-
 drivers/infiniband/core/cma.c                  |  11 +-
 drivers/infiniband/core/core_priv.h            |  26 ++-
 drivers/infiniband/core/device.c               | 119 +++++++---
 drivers/infiniband/core/iwcm.c                 |  12 +-
 drivers/infiniband/core/iwpm_msg.c             |  20 +-
 drivers/infiniband/core/iwpm_util.c            |  15 +-
 drivers/infiniband/core/netlink.c              | 310 ++++++++++++------------
 drivers/infiniband/core/nldev.c                | 312 +++++++++++++++++++++++++
 drivers/infiniband/core/sa_query.c             |  18 +-
 drivers/infiniband/core/sysfs.c                |   4 +-
 drivers/infiniband/hw/cxgb3/iwch_provider.c    |   5 +-
 drivers/infiniband/hw/cxgb4/provider.c         |   5 +-
 drivers/infiniband/hw/hfi1/verbs.c             |   5 +-
 drivers/infiniband/hw/i40iw/i40iw_verbs.c      |   7 +-
 drivers/infiniband/hw/mlx4/main.c              |   5 +-
 drivers/infiniband/hw/mlx5/main.c              |   8 +-
 drivers/infiniband/hw/mthca/mthca_provider.c   |   5 +-
 drivers/infiniband/hw/nes/nes_verbs.c          |   5 +-
 drivers/infiniband/hw/ocrdma/ocrdma_main.c     |   5 +-
 drivers/infiniband/hw/qedr/main.c              |   5 +-
 drivers/infiniband/hw/usnic/usnic_ib_main.c    |   6 +-
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c |   5 +-
 drivers/infiniband/ulp/ipoib/ipoib_ethtool.c   |   3 +-
 include/rdma/ib_verbs.h                        |   8 +-
 include/rdma/rdma_netlink.h                    |  38 +--
 include/uapi/rdma/rdma_netlink.h               |  84 ++++++-
 28 files changed, 743 insertions(+), 318 deletions(-)
 create mode 100644 drivers/infiniband/core/nldev.c
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Leon Romanovsky]

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

The commit cea05eadded0 ("IB/core: Add flow control to the portmapper netlink calls")
changed netlink to be blocked for all RDMA clients. This workaround
worked perfectly for portmapper, but is not correct for the whole
NETLINK_RDMA family.

The request/response should always be blocking and asynchronous
notification should always be non-blocking.

It is library and user-space application to chose how to handle recvmgs,
as an example see nl_recvmsgs() and nl_socket_set_nonblocking() calls of
libnl library.

Send timeout is not needed too and can be configured with SO_SNDTIMEO socket
option.

This reverts commit cea05eadded0d4eb59f7be6e1f1560eb6bfde2bf.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/netlink.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index 94931c474d41..454d51e432db 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -230,10 +230,7 @@ static void ibnl_rcv(struct sk_buff *skb)
 int ibnl_unicast(struct sk_buff *skb, struct nlmsghdr *nlh,
 			__u32 pid)
 {
-	int err;
-
-	err = netlink_unicast(nls, skb, pid, 0);
-	return (err < 0) ? err : 0;
+	return nlmsg_unicast(nls, skb, pid);
 }
 EXPORT_SYMBOL(ibnl_unicast);
 
@@ -256,7 +253,6 @@ int __init ibnl_init(void)
 		return -ENOMEM;
 	}
 
-	nls->sk_sndtimeo = 10 * HZ;
 	return 0;
 }
 
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 02/33] RDMA/netlink: Remove netlink clients infrastructure

[Leon Romanovsky]

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

RDMA netlink has complicated infrastructure to add and remove netlink
clients to NETLINK_RDMA family. This complicates the code and not in
use because not many clients are available (3 clients) and most of them
(2 clients) are statically compiled together with netlink.c.

The following patch refactors RDMA netlink and opens door for the future
patches which will be able to get rid of a lot of dead iwcm* code.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Chien Tin Tung <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/core/cma.c       |   6 +-
 drivers/infiniband/core/core_priv.h |   4 +-
 drivers/infiniband/core/device.c    |  45 +++------
 drivers/infiniband/core/iwcm.c      |  10 +-
 drivers/infiniband/core/netlink.c   | 185 +++++++++++++++++-------------------
 include/rdma/rdma_netlink.h         |  13 +--
 6 files changed, 112 insertions(+), 151 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index ca4135c596ba..2a16a559bdda 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -4512,9 +4512,7 @@ static int __init cma_init(void)
 	if (ret)
 		goto err;
 
-	if (ibnl_add_client(RDMA_NL_RDMA_CM, ARRAY_SIZE(cma_cb_table),
-			    cma_cb_table))
-		pr_warn("RDMA CMA: failed to add netlink callback\n");
+	rdma_nl_register(RDMA_NL_RDMA_CM, cma_cb_table);
 	cma_configfs_init();
 
 	return 0;
@@ -4531,7 +4529,7 @@ static int __init cma_init(void)
 static void __exit cma_cleanup(void)
 {
 	cma_configfs_exit();
-	ibnl_remove_client(RDMA_NL_RDMA_CM);
+	rdma_nl_unregister(RDMA_NL_RDMA_CM);
 	ib_unregister_client(&cma_client);
 	unregister_netdevice_notifier(&cma_nb);
 	rdma_addr_unregister_client(&addr_client);
diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index 11ae67514e13..e759c27113cd 100644
--- a/drivers/infiniband/core/core_priv.h
+++ b/drivers/infiniband/core/core_priv.h
@@ -179,8 +179,8 @@ void ib_mad_cleanup(void);
 int ib_sa_init(void);
 void ib_sa_cleanup(void);
 
-int ibnl_init(void);
-void ibnl_cleanup(void);
+int rdma_nl_init(void);
+void rdma_nl_exit(void);
 
 /**
  * Check if there are any listeners to the netlink group
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index a5dfab6adf49..298258c40d30 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -1086,29 +1086,15 @@ struct net_device *ib_get_net_dev_by_params(struct ib_device *dev,
 }
 EXPORT_SYMBOL(ib_get_net_dev_by_params);
 
-static struct ibnl_client_cbs ibnl_ls_cb_table[] = {
+static const struct ibnl_client_cbs ibnl_ls_cb_table[] = {
 	[RDMA_NL_LS_OP_RESOLVE] = {
-		.dump = ib_nl_handle_resolve_resp,
-		.module = THIS_MODULE },
+		.dump = ib_nl_handle_resolve_resp},
 	[RDMA_NL_LS_OP_SET_TIMEOUT] = {
-		.dump = ib_nl_handle_set_timeout,
-		.module = THIS_MODULE },
+		.dump = ib_nl_handle_set_timeout},
 	[RDMA_NL_LS_OP_IP_RESOLVE] = {
-		.dump = ib_nl_handle_ip_res_resp,
-		.module = THIS_MODULE },
+		.dump = ib_nl_handle_ip_res_resp},
 };
 
-static int ib_add_ibnl_clients(void)
-{
-	return ibnl_add_client(RDMA_NL_LS, ARRAY_SIZE(ibnl_ls_cb_table),
-			       ibnl_ls_cb_table);
-}
-
-static void ib_remove_ibnl_clients(void)
-{
-	ibnl_remove_client(RDMA_NL_LS);
-}
-
 static int __init ib_core_init(void)
 {
 	int ret;
@@ -1130,9 +1116,9 @@ static int __init ib_core_init(void)
 		goto err_comp;
 	}
 
-	ret = ibnl_init();
+	ret = rdma_nl_init();
 	if (ret) {
-		pr_warn("Couldn't init IB netlink interface\n");
+		pr_warn("Couldn't init IB netlink interface %d\n", ret);
 		goto err_sysfs;
 	}
 
@@ -1154,24 +1140,17 @@ static int __init ib_core_init(void)
 		goto err_mad;
 	}
 
-	ret = ib_add_ibnl_clients();
-	if (ret) {
-		pr_warn("Couldn't register ibnl clients\n");
-		goto err_sa;
-	}
-
 	ret = register_lsm_notifier(&ibdev_lsm_nb);
 	if (ret) {
 		pr_warn("Couldn't register LSM notifier. ret %d\n", ret);
-		goto err_ibnl_clients;
+		goto err_sa;
 	}
 
+	rdma_nl_register(RDMA_NL_LS, ibnl_ls_cb_table);
 	ib_cache_setup();
 
 	return 0;
 
-err_ibnl_clients:
-	ib_remove_ibnl_clients();
 err_sa:
 	ib_sa_cleanup();
 err_mad:
@@ -1179,7 +1158,7 @@ static int __init ib_core_init(void)
 err_addr:
 	addr_cleanup();
 err_ibnl:
-	ibnl_cleanup();
+	rdma_nl_exit();
 err_sysfs:
 	class_unregister(&ib_class);
 err_comp:
@@ -1191,13 +1170,13 @@ static int __init ib_core_init(void)
 
 static void __exit ib_core_cleanup(void)
 {
-	unregister_lsm_notifier(&ibdev_lsm_nb);
 	ib_cache_cleanup();
-	ib_remove_ibnl_clients();
+	rdma_nl_unregister(RDMA_NL_LS);
+	unregister_lsm_notifier(&ibdev_lsm_nb);
 	ib_sa_cleanup();
 	ib_mad_cleanup();
 	addr_cleanup();
-	ibnl_cleanup();
+	rdma_nl_exit();
 	class_unregister(&ib_class);
 	destroy_workqueue(ib_comp_wq);
 	/* Make sure that any pending umem accounting work is done. */
diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c
index 31661b5c1743..8599271d8be6 100644
--- a/drivers/infiniband/core/iwcm.c
+++ b/drivers/infiniband/core/iwcm.c
@@ -1175,12 +1175,8 @@ static int __init iw_cm_init(void)
 	ret = iwpm_init(RDMA_NL_IWCM);
 	if (ret)
 		pr_err("iw_cm: couldn't init iwpm\n");
-
-	ret = ibnl_add_client(RDMA_NL_IWCM, ARRAY_SIZE(iwcm_nl_cb_table),
-			      iwcm_nl_cb_table);
-	if (ret)
-		pr_err("iw_cm: couldn't register netlink callbacks\n");
-
+	else
+		rdma_nl_register(RDMA_NL_IWCM, iwcm_nl_cb_table);
 	iwcm_wq = alloc_ordered_workqueue("iw_cm_wq", WQ_MEM_RECLAIM);
 	if (!iwcm_wq)
 		return -ENOMEM;
@@ -1200,7 +1196,7 @@ static void __exit iw_cm_cleanup(void)
 {
 	unregister_net_sysctl_table(iwcm_ctl_table_hdr);
 	destroy_workqueue(iwcm_wq);
-	ibnl_remove_client(RDMA_NL_IWCM);
+	rdma_nl_unregister(RDMA_NL_IWCM);
 	iwpm_exit(RDMA_NL_IWCM);
 }
 
diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index 454d51e432db..31f4d33149bc 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -39,16 +39,13 @@
 #include <rdma/rdma_netlink.h>
 #include "core_priv.h"
 
-struct ibnl_client {
-	struct list_head		list;
-	int				index;
-	int				nops;
-	const struct ibnl_client_cbs   *cb_table;
-};
+#include "core_priv.h"
 
-static DEFINE_MUTEX(ibnl_mutex);
+static DEFINE_MUTEX(rdma_nl_mutex);
 static struct sock *nls;
-static LIST_HEAD(client_list);
+static struct {
+	const struct ibnl_client_cbs   *cb_table;
+} rdma_nl_types[RDMA_NL_NUM_CLIENTS];
 
 int ibnl_chk_listeners(unsigned int group)
 {
@@ -57,58 +54,74 @@ int ibnl_chk_listeners(unsigned int group)
 	return 0;
 }
 
-int ibnl_add_client(int index, int nops,
-		    const struct ibnl_client_cbs cb_table[])
+static bool is_nl_msg_valid(unsigned int type, unsigned int op)
 {
-	struct ibnl_client *cur;
-	struct ibnl_client *nl_client;
+	static const unsigned int max_num_ops[RDMA_NL_NUM_CLIENTS - 1] = {
+				  RDMA_NL_RDMA_CM_NUM_OPS,
+				  RDMA_NL_IWPM_NUM_OPS,
+				  0,
+				  RDMA_NL_LS_NUM_OPS,
+				  0 };
 
-	nl_client = kmalloc(sizeof *nl_client, GFP_KERNEL);
-	if (!nl_client)
-		return -ENOMEM;
+	/*
+	 * This BUILD_BUG_ON is intended to catch addition of new
+	 * RDMA netlink protocol without updating the array above.
+	 */
+	BUILD_BUG_ON(RDMA_NL_NUM_CLIENTS != 6);
 
-	nl_client->index	= index;
-	nl_client->nops		= nops;
-	nl_client->cb_table	= cb_table;
+	if (type > RDMA_NL_NUM_CLIENTS - 1)
+		return false;
 
-	mutex_lock(&ibnl_mutex);
+	return (op < max_num_ops[type - 1]) ? true : false;
+}
 
-	list_for_each_entry(cur, &client_list, list) {
-		if (cur->index == index) {
-			pr_warn("Client for %d already exists\n", index);
-			mutex_unlock(&ibnl_mutex);
-			kfree(nl_client);
-			return -EINVAL;
-		}
-	}
+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)
+		return false;
+	return true;
+}
 
-	list_add_tail(&nl_client->list, &client_list);
+void rdma_nl_register(unsigned int index,
+		      const struct ibnl_client_cbs cb_table[])
+{
+	mutex_lock(&rdma_nl_mutex);
+	if (!is_nl_msg_valid(index, 0)) {
+		/*
+		 * All clients are not interesting in success/failure of
+		 * this call. They want to see the print to error log and
+		 * continue their initialization. Print warning for them,
+		 * because it is programmer's error to be here.
+		 */
+		mutex_unlock(&rdma_nl_mutex);
+		WARN(true,
+		     "The not-valid %u index was supplied to RDMA netlink\n",
+		     index);
+		return;
+	}
 
-	mutex_unlock(&ibnl_mutex);
+	if (rdma_nl_types[index].cb_table) {
+		mutex_unlock(&rdma_nl_mutex);
+		WARN(true,
+		     "The %u index is already registered in RDMA netlink\n",
+		     index);
+		return;
+	}
 
-	return 0;
+	rdma_nl_types[index].cb_table = cb_table;
+	mutex_unlock(&rdma_nl_mutex);
 }
-EXPORT_SYMBOL(ibnl_add_client);
+EXPORT_SYMBOL(rdma_nl_register);
 
-int ibnl_remove_client(int index)
+void rdma_nl_unregister(unsigned int index)
 {
-	struct ibnl_client *cur, *next;
-
-	mutex_lock(&ibnl_mutex);
-	list_for_each_entry_safe(cur, next, &client_list, list) {
-		if (cur->index == index) {
-			list_del(&(cur->list));
-			mutex_unlock(&ibnl_mutex);
-			kfree(cur);
-			return 0;
-		}
-	}
-	pr_warn("Can't remove callback for client idx %d. Not found\n", index);
-	mutex_unlock(&ibnl_mutex);
-
-	return -EINVAL;
+	mutex_lock(&rdma_nl_mutex);
+	rdma_nl_types[index].cb_table = NULL;
+	mutex_unlock(&rdma_nl_mutex);
 }
-EXPORT_SYMBOL(ibnl_remove_client);
+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)
@@ -149,45 +162,31 @@ EXPORT_SYMBOL(ibnl_put_attr);
 static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 			struct netlink_ext_ack *extack)
 {
-	struct ibnl_client *client;
 	int type = nlh->nlmsg_type;
-	int index = RDMA_NL_GET_CLIENT(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 = {};
 
-	list_for_each_entry(client, &client_list, list) {
-		if (client->index == index) {
-			if (op >= client->nops || !client->cb_table[op].dump)
-				return -EINVAL;
-
-			/*
-			 * 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)) {
-				struct netlink_callback cb = {
-					.skb = skb,
-					.nlh = nlh,
-					.dump = client->cb_table[op].dump,
-					.module = client->cb_table[op].module,
-				};
-
-				return cb.dump(skb, &cb);
-			}
-
-			{
-				struct netlink_dump_control c = {
-					.dump = client->cb_table[op].dump,
-					.module = client->cb_table[op].module,
-				};
-				return netlink_dump_start(nls, skb, nlh, &c);
-			}
-		}
+	if (!is_nl_valid(index, op))
+		return -EINVAL;
+
+	/*
+	 * 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 = rdma_nl_types[index].cb_table[op].dump;
+		cb.module = rdma_nl_types[index].cb_table[op].module;
+		return cb.dump(skb, &cb);
 	}
 
-	pr_info("Index %d wasn't found in client list\n", index);
-	return -EINVAL;
+	c.dump = rdma_nl_types[index].cb_table[op].dump;
+	c.module = rdma_nl_types[index].cb_table[op].module;
+	return netlink_dump_start(nls, skb, nlh, &c);
 }
 
 static void ibnl_rcv_reply_skb(struct sk_buff *skb)
@@ -221,10 +220,10 @@ static void ibnl_rcv_reply_skb(struct sk_buff *skb)
 
 static void ibnl_rcv(struct sk_buff *skb)
 {
-	mutex_lock(&ibnl_mutex);
+	mutex_lock(&rdma_nl_mutex);
 	ibnl_rcv_reply_skb(skb);
 	netlink_rcv_skb(skb, &ibnl_rcv_msg);
-	mutex_unlock(&ibnl_mutex);
+	mutex_unlock(&rdma_nl_mutex);
 }
 
 int ibnl_unicast(struct sk_buff *skb, struct nlmsghdr *nlh,
@@ -241,31 +240,25 @@ int ibnl_multicast(struct sk_buff *skb, struct nlmsghdr *nlh,
 }
 EXPORT_SYMBOL(ibnl_multicast);
 
-int __init ibnl_init(void)
+int __init rdma_nl_init(void)
 {
 	struct netlink_kernel_cfg cfg = {
 		.input	= ibnl_rcv,
 	};
 
 	nls = netlink_kernel_create(&init_net, NETLINK_RDMA, &cfg);
-	if (!nls) {
-		pr_warn("Failed to create netlink socket\n");
+	if (!nls)
 		return -ENOMEM;
-	}
 
 	return 0;
 }
 
-void ibnl_cleanup(void)
+void rdma_nl_exit(void)
 {
-	struct ibnl_client *cur, *next;
+	int idx;
 
-	mutex_lock(&ibnl_mutex);
-	list_for_each_entry_safe(cur, next, &client_list, list) {
-		list_del(&(cur->list));
-		kfree(cur);
-	}
-	mutex_unlock(&ibnl_mutex);
+	for (idx = 0; idx < RDMA_NL_NUM_CLIENTS; idx++)
+		rdma_nl_unregister(idx);
 
 	netlink_kernel_release(nls);
 }
diff --git a/include/rdma/rdma_netlink.h b/include/rdma/rdma_netlink.h
index 348c102cb5f6..162e58f118e7 100644
--- a/include/rdma/rdma_netlink.h
+++ b/include/rdma/rdma_netlink.h
@@ -11,23 +11,18 @@ struct ibnl_client_cbs {
 };
 
 /**
- * Add a a client to the list of IB netlink exporters.
+ * Register client in RDMA netlink.
  * @index: Index of the added client
- * @nops: Number of supported ops by the added client.
  * @cb_table: A table for op->callback
- *
- * Returns 0 on success or a negative error code.
  */
-int ibnl_add_client(int index, int nops,
-		    const struct ibnl_client_cbs cb_table[]);
+void rdma_nl_register(unsigned int index,
+		      const struct ibnl_client_cbs cb_table[]);
 
 /**
  * Remove a client from IB netlink.
  * @index: Index of the removed IB client.
- *
- * Returns 0 on success or a negative error code.
  */
-int ibnl_remove_client(int index);
+void rdma_nl_unregister(unsigned int index);
 
 /**
  * Put a new message in a supplied skb.
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 03/33] RDMA/netlink: Remove redundant owner option for netlink callbacks

[Leon Romanovsky]

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

Owner field is not needed to be set because netlink is part of ib_core
which will be unloaded last after all other modules are unloaded.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/cma.c     | 3 +--
 drivers/infiniband/core/netlink.c | 2 --
 include/rdma/rdma_netlink.h       | 1 -
 3 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 2a16a559bdda..0c85f140e616 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -4459,8 +4459,7 @@ static int cma_get_id_stats(struct sk_buff *skb, struct netlink_callback *cb)
 }
 
 static const struct ibnl_client_cbs cma_cb_table[] = {
-	[RDMA_NL_RDMA_CM_ID_STATS] = { .dump = cma_get_id_stats,
-				       .module = THIS_MODULE },
+	[RDMA_NL_RDMA_CM_ID_STATS] = { .dump = cma_get_id_stats},
 };
 
 static int cma_init_net(struct net *net)
diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index 31f4d33149bc..6cb534a595ea 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -180,12 +180,10 @@ static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 		cb.skb = skb;
 		cb.nlh = nlh;
 		cb.dump = rdma_nl_types[index].cb_table[op].dump;
-		cb.module = rdma_nl_types[index].cb_table[op].module;
 		return cb.dump(skb, &cb);
 	}
 
 	c.dump = rdma_nl_types[index].cb_table[op].dump;
-	c.module = rdma_nl_types[index].cb_table[op].module;
 	return netlink_dump_start(nls, skb, nlh, &c);
 }
 
diff --git a/include/rdma/rdma_netlink.h b/include/rdma/rdma_netlink.h
index 162e58f118e7..7690aaabb958 100644
--- a/include/rdma/rdma_netlink.h
+++ b/include/rdma/rdma_netlink.h
@@ -7,7 +7,6 @@
 
 struct ibnl_client_cbs {
 	int (*dump)(struct sk_buff *skb, struct netlink_callback *nlcb);
-	struct module *module;
 };
 
 /**
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 04/33] RDMA/netlink: Avoid double pass for RDMA netlink messages

[Leon Romanovsky]

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

The standard netlink_rcv_skb function skips messages without
NLM_F_REQUEST flag in it, while SA netlink client issues them.

In commit bc10ed7d3d19 ("IB/core: Add rdma netlink helper functions")
the local function was introduced to allow such messages.

This led to double pass for every incoming message.

In this patch, we unify that local implementation and netlink_rcv_skb
functions, so there will be no need for double pass anymore.

As a outcome, this combined function gained more strict check
for NLM_F_REQUEST flag and it is now allowed for SA pathquery
client only.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/netlink.c | 62 +++++++++++++++++++++++++++------------
 1 file changed, 44 insertions(+), 18 deletions(-)

diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index 6cb534a595ea..86337d5e7551 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -159,8 +159,8 @@ int ibnl_put_attr(struct sk_buff *skb, struct nlmsghdr *nlh,
 }
 EXPORT_SYMBOL(ibnl_put_attr);
 
-static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
-			struct netlink_ext_ack *extack)
+static int rdma_nl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
+			   struct netlink_ext_ack *extack)
 {
 	int type = nlh->nlmsg_type;
 	unsigned int index = RDMA_NL_GET_CLIENT(type);
@@ -187,40 +187,66 @@ static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 	return netlink_dump_start(nls, skb, nlh, &c);
 }
 
-static void ibnl_rcv_reply_skb(struct sk_buff *skb)
+/*
+ * This function is similar to netlink_rcv_skb with one exception:
+ * It calls to the callback for the netlink messages without NLM_F_REQUEST
+ * flag. These messages are intended for RDMA_NL_LS consumer, so it is allowed
+ * for that consumer only.
+ */
+static int rdma_nl_rcv_skb(struct sk_buff *skb, int (*cb)(struct sk_buff *,
+						   struct nlmsghdr *,
+						   struct netlink_ext_ack *))
 {
+	struct netlink_ext_ack extack = {};
 	struct nlmsghdr *nlh;
-	int msglen;
+	int err;
 
-	/*
-	 * Process responses until there is no more message or the first
-	 * request. Generally speaking, it is not recommended to mix responses
-	 * with requests.
-	 */
 	while (skb->len >= nlmsg_total_size(0)) {
+		int msglen;
+
 		nlh = nlmsg_hdr(skb);
+		err = 0;
 
 		if (nlh->nlmsg_len < NLMSG_HDRLEN || skb->len < nlh->nlmsg_len)
-			return;
+			return 0;
 
-		/* Handle response only */
-		if (nlh->nlmsg_flags & NLM_F_REQUEST)
-			return;
+		/*
+		 * Generally speaking, the only requests are handled
+		 * by the kernel, but RDMA_NL_LS is different, because it
+		 * runs backward netlink scheme. Kernel initiates messages
+		 * and waits for reply with data to keep pathrecord cache
+		 * in sync.
+		 */
+		if (!(nlh->nlmsg_flags & NLM_F_REQUEST) &&
+		    (RDMA_NL_GET_CLIENT(nlh->nlmsg_type) != RDMA_NL_LS))
+			goto ack;
+
+		/* Skip control messages */
+		if (nlh->nlmsg_type < NLMSG_MIN_TYPE)
+			goto ack;
+
+		err = cb(skb, nlh, &extack);
+		if (err == -EINTR)
+			goto skip;
 
-		ibnl_rcv_msg(skb, nlh, NULL);
+ack:
+		if (nlh->nlmsg_flags & NLM_F_ACK || err)
+			netlink_ack(skb, nlh, err, &extack);
 
+skip:
 		msglen = NLMSG_ALIGN(nlh->nlmsg_len);
 		if (msglen > skb->len)
 			msglen = skb->len;
 		skb_pull(skb, msglen);
 	}
+
+	return 0;
 }
 
-static void ibnl_rcv(struct sk_buff *skb)
+static void rdma_nl_rcv(struct sk_buff *skb)
 {
 	mutex_lock(&rdma_nl_mutex);
-	ibnl_rcv_reply_skb(skb);
-	netlink_rcv_skb(skb, &ibnl_rcv_msg);
+	rdma_nl_rcv_skb(skb, &rdma_nl_rcv_msg);
 	mutex_unlock(&rdma_nl_mutex);
 }
 
@@ -241,7 +267,7 @@ EXPORT_SYMBOL(ibnl_multicast);
 int __init rdma_nl_init(void)
 {
 	struct netlink_kernel_cfg cfg = {
-		.input	= ibnl_rcv,
+		.input	= rdma_nl_rcv,
 	};
 
 	nls = netlink_kernel_create(&init_net, NETLINK_RDMA, &cfg);
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 05/33] RDMA/iwcm: Remove useless check of nelink client validity

[Leon Romanovsky]

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

RDMA netlink implementation guarantees that supplied
client number is in allowed range.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Chien Tin Tung <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/core/iwpm_util.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/infiniband/core/iwpm_util.c b/drivers/infiniband/core/iwpm_util.c
index f13870e69ccd..32ca2aaa4e3b 100644
--- a/drivers/infiniband/core/iwpm_util.c
+++ b/drivers/infiniband/core/iwpm_util.c
@@ -54,8 +54,6 @@ static struct iwpm_admin_data iwpm_admin;
 int iwpm_init(u8 nl_client)
 {
 	int ret = 0;
-	if (iwpm_valid_client(nl_client))
-		return -EINVAL;
 	mutex_lock(&iwpm_admin_lock);
 	if (atomic_read(&iwpm_admin.refcount) == 0) {
 		iwpm_hash_bucket = kzalloc(IWPM_MAPINFO_HASH_SIZE *
@@ -383,15 +381,11 @@ int iwpm_get_nlmsg_seq(void)
 
 int iwpm_valid_client(u8 nl_client)
 {
-	if (nl_client >= RDMA_NL_NUM_CLIENTS)
-		return 0;
 	return iwpm_admin.client_list[nl_client];
 }
 
 void iwpm_set_valid(u8 nl_client, int valid)
 {
-	if (nl_client >= RDMA_NL_NUM_CLIENTS)
-		return;
 	iwpm_admin.client_list[nl_client] = valid;
 }
 
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 06/33] RDMA/iwcm: Remove extra EXPORT_SYMBOLS

[Leon Romanovsky]

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

The iwcm exports functions which are not used outside of ib_core.
This patch simply removes these EXPORT_SYMBOLS.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Chien Tin Tung <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/core/iwpm_msg.c  | 12 ------------
 drivers/infiniband/core/iwpm_util.c |  5 -----
 2 files changed, 17 deletions(-)

diff --git a/drivers/infiniband/core/iwpm_msg.c b/drivers/infiniband/core/iwpm_msg.c
index a0e7c16d8bd8..1fab707b1f68 100644
--- a/drivers/infiniband/core/iwpm_msg.c
+++ b/drivers/infiniband/core/iwpm_msg.c
@@ -42,7 +42,6 @@ int iwpm_valid_pid(void)
 {
 	return iwpm_user_pid > 0;
 }
-EXPORT_SYMBOL(iwpm_valid_pid);
 
 /*
  * iwpm_register_pid - Send a netlink query to user space
@@ -122,7 +121,6 @@ int iwpm_register_pid(struct iwpm_dev_data *pm_msg, u8 nl_client)
 		iwpm_free_nlmsg_request(&nlmsg_request->kref);
 	return ret;
 }
-EXPORT_SYMBOL(iwpm_register_pid);
 
 /*
  * iwpm_add_mapping - Send a netlink add mapping message
@@ -191,7 +189,6 @@ int iwpm_add_mapping(struct iwpm_sa_data *pm_msg, u8 nl_client)
 		iwpm_free_nlmsg_request(&nlmsg_request->kref);
 	return ret;
 }
-EXPORT_SYMBOL(iwpm_add_mapping);
 
 /*
  * iwpm_add_and_query_mapping - Send a netlink add and query
@@ -267,7 +264,6 @@ int iwpm_add_and_query_mapping(struct iwpm_sa_data *pm_msg, u8 nl_client)
 		iwpm_free_nlmsg_request(&nlmsg_request->kref);
 	return ret;
 }
-EXPORT_SYMBOL(iwpm_add_and_query_mapping);
 
 /*
  * iwpm_remove_mapping - Send a netlink remove mapping message
@@ -328,7 +324,6 @@ int iwpm_remove_mapping(struct sockaddr_storage *local_addr, u8 nl_client)
 		dev_kfree_skb_any(skb);
 	return ret;
 }
-EXPORT_SYMBOL(iwpm_remove_mapping);
 
 /* netlink attribute policy for the received response to register pid request */
 static const struct nla_policy resp_reg_policy[IWPM_NLA_RREG_PID_MAX] = {
@@ -397,7 +392,6 @@ int iwpm_register_pid_cb(struct sk_buff *skb, struct netlink_callback *cb)
 	up(&nlmsg_request->sem);
 	return 0;
 }
-EXPORT_SYMBOL(iwpm_register_pid_cb);
 
 /* netlink attribute policy for the received response to add mapping request */
 static const struct nla_policy resp_add_policy[IWPM_NLA_RMANAGE_MAPPING_MAX] = {
@@ -466,7 +460,6 @@ int iwpm_add_mapping_cb(struct sk_buff *skb, struct netlink_callback *cb)
 	up(&nlmsg_request->sem);
 	return 0;
 }
-EXPORT_SYMBOL(iwpm_add_mapping_cb);
 
 /* netlink attribute policy for the response to add and query mapping request
  * and response with remote address info */
@@ -558,7 +551,6 @@ int iwpm_add_and_query_mapping_cb(struct sk_buff *skb,
 	up(&nlmsg_request->sem);
 	return 0;
 }
-EXPORT_SYMBOL(iwpm_add_and_query_mapping_cb);
 
 /*
  * iwpm_remote_info_cb - Process a port mapper message, containing
@@ -627,7 +619,6 @@ int iwpm_remote_info_cb(struct sk_buff *skb, struct netlink_callback *cb)
 			"remote_info: Mapped remote sockaddr:");
 	return ret;
 }
-EXPORT_SYMBOL(iwpm_remote_info_cb);
 
 /* netlink attribute policy for the received request for mapping info */
 static const struct nla_policy resp_mapinfo_policy[IWPM_NLA_MAPINFO_REQ_MAX] = {
@@ -677,7 +668,6 @@ int iwpm_mapping_info_cb(struct sk_buff *skb, struct netlink_callback *cb)
 	ret = iwpm_send_mapinfo(nl_client, iwpm_user_pid);
 	return ret;
 }
-EXPORT_SYMBOL(iwpm_mapping_info_cb);
 
 /* netlink attribute policy for the received mapping info ack */
 static const struct nla_policy ack_mapinfo_policy[IWPM_NLA_MAPINFO_NUM_MAX] = {
@@ -707,7 +697,6 @@ int iwpm_ack_mapping_info_cb(struct sk_buff *skb, struct netlink_callback *cb)
 	atomic_set(&echo_nlmsg_seq, cb->nlh->nlmsg_seq);
 	return 0;
 }
-EXPORT_SYMBOL(iwpm_ack_mapping_info_cb);
 
 /* netlink attribute policy for the received port mapper error message */
 static const struct nla_policy map_error_policy[IWPM_NLA_ERR_MAX] = {
@@ -751,4 +740,3 @@ int iwpm_mapping_error_cb(struct sk_buff *skb, struct netlink_callback *cb)
 	up(&nlmsg_request->sem);
 	return 0;
 }
-EXPORT_SYMBOL(iwpm_mapping_error_cb);
diff --git a/drivers/infiniband/core/iwpm_util.c b/drivers/infiniband/core/iwpm_util.c
index 32ca2aaa4e3b..c46442ac71a2 100644
--- a/drivers/infiniband/core/iwpm_util.c
+++ b/drivers/infiniband/core/iwpm_util.c
@@ -81,7 +81,6 @@ int iwpm_init(u8 nl_client)
 	}
 	return ret;
 }
-EXPORT_SYMBOL(iwpm_init);
 
 static void free_hash_bucket(void);
 static void free_reminfo_bucket(void);
@@ -107,7 +106,6 @@ int iwpm_exit(u8 nl_client)
 	iwpm_set_registration(nl_client, IWPM_REG_UNDEF);
 	return 0;
 }
-EXPORT_SYMBOL(iwpm_exit);
 
 static struct hlist_head *get_mapinfo_hash_bucket(struct sockaddr_storage *,
 					       struct sockaddr_storage *);
@@ -146,7 +144,6 @@ int iwpm_create_mapinfo(struct sockaddr_storage *local_sockaddr,
 	spin_unlock_irqrestore(&iwpm_mapinfo_lock, flags);
 	return ret;
 }
-EXPORT_SYMBOL(iwpm_create_mapinfo);
 
 int iwpm_remove_mapinfo(struct sockaddr_storage *local_sockaddr,
 			struct sockaddr_storage *mapped_local_addr)
@@ -182,7 +179,6 @@ int iwpm_remove_mapinfo(struct sockaddr_storage *local_sockaddr,
 	spin_unlock_irqrestore(&iwpm_mapinfo_lock, flags);
 	return ret;
 }
-EXPORT_SYMBOL(iwpm_remove_mapinfo);
 
 static void free_hash_bucket(void)
 {
@@ -295,7 +291,6 @@ int iwpm_get_remote_info(struct sockaddr_storage *mapped_loc_addr,
 	spin_unlock_irqrestore(&iwpm_reminfo_lock, flags);
 	return ret;
 }
-EXPORT_SYMBOL(iwpm_get_remote_info);
 
 struct iwpm_nlmsg_request *iwpm_get_nlmsg_request(__u32 nlmsg_seq,
 					u8 nl_client, gfp_t gfp)
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 07/33] RDMA/netlink: Add flag to consolidate common handing

[Leon Romanovsky]

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

Add ability to provide flags to control RDMA netlink callbacks
and convert addr.c and sa_query.c to be first users of such
infrastructure. It allows to move their CAP_NET_ADMIN checks
into netlink core.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 drivers/infiniband/core/addr.c     |  3 +--
 drivers/infiniband/core/device.c   | 12 +++++++++---
 drivers/infiniband/core/netlink.c  |  4 ++++
 drivers/infiniband/core/sa_query.c |  6 ++----
 include/rdma/rdma_netlink.h        |  6 ++++++
 5 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index 01236cef7bfb..9f3339861ec5 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -134,8 +134,7 @@ int ib_nl_handle_ip_res_resp(struct sk_buff *skb,
 	const struct nlmsghdr *nlh = (struct nlmsghdr *)cb->nlh;
 
 	if ((nlh->nlmsg_flags & NLM_F_REQUEST) ||
-	    !(NETLINK_CB(skb).sk) ||
-	    !netlink_capable(skb, CAP_NET_ADMIN))
+	    !(NETLINK_CB(skb).sk))
 		return -EPERM;
 
 	if (ib_nl_is_good_ip_resp(nlh))
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 298258c40d30..1c5276e6e7f6 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -1088,11 +1088,17 @@ EXPORT_SYMBOL(ib_get_net_dev_by_params);
 
 static const struct ibnl_client_cbs ibnl_ls_cb_table[] = {
 	[RDMA_NL_LS_OP_RESOLVE] = {
-		.dump = ib_nl_handle_resolve_resp},
+		.dump = ib_nl_handle_resolve_resp,
+		.flags = RDMA_NL_ADMIN_PERM,
+	},
 	[RDMA_NL_LS_OP_SET_TIMEOUT] = {
-		.dump = ib_nl_handle_set_timeout},
+		.dump = ib_nl_handle_set_timeout,
+		.flags = RDMA_NL_ADMIN_PERM,
+	},
 	[RDMA_NL_LS_OP_IP_RESOLVE] = {
-		.dump = ib_nl_handle_ip_res_resp},
+		.dump = ib_nl_handle_ip_res_resp,
+		.flags = RDMA_NL_ADMIN_PERM,
+	},
 };
 
 static int __init ib_core_init(void)
diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index 86337d5e7551..a0e25689d3e7 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -171,6 +171,10 @@ static int rdma_nl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (!is_nl_valid(index, op))
 		return -EINVAL;
 
+	if ((rdma_nl_types[index].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.
diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index 70fa4cabe48e..b499f4422f41 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -1033,8 +1033,7 @@ int ib_nl_handle_set_timeout(struct sk_buff *skb,
 	int ret;
 
 	if (!(nlh->nlmsg_flags & NLM_F_REQUEST) ||
-	    !(NETLINK_CB(skb).sk) ||
-	    !netlink_capable(skb, CAP_NET_ADMIN))
+	    !(NETLINK_CB(skb).sk))
 		return -EPERM;
 
 	ret = nla_parse(tb, LS_NLA_TYPE_MAX - 1, nlmsg_data(nlh),
@@ -1109,8 +1108,7 @@ int ib_nl_handle_resolve_resp(struct sk_buff *skb,
 	int ret;
 
 	if ((nlh->nlmsg_flags & NLM_F_REQUEST) ||
-	    !(NETLINK_CB(skb).sk) ||
-	    !netlink_capable(skb, CAP_NET_ADMIN))
+	    !(NETLINK_CB(skb).sk))
 		return -EPERM;
 
 	spin_lock_irqsave(&ib_nl_request_lock, flags);
diff --git a/include/rdma/rdma_netlink.h b/include/rdma/rdma_netlink.h
index 7690aaabb958..79d8802f3ef7 100644
--- a/include/rdma/rdma_netlink.h
+++ b/include/rdma/rdma_netlink.h
@@ -7,6 +7,12 @@
 
 struct ibnl_client_cbs {
 	int (*dump)(struct sk_buff *skb, struct netlink_callback *nlcb);
+	u8 flags;
+};
+
+enum rdma_nl_flags {
+	/* Require CAP_NET_ADMIN */
+	RDMA_NL_ADMIN_PERM	= 1 << 0,
 };
 
 /**
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 08/33] RDMA/netlink: Simplify the put_msg and put_attr

[Leon Romanovsky]

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

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

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 drivers/infiniband/core/netlink.c | 31 +++++++++----------------------
 1 file changed, 9 insertions(+), 22 deletions(-)

diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index a0e25689d3e7..19476a584de3 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -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);
 
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 09/33] RDMA/netlink: Rename and remove redundant parameter from ibnl_unicast

[Leon Romanovsky]

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

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

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 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(-)

diff --git a/drivers/infiniband/core/iwpm_msg.c b/drivers/infiniband/core/iwpm_msg.c
index 1fab707b1f68..8f84557d04e3 100644
--- a/drivers/infiniband/core/iwpm_msg.c
+++ b/drivers/infiniband/core/iwpm_msg.c
@@ -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;
diff --git a/drivers/infiniband/core/iwpm_util.c b/drivers/infiniband/core/iwpm_util.c
index c46442ac71a2..c81c55942626 100644
--- a/drivers/infiniband/core/iwpm_util.c
+++ b/drivers/infiniband/core/iwpm_util.c
@@ -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;
diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index 19476a584de3..95e2453753ff 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -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)
diff --git a/include/rdma/rdma_netlink.h b/include/rdma/rdma_netlink.h
index 79d8802f3ef7..cada662baf16 100644
--- a/include/rdma/rdma_netlink.h
+++ b/include/rdma/rdma_netlink.h
@@ -56,12 +56,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.
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 10/33] RDMA/netlink: Rename and remove redundant parameter from ibnl_multicast

[Leon Romanovsky]

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

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/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 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(-)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index 9f3339861ec5..30cf764824ec 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -184,7 +184,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.
diff --git a/drivers/infiniband/core/iwpm_msg.c b/drivers/infiniband/core/iwpm_msg.c
index 8f84557d04e3..561f312ea35a 100644
--- a/drivers/infiniband/core/iwpm_msg.c
+++ b/drivers/infiniband/core/iwpm_msg.c
@@ -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;
diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index 95e2453753ff..83e62ecddde1 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -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)
 {
diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index b499f4422f41..977f64d0e983 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -861,7 +861,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
diff --git a/include/rdma/rdma_netlink.h b/include/rdma/rdma_netlink.h
index cada662baf16..f10607ce1cf0 100644
--- a/include/rdma/rdma_netlink.h
+++ b/include/rdma/rdma_netlink.h
@@ -64,12 +64,10 @@ 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);
 
 #endif /* _RDMA_NETLINK_H */
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 11/33] RDMA/netlink: Simplify and rename ibnl_chk_listeners

[Leon Romanovsky]

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

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/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 drivers/infiniband/core/addr.c     | 2 +-
 drivers/infiniband/core/netlink.c  | 7 +++----
 drivers/infiniband/core/sa_query.c | 2 +-
 include/rdma/rdma_netlink.h        | 6 ++++++
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index 30cf764824ec..7310ece99cd9 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -325,7 +325,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 */
diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index 83e62ecddde1..d1d187f27297 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -47,12 +47,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(rdma_nl_chk_listeners);
 
 static bool is_nl_msg_valid(unsigned int type, unsigned int op)
 {
diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index 977f64d0e983..2cc85c2b74b7 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -1418,7 +1418,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;
 		}
diff --git a/include/rdma/rdma_netlink.h b/include/rdma/rdma_netlink.h
index f10607ce1cf0..afc364c3220c 100644
--- a/include/rdma/rdma_netlink.h
+++ b/include/rdma/rdma_netlink.h
@@ -70,4 +70,10 @@ int rdma_nl_unicast(struct sk_buff *skb, u32 pid);
  */
 int rdma_nl_multicast(struct sk_buff *skb, unsigned int group, gfp_t flags);
 
+/**
+ * Check if there are any listeners to the netlink group
+ * @group: the netlink group ID
+ * Returns 0 on success or a negative for no listeners.
+ */
+int rdma_nl_chk_listeners(unsigned int group);
 #endif /* _RDMA_NETLINK_H */
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 12/33] RDMA/netlink: Rename netlink callback struct

[Leon Romanovsky]

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

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/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 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(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 0c85f140e616..d8edd8b11561 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -4458,7 +4458,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},
 };
 
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 1c5276e6e7f6..a1d5798fa4d7 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -1086,7 +1086,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,
diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c
index 8599271d8be6..452a3115e3e6 100644
--- a/drivers/infiniband/core/iwcm.c
+++ b/drivers/infiniband/core/iwcm.c
@@ -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},
diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index d1d187f27297..4d8c8b5583c6 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -44,7 +44,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)
@@ -84,7 +84,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)) {
diff --git a/include/rdma/rdma_netlink.h b/include/rdma/rdma_netlink.h
index afc364c3220c..17565a2765c4 100644
--- a/include/rdma/rdma_netlink.h
+++ b/include/rdma/rdma_netlink.h
@@ -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;
 };
@@ -21,7 +21,7 @@ enum rdma_nl_flags {
  * @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.
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 13/33] RDMA/core: Add iterator over ib_devices

[Leon Romanovsky]

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

The coming nldev needs to iterate over all IB devices in the system
and in order to 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.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 drivers/infiniband/core/core_priv.h |  8 ++++++++
 drivers/infiniband/core/device.c    | 25 +++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index e759c27113cd..0c175590cf92 100644
--- a/drivers/infiniband/core/core_priv.h
+++ b/drivers/infiniband/core/core_priv.h
@@ -102,6 +102,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
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index a1d5798fa4d7..54143476a020 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -893,6 +893,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
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 14/33] RDMA/core: Add and expose static device index

[Leon Romanovsky]

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

This patch adds static device index in similar fashion to
already available in netdev world (struct net->ifindex).

In downstream patches, the RDMA nelink will use this idx-to-ib_device
conversion, so as part of this commit, we are exposing the translation
function to be visible for IB/core users.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/core_priv.h |  5 +++++
 drivers/infiniband/core/device.c    | 37 ++++++++++++++++++++++++++++++++++++-
 include/rdma/ib_verbs.h             |  2 ++
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index 0c175590cf92..cbdcc81e1df8 100644
--- a/drivers/infiniband/core/core_priv.h
+++ b/drivers/infiniband/core/core_priv.h
@@ -309,4 +309,9 @@ static inline int ib_mad_enforce_security(struct ib_mad_agent_private *map,
 	return 0;
 }
 #endif
+
+struct ib_device *__ib_device_get_by_index(u32 ifindex);
+/* RDMA device netlink */
+void nldev_init(void);
+void nldev_exit(void);
 #endif /* _CORE_PRIV_H */
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 54143476a020..41eebd6ca0ff 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -134,6 +134,17 @@ static int ib_device_check_mandatory(struct ib_device *device)
 	return 0;
 }
 
+struct ib_device *__ib_device_get_by_index(u32 index)
+{
+	struct ib_device *device;
+
+	list_for_each_entry(device, &device_list, core_list)
+		if (device->index == index)
+			return device;
+
+	return NULL;
+}
+
 static struct ib_device *__ib_device_get_by_name(const char *name)
 {
 	struct ib_device *device;
@@ -145,7 +156,6 @@ static struct ib_device *__ib_device_get_by_name(const char *name)
 	return NULL;
 }
 
-
 static int alloc_name(char *name)
 {
 	unsigned long *inuse;
@@ -395,6 +405,30 @@ static int ib_security_change(struct notifier_block *nb, unsigned long event,
 }
 
 /**
+ *	__dev_new_index	-	allocate an device index
+ *
+ *	Returns a suitable unique value for a new device interface
+ *	number.  It assumes that there are less than 2^32-1 ib devices
+ *	will be present in the system.
+ */
+static u32 __dev_new_index(void)
+{
+	/*
+	 * The device index to allow stable naming.
+	 * Similar to struct net -> ifindex.
+	 */
+	static u32 index;
+
+	for (;;) {
+		if (!(++index))
+			index = 1;
+
+		if (!__ib_device_get_by_index(index))
+			return index;
+	}
+}
+
+/**
  * ib_register_device - Register an IB device with IB core
  * @device:Device to register
  *
@@ -492,6 +526,7 @@ int ib_register_device(struct ib_device *device,
 		if (client->add && !add_client_context(device, client))
 			client->add(device);
 
+	device->index = __dev_new_index();
 	down_write(&lists_rwsem);
 	list_add_tail(&device->core_list, &device_list);
 	up_write(&lists_rwsem);
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 1082b4c81b2c..3391df5fdc9c 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2298,6 +2298,8 @@ struct ib_device {
 	struct rdmacg_device         cg_device;
 #endif
 
+	u32                          index;
+
 	/**
 	 * The following mandatory functions are used only at device
 	 * registration.  Keep functions such as these at the end of this
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 15/33] RDMA/netlink: Add and implement doit netlink callback

[Leon Romanovsky]

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

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/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 drivers/infiniband/core/netlink.c | 19 ++++++++++++++-----
 include/rdma/rdma_netlink.h       |  2 ++
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index 4d8c8b5583c6..f98f9fe86d0a 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -76,9 +76,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;
 }
@@ -153,6 +157,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;
@@ -171,10 +176,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);
 }
 
 /*
diff --git a/include/rdma/rdma_netlink.h b/include/rdma/rdma_netlink.h
index 17565a2765c4..b8b3528ebc08 100644
--- a/include/rdma/rdma_netlink.h
+++ b/include/rdma/rdma_netlink.h
@@ -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;
 };
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 16/33] RDMA/netlink: Reduce indirection access to cb_table

[Leon Romanovsky]

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

Introduce intermediate variable to store access to fields
of cb_table.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 drivers/infiniband/core/netlink.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index f98f9fe86d0a..6c3bb2693afa 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -157,12 +157,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;
 
@@ -174,14 +177,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;
 
 }
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 17/33] RDMA/netlink: Convert LS to doit callback

[Leon Romanovsky]

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

RDMA_NL_LS protocol is actually does 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().

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 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(-)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index 7310ece99cd9..16b3bdffeb96 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -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;
diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index cbdcc81e1df8..57beb1cceda3 100644
--- a/drivers/infiniband/core/core_priv.h
+++ b/drivers/infiniband/core/core_priv.h
@@ -198,11 +198,14 @@ void rdma_nl_exit(void);
 int ibnl_chk_listeners(unsigned int group);
 
 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);
 
 int ib_get_cached_subnet_prefix(struct ib_device *device,
 				u8                port_num,
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 41eebd6ca0ff..5a3ff941c86c 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -1148,15 +1148,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,
 	},
 };
diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index 6c3bb2693afa..3efa161bf4a6 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -155,38 +155,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;
+	/* FIXME: 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;
 }
 
 /*
diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index 2cc85c2b74b7..da29e2863c84 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -1021,9 +1021,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;
@@ -1097,9 +1097,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;
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 18/33] RDMA/netlink: Update copyright

[Leon Romanovsky]

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

Add Mellanox to the copyright header.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 drivers/infiniband/core/netlink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index 3efa161bf4a6..2ca523c7d14a 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -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
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 19/33] RDMA/netlink: Add netlink device definitions to UAPI

[Leon Romanovsky]

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

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 deletes the old defines.

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 index (RDMA_NLDEV_ATTR_DEV_INDEX)
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/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 drivers/infiniband/core/netlink.c |  2 +-
 include/uapi/rdma/rdma_netlink.h  | 39 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index 2ca523c7d14a..a70f3e5483f6 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -61,7 +61,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
diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
index 02fe8390c18f..1d064fedfa9a 100644
--- a/include/uapi/rdma/rdma_netlink.h
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -8,7 +8,7 @@ enum {
 	RDMA_NL_IWCM,
 	RDMA_NL_RSVD,
 	RDMA_NL_LS,	/* RDMA Local Services */
-	RDMA_NL_I40IW,
+	RDMA_NL_NLDEV,	/* RDMA device interface */
 	RDMA_NL_NUM_CLIENTS
 };
 
@@ -222,4 +222,41 @@ struct rdma_nla_ls_gid {
 	__u8		gid[16];
 };
 
+enum rdma_nldev_command {
+	RDMA_NLDEV_CMD_UNSPEC,
+
+	RDMA_NLDEV_CMD_GET, /* can dump */
+	RDMA_NLDEV_CMD_SET,
+	RDMA_NLDEV_CMD_NEW,
+	RDMA_NLDEV_CMD_DEL,
+
+	RDMA_NLDEV_CMD_PORT_GET, /* can dump */
+	RDMA_NLDEV_CMD_PORT_SET,
+	RDMA_NLDEV_CMD_PORT_NEW,
+	RDMA_NLDEV_CMD_PORT_DEL,
+
+	RDMA_NLDEV_NUM_OPS
+};
+
+enum rdma_nldev_attr {
+	/* don't change the order or add anything between, this is ABI! */
+	RDMA_NLDEV_ATTR_UNSPEC,
+
+	/* Identifier for ib_device */
+	RDMA_NLDEV_ATTR_DEV_INDEX,		/* u32 */
+
+	RDMA_NLDEV_ATTR_DEV_NAME,		/* string */
+	/*
+	 * Device index together with port index are identifiers
+	 * for port/link properties.
+	 *
+	 * For RDMA_NLDEV_CMD_GET comamnd, port index will return number
+	 * of available ports in ib_device, while for port specific operations,
+	 * it will be real port index as it appears in sysfs. Port index follows
+	 * sysfs notation and starts from 1 for the first port.
+	 */
+	RDMA_NLDEV_ATTR_PORT_INDEX,		/* u32 */
+
+	RDMA_NLDEV_ATTR_MAX
+};
 #endif /* _UAPI_RDMA_NETLINK_H */
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 20/33] RDMA/netlink: Add nldev initialization flows

[Leon Romanovsky]

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

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

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 drivers/infiniband/core/Makefile |  3 ++-
 drivers/infiniband/core/device.c |  2 ++
 drivers/infiniband/core/nldev.c  | 45 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 drivers/infiniband/core/nldev.c

diff --git a/drivers/infiniband/core/Makefile b/drivers/infiniband/core/Makefile
index e3cdafff8ece..920609a0872e 100644
--- a/drivers/infiniband/core/Makefile
+++ b/drivers/infiniband/core/Makefile
@@ -11,7 +11,8 @@ 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 \
-				security.o
+				security.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
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 5a3ff941c86c..fca126aa001b 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -1212,6 +1212,7 @@ static int __init ib_core_init(void)
 		goto err_sa;
 	}
 
+	nldev_init();
 	rdma_nl_register(RDMA_NL_LS, ibnl_ls_cb_table);
 	ib_cache_setup();
 
@@ -1237,6 +1238,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);
 	unregister_lsm_notifier(&ibdev_lsm_nb);
 	ib_sa_cleanup();
diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
new file mode 100644
index 000000000000..1d1e4f214874
--- /dev/null
+++ b/drivers/infiniband/core/nldev.c
@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the names of the copyright holders nor the names of its
+ *    contributors may be used to endorse or promote products derived from
+ *    this software without specific prior written permission.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2 as published by the Free
+ * Software Foundation.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <rdma/rdma_netlink.h>
+
+#include "core_priv.h"
+
+void __init nldev_init(void)
+{
+	rdma_nl_register(RDMA_NL_NLDEV, NULL);
+}
+
+void __exit nldev_exit(void)
+{
+	rdma_nl_unregister(RDMA_NL_NLDEV);
+}
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 21/33] RDMA/netlink: Implement nldev device dumpit calback

[Leon Romanovsky]

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

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

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 drivers/infiniband/core/nldev.c | 64 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 1d1e4f214874..4b8b0f5d5344 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -30,13 +30,75 @@
  * 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_INDEX]     = { .type = NLA_U32 },
+	[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_u32(msg, RDMA_NLDEV_ATTR_DEV_INDEX, device->index))
+		return -EMSGSIZE;
+	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)
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 22/33] RDMA/netlink: Add nldev device doit implementation

[Leon Romanovsky]

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

Provide ability to query specific device.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 drivers/infiniband/core/nldev.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 4b8b0f5d5344..666940f2b49a 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -53,6 +53,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];
+	struct ib_device *device;
+	struct sk_buff *msg;
+	u32 index;
+	int err;
+
+	err = nlmsg_parse(nlh, 0, tb, RDMA_NLDEV_ATTR_MAX - 1,
+			  nldev_policy, extack);
+	if (err || !tb[RDMA_NLDEV_ATTR_DEV_INDEX])
+		return -EINVAL;
+
+	index = nla_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]);
+
+	device = __ib_device_get_by_index(index);
+	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,
@@ -92,6 +131,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,
 	},
 };
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 23/33] RDMA/netlink: Add nldev port dumpit implementation

[Leon Romanovsky]

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

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

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 drivers/infiniband/core/nldev.c | 61 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 666940f2b49a..4413eda8c435 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -53,6 +53,18 @@ 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_u32(msg, RDMA_NLDEV_ATTR_DEV_INDEX, device->index))
+		return -EMSGSIZE;
+	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)
 {
@@ -129,11 +141,60 @@ 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];
+	struct ib_device *device;
+	int start = cb->args[0];
+	struct nlmsghdr *nlh;
+	u32 idx = 0;
+	u32 ifindex;
+	int err;
+	u32 p;
+
+	err = nlmsg_parse(cb->nlh, 0, tb, RDMA_NLDEV_ATTR_MAX - 1,
+			  nldev_policy, NULL);
+	if (err || !tb[RDMA_NLDEV_ATTR_DEV_INDEX])
+		return -EINVAL;
+
+	ifindex = nla_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]);
+	device = __ib_device_get_by_index(ifindex);
+	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)
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 24/33] RDMA/netlink: Implement nldev port doit callback

[Leon Romanovsky]

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

Provide ability to get specific to device and port information.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 drivers/infiniband/core/nldev.c | 44 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 4413eda8c435..533ac72a05b8 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -141,6 +141,49 @@ 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];
+	struct ib_device *device;
+	struct sk_buff *msg;
+	u32 index;
+	u32 port;
+	int err;
+
+	err = nlmsg_parse(nlh, 0, tb, RDMA_NLDEV_ATTR_MAX - 1,
+			  nldev_policy, extack);
+	if (err || !tb[RDMA_NLDEV_ATTR_PORT_INDEX])
+		return -EINVAL;
+
+	index = nla_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]);
+	device = __ib_device_get_by_index(index);
+	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)
 {
@@ -193,6 +236,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,
 	},
 };
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 25/33] RDMA/netlink: Expose device and port capability masks

[Leon Romanovsky]

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

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/w@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
 drivers/infiniband/core/nldev.c  | 19 +++++++++++++++++++
 include/uapi/rdma/rdma_netlink.h |  5 +++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 533ac72a05b8..21e75928400c 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -50,18 +50,37 @@ 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_u32(msg, RDMA_NLDEV_ATTR_DEV_INDEX, device->index))
 		return -EMSGSIZE;
 	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;
 }
 
diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
index 1d064fedfa9a..8fc5278d07d6 100644
--- a/include/uapi/rdma/rdma_netlink.h
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -257,6 +257,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 */
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 26/33] RDMA: Simplify get firmware interface

[Leon Romanovsky]

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

There is a need to forward FW version to user space
application through RDMA netlink. In order to make it safe, there
is need to declare nla_policy and limit the size of FW string.

The new define IB_FW_VERSION_NAME_MAX will limit the size of
FW version string. That define was chosen to be equal to
ETHTOOL_FWVERS_LEN, because many drivers anyway are limited
by that value indirectly.

The introduction of this define allows us to remove the string size
from get_fw_str function signature.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/device.c               | 4 ++--
 drivers/infiniband/core/sysfs.c                | 4 ++--
 drivers/infiniband/hw/cxgb3/iwch_provider.c    | 5 ++---
 drivers/infiniband/hw/cxgb4/provider.c         | 5 ++---
 drivers/infiniband/hw/hfi1/verbs.c             | 5 ++---
 drivers/infiniband/hw/i40iw/i40iw_verbs.c      | 7 +++----
 drivers/infiniband/hw/mlx4/main.c              | 5 ++---
 drivers/infiniband/hw/mlx5/main.c              | 8 ++++----
 drivers/infiniband/hw/mthca/mthca_provider.c   | 5 ++---
 drivers/infiniband/hw/nes/nes_verbs.c          | 5 ++---
 drivers/infiniband/hw/ocrdma/ocrdma_main.c     | 5 ++---
 drivers/infiniband/hw/qedr/main.c              | 5 ++---
 drivers/infiniband/hw/usnic/usnic_ib_main.c    | 6 ++----
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c | 5 ++---
 drivers/infiniband/ulp/ipoib/ipoib_ethtool.c   | 3 +--
 include/rdma/ib_verbs.h                        | 6 ++++--
 16 files changed, 36 insertions(+), 47 deletions(-)

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index fca126aa001b..75220569b00e 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -336,10 +336,10 @@ static int read_port_immutable(struct ib_device *device)
 	return 0;
 }
 
-void ib_get_device_fw_str(struct ib_device *dev, char *str, size_t str_len)
+void ib_get_device_fw_str(struct ib_device *dev, char *str)
 {
 	if (dev->get_dev_fw_str)
-		dev->get_dev_fw_str(dev, str, str_len);
+		dev->get_dev_fw_str(dev, str);
 	else
 		str[0] = '\0';
 }
diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
index 7ebe1ef23652..abc5ab581f82 100644
--- a/drivers/infiniband/core/sysfs.c
+++ b/drivers/infiniband/core/sysfs.c
@@ -1210,8 +1210,8 @@ static ssize_t show_fw_ver(struct device *device, struct device_attribute *attr,
 {
 	struct ib_device *dev = container_of(device, struct ib_device, dev);
 
-	ib_get_device_fw_str(dev, buf, PAGE_SIZE);
-	strlcat(buf, "\n", PAGE_SIZE);
+	ib_get_device_fw_str(dev, buf);
+	strlcat(buf, "\n", IB_FW_VERSION_NAME_MAX);
 	return strlen(buf);
 }
 
diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c
index 0cd0c1fa27d4..099e76f3758a 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_provider.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c
@@ -1336,8 +1336,7 @@ static int iwch_port_immutable(struct ib_device *ibdev, u8 port_num,
 	return 0;
 }
 
-static void get_dev_fw_ver_str(struct ib_device *ibdev, char *str,
-			       size_t str_len)
+static void get_dev_fw_ver_str(struct ib_device *ibdev, char *str)
 {
 	struct iwch_dev *iwch_dev = to_iwch_dev(ibdev);
 	struct ethtool_drvinfo info;
@@ -1345,7 +1344,7 @@ static void get_dev_fw_ver_str(struct ib_device *ibdev, char *str,
 
 	pr_debug("%s dev 0x%p\n", __func__, iwch_dev);
 	lldev->ethtool_ops->get_drvinfo(lldev, &info);
-	snprintf(str, str_len, "%s", info.fw_version);
+	snprintf(str, IB_FW_VERSION_NAME_MAX, "%s", info.fw_version);
 }
 
 int iwch_register_device(struct iwch_dev *dev)
diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c
index 0771e9a4d061..346e8334279a 100644
--- a/drivers/infiniband/hw/cxgb4/provider.c
+++ b/drivers/infiniband/hw/cxgb4/provider.c
@@ -517,14 +517,13 @@ static int c4iw_port_immutable(struct ib_device *ibdev, u8 port_num,
 	return 0;
 }
 
-static void get_dev_fw_str(struct ib_device *dev, char *str,
-			   size_t str_len)
+static void get_dev_fw_str(struct ib_device *dev, char *str)
 {
 	struct c4iw_dev *c4iw_dev = container_of(dev, struct c4iw_dev,
 						 ibdev);
 	pr_debug("%s dev 0x%p\n", __func__, dev);
 
-	snprintf(str, str_len, "%u.%u.%u.%u",
+	snprintf(str, IB_FW_VERSION_NAME_MAX, "%u.%u.%u.%u",
 		 FW_HDR_FW_VER_MAJOR_G(c4iw_dev->rdev.lldi.fw_vers),
 		 FW_HDR_FW_VER_MINOR_G(c4iw_dev->rdev.lldi.fw_vers),
 		 FW_HDR_FW_VER_MICRO_G(c4iw_dev->rdev.lldi.fw_vers),
diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
index dc51bf247006..c88c03c11555 100644
--- a/drivers/infiniband/hw/hfi1/verbs.c
+++ b/drivers/infiniband/hw/hfi1/verbs.c
@@ -1561,14 +1561,13 @@ static void init_ibport(struct hfi1_pportdata *ppd)
 	RCU_INIT_POINTER(ibp->rvp.qp[1], NULL);
 }
 
-static void hfi1_get_dev_fw_str(struct ib_device *ibdev, char *str,
-				size_t str_len)
+static void hfi1_get_dev_fw_str(struct ib_device *ibdev, char *str)
 {
 	struct rvt_dev_info *rdi = ib_to_rvt(ibdev);
 	struct hfi1_ibdev *dev = dev_from_rdi(rdi);
 	u32 ver = dd_from_dev(dev)->dc8051_ver;
 
-	snprintf(str, str_len, "%u.%u.%u", dc8051_ver_maj(ver),
+	snprintf(str, IB_FW_VERSION_NAME_MAX, "%u.%u.%u", dc8051_ver_maj(ver),
 		 dc8051_ver_min(ver), dc8051_ver_patch(ver));
 }
 
diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
index 02d871db7ca5..1aa411034a27 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
@@ -2584,13 +2584,12 @@ static const char * const i40iw_hw_stat_names[] = {
 		"iwRdmaInv"
 };
 
-static void i40iw_get_dev_fw_str(struct ib_device *dev, char *str,
-				 size_t str_len)
+static void i40iw_get_dev_fw_str(struct ib_device *dev, char *str)
 {
 	u32 firmware_version = I40IW_FW_VERSION;
 
-	snprintf(str, str_len, "%u.%u", firmware_version,
-		       (firmware_version & 0x000000ff));
+	snprintf(str, IB_FW_VERSION_NAME_MAX, "%u.%u", firmware_version,
+		 (firmware_version & 0x000000ff));
 }
 
 /**
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index 1f25a37eb056..c636842c5be0 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -2587,12 +2587,11 @@ static int mlx4_port_immutable(struct ib_device *ibdev, u8 port_num,
 	return 0;
 }
 
-static void get_fw_ver_str(struct ib_device *device, char *str,
-			   size_t str_len)
+static void get_fw_ver_str(struct ib_device *device, char *str)
 {
 	struct mlx4_ib_dev *dev =
 		container_of(device, struct mlx4_ib_dev, ib_dev);
-	snprintf(str, str_len, "%d.%d.%d",
+	snprintf(str, IB_FW_VERSION_NAME_MAX, "%d.%d.%d",
 		 (int) (dev->dev->caps.fw_ver >> 32),
 		 (int) (dev->dev->caps.fw_ver >> 16) & 0xffff,
 		 (int) dev->dev->caps.fw_ver & 0xffff);
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 9279631d8da0..0a5a4e3fa66d 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -3285,13 +3285,13 @@ static int mlx5_port_immutable(struct ib_device *ibdev, u8 port_num,
 	return 0;
 }
 
-static void get_dev_fw_str(struct ib_device *ibdev, char *str,
-			   size_t str_len)
+static void get_dev_fw_str(struct ib_device *ibdev, char *str)
 {
 	struct mlx5_ib_dev *dev =
 		container_of(ibdev, struct mlx5_ib_dev, ib_dev);
-	snprintf(str, str_len, "%d.%d.%04d", fw_rev_maj(dev->mdev),
-		       fw_rev_min(dev->mdev), fw_rev_sub(dev->mdev));
+	snprintf(str, IB_FW_VERSION_NAME_MAX, "%d.%d.%04d",
+		 fw_rev_maj(dev->mdev), fw_rev_min(dev->mdev),
+		 fw_rev_sub(dev->mdev));
 }
 
 static int mlx5_eth_lag_init(struct mlx5_ib_dev *dev)
diff --git a/drivers/infiniband/hw/mthca/mthca_provider.c b/drivers/infiniband/hw/mthca/mthca_provider.c
index c197cd9b193f..eae9bffd45d4 100644
--- a/drivers/infiniband/hw/mthca/mthca_provider.c
+++ b/drivers/infiniband/hw/mthca/mthca_provider.c
@@ -1178,12 +1178,11 @@ static int mthca_port_immutable(struct ib_device *ibdev, u8 port_num,
 	return 0;
 }
 
-static void get_dev_fw_str(struct ib_device *device, char *str,
-			   size_t str_len)
+static void get_dev_fw_str(struct ib_device *device, char *str)
 {
 	struct mthca_dev *dev =
 		container_of(device, struct mthca_dev, ib_dev);
-	snprintf(str, str_len, "%d.%d.%d",
+	snprintf(str, IB_FW_VERSION_NAME_MAX, "%d.%d.%d",
 		 (int) (dev->fw_ver >> 32),
 		 (int) (dev->fw_ver >> 16) & 0xffff,
 		 (int) dev->fw_ver & 0xffff);
diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
index 25dcd7573df9..c2943e39d2f9 100644
--- a/drivers/infiniband/hw/nes/nes_verbs.c
+++ b/drivers/infiniband/hw/nes/nes_verbs.c
@@ -3672,15 +3672,14 @@ static int nes_port_immutable(struct ib_device *ibdev, u8 port_num,
 	return 0;
 }
 
-static void get_dev_fw_str(struct ib_device *dev, char *str,
-			   size_t str_len)
+static void get_dev_fw_str(struct ib_device *dev, char *str)
 {
 	struct nes_ib_device *nesibdev =
 			container_of(dev, struct nes_ib_device, ibdev);
 	struct nes_vnic *nesvnic = nesibdev->nesvnic;
 
 	nes_debug(NES_DBG_INIT, "\n");
-	snprintf(str, str_len, "%u.%u",
+	snprintf(str, IB_FW_VERSION_NAME_MAX, "%u.%u",
 		 (nesvnic->nesdev->nesadapter->firmware_version >> 16),
 		 (nesvnic->nesdev->nesadapter->firmware_version & 0x000000ff));
 }
diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_main.c b/drivers/infiniband/hw/ocrdma/ocrdma_main.c
index 757c65816295..fbfbd9e96147 100644
--- a/drivers/infiniband/hw/ocrdma/ocrdma_main.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_main.c
@@ -107,12 +107,11 @@ static int ocrdma_port_immutable(struct ib_device *ibdev, u8 port_num,
 	return 0;
 }
 
-static void get_dev_fw_str(struct ib_device *device, char *str,
-			   size_t str_len)
+static void get_dev_fw_str(struct ib_device *device, char *str)
 {
 	struct ocrdma_dev *dev = get_ocrdma_dev(device);
 
-	snprintf(str, str_len, "%s", &dev->attr.fw_ver[0]);
+	snprintf(str, IB_FW_VERSION_NAME_MAX, "%s", &dev->attr.fw_ver[0]);
 }
 
 static int ocrdma_register_device(struct ocrdma_dev *dev)
diff --git a/drivers/infiniband/hw/qedr/main.c b/drivers/infiniband/hw/qedr/main.c
index 199b6edbef92..97d033f51dc9 100644
--- a/drivers/infiniband/hw/qedr/main.c
+++ b/drivers/infiniband/hw/qedr/main.c
@@ -68,13 +68,12 @@ static enum rdma_link_layer qedr_link_layer(struct ib_device *device,
 	return IB_LINK_LAYER_ETHERNET;
 }
 
-static void qedr_get_dev_fw_str(struct ib_device *ibdev, char *str,
-				size_t str_len)
+static void qedr_get_dev_fw_str(struct ib_device *ibdev, char *str)
 {
 	struct qedr_dev *qedr = get_qedr_dev(ibdev);
 	u32 fw_ver = (u32)qedr->attr.fw_ver;
 
-	snprintf(str, str_len, "%d. %d. %d. %d",
+	snprintf(str, IB_FW_VERSION_NAME_MAX, "%d. %d. %d. %d",
 		 (fw_ver >> 24) & 0xFF, (fw_ver >> 16) & 0xFF,
 		 (fw_ver >> 8) & 0xFF, fw_ver & 0xFF);
 }
diff --git a/drivers/infiniband/hw/usnic/usnic_ib_main.c b/drivers/infiniband/hw/usnic/usnic_ib_main.c
index e69c8e476a2b..e86700f994cb 100644
--- a/drivers/infiniband/hw/usnic/usnic_ib_main.c
+++ b/drivers/infiniband/hw/usnic/usnic_ib_main.c
@@ -333,9 +333,7 @@ static int usnic_port_immutable(struct ib_device *ibdev, u8 port_num,
 	return 0;
 }
 
-static void usnic_get_dev_fw_str(struct ib_device *device,
-				 char *str,
-				 size_t str_len)
+static void usnic_get_dev_fw_str(struct ib_device *device, char *str)
 {
 	struct usnic_ib_dev *us_ibdev =
 		container_of(device, struct usnic_ib_dev, ib_dev);
@@ -345,7 +343,7 @@ static void usnic_get_dev_fw_str(struct ib_device *device,
 	us_ibdev->netdev->ethtool_ops->get_drvinfo(us_ibdev->netdev, &info);
 	mutex_unlock(&us_ibdev->usdev_lock);
 
-	snprintf(str, str_len, "%s", info.fw_version);
+	snprintf(str, IB_FW_VERSION_NAME_MAX, "%s", info.fw_version);
 }
 
 /* Start of PF discovery section */
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
index e76565280afa..7f29e4db28a1 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
@@ -102,12 +102,11 @@ static struct device_attribute *pvrdma_class_attributes[] = {
 	&dev_attr_board_id
 };
 
-static void pvrdma_get_fw_ver_str(struct ib_device *device, char *str,
-				  size_t str_len)
+static void pvrdma_get_fw_ver_str(struct ib_device *device, char *str)
 {
 	struct pvrdma_dev *dev =
 		container_of(device, struct pvrdma_dev, ib_dev);
-	snprintf(str, str_len, "%d.%d.%d\n",
+	snprintf(str, IB_FW_VERSION_NAME_MAX, "%d.%d.%d\n",
 		 (int) (dev->dsr->caps.fw_ver >> 32),
 		 (int) (dev->dsr->caps.fw_ver >> 16) & 0xffff,
 		 (int) dev->dsr->caps.fw_ver & 0xffff);
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
index 7871379342f4..98e30b41e436 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
@@ -62,8 +62,7 @@ static void ipoib_get_drvinfo(struct net_device *netdev,
 {
 	struct ipoib_dev_priv *priv = ipoib_priv(netdev);
 
-	ib_get_device_fw_str(priv->ca, drvinfo->fw_version,
-			     sizeof(drvinfo->fw_version));
+	ib_get_device_fw_str(priv->ca, drvinfo->fw_version);
 
 	strlcpy(drvinfo->bus_info, dev_name(priv->ca->dev.parent),
 		sizeof(drvinfo->bus_info));
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 3391df5fdc9c..e0e87a1f66fb 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -64,6 +64,8 @@
 #include <linux/cgroup_rdma.h>
 #include <uapi/rdma/ib_user_verbs.h>
 
+#define IB_FW_VERSION_NAME_MAX	ETHTOOL_FWVERS_LEN
+
 extern struct workqueue_struct *ib_wq;
 extern struct workqueue_struct *ib_comp_wq;
 
@@ -2307,7 +2309,7 @@ struct ib_device {
 	 * in fast paths.
 	 */
 	int (*get_port_immutable)(struct ib_device *, u8, struct ib_port_immutable *);
-	void (*get_dev_fw_str)(struct ib_device *, char *str, size_t str_len);
+	void (*get_dev_fw_str)(struct ib_device *, char *str);
 };
 
 struct ib_client {
@@ -2343,7 +2345,7 @@ struct ib_client {
 struct ib_device *ib_alloc_device(size_t size);
 void ib_dealloc_device(struct ib_device *device);
 
-void ib_get_device_fw_str(struct ib_device *device, char *str, size_t str_len);
+void ib_get_device_fw_str(struct ib_device *device, char *str);
 
 int ib_register_device(struct ib_device *device,
 		       int (*port_callback)(struct ib_device *,
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 27/33] RDMA/netlink: Export FW version

[Leon Romanovsky]

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

Add FW version to the device properties exported
by RDMA netlink, to be used by RDMAtool.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/nldev.c  | 9 +++++++++
 include/uapi/rdma/rdma_netlink.h | 4 ++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 21e75928400c..40d5f0eb16f5 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -40,10 +40,14 @@ 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 },
+	[RDMA_NLDEV_ATTR_FW_VERSION]	= { .type = NLA_NUL_STRING,
+					    .len = IB_FW_VERSION_NAME_MAX - 1},
 };
 
 static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
 {
+	char fw[IB_FW_VERSION_NAME_MAX];
+
 	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_DEV_INDEX, device->index))
 		return -EMSGSIZE;
 	if (nla_put_string(msg, RDMA_NLDEV_ATTR_DEV_NAME, device->name))
@@ -56,6 +60,11 @@ static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
 			      device->attrs.device_cap_flags, 0))
 		return -EMSGSIZE;
 
+	ib_get_device_fw_str(device, fw);
+	/* Device without FW has strlen(fw) */
+	if (strlen(fw) && nla_put_string(msg, RDMA_NLDEV_ATTR_FW_VERSION, fw))
+		return -EMSGSIZE;
+
 	return 0;
 }
 
diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
index 8fc5278d07d6..fec735f73eee 100644
--- a/include/uapi/rdma/rdma_netlink.h
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -262,6 +262,10 @@ enum rdma_nldev_attr {
 	 */
 	RDMA_NLDEV_ATTR_CAP_FLAGS,		/* u64 */
 
+	/*
+	 * FW version
+	 */
+	RDMA_NLDEV_ATTR_FW_VERSION,		/* string */
 	RDMA_NLDEV_ATTR_MAX
 };
 #endif /* _UAPI_RDMA_NETLINK_H */
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 28/33] RDMA/netlink: Export node_guid and sys_image_guid

[Leon Romanovsky]

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

Add Node GUID and system image GUID to the device properties
exported by RDMA netlink, to be used by RDMAtool.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/nldev.c  |  8 ++++++++
 include/uapi/rdma/rdma_netlink.h | 13 +++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 40d5f0eb16f5..fde0138fff5f 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -42,6 +42,8 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
 	[RDMA_NLDEV_ATTR_PORT_INDEX]	= { .type = NLA_U32 },
 	[RDMA_NLDEV_ATTR_FW_VERSION]	= { .type = NLA_NUL_STRING,
 					    .len = IB_FW_VERSION_NAME_MAX - 1},
+	[RDMA_NLDEV_ATTR_NODE_GUID]	= { .type = NLA_U64 },
+	[RDMA_NLDEV_ATTR_SYS_IMAGE_GUID] = { .type = NLA_U64 },
 };
 
 static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
@@ -65,6 +67,12 @@ static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
 	if (strlen(fw) && nla_put_string(msg, RDMA_NLDEV_ATTR_FW_VERSION, fw))
 		return -EMSGSIZE;
 
+	if (nla_put_u64_64bit(msg, RDMA_NLDEV_ATTR_NODE_GUID,
+			      be64_to_cpu(device->node_guid), 0))
+		return -EMSGSIZE;
+	if (nla_put_u64_64bit(msg, RDMA_NLDEV_ATTR_SYS_IMAGE_GUID,
+			      be64_to_cpu(device->attrs.sys_image_guid), 0))
+		return -EMSGSIZE;
 	return 0;
 }
 
diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
index fec735f73eee..b34f7745daf0 100644
--- a/include/uapi/rdma/rdma_netlink.h
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -266,6 +266,19 @@ enum rdma_nldev_attr {
 	 * FW version
 	 */
 	RDMA_NLDEV_ATTR_FW_VERSION,		/* string */
+
+	/*
+	 * Node GUID (in host byte order) associated with the RDMA device.
+	 */
+	RDMA_NLDEV_ATTR_NODE_GUID,			/* u64 */
+
+	/*
+	 * System image GUID (in host byte order) associated with
+	 * this RDMA device and other devices which are part of a
+	 * single system.
+	 */
+	RDMA_NLDEV_ATTR_SYS_IMAGE_GUID,		/* u64 */
+
 	RDMA_NLDEV_ATTR_MAX
 };
 #endif /* _UAPI_RDMA_NETLINK_H */
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 29/33] RDMA/netlink: Advertise IB subnet prefix

[Leon Romanovsky]

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

Add IB subnet prefix to the port properties exported
by RDMA netlink.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/nldev.c  | 5 +++++
 include/uapi/rdma/rdma_netlink.h | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index fde0138fff5f..c4884841e2fe 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -44,6 +44,7 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
 					    .len = IB_FW_VERSION_NAME_MAX - 1},
 	[RDMA_NLDEV_ATTR_NODE_GUID]	= { .type = NLA_U64 },
 	[RDMA_NLDEV_ATTR_SYS_IMAGE_GUID] = { .type = NLA_U64 },
+	[RDMA_NLDEV_ATTR_SUBNET_PREFIX]	= { .type = NLA_U64 },
 };
 
 static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
@@ -97,6 +98,10 @@ static int fill_port_info(struct sk_buff *msg,
 	if (nla_put_u64_64bit(msg, RDMA_NLDEV_ATTR_CAP_FLAGS,
 			      (u64)attr.port_cap_flags, 0))
 		return -EMSGSIZE;
+	if (rdma_protocol_ib(device, port) &&
+	    nla_put_u64_64bit(msg, RDMA_NLDEV_ATTR_SUBNET_PREFIX,
+			      attr.subnet_prefix, 0))
+		return -EMSGSIZE;
 
 	return 0;
 }
diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
index b34f7745daf0..24712d810dca 100644
--- a/include/uapi/rdma/rdma_netlink.h
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -279,6 +279,11 @@ enum rdma_nldev_attr {
 	 */
 	RDMA_NLDEV_ATTR_SYS_IMAGE_GUID,		/* u64 */
 
+	/*
+	 * Subnet prefix (in host byte order)
+	 */
+	RDMA_NLDEV_ATTR_SUBNET_PREFIX,		/* u64 */
+
 	RDMA_NLDEV_ATTR_MAX
 };
 #endif /* _UAPI_RDMA_NETLINK_H */
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 30/33] RDMA/netink: Export lids and sm_lids

[Leon Romanovsky]

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

According to the IB specification, the LID and SM_LID
are 16-bit wide, but to support OmniPath users, export
it as 32-bit value from the beginning.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/nldev.c  | 9 ++++++++-
 include/uapi/rdma/rdma_netlink.h | 8 ++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index c4884841e2fe..ce733bf33ed9 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -45,6 +45,8 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
 	[RDMA_NLDEV_ATTR_NODE_GUID]	= { .type = NLA_U64 },
 	[RDMA_NLDEV_ATTR_SYS_IMAGE_GUID] = { .type = NLA_U64 },
 	[RDMA_NLDEV_ATTR_SUBNET_PREFIX]	= { .type = NLA_U64 },
+	[RDMA_NLDEV_ATTR_LID]		= { .type = NLA_U32 },
+	[RDMA_NLDEV_ATTR_SM_LID]	= { .type = NLA_U32 },
 };
 
 static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
@@ -102,7 +104,12 @@ static int fill_port_info(struct sk_buff *msg,
 	    nla_put_u64_64bit(msg, RDMA_NLDEV_ATTR_SUBNET_PREFIX,
 			      attr.subnet_prefix, 0))
 		return -EMSGSIZE;
-
+	if (rdma_protocol_ib(device, port)) {
+		if (nla_put_u32(msg, RDMA_NLDEV_ATTR_LID, attr.lid))
+			return -EMSGSIZE;
+		if (nla_put_u32(msg, RDMA_NLDEV_ATTR_SM_LID, attr.sm_lid))
+			return -EMSGSIZE;
+	}
 	return 0;
 }
 
diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
index 24712d810dca..82e0d64346e1 100644
--- a/include/uapi/rdma/rdma_netlink.h
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -284,6 +284,14 @@ enum rdma_nldev_attr {
 	 */
 	RDMA_NLDEV_ATTR_SUBNET_PREFIX,		/* u64 */
 
+	/*
+	 * Local Identifier (LID),
+	 * According to IB specification, It is 16-bit address assigned
+	 * by the Subnet Manager. Extended to be 32-bit for OmniPath users.
+	 */
+	RDMA_NLDEV_ATTR_LID,			/* u32 */
+	RDMA_NLDEV_ATTR_SM_LID,			/* u32 */
+
 	RDMA_NLDEV_ATTR_MAX
 };
 #endif /* _UAPI_RDMA_NETLINK_H */
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 31/33] RDMA/netlink: Export LID mask control (LMC)

[Leon Romanovsky]

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

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/nldev.c  | 3 +++
 include/uapi/rdma/rdma_netlink.h | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index ce733bf33ed9..f968a40ef5d3 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -47,6 +47,7 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
 	[RDMA_NLDEV_ATTR_SUBNET_PREFIX]	= { .type = NLA_U64 },
 	[RDMA_NLDEV_ATTR_LID]		= { .type = NLA_U32 },
 	[RDMA_NLDEV_ATTR_SM_LID]	= { .type = NLA_U32 },
+	[RDMA_NLDEV_ATTR_LMC]		= { .type = NLA_U8 },
 };
 
 static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
@@ -109,6 +110,8 @@ static int fill_port_info(struct sk_buff *msg,
 			return -EMSGSIZE;
 		if (nla_put_u32(msg, RDMA_NLDEV_ATTR_SM_LID, attr.sm_lid))
 			return -EMSGSIZE;
+		if (nla_put_u8(msg, RDMA_NLDEV_ATTR_LMC, attr.lmc))
+			return -EMSGSIZE;
 	}
 	return 0;
 }
diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
index 82e0d64346e1..b85a08a7255f 100644
--- a/include/uapi/rdma/rdma_netlink.h
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -292,6 +292,11 @@ enum rdma_nldev_attr {
 	RDMA_NLDEV_ATTR_LID,			/* u32 */
 	RDMA_NLDEV_ATTR_SM_LID,			/* u32 */
 
+	/*
+	 * LID mask control (LMC)
+	 */
+	RDMA_NLDEV_ATTR_LMC,			/* u8 */
+
 	RDMA_NLDEV_ATTR_MAX
 };
 #endif /* _UAPI_RDMA_NETLINK_H */
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 32/33] RDMA/netlink: Provide port state and physical link state

[Leon Romanovsky]

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

Add port state and physical link state to the users of RDMA netlink.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/nldev.c  | 6 ++++++
 include/uapi/rdma/rdma_netlink.h | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index f968a40ef5d3..30fe1e9ff5ce 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -48,6 +48,8 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
 	[RDMA_NLDEV_ATTR_LID]		= { .type = NLA_U32 },
 	[RDMA_NLDEV_ATTR_SM_LID]	= { .type = NLA_U32 },
 	[RDMA_NLDEV_ATTR_LMC]		= { .type = NLA_U8 },
+	[RDMA_NLDEV_ATTR_PORT_STATE]	= { .type = NLA_U8 },
+	[RDMA_NLDEV_ATTR_PORT_PHYS_STATE] = { .type = NLA_U8 },
 };
 
 static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
@@ -113,6 +115,10 @@ static int fill_port_info(struct sk_buff *msg,
 		if (nla_put_u8(msg, RDMA_NLDEV_ATTR_LMC, attr.lmc))
 			return -EMSGSIZE;
 	}
+	if (nla_put_u8(msg, RDMA_NLDEV_ATTR_PORT_STATE, attr.state))
+		return -EMSGSIZE;
+	if (nla_put_u8(msg, RDMA_NLDEV_ATTR_PORT_PHYS_STATE, attr.phys_state))
+		return -EMSGSIZE;
 	return 0;
 }
 
diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
index b85a08a7255f..ea8e1f01ba0d 100644
--- a/include/uapi/rdma/rdma_netlink.h
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -297,6 +297,9 @@ enum rdma_nldev_attr {
 	 */
 	RDMA_NLDEV_ATTR_LMC,			/* u8 */
 
+	RDMA_NLDEV_ATTR_PORT_STATE,		/* u8 */
+	RDMA_NLDEV_ATTR_PORT_PHYS_STATE,	/* u8 */
+
 	RDMA_NLDEV_ATTR_MAX
 };
 #endif /* _UAPI_RDMA_NETLINK_H */
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[rdma-next 33/33] RDMA/netlink: Export node_type

[Leon Romanovsky]

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

Add ability to get node_type for RDAM netlink users.

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/nldev.c  | 3 +++
 include/uapi/rdma/rdma_netlink.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 30fe1e9ff5ce..7f04ef89398c 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -50,6 +50,7 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
 	[RDMA_NLDEV_ATTR_LMC]		= { .type = NLA_U8 },
 	[RDMA_NLDEV_ATTR_PORT_STATE]	= { .type = NLA_U8 },
 	[RDMA_NLDEV_ATTR_PORT_PHYS_STATE] = { .type = NLA_U8 },
+	[RDMA_NLDEV_ATTR_DEV_NODE_TYPE] = { .type = NLA_U8 },
 };
 
 static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
@@ -79,6 +80,8 @@ static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
 	if (nla_put_u64_64bit(msg, RDMA_NLDEV_ATTR_SYS_IMAGE_GUID,
 			      be64_to_cpu(device->attrs.sys_image_guid), 0))
 		return -EMSGSIZE;
+	if (nla_put_u8(msg, RDMA_NLDEV_ATTR_DEV_NODE_TYPE, device->node_type))
+		return -EMSGSIZE;
 	return 0;
 }
 
diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
index ea8e1f01ba0d..8857c5de0fec 100644
--- a/include/uapi/rdma/rdma_netlink.h
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -300,6 +300,8 @@ enum rdma_nldev_attr {
 	RDMA_NLDEV_ATTR_PORT_STATE,		/* u8 */
 	RDMA_NLDEV_ATTR_PORT_PHYS_STATE,	/* u8 */
 
+	RDMA_NLDEV_ATTR_DEV_NODE_TYPE,		/* u8 */
+
 	RDMA_NLDEV_ATTR_MAX
 };
 #endif /* _UAPI_RDMA_NETLINK_H */
-- 
2.13.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Chien Tin Tung]

On Tue, Aug 01, 2017 at 03:05:04PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> The commit cea05eadded0 ("IB/core: Add flow control to the portmapper netlink calls")
> changed netlink to be blocked for all RDMA clients. This workaround
> worked perfectly for portmapper, but is not correct for the whole
> NETLINK_RDMA family.

Leon,

I've already told you I'm opposed to the revert.  There is a patch that will work
with your usage of ibnl_unicast() but you chose to abandon that discussion on June 29, 2017
(RDMA/core: Add wait/retry version of ibnl_unicast).  Either you step up with good sound
technical evidence to convince me that patch won't work for you or you stop trying to 
break existing functionality with this revert.

Chien
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Leon Romanovsky]

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

On Tue, Aug 01, 2017 at 08:38:32AM -0500, Chien Tin Tung wrote:
> On Tue, Aug 01, 2017 at 03:05:04PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > The commit cea05eadded0 ("IB/core: Add flow control to the portmapper netlink calls")
> > changed netlink to be blocked for all RDMA clients. This workaround
> > worked perfectly for portmapper, but is not correct for the whole
> > NETLINK_RDMA family.
>
> Leon,
>
> I've already told you I'm opposed to the revert.  There is a patch that will work
> with your usage of ibnl_unicast() but you chose to abandon that discussion on June 29, 2017
> (RDMA/core: Add wait/retry version of ibnl_unicast).  Either you step up with good sound
> technical evidence to convince me that patch won't work for you or you stop trying to
> break existing functionality with this revert.

Chien,

You never explained us what exactly your original patch fixed and why it
should be fixed in kernel and not in user space. I saw that our
discussion wasn't useful and brought bad blood instead of good will,
so I stepped out in waited for RDMA maintainer (Doug) step in.

I never heard anything from Doug on the matter, so proceeded and
resubmitted it.

Thanks

>
> Chien

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

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Chien Tin Tung]

On Tue, Aug 01, 2017 at 05:10:23PM +0300, Leon Romanovsky wrote:
> On Tue, Aug 01, 2017 at 08:38:32AM -0500, Chien Tin Tung wrote:
> > On Tue, Aug 01, 2017 at 03:05:04PM +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > >
> > > The commit cea05eadded0 ("IB/core: Add flow control to the portmapper netlink calls")
> > > changed netlink to be blocked for all RDMA clients. This workaround
> > > worked perfectly for portmapper, but is not correct for the whole
> > > NETLINK_RDMA family.
> >
> > Leon,
> >
> > I've already told you I'm opposed to the revert.  There is a patch that will work
> > with your usage of ibnl_unicast() but you chose to abandon that discussion on June 29, 2017
> > (RDMA/core: Add wait/retry version of ibnl_unicast).  Either you step up with good sound
> > technical evidence to convince me that patch won't work for you or you stop trying to
> > break existing functionality with this revert.
> 
> Chien,
> 
> You never explained us what exactly your original patch fixed and why it
> should be fixed in kernel and not in user space. I saw that our
> discussion wasn't useful and brought bad blood instead of good will,
> so I stepped out in waited for RDMA maintainer (Doug) step in.

Why do I need to explain the original patch?  It was accepted many kernels
ago.  Your questions on the original were based on false assumptions and
facts which I've proven over and over.  You are right in that I do
not want to revisit those either.  There is a patch that can solve
the problem you are facing but yet you insist on the revert.  This is
very puzzling to me.  You simply refuse to move this forward.  That
is your choice but revert is a no go for me.

> I never heard anything from Doug on the matter, so proceeded and
> resubmitted it.

Fine.  Your choice to resubmit and my choice to say no to the revert.

Chien


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Christopher Lameter]

On Tue, 1 Aug 2017, Chien Tin Tung wrote:

> Why do I need to explain the original patch?  It was accepted many kernels
> ago.  Your questions on the original were based on false assumptions and
> facts which I've proven over and over.  You are right in that I do
> not want to revisit those either.  There is a patch that can solve
> the problem you are facing but yet you insist on the revert.  This is
> very puzzling to me.  You simply refuse to move this forward.  That
> is your choice but revert is a no go for me.

Ok this is pretty confusing to someone not involved in the prior
discussions. Could both of your stop attacking each other and start
talking about the technical issues?

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Leon Romanovsky]

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

On Tue, Aug 01, 2017 at 09:22:47AM -0500, Christopher Lameter wrote:
> On Tue, 1 Aug 2017, Chien Tin Tung wrote:
>
> > Why do I need to explain the original patch?  It was accepted many kernels
> > ago.  Your questions on the original were based on false assumptions and
> > facts which I've proven over and over.  You are right in that I do
> > not want to revisit those either.  There is a patch that can solve
> > the problem you are facing but yet you insist on the revert.  This is
> > very puzzling to me.  You simply refuse to move this forward.  That
> > is your choice but revert is a no go for me.
>
> Ok this is pretty confusing to someone not involved in the prior
> discussions. Could both of your stop attacking each other and start
> talking about the technical issues?
>

Christopher,

I'm reverting the patch which made two things to RDMA netlink - It made
the netlink stack blocking, which shouldn't. And reinvented the wheel with
setting timeout on socket, instead of using proper and already exposed API.

I tried to explain it in commit message.

Chien sent patch which supposed to replace my revert, but both of his and
my patch wasn't accepted without any reason, so I proceed.

Thanks

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

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Chien Tin Tung]

On Tue, Aug 01, 2017 at 09:22:47AM -0500, Christopher Lameter wrote:
> On Tue, 1 Aug 2017, Chien Tin Tung wrote:
> 
> > Why do I need to explain the original patch?  It was accepted many kernels
> > ago.  Your questions on the original were based on false assumptions and
> > facts which I've proven over and over.  You are right in that I do
> > not want to revisit those either.  There is a patch that can solve
> > the problem you are facing but yet you insist on the revert.  This is
> > very puzzling to me.  You simply refuse to move this forward.  That
> > is your choice but revert is a no go for me.
> 
> Ok this is pretty confusing to someone not involved in the prior
> discussions. Could both of your stop attacking each other and start
> talking about the technical issues?

There's been multiple threads over this revert.  This one is the
latest Leon walked away from: https://patchwork.kernel.org/patch/9814367/

I'm simply asking for proof/evidence/facts to back up Leon's claims.
Show me the code/stack trace/whatever and I will be happy to admit wrong
publicly on the list and move on.  I do have better things to do than
to NACK a revert.

Chien
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Bart Van Assche]

On Tue, 2017-08-01 at 10:15 -0500, Chien Tin Tung wrote:
> On Tue, Aug 01, 2017 at 09:22:47AM -0500, Christopher Lameter wrote:
> > On Tue, 1 Aug 2017, Chien Tin Tung wrote:
> > 
> > > Why do I need to explain the original patch?  It was accepted many kernels
> > > ago.  Your questions on the original were based on false assumptions and
> > > facts which I've proven over and over.  You are right in that I do
> > > not want to revisit those either.  There is a patch that can solve
> > > the problem you are facing but yet you insist on the revert.  This is
> > > very puzzling to me.  You simply refuse to move this forward.  That
> > > is your choice but revert is a no go for me.
> > 
> > Ok this is pretty confusing to someone not involved in the prior
> > discussions. Could both of your stop attacking each other and start
> > talking about the technical issues?
> 
> There's been multiple threads over this revert.  This one is the
> latest Leon walked away from: https://patchwork.kernel.org/patch/9814367/
> 
> I'm simply asking for proof/evidence/facts to back up Leon's claims.
> Show me the code/stack trace/whatever and I will be happy to admit wrong
> publicly on the list and move on.  I do have better things to do than
> to NACK a revert.

Hello Chien,

What Leon wrote, namely that calls that send netlink data from kernel to user
space should be non-blocking makes sense to me. So please be more constructive
than replying with "NAK".

Thanks,

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Chien Tin Tung]

On Tue, Aug 01, 2017 at 03:20:11PM +0000, Bart Van Assche wrote:
> On Tue, 2017-08-01 at 10:15 -0500, Chien Tin Tung wrote:
> > On Tue, Aug 01, 2017 at 09:22:47AM -0500, Christopher Lameter wrote:
> > > On Tue, 1 Aug 2017, Chien Tin Tung wrote:
> > > 
> > > > Why do I need to explain the original patch?  It was accepted many kernels
> > > > ago.  Your questions on the original were based on false assumptions and
> > > > facts which I've proven over and over.  You are right in that I do
> > > > not want to revisit those either.  There is a patch that can solve
> > > > the problem you are facing but yet you insist on the revert.  This is
> > > > very puzzling to me.  You simply refuse to move this forward.  That
> > > > is your choice but revert is a no go for me.
> > > 
> > > Ok this is pretty confusing to someone not involved in the prior
> > > discussions. Could both of your stop attacking each other and start
> > > talking about the technical issues?
> > 
> > There's been multiple threads over this revert.  This one is the
> > latest Leon walked away from: https://patchwork.kernel.org/patch/9814367/
> > 
> > I'm simply asking for proof/evidence/facts to back up Leon's claims.
> > Show me the code/stack trace/whatever and I will be happy to admit wrong
> > publicly on the list and move on.  I do have better things to do than
> > to NACK a revert.
> 
> Hello Chien,
> 
> What Leon wrote, namely that calls that send netlink data from kernel to user
> space should be non-blocking makes sense to me.

That's been addressed in so many emails I won't rehash.  If _no_one_ should
block (it is actually a one shot retry with a timeout) sending Netlink message
from kernel to user, why don't Leon or you patch that code out?  All of
it, not just ibnl_unicast().

> So please be more constructive than replying with "NAK".

I've sent so many emails (some you were CC'd), so I'm not sure how
much more constructive I can be.  BTW, did you see the one with my
attempt at world peace (https://www.spinics.net/lists/linux-rdma/msg50591.html)?

Here are the relevant threads for people that are interested in participating
in this discussion.

https://patchwork.kernel.org/patch/9814367/

https://patchwork.kernel.org/patch/9752855/

Chien
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Bart Van Assche]

On Tue, 2017-08-01 at 11:21 -0500, Chien Tin Tung wrote:
> On Tue, Aug 01, 2017 at 03:20:11PM +0000, Bart Van Assche wrote:
> > What Leon wrote, namely that calls that send netlink data from kernel to user
> > space should be non-blocking makes sense to me.
> 
> That's been addressed in so many emails I won't rehash.  If _no_one_ should
> block (it is actually a one shot retry with a timeout) sending Netlink message
> from kernel to user, why don't Leon or you patch that code out?  All of
> it, not just ibnl_unicast().
> 
> > So please be more constructive than replying with "NAK".
> 
> I've sent so many emails (some you were CC'd), so I'm not sure how
> much more constructive I can be.  BTW, did you see the one with my
> attempt at world peace (https://www.spinics.net/lists/linux-rdma/msg50591.html)?
> 
> Here are the relevant threads for people that are interested in participating
> in this discussion.
> 
> https://patchwork.kernel.org/patch/9814367/
> 
> https://patchwork.kernel.org/patch/9752855/

Hello Chien,

Yes, I had read these e-mails but I do not agree with all of what was written
in these e-mails. I'm not sure whether you are aware of the original design
goal of the netlink mechanism? It was designed on purpose to be unreliable
such that sending information from the kernel to user space would never block.
If sending data over a netlink socket can block then analyzing whether or not
any deadlocks can occur is only possible by examining kernel and user space
together. If sending data over a netlink socket never blocks then a only the
kernel has to be considered when analyzing whether or not a deadlock can occur.
We do not want that userspace can cause the kernel to lock up. So it's very
strange to me to see that today the kernel has facilities for both blocking
and non-blocking sends over netlink sockets. My opinion here is that any
client that needs reliable communication from kernel to user space should use
another mechanism than netlink sockets. Additionally, since the kernel reports
netlink socket buffer overflows to user space through the ENOBUFS error code,
what's so hard about detecting that error code in user space and
resynchronizing state if recv() fails with ENOBUFS?

Is your concern perhaps about the code in iwpmd/iwarp_pm_server.c? From what
I see in process_iwpm_netlink_msg() it seems like all receive errors are
treated equal and no state resynchronization occurs if the kernel reports
ENOBUFS?

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Chien Tin Tung]

On Tue, Aug 01, 2017 at 04:55:09PM +0000, Bart Van Assche wrote:
> On Tue, 2017-08-01 at 11:21 -0500, Chien Tin Tung wrote:
> > On Tue, Aug 01, 2017 at 03:20:11PM +0000, Bart Van Assche wrote:
> > > What Leon wrote, namely that calls that send netlink data from kernel to user
> > > space should be non-blocking makes sense to me.
> > 
> > That's been addressed in so many emails I won't rehash.  If _no_one_ should
> > block (it is actually a one shot retry with a timeout) sending Netlink message
> > from kernel to user, why don't Leon or you patch that code out?  All of
> > it, not just ibnl_unicast().
> > 
> > > So please be more constructive than replying with "NAK".
> > 
> > I've sent so many emails (some you were CC'd), so I'm not sure how
> > much more constructive I can be.  BTW, did you see the one with my
> > attempt at world peace (https://www.spinics.net/lists/linux-rdma/msg50591.html)?
> > 
> > Here are the relevant threads for people that are interested in participating
> > in this discussion.
> > 
> > https://patchwork.kernel.org/patch/9814367/
> > 
> > https://patchwork.kernel.org/patch/9752855/
> 
> Hello Chien,
> 
> Yes, I had read these e-mails but I do not agree with all of what was written
> in these e-mails. I'm not sure whether you are aware of the original design
> goal of the netlink mechanism? It was designed on purpose to be unreliable
> such that sending information from the kernel to user space would never block.

Please show me a feference to Netlink design document/email on mailing list
that specifically disallowed this?

> If sending data over a netlink socket can block then analyzing whether or not
> any deadlocks can occur is only possible by examining kernel and user space
> together. If sending data over a netlink socket never blocks then a only the
> kernel has to be considered when analyzing whether or not a deadlock can occur.
> We do not want that userspace can cause the kernel to lock up. So it's very
> strange to me to see that today the kernel has facilities for both blocking
> and non-blocking sends over netlink sockets.

So here we go again.  I will rehash my arguments.

1) There is NO deadlock with ibnl_unicast(). It is single shot retry with timeout.
As I've requested before, people should stop using terms that are NOT applicable
to this situation.  You can make other arguments but not this one.

2) With the patch from Mustafa it will separate out messages, one with the retry
and the other without, as designed in Netlink.  what's wrong with this solution?
Has Leon tried the patch?  Does it not work for his situation?

> My opinion here is that any
> client that needs reliable communication from kernel to user space should use
> another mechanism than netlink sockets. Additionally, since the kernel reports
> netlink socket buffer overflows to user space through the ENOBUFS error code,
> what's so hard about detecting that error code in user space and
> resynchronizing state if recv() fails with ENOBUFS?

Incorrect analysis on how the message is triggered.  The message originates
from kernel to user.  It is not trying to guarantee delivery as it is single
retry with timeout.  Yes, I've stated these facts in previous threads.

> 
> Is your concern perhaps about the code in iwpmd/iwarp_pm_server.c? From what
> I see in process_iwpm_netlink_msg() it seems like all receive errors are
> treated equal and no state resynchronization occurs if the kernel reports
> ENOBUFS?

you need to visit the kernel code flow not user.  I've laid out the code flow
for everyone to see in a previous email, not doing it again.

Chien
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Bart Van Assche]

On Tue, 2017-08-01 at 12:14 -0500, Chien Tin Tung wrote:
> On Tue, Aug 01, 2017 at 04:55:09PM +0000, Bart Van Assche wrote:
> > Yes, I had read these e-mails but I do not agree with all of what was written
> > in these e-mails. I'm not sure whether you are aware of the original design
> > goal of the netlink mechanism? It was designed on purpose to be unreliable
> > such that sending information from the kernel to user space would never block.
> 
> Please show me a feference to Netlink design document/email on mailing list
> that specifically disallowed this?

As you probably know kernel developers do not write design documents. But there
is plenty of evidence on the web that the netlink mechanism was designed to be
unreliable. A quote from a Linux Journal article from 2005
(http://www.linuxjournal.com/article/7356): "Netlink is asynchronous because,
as with any other socket API, it provides a socket queue to smooth the burst
of messages."

>From LWN.net article from 2005 (https://lwn.net/Articles/131802/):
"netlink is an unreliable service".

>From RFC 3549, co-authored by Alexey Kuznetsov, one of the netlink authors
(https://tools.ietf.org/html/rfc3549): "By default, however, Netlink provides
an unreliable communication."

>From the netlink man page (http://man7.org/linux/man-pages/man7/netlink.7.html):
"However, reliable transmissions from kernel to user are impossible in any case.
The kernel can't send a netlink message if the socket buffer is full: the message
will be dropped and the kernel and the user-space process will no longer have
the same view of kernel state. It is up to the application to detect when this
happens (via the ENOBUFS error returned by recvmsg(2)) and resynchronize."

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Chien Tin Tung]

On Tue, Aug 01, 2017 at 05:28:54PM +0000, Bart Van Assche wrote:
> On Tue, 2017-08-01 at 12:14 -0500, Chien Tin Tung wrote:
> > On Tue, Aug 01, 2017 at 04:55:09PM +0000, Bart Van Assche wrote:
> > > Yes, I had read these e-mails but I do not agree with all of what was written
> > > in these e-mails. I'm not sure whether you are aware of the original design
> > > goal of the netlink mechanism? It was designed on purpose to be unreliable
> > > such that sending information from the kernel to user space would never block.
> > 
> > Please show me a feference to Netlink design document/email on mailing list
> > that specifically disallowed this?
> 
> As you probably know kernel developers do not write design documents. But there
> is plenty of evidence on the web that the netlink mechanism was designed to be
> unreliable. A quote from a Linux Journal article from 2005
> (http://www.linuxjournal.com/article/7356): "Netlink is asynchronous because,
> as with any other socket API, it provides a socket queue to smooth the burst
> of messages."

So what happens when the queue is full?  you can fail at that point or you
can choose the 1-shot retry with timeout as we have done.  What is _wrong_
with the 1-shot retry?

> From LWN.net article from 2005 (https://lwn.net/Articles/131802/):
> "netlink is an unreliable service".

fine again, already stated ibnl_unicast is not a guaranteed delivery of
Netlink messages.  I can only say this so many times.

> From RFC 3549, co-authored by Alexey Kuznetsov, one of the netlink authors
> (https://tools.ietf.org/html/rfc3549): "By default, however, Netlink provides
> an unreliable communication."

Same as above.

> From the netlink man page (http://man7.org/linux/man-pages/man7/netlink.7.html):
> "However, reliable transmissions from kernel to user are impossible in any case.
> The kernel can't send a netlink message if the socket buffer is full: the message
> will be dropped and the kernel and the user-space process will no longer have
> the same view of kernel state. It is up to the application to detect when this
> happens (via the ENOBUFS error returned by recvmsg(2)) and resynchronize."

Same as above.

It is so convenient for you and Leon to delete my questions that you don't
want to address.  

So, here they are again.

The Netlink message we want 1-shot retry is triggered from kernel to user, not
user->kernel->user.  Please get this right.  Mustafa's patch would separate
out messages that's user->kernel->user.  What's wrong with that solution?
[rehash] The original patch to add 1-shot retry was to split ibnl_unicast into
two, one with retry, the other without, which Leon shot down.  So please don't
ask how we ended up here.


I see two path out of this.

1) Either Leon/you patch out all blocking Netlink code from the kernel,
including the 1-shot retry mechanism ibln_unicast is using.  I promise I won't
comment on the patch.  I will let Netlink maintainer handle that matter.

2) Accept Mustafa's patch.

It is time to move on from this discussion.

Chien
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Bart Van Assche]

On Tue, 2017-08-01 at 12:52 -0500, Chien Tin Tung wrote:
> On Tue, Aug 01, 2017 at 05:28:54PM +0000, Bart Van Assche wrote:
> > On Tue, 2017-08-01 at 12:14 -0500, Chien Tin Tung wrote:
> > > On Tue, Aug 01, 2017 at 04:55:09PM +0000, Bart Van Assche wrote:
> > > > Yes, I had read these e-mails but I do not agree with all of what was written
> > > > in these e-mails. I'm not sure whether you are aware of the original design
> > > > goal of the netlink mechanism? It was designed on purpose to be unreliable
> > > > such that sending information from the kernel to user space would never block.
> > > 
> > > Please show me a feference to Netlink design document/email on mailing list
> > > that specifically disallowed this?
> > 
> > As you probably know kernel developers do not write design documents. But there
> > is plenty of evidence on the web that the netlink mechanism was designed to be
> > unreliable. A quote from a Linux Journal article from 2005
> > (http://www.linuxjournal.com/article/7356): "Netlink is asynchronous because,
> > as with any other socket API, it provides a socket queue to smooth the burst
> > of messages."
> 
> So what happens when the queue is full?  you can fail at that point or you
> can choose the 1-shot retry with timeout as we have done.  What is _wrong_
> with the 1-shot retry?

Why is the one-shot retry necessary? What aspect of the portmapper design makes
this one-shot retry necessary? Why is this one-shot retry only necessary for the
portmapper and not for any other Netlink client?

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Chien Tin Tung]

On Tue, Aug 01, 2017 at 05:58:29PM +0000, Bart Van Assche wrote:
> On Tue, 2017-08-01 at 12:52 -0500, Chien Tin Tung wrote:
> > On Tue, Aug 01, 2017 at 05:28:54PM +0000, Bart Van Assche wrote:
> > > On Tue, 2017-08-01 at 12:14 -0500, Chien Tin Tung wrote:
> > > > On Tue, Aug 01, 2017 at 04:55:09PM +0000, Bart Van Assche wrote:
> > > > > Yes, I had read these e-mails but I do not agree with all of what was written
> > > > > in these e-mails. I'm not sure whether you are aware of the original design
> > > > > goal of the netlink mechanism? It was designed on purpose to be unreliable
> > > > > such that sending information from the kernel to user space would never block.
> > > > 
> > > > Please show me a feference to Netlink design document/email on mailing list
> > > > that specifically disallowed this?
> > > 
> > > As you probably know kernel developers do not write design documents. But there
> > > is plenty of evidence on the web that the netlink mechanism was designed to be
> > > unreliable. A quote from a Linux Journal article from 2005
> > > (http://www.linuxjournal.com/article/7356): "Netlink is asynchronous because,
> > > as with any other socket API, it provides a socket queue to smooth the burst
> > > of messages."
> > 
> > So what happens when the queue is full?  you can fail at that point or you
> > can choose the 1-shot retry with timeout as we have done.  What is _wrong_
> > with the 1-shot retry?
> 
> Why is the one-shot retry necessary? What aspect of the portmapper design makes
> this one-shot retry necessary? Why is this one-shot retry only necessary for the
> portmapper and not for any other Netlink client?

You are really asking why the 1-shot retry code is in Netlink.  We didn't code
it, we are simply using it with ibnl_unicast.  It being used as designed, when
the queue is full, try it one more time within the timeout parameter.  Other clients
can use it too, I'm not stopping them.

Chien
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Jason Gunthorpe]

On Tue, Aug 01, 2017 at 10:15:11AM -0500, Chien Tin Tung wrote:
> On Tue, Aug 01, 2017 at 09:22:47AM -0500, Christopher Lameter wrote:
> > On Tue, 1 Aug 2017, Chien Tin Tung wrote:
> > 
> > > Why do I need to explain the original patch?  It was accepted many kernels
> > > ago.  Your questions on the original were based on false assumptions and
> > > facts which I've proven over and over.  You are right in that I do
> > > not want to revisit those either.  There is a patch that can solve
> > > the problem you are facing but yet you insist on the revert.  This is
> > > very puzzling to me.  You simply refuse to move this forward.  That
> > > is your choice but revert is a no go for me.
> > 
> > Ok this is pretty confusing to someone not involved in the prior
> > discussions. Could both of your stop attacking each other and start
> > talking about the technical issues?
> 
> There's been multiple threads over this revert.  This one is the
> latest Leon walked away from: https://patchwork.kernel.org/patch/9814367/
> 
> I'm simply asking for proof/evidence/facts to back up Leon's claims.
> Show me the code/stack trace/whatever and I will be happy to admit wrong
> publicly on the list and move on.  I do have better things to do than
> to NACK a revert.

Well, I think the design of the netlink protocol in iwpmd is pretty
strange, but audit netlink is the other major user of blocking
netlink_uncast to achive some reliability, so it should be workable
here as well.

Presumably the iwpmd kernel side is designed to handle the locking
right to avoid deadlocks.

I'm not sure why there is so much noise about this - yes, iwpmd is
really weird, but it is a UAPI, we can't change it and we can't demand
they change. For better or worse the protocol is based on
'near-reliable' netlink delivery of messages and we are stuck with
that.

Leon is also right that every other user of netlink_unicast in rdma
should be using the non-blocking version.

So, the above patch seems like a sensible approach to me..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Chien Tin Tung]

On Tue, Aug 01, 2017 at 01:58:40PM -0600, Jason Gunthorpe wrote:
> On Tue, Aug 01, 2017 at 10:15:11AM -0500, Chien Tin Tung wrote:
> > On Tue, Aug 01, 2017 at 09:22:47AM -0500, Christopher Lameter wrote:
> > > On Tue, 1 Aug 2017, Chien Tin Tung wrote:
> > > 
> > > > Why do I need to explain the original patch?  It was accepted many kernels
> > > > ago.  Your questions on the original were based on false assumptions and
> > > > facts which I've proven over and over.  You are right in that I do
> > > > not want to revisit those either.  There is a patch that can solve
> > > > the problem you are facing but yet you insist on the revert.  This is
> > > > very puzzling to me.  You simply refuse to move this forward.  That
> > > > is your choice but revert is a no go for me.
> > > 
> > > Ok this is pretty confusing to someone not involved in the prior
> > > discussions. Could both of your stop attacking each other and start
> > > talking about the technical issues?
> > 
> > There's been multiple threads over this revert.  This one is the
> > latest Leon walked away from: https://patchwork.kernel.org/patch/9814367/
> > 
> > I'm simply asking for proof/evidence/facts to back up Leon's claims.
> > Show me the code/stack trace/whatever and I will be happy to admit wrong
> > publicly on the list and move on.  I do have better things to do than
> > to NACK a revert.
> 
> Well, I think the design of the netlink protocol in iwpmd is pretty
> strange, but audit netlink is the other major user of blocking
> netlink_uncast to achive some reliability, so it should be workable
> here as well.

Agreed, somewhat.  I'm all for making things better but the basis of
any conversation needs to start with correct assumptions.

> Presumably the iwpmd kernel side is designed to handle the locking
> right to avoid deadlocks.

Please stop using the word deadlock in relation to usage of 1-shot
retry in ibnl_unicast.  It does not and will not cause deadlock period.
It will wait for a buffer within timeout value but that's it.

> I'm not sure why there is so much noise about this - yes, iwpmd is
> really weird, but it is a UAPI, we can't change it and we can't demand
> they change. For better or worse the protocol is based on
> 'near-reliable' netlink delivery of messages and we are stuck with
> that.
> 
> Leon is also right that every other user of netlink_unicast in rdma
> should be using the non-blocking version.

In Mustafa's patch, there are calls to both versions of ibnl_unicast.
There are situations where the retry version of ibnl_unicast
is not desirable, note I didn't say wrong.

Here is the reality.

The retry version of ibnl_unicast will only wait when there is
no buffer to send the Netlink message.  It does not
interfere with other Netlink calls.  For that matter, other
calls to transmit a Netlink message will fail as there are no
buffers available.  If a buffer does not become available within
timeout parameter, you have bigger problems.

> So, the above patch seems like a sensible approach to me..

Finally.  Thank you.

Chien
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Jason Gunthorpe]

On Tue, Aug 01, 2017 at 03:38:15PM -0500, Chien Tin Tung wrote:

> > Presumably the iwpmd kernel side is designed to handle the locking
> > right to avoid deadlocks.
> 
> Please stop using the word deadlock in relation to usage of 1-shot
> retry in ibnl_unicast.  It does not and will not cause deadlock period.
> It will wait for a buffer within timeout value but that's it.

I think you are arguing semantics here.

My understanding with netlink (and it has been years since I've
looked) is that there is way for netlink callbacks to be run within
the context of the user thread calling send(). In this case calling
blocking netlink_unicast within such a callback will 'deadlock', as
netlink_unicast waits for the current thread to call recv(), which can
never happen.

We still call this a deadlock even though the sndtimer rescues
the overall system, as the system was in a deadlocked state for
a time.

> > Leon is also right that every other user of netlink_unicast in rdma
> > should be using the non-blocking version.
> 
> In Mustafa's patch, there are calls to both versions of ibnl_unicast.
> There are situations where the retry version of ibnl_unicast
> is not desirable, note I didn't say wrong.

I haven't studied closely, but if any of those blocking
netlink_unicasts are in netlink callbacks, it would need very
careful inspection to determine if that is OK.

> buffers available.  If a buffer does not become available within
> timeout parameter, you have bigger problems.

Well.. It is problematic to create that situation in the first place.

Usually you would never run unsolicited netlink traffic and
request/response traffic over the same socket. Having the kernel
randomly drop responses because the sndbuf is full of unsolicited
traffic is really problematic.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Leon Romanovsky]

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

On Tue, Aug 01, 2017 at 01:58:40PM -0600, Jason Gunthorpe wrote:
>
> I'm not sure why there is so much noise about this - yes, iwpmd is
> really weird, but it is a UAPI, we can't change it and we can't demand
> they change.

If you claim that it is UAPI change, we MUST revert this patch, because
reverted patch CHANGED UAPI.

Thanks

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

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Jason Gunthorpe]

On Wed, Aug 02, 2017 at 06:44:38AM +0300, Leon Romanovsky wrote:
> On Tue, Aug 01, 2017 at 01:58:40PM -0600, Jason Gunthorpe wrote:
> >
> > I'm not sure why there is so much noise about this - yes, iwpmd is
> > really weird, but it is a UAPI, we can't change it and we can't demand
> > they change.
> 
> If you claim that it is UAPI change, we MUST revert this patch, because
> reverted patch CHANGED UAPI.

That is not how I read it.. The UAPI was intended to be lossless and
there was a kernel bug that made it more lossy than expected, that is
what that original patch was addressing.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Leon Romanovsky]

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

On Wed, Aug 02, 2017 at 09:58:56AM -0600, Jason Gunthorpe wrote:
> On Wed, Aug 02, 2017 at 06:44:38AM +0300, Leon Romanovsky wrote:
> > On Tue, Aug 01, 2017 at 01:58:40PM -0600, Jason Gunthorpe wrote:
> > >
> > > I'm not sure why there is so much noise about this - yes, iwpmd is
> > > really weird, but it is a UAPI, we can't change it and we can't demand
> > > they change.
> >
> > If you claim that it is UAPI change, we MUST revert this patch, because
> > reverted patch CHANGED UAPI.
>
> That is not how I read it.. The UAPI was intended to be lossless and
> there was a kernel bug that made it more lossy than expected, that is
> what that original patch was addressing.

The catch 22 here is in the fact that there is no kernel bug. I assume
and according to Bart's questions (but better to ask him) he thinks the same,
that the bug is in protocol layer and/or user space part. There is no
visible kernel bug.

This is exactly what we are asking from Chien to provide the details.

Thanks

>
> Jason

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

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Jason Gunthorpe]

On Wed, Aug 02, 2017 at 07:29:38PM +0300, Leon Romanovsky wrote:
> On Wed, Aug 02, 2017 at 09:58:56AM -0600, Jason Gunthorpe wrote:
> > On Wed, Aug 02, 2017 at 06:44:38AM +0300, Leon Romanovsky wrote:
> > > On Tue, Aug 01, 2017 at 01:58:40PM -0600, Jason Gunthorpe wrote:
> > > >
> > > > I'm not sure why there is so much noise about this - yes, iwpmd is
> > > > really weird, but it is a UAPI, we can't change it and we can't demand
> > > > they change.
> > >
> > > If you claim that it is UAPI change, we MUST revert this patch, because
> > > reverted patch CHANGED UAPI.
> >
> > That is not how I read it.. The UAPI was intended to be lossless and
> > there was a kernel bug that made it more lossy than expected, that is
> > what that original patch was addressing.
> 
> The catch 22 here is in the fact that there is no kernel bug. I assume
> and according to Bart's questions (but better to ask him) he thinks the same,
> that the bug is in protocol layer and/or user space part. There is no
> visible kernel bug.

No, I disagree.. It is pretty clear how iwpm works, it it pushes
unsolicted messaged directly to the userspace daemon from the kernel
and expects the daemon to receive those messages.

With such a scheme it is really important for the kernel to do
everything it can to minimize the risk of message loss, and using the
blocking sending for unsolicited messages is certainly part of it -
that is what audit does for instance.

The discussion really got into the weeds when people brought up
O_NONBLOCK or ENOBUFS, or any other user space change as that has
nothing to do with pushing unsolicited messages from the kernel to
userspace.

Arguing that is a 'protocol bug' doesn't make much sense, the protocol
is a uAPI, so it is up to the kernel to provide the best
implementation possible, which in this case means working to minimize
loss of the messages..

Noting again, that this is *ONLY* talking about the unsolicted
messages the iwpm sends toward userspace. IMHO, use of blocking send
in other contexts, such as dump callbacks, is an error.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Bart Van Assche]

On Wed, 2017-08-02 at 10:45 -0600, Jason Gunthorpe wrote:
> No, I disagree.. It is pretty clear how iwpm works, it it pushes
> unsolicted messaged directly to the userspace daemon from the kernel
> and expects the daemon to receive those messages.
> 
> With such a scheme it is really important for the kernel to do
> everything it can to minimize the risk of message loss, and using the
> blocking sending for unsolicited messages is certainly part of it -
> that is what audit does for instance.
> 
> The discussion really got into the weeds when people brought up
> O_NONBLOCK or ENOBUFS, or any other user space change as that has
> nothing to do with pushing unsolicited messages from the kernel to
> userspace.
> 
> Arguing that is a 'protocol bug' doesn't make much sense, the protocol
> is a uAPI, so it is up to the kernel to provide the best
> implementation possible, which in this case means working to minimize
> loss of the messages..
> 
> Noting again, that this is *ONLY* talking about the unsolicted
> messages the iwpm sends toward userspace. IMHO, use of blocking send
> in other contexts, such as dump callbacks, is an error.

Hello Jason,

Although I do not object against the "RDMA/core: Add wait/retry version
of ibnl_unicast" patch, I hope that you realize that it is an ugly hack
instead of a proper fix. Anything that makes user space wait longer than
the socket timeout, e.g. heavy swapping activity or running the user
space software under a debugger, will cause delivery of the netlink
message from kernel to user to fail anyway.

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Jason Gunthorpe]

On Wed, Aug 02, 2017 at 04:51:01PM +0000, Bart Van Assche wrote:
 
> Although I do not object against the "RDMA/core: Add wait/retry version
> of ibnl_unicast" patch, I hope that you realize that it is an ugly hack
> instead of a proper fix. Anything that makes user space wait longer than
> the socket timeout, e.g. heavy swapping activity or running the user
> space software under a debugger, will cause delivery of the netlink
> message from kernel to user to fail anyway.

Yes, I assume the iwpm people are aware of this as well, this is why I
used the phrase 'minimize loss'.

I'm not sure I'd call it a hack though. There is no proper fix here,
messages from kernel to user space must be delivered and processed for
iwpm to function as designed. Maximizing the chance of delivering
unsolicited messages via blocking netlink_unicast is going to be a
necessary component of any design...

The big point in all of this discussion is that none of this allows
iwpmd to ignore lost messages and it needs to have a sane well thought
out process for resynchronizing with the kernel. Particularly, the
flow that causes sockets to closed must have some kind of resync.

But even if all that loss resynchronization works perfectly, it is
still very desirable to avoid triggering it!

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Bart Van Assche]

On Wed, 2017-08-02 at 11:08 -0600, Jason Gunthorpe wrote:
> On Wed, Aug 02, 2017 at 04:51:01PM +0000, Bart Van Assche wrote:
>  
> > Although I do not object against the "RDMA/core: Add wait/retry version
> > of ibnl_unicast" patch, I hope that you realize that it is an ugly hack
> > instead of a proper fix. Anything that makes user space wait longer than
> > the socket timeout, e.g. heavy swapping activity or running the user
> > space software under a debugger, will cause delivery of the netlink
> > message from kernel to user to fail anyway.
> 
> Yes, I assume the iwpm people are aware of this as well, this is why I
> used the phrase 'minimize loss'.
> 
> I'm not sure I'd call it a hack though. There is no proper fix here,
> messages from kernel to user space must be delivered and processed for
> iwpm to function as designed. Maximizing the chance of delivering
> unsolicited messages via blocking netlink_unicast is going to be a
> necessary component of any design...
> 
> The big point in all of this discussion is that none of this allows
> iwpmd to ignore lost messages and it needs to have a sane well thought
> out process for resynchronizing with the kernel. Particularly, the
> flow that causes sockets to closed must have some kind of resync.
> 
> But even if all that loss resynchronization works perfectly, it is
> still very desirable to avoid triggering it!

Hello Jason,

Because it is not possible to make the current iwpm netlink code work 100%
reliably, my proposal is to add a new and redesigned interface for iwpm
communication between user space and kernel and to deprecate the current API.

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Jason Gunthorpe]

On Wed, Aug 02, 2017 at 05:11:33PM +0000, Bart Van Assche wrote:

> Because it is not possible to make the current iwpm netlink code work 100%
> reliably,

Well, I don't know enough about what iwpm has exactly done in those
details to be sure of that statement.. I'm not sure what the message
loss strategy is. It looks like loss on connect() just refuses the
connection, which seems fine, but loss on close() looks troubled? Not
sure.

I'm not really inclined to look into iwpm in any detail, but I really
want to see Leon's patch series progress, so overcoming that hurdle by
using the patch Chien pointed out seems perfectly fine to me.

> my proposal is to add a new and redesigned interface for iwpm
> communication between user space and kernel and to deprecate the
> current API.

What would you do differently?

And even if this is the best path, the use of blocking send in the
compat uAPI code is *still* very appropriate.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Doug Ledford]

On Tue, 2017-08-01 at 17:10 +0300, Leon Romanovsky wrote:
> On Tue, Aug 01, 2017 at 08:38:32AM -0500, Chien Tin Tung wrote:
> > On Tue, Aug 01, 2017 at 03:05:04PM +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > 
> > > The commit cea05eadded0 ("IB/core: Add flow control to the
> > > portmapper netlink calls")
> > > changed netlink to be blocked for all RDMA clients. This
> > > workaround
> > > worked perfectly for portmapper, but is not correct for the whole
> > > NETLINK_RDMA family.
> > 
> > Leon,
> > 
> > I've already told you I'm opposed to the revert.  There is a patch
> > that will work
> > with your usage of ibnl_unicast() but you chose to abandon that
> > discussion on June 29, 2017
> > (RDMA/core: Add wait/retry version of ibnl_unicast).  Either you
> > step up with good sound
> > technical evidence to convince me that patch won't work for you or
> > you stop trying to
> > break existing functionality with this revert.
> 
> Chien,
> 
> You never explained us what exactly your original patch fixed and why
> it
> should be fixed in kernel and not in user space. I saw that our
> discussion wasn't useful and brought bad blood instead of good will,
> so I stepped out in waited for RDMA maintainer (Doug) step in.
> 
> I never heard anything from Doug on the matter, so proceeded and
> resubmitted it.

What makes you think *I* want to be in the middle of a school yard
squabble like this?

Regardless,  here's what I think:

1) The blocking/timeout functionality that Chien is using is in the
netlink core, not something he grafted onto netlink himself.  Since
it's a core netlink feature, I see no reason he shouldn't be allowed to
use it.

2) We don't break API for user space programs.  Reverting this could
have a significant deleterious influence on programs already in the
wild.  While those programs should resync on a failure, clearly,
failure is much more frequent without this patch than with it, and as a
resync can be expensive, it is a judgment call whether it is preferable
to resync more frequently because of dropped packets, or utilize a
backlog queue to reduce the frequency of resyncs.  Clearly, Intel is of
the position that the backlog queue is worth avoiding the resyncs.

3) Chien submitted a patch that would allow just the iwpmd to get the
behavior they find preferable, while allowing Leon's new code to get
the behavior Leon wants.

Given that #1 and #2 are true and that #3 provides a reasonable
solution, the revert is rejected.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Leon Romanovsky]

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

On Wed, Aug 02, 2017 at 01:57:41PM -0400, Doug Ledford wrote:
> On Tue, 2017-08-01 at 17:10 +0300, Leon Romanovsky wrote:
> > On Tue, Aug 01, 2017 at 08:38:32AM -0500, Chien Tin Tung wrote:
> > > On Tue, Aug 01, 2017 at 03:05:04PM +0300, Leon Romanovsky wrote:
> > > > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > >
> > > > The commit cea05eadded0 ("IB/core: Add flow control to the
> > > > portmapper netlink calls")
> > > > changed netlink to be blocked for all RDMA clients. This
> > > > workaround
> > > > worked perfectly for portmapper, but is not correct for the whole
> > > > NETLINK_RDMA family.
> > >
> > > Leon,
> > >
> > > I've already told you I'm opposed to the revert.  There is a patch
> > > that will work
> > > with your usage of ibnl_unicast() but you chose to abandon that
> > > discussion on June 29, 2017
> > > (RDMA/core: Add wait/retry version of ibnl_unicast).  Either you
> > > step up with good sound
> > > technical evidence to convince me that patch won't work for you or
> > > you stop trying to
> > > break existing functionality with this revert.
> >
> > Chien,
> >
> > You never explained us what exactly your original patch fixed and why
> > it
> > should be fixed in kernel and not in user space. I saw that our
> > discussion wasn't useful and brought bad blood instead of good will,
> > so I stepped out in waited for RDMA maintainer (Doug) step in.
> >
> > I never heard anything from Doug on the matter, so proceeded and
> > resubmitted it.
>
> What makes you think *I* want to be in the middle of a school yard
> squabble like this?

It is not "I" want, but "I" expect and "I" demand from the maintainer.
It is his responsibility to drive to the resolution for the important
core patches.

This revert was discussed for something like month+, as an outcome of
it, Chien reviewed and sent much more improved version of their original
patch, without socket timeout and with minimal blocking places.

So please, don't say "school yard", especially after your silence.

Thanks

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

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Doug Ledford]

On Wed, 2017-08-02 at 21:54 +0300, Leon Romanovsky wrote:
> On Wed, Aug 02, 2017 at 01:57:41PM -0400, Doug Ledford wrote:
> > On Tue, 2017-08-01 at 17:10 +0300, Leon Romanovsky wrote:
> > > On Tue, Aug 01, 2017 at 08:38:32AM -0500, Chien Tin Tung wrote:
> > > > On Tue, Aug 01, 2017 at 03:05:04PM +0300, Leon Romanovsky
> > > > wrote:
> > > > > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > > > 
> > > > > The commit cea05eadded0 ("IB/core: Add flow control to the
> > > > > portmapper netlink calls")
> > > > > changed netlink to be blocked for all RDMA clients. This
> > > > > workaround
> > > > > worked perfectly for portmapper, but is not correct for the
> > > > > whole
> > > > > NETLINK_RDMA family.
> > > > 
> > > > Leon,
> > > > 
> > > > I've already told you I'm opposed to the revert.  There is a
> > > > patch
> > > > that will work
> > > > with your usage of ibnl_unicast() but you chose to abandon that
> > > > discussion on June 29, 2017
> > > > (RDMA/core: Add wait/retry version of ibnl_unicast).  Either
> > > > you
> > > > step up with good sound
> > > > technical evidence to convince me that patch won't work for you
> > > > or
> > > > you stop trying to
> > > > break existing functionality with this revert.
> > > 
> > > Chien,
> > > 
> > > You never explained us what exactly your original patch fixed and
> > > why
> > > it
> > > should be fixed in kernel and not in user space. I saw that our
> > > discussion wasn't useful and brought bad blood instead of good
> > > will,
> > > so I stepped out in waited for RDMA maintainer (Doug) step in.
> > > 
> > > I never heard anything from Doug on the matter, so proceeded and
> > > resubmitted it.
> > 
> > What makes you think *I* want to be in the middle of a school yard
> > squabble like this?
> 
> It is not "I" want, but "I" expect and "I" demand from the
> maintainer.

I'm a big fan of the movie Draft Day.  You may not have ever seen it,
but I really enjoy all of the things that happen in the course of one
day.  In that movie, Vontae Mack is afraid he won't get selected 7th by
Sonny Weaver Jr.  When Sonny Weaver Jr. trades up for the 1st pick,
Vontae assumes it's because he's going to try and take the popular
pick, and so Vontae takes to his twitter account and tells Sonny Weaver
Jr. off.  Sonny gives Vontae a call, in which he tells Vontae that he
needs to put his twitter account down and take a chill pill, because
"as much as going to twitter is your God given right, General Managers
hate that shit because now they know that you'll put whatever innane
thought goes through your mind out there for the entire world to see."
(or something like that, it's from memory).

You may be wondering what my point here is.  It's simple.  It may be
your God given right to force a maintainer to make a call like this,
but maintainers hate that shit, because it's usually a sign of
dysfunction in the community and failure to work cooperatively to
arrive at a communally approved resolution.  What's more, it puts the
maintainer in a bad position of having to deal with possible political
crap and everything else.  So, yes, it's your God given right, but it
better be your last resort.

> It is his responsibility to drive to the resolution for the important
> core patches.

This was really only a core patch of minor importance to begin with,
and once Chien wrote a patch to give you what you wanted without
breaking the existing user space packages, it really became a trivial
core patch.  Hardly worthy of pulling the "I demand you make a
maintainer call" card.

> This revert was discussed for something like month+, as an outcome of
> it, Chien reviewed and sent much more improved version of their
> original
> patch, without socket timeout and with minimal blocking places.

That would have been a perfect moment for you to work with Chien to
take his patch as the first of your series instead of sticking to the
revert.

> So please, don't say "school yard",

I stand by my statement.  So many school yard arguments make no real
sense, and given you had every opportunity to resolve this problem with
a community viable alternative patch, and in so doing would have
actually fulfilled the edict that "we don't break user space" instead
of violating it as you were doing, the metaphor seems entirely
appropriate.

>  especially after your silence.

I want to address this separate from everything else.

With as much hostility and accusations as I saw fly on the original
submission, several things happened:

1) The submission got bumped to the bottom of my queue because settling
squables is not the sort of thing maintainers want to deal with.
2) The submission got bumped to the bottom of my queue so you would
have a chance to work things out and come to a mutually agreed
solution.  Such resolution is in the best interest of both the
community and the technical correctness of the fix as, generally, an
agreed upon fix will have had more community input and refinement prior
to being accepted.
3) The submission will get put on hold for a short while regardless of
points 1 and 2 just so you have a moment to settle down, rethink things
a little bit, and decide if you really want to pull that "put the
maintainer in this position" trigger.  Have you really examined all the
possible solutions?  Is there really no superior alternative?  Is there
really no way to come to agreement with the other community member(s)
you are disagreeing with?

In the end, there will always be those things that a maintainer will
simply need to make a call on because there won't be a clear right or
wrong side and it just becomes a judgment call.  But I don't think this
one even came close to being that unclear, so again I'm confused by
your insistence on going this route instead of just working things out
with Chien.


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Leon Romanovsky]

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

On Wed, Aug 02, 2017 at 08:19:35PM -0400, Doug Ledford wrote:
> On Wed, 2017-08-02 at 21:54 +0300, Leon Romanovsky wrote:
> > On Wed, Aug 02, 2017 at 01:57:41PM -0400, Doug Ledford wrote:
> > > On Tue, 2017-08-01 at 17:10 +0300, Leon Romanovsky wrote:
> > > > On Tue, Aug 01, 2017 at 08:38:32AM -0500, Chien Tin Tung wrote:
> > > > > On Tue, Aug 01, 2017 at 03:05:04PM +0300, Leon Romanovsky
> > > > > wrote:
> > > > > > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > > > >
> > > > > > The commit cea05eadded0 ("IB/core: Add flow control to the
> > > > > > portmapper netlink calls")
> > > > > > changed netlink to be blocked for all RDMA clients. This
> > > > > > workaround
> > > > > > worked perfectly for portmapper, but is not correct for the
> > > > > > whole
> > > > > > NETLINK_RDMA family.
> > > > >
> > > > > Leon,
> > > > >
> > > > > I've already told you I'm opposed to the revert.  There is a
> > > > > patch
> > > > > that will work
> > > > > with your usage of ibnl_unicast() but you chose to abandon that
> > > > > discussion on June 29, 2017
> > > > > (RDMA/core: Add wait/retry version of ibnl_unicast).  Either
> > > > > you
> > > > > step up with good sound
> > > > > technical evidence to convince me that patch won't work for you
> > > > > or
> > > > > you stop trying to
> > > > > break existing functionality with this revert.
> > > >
> > > > Chien,
> > > >
> > > > You never explained us what exactly your original patch fixed and
> > > > why
> > > > it
> > > > should be fixed in kernel and not in user space. I saw that our
> > > > discussion wasn't useful and brought bad blood instead of good
> > > > will,
> > > > so I stepped out in waited for RDMA maintainer (Doug) step in.
> > > >
> > > > I never heard anything from Doug on the matter, so proceeded and
> > > > resubmitted it.
> > >
> > > What makes you think *I* want to be in the middle of a school yard
> > > squabble like this?
> >
> > It is not "I" want, but "I" expect and "I" demand from the
> > maintainer.
>
> I'm a big fan of the movie Draft Day.  You may not have ever seen it,
> but I really enjoy all of the things that happen in the course of one
> day.  In that movie, Vontae Mack is afraid he won't get selected 7th by
> Sonny Weaver Jr.  When Sonny Weaver Jr. trades up for the 1st pick,
> Vontae assumes it's because he's going to try and take the popular
> pick, and so Vontae takes to his twitter account and tells Sonny Weaver
> Jr. off.  Sonny gives Vontae a call, in which he tells Vontae that he
> needs to put his twitter account down and take a chill pill, because
> "as much as going to twitter is your God given right, General Managers
> hate that shit because now they know that you'll put whatever innane
> thought goes through your mind out there for the entire world to see."
> (or something like that, it's from memory).
>
> You may be wondering what my point here is.  It's simple.  It may be
> your God given right to force a maintainer to make a call like this,
> but maintainers hate that shit, because it's usually a sign of
> dysfunction in the community and failure to work cooperatively to
> arrive at a communally approved resolution.  What's more, it puts the
> maintainer in a bad position of having to deal with possible political
> crap and everything else.  So, yes, it's your God given right, but it
> better be your last resort.

Are you kidding me? Reply to the series from the maintainer that "it is
postponed till both sides are happy" is too much for you?

"Politics" - it is so sticky word here. Every disagreement in the mailing list
immediately claimed "for politic reasons" and everyone is hiding in the bushes.

So yes, the community doesn't work, and I don't see desire to fix it
from the all participants of this mailing list.

>
> > It is his responsibility to drive to the resolution for the important
> > core patches.
>
> This was really only a core patch of minor importance to begin with,
> and once Chien wrote a patch to give you what you wanted without
> breaking the existing user space packages, it really became a trivial
> core patch.  Hardly worthy of pulling the "I demand you make a
> maintainer call" card.

No, Chien never explained why he needs this fix and what this fix is actually fixing.

>
> > This revert was discussed for something like month+, as an outcome of
> > it, Chien reviewed and sent much more improved version of their
> > original
> > patch, without socket timeout and with minimal blocking places.
>
> That would have been a perfect moment for you to work with Chien to
> take his patch as the first of your series instead of sticking to the
> revert.

I can't work on the solution if other side didn't explain why it is needed.

>
> > So please, don't say "school yard",
>
> I stand by my statement.  So many school yard arguments make no real
> sense, and given you had every opportunity to resolve this problem with
> a community viable alternative patch, and in so doing would have
> actually fulfilled the edict that "we don't break user space" instead
> of violating it as you were doing, the metaphor seems entirely
> appropriate.

And the fact that the reverted patch broke the user space behavior
before, didn't bother you?

>
> >  especially after your silence.
>
> I want to address this separate from everything else.
>
> With as much hostility and accusations as I saw fly on the original
> submission, several things happened:
>
> 1) The submission got bumped to the bottom of my queue because settling
> squables is not the sort of thing maintainers want to deal with.
> 2) The submission got bumped to the bottom of my queue so you would
> have a chance to work things out and come to a mutually agreed
> solution.  Such resolution is in the best interest of both the
> community and the technical correctness of the fix as, generally, an
> agreed upon fix will have had more community input and refinement prior
> to being accepted.
> 3) The submission will get put on hold for a short while regardless of
> points 1 and 2 just so you have a moment to settle down, rethink things
> a little bit, and decide if you really want to pull that "put the
> maintainer in this position" trigger.  Have you really examined all the
> possible solutions?  Is there really no superior alternative?  Is there
> really no way to come to agreement with the other community member(s)
> you are disagreeing with?

And did you go to the mailing list and write something like that "the
series on hold till solution will be found", like it was with hfi1-vnic
and ipoib?

The Answer is no, you didn't do it, you silently ignored discussion, understood
that the series is stuck and continued down the road.

>
> In the end, there will always be those things that a maintainer will
> simply need to make a call on because there won't be a clear right or
> wrong side and it just becomes a judgment call.  But I don't think this
> one even came close to being that unclear, so again I'm confused by
> your insistence on going this route instead of just working things out
> with Chien.

Because, I truly believe that they proposed the nasty hack, which
doesn't fix the real problem - inability to deal with losses of netlink
messages.

And I realized it after I started to work on RDMA netlink and read RFC
and design documents and assumptions about netlink.

In my opinion, it is one of the small examples why RDMA subsystem is so
hated. It is full of "custom" solutions to known problems and many of
those "custom" solutions are mix of hacks and band aids.

There is no need to reply on it.
It is my personal opinion and my view of the RDMA subsystem as a newcomer.

I'm OOO till Monday.
Happy weekends.

>
>
> --
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>     GPG KeyID: B826A3330E572FDD
>     Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>

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

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Doug Ledford]

On Thu, 2017-08-03 at 08:10 +0300, Leon Romanovsky wrote:
> On Wed, Aug 02, 2017 at 08:19:35PM -0400, Doug Ledford wrote:
> > On Wed, 2017-08-02 at 21:54 +0300, Leon Romanovsky wrote:
> > > 
> 
> Are you kidding me? Reply to the series from the maintainer that "it
> is
> postponed till both sides are happy" is too much for you?

I was a bit busy the last few weeks fixing a mess of my own.  And I'm
still extremely busy working on getting our test cluster in Red Hat
back operational....it's been down now for almost four weeks in total
(well, planning and downtime has now extended to four weeks, only three
weeks have been downtime).  I don't delete emails from my linux-rdma
folder until they've been fully processed and won't be needed any more.
 Between the problems I created for myself by missing the merge window,
and the downtime of the Red Hat cluster I've been working on, I have
1500 emails in that folder.  So, yes, I punted on what looked like a
squabble and worked on other stuff.

> > 
> > > It is his responsibility to drive to the resolution for the
> > > important
> > > core patches.
> > 
> > This was really only a core patch of minor importance to begin
> > with,
> > and once Chien wrote a patch to give you what you wanted without
> > breaking the existing user space packages, it really became a
> > trivial
> > core patch.  Hardly worthy of pulling the "I demand you make a
> > maintainer call" card.
> 
> No, Chien never explained why he needs this fix and what this fix is
> actually fixing.

I understood his explanation.  He wasn't as clear as he could have
been, but then again he's ESL just like you (English as Second
Language, it's the term they use in my wife's job for situations like
this).  I'd be happy to explain if it will help.

> > 
> > > This revert was discussed for something like month+, as an
> > > outcome of
> > > it, Chien reviewed and sent much more improved version of their
> > > original
> > > patch, without socket timeout and with minimal blocking places.
> > 
> > That would have been a perfect moment for you to work with Chien to
> > take his patch as the first of your series instead of sticking to
> > the
> > revert.
> 
> I can't work on the solution if other side didn't explain why it is
> needed.

He's ESL just like you are, with both of you I expect unintentional
shortcomings in explanations, and when that happens I attempt to
clarify for the sake of all parties.  He tried, as best he could, to
give you what you asked for.  I got it, if you need help understanding
it I'd be glad to help.

> > 
> > > So please, don't say "school yard",
> > 
> > I stand by my statement.  So many school yard arguments make no
> > real
> > sense, and given you had every opportunity to resolve this problem
> > with
> > a community viable alternative patch, and in so doing would have
> > actually fulfilled the edict that "we don't break user space"
> > instead
> > of violating it as you were doing, the metaphor seems entirely
> > appropriate.
> 
> And the fact that the reverted patch broke the user space behavior
> before, didn't bother you?

Breakage how?  If you mean that the fact that it broke the behavior
pattern you expected, that breakage doesn't really exist until your
patches are accepted, so it's not really broke right now, it just isn't
what you want.

> > 
> > >  especially after your silence.
> > 
> > I want to address this separate from everything else.
> > 
> > With as much hostility and accusations as I saw fly on the original
> > submission, several things happened:
> > 
> > 1) The submission got bumped to the bottom of my queue because
> > settling
> > squables is not the sort of thing maintainers want to deal with.
> > 2) The submission got bumped to the bottom of my queue so you would
> > have a chance to work things out and come to a mutually agreed
> > solution.  Such resolution is in the best interest of both the
> > community and the technical correctness of the fix as, generally,
> > an
> > agreed upon fix will have had more community input and refinement
> > prior
> > to being accepted.
> > 3) The submission will get put on hold for a short while regardless
> > of
> > points 1 and 2 just so you have a moment to settle down, rethink
> > things
> > a little bit, and decide if you really want to pull that "put the
> > maintainer in this position" trigger.  Have you really examined all
> > the
> > possible solutions?  Is there really no superior alternative?  Is
> > there
> > really no way to come to agreement with the other community
> > member(s)
> > you are disagreeing with?
> 
> And did you go to the mailing list and write something like that "the
> series on hold till solution will be found", like it was with hfi1-
> vnic
> and ipoib?

No, and if you recall, there was a decent period of silence on that
before I told you guys to work it out too.

> The Answer is no, you didn't do it, you silently ignored discussion,
> understood
> that the series is stuck and continued down the road.
> 
> > 
> > In the end, there will always be those things that a maintainer
> > will
> > simply need to make a call on because there won't be a clear right
> > or
> > wrong side and it just becomes a judgment call.  But I don't think
> > this
> > one even came close to being that unclear, so again I'm confused by
> > your insistence on going this route instead of just working things
> > out
> > with Chien.
> 
> Because, I truly believe that they proposed the nasty hack, which
> doesn't fix the real problem - inability to deal with losses of
> netlink
> messages.

In so much as the iwpmd needs to be modified to do a resync, I agree
with you.  But, the fix they made caused the system to be "reliable
enough" that the problem more or less disappeared.  It is true that,
given enough load, the problem could resurface, but that hasn't
happened yet in the real world.

However, iwpmd is part of rdma-core now, so *anyone* can fix it to do
the resyncs as needed (and I don't know, maybe it does, I just did a
quick grep for ENOBUFS to see if they capture that error and do a
resync on it, and saw no hits for that with grep).

> And I realized it after I started to work on RDMA netlink and read
> RFC
> and design documents and assumptions about netlink.
> 
> In my opinion, it is one of the small examples why RDMA subsystem is
> so
> hated. It is full of "custom" solutions to known problems and many of
> those "custom" solutions are mix of hacks and band aids.

I don't believe this solution was as custom as you say it is.  The
netlink core supports it and we aren't the only consumer.  That makes
it a bit more generic than you paint it here IMO.

> There is no need to reply on it.
> It is my personal opinion and my view of the RDMA subsystem as a
> newcomer.
> 
> I'm OOO till Monday.
> Happy weekends.
> 
> > 
> > 
> > --
> > Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >     GPG KeyID: B826A3330E572FDD
> >     Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57
> > 2FDD
> > 
-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Doug Ledford]

On Thu, 2017-08-03 at 08:22 -0400, Doug Ledford wrote:
> On Thu, 2017-08-03 at 08:10 +0300, Leon Romanovsky wrote:
> > 
> > Because, I truly believe that they proposed the nasty hack, which
> > doesn't fix the real problem - inability to deal with losses of
> > netlink
> > messages.
> 
> In so much as the iwpmd needs to be modified to do a resync, I agree
> with you.  But, the fix they made caused the system to be "reliable
> enough" that the problem more or less disappeared.  It is true that,
> given enough load, the problem could resurface, but that hasn't
> happened yet in the real world.
> 
> However, iwpmd is part of rdma-core now, so *anyone* can fix it to do
> the resyncs as needed (and I don't know, maybe it does, I just did a
> quick grep for ENOBUFS to see if they capture that error and do a
> resync on it, and saw no hits for that with grep).

And in case this didn't sink in from the above statement, allow me to
make it explicit:

Does it not strike you how much of a failure it was on your part to
submit a kernel patch that breaks iwpmd, code that is now part of rdma-
core, FOR WHICH YOU ARE COMAINTAINER, without the corresponding patches
to rdma-core to fix the real issue?  You don't get to tell Chien to "go
fix his software" because his software is part of rdma-core for which
you are co-maintainer and ultimately you bear responsibility for rdma-
core as a whole whether Chien fixes his stuff or not!  Your job is to
make sure that rdma-core is never broken in the way you were going to
break it.  You had a conflict of interest here between your kernel work
and your user space responsibilities and you did not choose well how to
resolve that conflict.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Leon Romanovsky]

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

On Thu, Aug 03, 2017 at 08:42:39AM -0400, Doug Ledford wrote:
> On Thu, 2017-08-03 at 08:22 -0400, Doug Ledford wrote:
> > On Thu, 2017-08-03 at 08:10 +0300, Leon Romanovsky wrote:
> > >
> > > Because, I truly believe that they proposed the nasty hack, which
> > > doesn't fix the real problem - inability to deal with losses of
> > > netlink
> > > messages.
> >
> > In so much as the iwpmd needs to be modified to do a resync, I agree
> > with you.  But, the fix they made caused the system to be "reliable
> > enough" that the problem more or less disappeared.  It is true that,
> > given enough load, the problem could resurface, but that hasn't
> > happened yet in the real world.
> >
> > However, iwpmd is part of rdma-core now, so *anyone* can fix it to do
> > the resyncs as needed (and I don't know, maybe it does, I just did a
> > quick grep for ENOBUFS to see if they capture that error and do a
> > resync on it, and saw no hits for that with grep).
>
> And in case this didn't sink in from the above statement, allow me to
> make it explicit:
>
> Does it not strike you how much of a failure it was on your part to
> submit a kernel patch that breaks iwpmd, code that is now part of rdma-
> core, FOR WHICH YOU ARE COMAINTAINER, without the corresponding patches
> to rdma-core to fix the real issue?  You don't get to tell Chien to "go
> fix his software" because his software is part of rdma-core for which
> you are co-maintainer and ultimately you bear responsibility for rdma-
> core as a whole whether Chien fixes his stuff or not!  Your job is to
> make sure that rdma-core is never broken in the way you were going to
> break it.  You had a conflict of interest here between your kernel work
> and your user space responsibilities and you did not choose well how to
> resolve that conflict.

Ohh, my god.

Doug,
There is no ANY breakage of UAPI and this was used as an argument
without ANY supportive claim behind of it. Just for the fun, I used the
same claim, but it looks like I was supposed to cut it once it was
claimed and not silently skip.

The iwpmd in rdma-core didn't get ANY patches BEFORE the patch in question
and AFTER.

So yes, I do responsible for the rdma-core and I'm enforcing the same policy
as we are using kernel - push companies to fix the root cause
and invest in the common infrastructure.

Chien is supported by huge corporation with many developers working on iWARP stack
and it seems very natural to demand from them the right solution.

Publicly, I tried it twice for the rdma-core:
1. One company stepped extra mile and improved the infrastructure.
2. Another company vanished into the dark and their supper important
feature wasn't so important at the end.

Thanks

>
> --
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>     GPG KeyID: B826A3330E572FDD
>     Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>

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

Re: [rdma-next 01/33] Revert "IB/core: Add flow control to the portmapper netlink calls"

[Bart Van Assche]

On Sun, 2017-08-06 at 10:47 +0300, Leon Romanovsky wrote:
> Chien is supported by huge corporation with many developers working on iWARP stack
> and it seems very natural to demand from them the right solution.

Let's see whether or not the interface between kernel and iwpmd gets fixed
by someone who really cares about iWARP. If this interface does not get fixed
maybe that tells us something about the economic importance today of iWARP
adapters ...

Bart.