Skip to contentSkip to conversation

RDMA core, drivers and IPoIB fixes

GIT PULL In review Leon Romanovsky 3 participants 39 messages Aug 17, 2017 – Aug 28, 2017 13 messages not in the archive

Subsystems (7)
lore
Share & tools
lore
#0 lore raw

Hi,

This is bunch of fixes to the RDMA stack which we found during code review, static checkers cleanups and by various code inspections.

There is one exception to this batch - patch from Feras to add statistics support to SRIOV VF in IPoIB.

However, the diff stat of that change is so small and I didn't find other batch to attach it.

 drivers/infiniband/ulp/ipoib/ipoib_main.c | 1 +
 1 file changed, 1 insertion(+)

Thanks

Changelog
v2->v3:
* Dropped node_type removal patch
* Dropped following pull request with cleanups and UAPI changes and integrate two leftover patches into this pull request.
v1->v2:
* Dropped patch to remove empty gid function from hns, till proper solution will be found.
* Added two new patches to rxe and mlx5 - add static annotation.

v0->v1:
* Patch 20: Reused memchr_inv function instead of open-coded variant
* Added Dennis's ROB tags

----------------------------------------------------------------
The following changes since commit 3e5f0881f17525e3b49835947a5e0cf2d681b1e2:

IB/hns: Avoid compile test under non 64bit environments (2017-08-14 11:16:53 -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-17

for you to fetch changes up to e2456daaf8b627e9b3a93115bd2366dfce9347fb:

RDMA/nes: Remove zeroed parameter from port query callback (2017-08-17 10:55:29 +0300)

Erez Shitrit (1):
IB/ipoib: Sync between remove_one to sysfs calls that use rtnl_lock

Feras Daoud (1):
IB/ipoib: Add get statistics support to SRIOV VF

Guy Levi (4):
IB/mlx4: Fix RSS QP type in creation verb
IB/mlx4: Fix struct mlx4_ib_create_wq alignment
IB/mlx4: Remove redundant attribute in mlx4_ib_create_qp_rss struct IB/mlx4: Check that reserved fields in mlx4_ib_create_qp_rss are zero

Kamal Heib (1):
IB/rxe: Make rxe_counter_name static

Leon Romanovsky (12):
IB/cma: Fix erroneous validation of supported default GID type RDMA/mlx4: Don't use uninitialized variable RDMA/(core,ulp): Convert register/unregister event handler to be void RDMA/core: Cleanup device capability enum
RDMA/core: Delete BUG() from unreachable flow
RDMA/core: Refactor get link layer wrapper
RDMA/mlx4: Remove gfp_mask argument from acquire_group call RDMA/usnic: Fix remove address space warning
RDMA/mthca: Make explicit conversion to 64bit value RDMA/mlx5: Limit scope of get vector affinity local function RDMA/mlx4: Properly annotate link layer variable RDMA/nes: Remove zeroed parameter from port query callback

Maor Gottlieb (2):
RDMA/mlx4: Fix create qp command alignment IB/mlx5: Add necessary delay drop assignment

Parav Pandit (1):
IB/uverbs: Introduce and use helper functions to copy ah attributes

Talat Batheesh (2):
IB/mlx4: Fix some spelling mistakes
IB/mlx5: Fix some spelling mistakes

 drivers/infiniband/core/cache.c                 |  23 ++---
 drivers/infiniband/core/cma.c                   |   4 +-
 drivers/infiniband/core/device.c                |   8 +-
 drivers/infiniband/core/sa_query.c              |   3 +-
 drivers/infiniband/core/uverbs_cmd.c            | 124 ++++++++++--------------
 drivers/infiniband/core/uverbs_main.c           |  13 +--
 drivers/infiniband/core/verbs.c                 |  34 +++----
 drivers/infiniband/hw/mlx4/alias_GUID.c         |   2 +-
 drivers/infiniband/hw/mlx4/cq.c                 |   2 +-
 drivers/infiniband/hw/mlx4/mcg.c                |   9 +-
 drivers/infiniband/hw/mlx4/qp.c                 |  22 ++---
 drivers/infiniband/hw/mlx5/cq.c                 |   2 +-
 drivers/infiniband/hw/mlx5/mad.c                |   2 +-
 drivers/infiniband/hw/mlx5/main.c               |   6 +-
 drivers/infiniband/hw/mthca/mthca_cmd.c         |   2 +-
 drivers/infiniband/hw/nes/nes_verbs.c           |   5 -
 drivers/infiniband/hw/usnic/usnic_fwd.c         |  12 +--
 drivers/infiniband/hw/usnic/usnic_fwd.h         |   2 +-
 drivers/infiniband/hw/usnic/usnic_ib_main.c     |  10 +-
 drivers/infiniband/sw/rxe/rxe_hw_counters.c     |   2 +-
 drivers/infiniband/ulp/ipoib/ipoib.h            |   1 +
 drivers/infiniband/ulp/ipoib/ipoib_cm.c         |   8 +-
 drivers/infiniband/ulp/ipoib/ipoib_main.c       |  16 ++-
 drivers/infiniband/ulp/ipoib/ipoib_vlan.c       |  22 ++++-
 drivers/infiniband/ulp/iser/iser_verbs.c        |   6 +-
 drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c |   7 +-
 drivers/infiniband/ulp/srpt/ib_srpt.c           |   5 +-
 include/rdma/ib_verbs.h                         |  10 +-
 include/uapi/rdma/mlx4-abi.h                    |   6 +-
 29 files changed, 154 insertions(+), 214 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

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

#2 lore raw

PATCH v3 1/24 IB/cma: Fix erroneous validation of supported default GID type

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

When rdma_cm is initializing a cma_device it checks if this device supports the preferred default GID type. This check was done in a wrong way and therefore sometimes rdma_cm is coming up with default GID type that is not supported by the device.

Fix that by checking for supported GID type properly.

Fixes 3c7f67d1880d ("IB/cma: Fix default RoCE type setting")
---
 drivers/infiniband/core/cma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/infiniband/core/cma.c +2 -2
index d8edd8b11561..f5f88b427328 100644
@@ -72,7 +72,7 @@ MODULE_LICENSE("Dual BSD/GPL");
 #define CMA_MAX_CM_RETRIES 15
 #define CMA_CM_MRA_SETTING (IB_CM_MRA_FLAG_DELAY | 24)
 #define CMA_IBOE_PACKET_LIFETIME 18
-#define CMA_PREFERRED_ROCE_GID_TYPE (1 << IB_GID_TYPE_ROCE_UDP_ENCAP)
+#define CMA_PREFERRED_ROCE_GID_TYPE IB_GID_TYPE_ROCE_UDP_ENCAP
 
 static const char * const cma_events[] = {
 	[RDMA_CM_EVENT_ADDR_RESOLVED]	 = "address resolved",
@@ -4282,7 +4282,7 @@ static void cma_add_one(struct ib_device *device)
 	for (i = rdma_start_port(device); i <= rdma_end_port(device); i++) {
 		supported_gids = roce_gid_type_mask_support(device, i);
 		WARN_ON(!supported_gids);
-		if (supported_gids & CMA_PREFERRED_ROCE_GID_TYPE)
+		if (supported_gids & (1 << CMA_PREFERRED_ROCE_GID_TYPE))
 			cma_dev->default_gid_type[i - rdma_start_port(device)] =
 				CMA_PREFERRED_ROCE_GID_TYPE;
 		else
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Reviewed-by
#4 lore raw

Re: [rdma-next v3 01/24] IB/cma: Fix erroneous validation of supported default GID type

Quoted 7 lines · Leon Romanovsky

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

When rdma_cm is initializing a cma_device it checks if this device supports the preferred default GID type. This check was done in a wrong way and therefore sometimes rdma_cm is coming up with default GID type that is not supported by the device.

Quoted 6 lines

Fix that by checking for supported GID type properly.

Fixes 3c7f67d1880d ("IB/cma: Fix default RoCE type setting")

---

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#5 lore raw

PATCH v3 2/24 IB/uverbs: Introduce and use helper functions to copy ah attributes

In reply to a message not in the archive

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

This patch introduces two helper functions to copy ah attributes from uverbs to internal ib_ah_attr structure and the other way during modify qp and query qp respectively.

---
 drivers/infiniband/core/uverbs_cmd.c | 124 ++++++++++++++---------------------
 1 file changed, 49 insertions(+), 75 deletions(-)
drivers/infiniband/core/uverbs_cmd.c +49 -75
index 670176b670a0..515425a50059 100644
@@ -1810,6 +1810,28 @@ ssize_t ib_uverbs_open_qp(struct ib_uverbs_file *file,
 	return ret;
 }
 
+static void copy_ah_attr_to_uverbs(struct ib_uverbs_qp_dest *uverb_attr,
+				   struct rdma_ah_attr *rdma_attr)
+{
+	const struct ib_global_route   *grh;
+
+	uverb_attr->dlid              = rdma_ah_get_dlid(rdma_attr);
+	uverb_attr->sl                = rdma_ah_get_sl(rdma_attr);
+	uverb_attr->src_path_bits     = rdma_ah_get_path_bits(rdma_attr);
+	uverb_attr->static_rate       = rdma_ah_get_static_rate(rdma_attr);
+	uverb_attr->is_global         = !!(rdma_ah_get_ah_flags(rdma_attr) &
+					 IB_AH_GRH);
+	if (uverb_attr->is_global) {
+		grh = rdma_ah_read_grh(rdma_attr);
+		memcpy(uverb_attr->dgid, grh->dgid.raw, 16);
+		uverb_attr->flow_label        = grh->flow_label;
+		uverb_attr->sgid_index        = grh->sgid_index;
+		uverb_attr->hop_limit         = grh->hop_limit;
+		uverb_attr->traffic_class     = grh->traffic_class;
+	}
+	uverb_attr->port_num          = rdma_ah_get_port_num(rdma_attr);
+}
+
 ssize_t ib_uverbs_query_qp(struct ib_uverbs_file *file,
 			   struct ib_device *ib_dev,
 			   const char __user *buf, int in_len,
@@ -1820,7 +1842,6 @@ ssize_t ib_uverbs_query_qp(struct ib_uverbs_file *file,
 	struct ib_qp                   *qp;
 	struct ib_qp_attr              *attr;
 	struct ib_qp_init_attr         *init_attr;
-	const struct ib_global_route   *grh;
 	int                            ret;
 
 	if (copy_from_user(&cmd, buf, sizeof cmd))
@@ -1870,39 +1891,8 @@ ssize_t ib_uverbs_query_qp(struct ib_uverbs_file *file,
 	resp.alt_port_num           = attr->alt_port_num;
 	resp.alt_timeout            = attr->alt_timeout;
 
-	resp.dest.dlid              = rdma_ah_get_dlid(&attr->ah_attr);
-	resp.dest.sl                = rdma_ah_get_sl(&attr->ah_attr);
-	resp.dest.src_path_bits     = rdma_ah_get_path_bits(&attr->ah_attr);
-	resp.dest.static_rate       = rdma_ah_get_static_rate(&attr->ah_attr);
-	resp.dest.is_global         = !!(rdma_ah_get_ah_flags(&attr->ah_attr) &
-					 IB_AH_GRH);
-	if (resp.dest.is_global) {
-		grh = rdma_ah_read_grh(&attr->ah_attr);
-		memcpy(resp.dest.dgid, grh->dgid.raw, 16);
-		resp.dest.flow_label        = grh->flow_label;
-		resp.dest.sgid_index        = grh->sgid_index;
-		resp.dest.hop_limit         = grh->hop_limit;
-		resp.dest.traffic_class     = grh->traffic_class;
-	}
-	resp.dest.port_num          = rdma_ah_get_port_num(&attr->ah_attr);
-
-	resp.alt_dest.dlid          = rdma_ah_get_dlid(&attr->alt_ah_attr);
-	resp.alt_dest.sl            = rdma_ah_get_sl(&attr->alt_ah_attr);
-	resp.alt_dest.src_path_bits = rdma_ah_get_path_bits(&attr->alt_ah_attr);
-	resp.alt_dest.static_rate
-			= rdma_ah_get_static_rate(&attr->alt_ah_attr);
-	resp.alt_dest.is_global
-			= !!(rdma_ah_get_ah_flags(&attr->alt_ah_attr) &
-						  IB_AH_GRH);
-	if (resp.alt_dest.is_global) {
-		grh = rdma_ah_read_grh(&attr->alt_ah_attr);
-		memcpy(resp.alt_dest.dgid, grh->dgid.raw, 16);
-		resp.alt_dest.flow_label    = grh->flow_label;
-		resp.alt_dest.sgid_index    = grh->sgid_index;
-		resp.alt_dest.hop_limit     = grh->hop_limit;
-		resp.alt_dest.traffic_class = grh->traffic_class;
-	}
-	resp.alt_dest.port_num      = rdma_ah_get_port_num(&attr->alt_ah_attr);
+	copy_ah_attr_to_uverbs(&resp.dest, &attr->ah_attr);
+	copy_ah_attr_to_uverbs(&resp.alt_dest, &attr->alt_ah_attr);
 
 	resp.max_send_wr            = init_attr->cap.max_send_wr;
 	resp.max_recv_wr            = init_attr->cap.max_recv_wr;
@@ -1936,6 +1926,28 @@ static int modify_qp_mask(enum ib_qp_type qp_type, int mask)
 	}
 }
 
+static void copy_ah_attr_from_uverbs(struct ib_device *dev,
+				     struct rdma_ah_attr *rdma_attr,
+				     struct ib_uverbs_qp_dest *uverb_attr)
+{
+	rdma_attr->type = rdma_ah_find_type(dev, uverb_attr->port_num);
+	if (uverb_attr->is_global) {
+		rdma_ah_set_grh(rdma_attr, NULL,
+				uverb_attr->flow_label,
+				uverb_attr->sgid_index,
+				uverb_attr->hop_limit,
+				uverb_attr->traffic_class);
+		rdma_ah_set_dgid_raw(rdma_attr, uverb_attr->dgid);
+	} else {
+		rdma_ah_set_ah_flags(rdma_attr, 0);
+	}
+	rdma_ah_set_dlid(rdma_attr, uverb_attr->dlid);
+	rdma_ah_set_sl(rdma_attr, uverb_attr->sl);
+	rdma_ah_set_path_bits(rdma_attr, uverb_attr->src_path_bits);
+	rdma_ah_set_static_rate(rdma_attr, uverb_attr->static_rate);
+	rdma_ah_set_port_num(rdma_attr, uverb_attr->port_num);
+}
+
 static int modify_qp(struct ib_uverbs_file *file,
 		     struct ib_uverbs_ex_modify_qp *cmd, struct ib_udata *udata)
 {
@@ -1982,47 +1994,9 @@ static int modify_qp(struct ib_uverbs_file *file,
 	attr->alt_timeout	  = cmd->base.alt_timeout;
 	attr->rate_limit	  = cmd->rate_limit;
 
-	attr->ah_attr.type = rdma_ah_find_type(qp->device,
-					       cmd->base.dest.port_num);
-	if (cmd->base.dest.is_global) {
-		rdma_ah_set_grh(&attr->ah_attr, NULL,
-				cmd->base.dest.flow_label,
-				cmd->base.dest.sgid_index,
-				cmd->base.dest.hop_limit,
-				cmd->base.dest.traffic_class);
-		rdma_ah_set_dgid_raw(&attr->ah_attr, cmd->base.dest.dgid);
-	} else {
-		rdma_ah_set_ah_flags(&attr->ah_attr, 0);
-	}
-	rdma_ah_set_dlid(&attr->ah_attr, cmd->base.dest.dlid);
-	rdma_ah_set_sl(&attr->ah_attr, cmd->base.dest.sl);
-	rdma_ah_set_path_bits(&attr->ah_attr, cmd->base.dest.src_path_bits);
-	rdma_ah_set_static_rate(&attr->ah_attr, cmd->base.dest.static_rate);
-	rdma_ah_set_port_num(&attr->ah_attr,
-			     cmd->base.dest.port_num);
-
-	attr->alt_ah_attr.type = rdma_ah_find_type(qp->device,
-						   cmd->base.dest.port_num);
-	if (cmd->base.alt_dest.is_global) {
-		rdma_ah_set_grh(&attr->alt_ah_attr, NULL,
-				cmd->base.alt_dest.flow_label,
-				cmd->base.alt_dest.sgid_index,
-				cmd->base.alt_dest.hop_limit,
-				cmd->base.alt_dest.traffic_class);
-		rdma_ah_set_dgid_raw(&attr->alt_ah_attr,
-				     cmd->base.alt_dest.dgid);
-	} else {
-		rdma_ah_set_ah_flags(&attr->alt_ah_attr, 0);
-	}
-
-	rdma_ah_set_dlid(&attr->alt_ah_attr, cmd->base.alt_dest.dlid);
-	rdma_ah_set_sl(&attr->alt_ah_attr, cmd->base.alt_dest.sl);
-	rdma_ah_set_path_bits(&attr->alt_ah_attr,
-			      cmd->base.alt_dest.src_path_bits);
-	rdma_ah_set_static_rate(&attr->alt_ah_attr,
-				cmd->base.alt_dest.static_rate);
-	rdma_ah_set_port_num(&attr->alt_ah_attr,
-			     cmd->base.alt_dest.port_num);
+	copy_ah_attr_from_uverbs(qp->device, &attr->ah_attr, &cmd->base.dest);
+	copy_ah_attr_from_uverbs(qp->device, &attr->alt_ah_attr,
+				 &cmd->base.alt_dest);
 
 	ret = ib_modify_qp_with_udata(qp, attr,
 				      modify_qp_mask(qp->qp_type,
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

#7 lore raw

Re: [rdma-next v3 02/24] IB/uverbs: Introduce and use helper functions to copy ah attributes

Quoted 6 lines · Leon Romanovsky

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

This patch introduces two helper functions to copy ah attributes from uverbs to internal ib_ah_attr structure and the other way during modify qp and query qp respectively.

Quoted 8 lines
---
 drivers/infiniband/core/uverbs_cmd.c | 124 ++++++++++++++-----------
----------
 1 file changed, 49 insertions(+), 75 deletions(-)

I had to fix this patch up considerably...

Quoted 35 lines
drivers/infiniband/core/uverbs_cmd.c +22 -0
index 670176b670a0..515425a50059 100644
@@ -1936,6 +1926,28 @@ static int modify_qp_mask(enum ib_qp_type
 qp_type, int mask)
 	}
 }
 
+static void copy_ah_attr_from_uverbs(struct ib_device *dev,
+				     struct rdma_ah_attr *rdma_attr,
+				     struct ib_uverbs_qp_dest
 *uverb_attr)
+{
+	rdma_attr->type = rdma_ah_find_type(dev, uverb_attr-
 >port_num);
+	if (uverb_attr->is_global) {
+		rdma_ah_set_grh(rdma_attr, NULL,
+				uverb_attr->flow_label,
+				uverb_attr->sgid_index,
+				uverb_attr->hop_limit,
+				uverb_attr->traffic_class);
+		rdma_ah_set_dgid_raw(rdma_attr, uverb_attr->dgid);
+	} else {
+		rdma_ah_set_ah_flags(rdma_attr, 0);
+	}
+	rdma_ah_set_dlid(rdma_attr, uverb_attr->dlid);
+	rdma_ah_set_sl(rdma_attr, uverb_attr->sl);
+	rdma_ah_set_path_bits(rdma_attr, uverb_attr->src_path_bits);
+	rdma_ah_set_static_rate(rdma_attr, uverb_attr->static_rate);
+	rdma_ah_set_port_num(rdma_attr, uverb_attr->port_num);
+}
+
Quoted 4 lines

static int modify_qp(struct ib_uverbs_file *file,
struct ib_uverbs_ex_modify_qp *cmd, struct
ib_udata *udata)
{

My current HEAD of my k.o/for-next (which is merged with my current head of k.o/for-4.13-rc and so should contain the most up to date code for the RDMA subsystem) includes this in the areas where it manually copies the ah attributes:

rdma_ah_set_make_grd(rdma_attr, false);

So I added it to the end of the helper function.

drivers/infiniband/core/uverbs_cmd.c static int modify_qp(struct ib_uverbs_file · line 1998

Quoted 8 lines
drivers/infiniband/core/uverbs_cmd.c +0 -2
@@ -1982,47 +1994,9 @@ static int modify_qp(struct ib_uverbs_file
 *file,
 	attr->alt_timeout	  = cmd->base.alt_timeout;
 	attr->rate_limit	  = cmd->rate_limit;
 
-	attr->ah_attr.type = rdma_ah_find_type(qp->device,
-					       cmd-
 >base.dest.port_num);

And we had a fix here where we only copy the ah attributes if the user set the flag for it, so what I ended up with looks like this:

        if (cmd->base.attr_mask & IB_QP_AV)
                copy_ah_attr_from_uverbs(qp->device, &attr->ah_attr,
                                         &cmd->base.dest);

        if (cmd->base.attr_mask & IB_QP_ALT_PATH)
                copy_ah_attr_from_uverbs(qp->device, &attr-

drivers/infiniband/core/uverbs_cmd.c static int modify_qp(struct ib_uverbs_file · line 1999

Quoted 1 line
drivers/infiniband/core/uverbs_cmd.c +0 -0
 alt_ah_attr,
                                         &cmd->base.alt_dest);
Signature · 8 lines
Doug Ledford <[email protected]>
    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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 1503607242.78641.48.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

#9 lore raw

Re: [rdma-next v3 02/24] IB/uverbs: Introduce and use helper functions to copy ah attributes

Quoted 79 lines · Doug Ledford
Quoted 6 lines · Leon Romanovsky

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

This patch introduces two helper functions to copy ah attributes from uverbs to internal ib_ah_attr structure and the other way during modify qp and query qp respectively.

Quoted 8 lines
---
 drivers/infiniband/core/uverbs_cmd.c | 124 ++++++++++++++-----------
----------
 1 file changed, 49 insertions(+), 75 deletions(-)

I had to fix this patch up considerably...

Quoted 35 lines
drivers/infiniband/core/uverbs_cmd.c +22 -0
index 670176b670a0..515425a50059 100644
@@ -1936,6 +1926,28 @@ static int modify_qp_mask(enum ib_qp_type
 qp_type, int mask)
 	}
 }
 
+static void copy_ah_attr_from_uverbs(struct ib_device *dev,
+				     struct rdma_ah_attr *rdma_attr,
+				     struct ib_uverbs_qp_dest
 *uverb_attr)
+{
+	rdma_attr->type = rdma_ah_find_type(dev, uverb_attr-
 >port_num);
+	if (uverb_attr->is_global) {
+		rdma_ah_set_grh(rdma_attr, NULL,
+				uverb_attr->flow_label,
+				uverb_attr->sgid_index,
+				uverb_attr->hop_limit,
+				uverb_attr->traffic_class);
+		rdma_ah_set_dgid_raw(rdma_attr, uverb_attr->dgid);
+	} else {
+		rdma_ah_set_ah_flags(rdma_attr, 0);
+	}
+	rdma_ah_set_dlid(rdma_attr, uverb_attr->dlid);
+	rdma_ah_set_sl(rdma_attr, uverb_attr->sl);
+	rdma_ah_set_path_bits(rdma_attr, uverb_attr->src_path_bits);
+	rdma_ah_set_static_rate(rdma_attr, uverb_attr->static_rate);
+	rdma_ah_set_port_num(rdma_attr, uverb_attr->port_num);
+}
+
Quoted 4 lines

static int modify_qp(struct ib_uverbs_file *file,
struct ib_uverbs_ex_modify_qp *cmd, struct
ib_udata *udata)
{

My current HEAD of my k.o/for-next (which is merged with my current head of k.o/for-4.13-rc and so should contain the most up to date code for the RDMA subsystem) includes this in the areas where it manually copies the ah attributes:

rdma_ah_set_make_grd(rdma_attr, false);

So I added it to the end of the helper function.

drivers/infiniband/core/uverbs_cmd.c static int modify_qp(struct ib_uverbs_file · line 1998

Quoted 8 lines
drivers/infiniband/core/uverbs_cmd.c +0 -2
@@ -1982,47 +1994,9 @@ static int modify_qp(struct ib_uverbs_file
 *file,
 	attr->alt_timeout	  = cmd->base.alt_timeout;
 	attr->rate_limit	  = cmd->rate_limit;
 
-	attr->ah_attr.type = rdma_ah_find_type(qp->device,
-					       cmd-
 >base.dest.port_num);

And we had a fix here where we only copy the ah attributes if the user set the flag for it, so what I ended up with looks like this:

Quoted 8 lines
        if (cmd->base.attr_mask & IB_QP_AV)
                copy_ah_attr_from_uverbs(qp->device, &attr->ah_attr,
                                         &cmd->base.dest);

        if (cmd->base.attr_mask & IB_QP_ALT_PATH)
                copy_ah_attr_from_uverbs(qp->device, &attr-

drivers/infiniband/core/uverbs_cmd.c static int modify_qp(struct ib_uverbs_file · line 1999

Quoted 1 line
drivers/infiniband/core/uverbs_cmd.c +0 -0
 alt_ah_attr,
                                         &cmd->base.alt_dest);

Doug,

It should be "qp" and not "qp->device" in copy_ah_attr_from_uverbs().

1998 if (cmd->base.attr_mask & IB_QP_AV)
1999 copy_ah_attr_from_uverbs(qp, &attr->ah_attr, &cmd->base.dest);
2000 if (cmd->base.attr_mask & IB_QP_ALT_PATH)
2001 copy_ah_attr_from_uverbs(qp, &attr->alt_ah_attr,
2002 &cmd->base.alt_dest);
2003 ret = ib_modify_qp_with_udata(qp, attr,
2004 modify_qp_mask(qp->qp_type,
2005 cmd->base.attr_mask),
2006 udata);

Thanks

Quoted 6 lines

--
Doug Ledford <[email protected]>

    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

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

#11 lore raw

Re: [rdma-next v3 02/24] IB/uverbs: Introduce and use helper functions to copy ah attributes

Quoted 26 lines · Leon Romanovsky
Quoted 25 lines · Doug Ledford
Quoted 10 lines · Leon Romanovsky

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

This patch introduces two helper functions to copy ah attributes from uverbs to internal ib_ah_attr structure and the other way during modify qp and query qp respectively.

Quoted 5 lines
---
 drivers/infiniband/core/uverbs_cmd.c | 124 ++++++++++++++-------
----
----------
 1 file changed, 49 insertions(+), 75 deletions(-)

I had to fix this patch up considerably...

Quoted 6 lines
drivers/infiniband/core/uverbs_cmd.c +0 -0
index 670176b670a0..515425a50059 100644
Quoted 6 lines
Quoted 6 lines

drivers/infiniband/core/uverbs_cmd.c static int modify_qp_mask(enum ib_qp_type · line 1930

Quoted 6 lines
drivers/infiniband/core/uverbs_cmd.c +1 -0
@@ -1936,6 +1926,28 @@ static int modify_qp_mask(enum ib_qp_type
 qp_type, int mask)
 	}
 }
 
+static void copy_ah_attr_from_uverbs(struct ib_device *dev,
                                             ^^^^^^^^^^^^^^^^^^^^
Quoted 33 lines

From your patch...

Quoted 10 lines

drivers/infiniband/core/uverbs_cmd.c static int modify_qp_mask(enum ib_qp_type · line 1934

Quoted 4 lines
drivers/infiniband/core/uverbs_cmd.c +2 -0
+				     struct rdma_ah_attr
 *rdma_attr,
+				     struct ib_uverbs_qp_dest
 

And we had a fix here where we only copy the ah attributes if the user
set the flag for it, so what I ended up with looks like this:

        if (cmd->base.attr_mask & IB_QP_AV)
                copy_ah_attr_from_uverbs(qp->device, &attr-
Quoted 8 lines
Quoted 1 line

ah_attr,

                                         &cmd->base.dest);

        if (cmd->base.attr_mask & IB_QP_ALT_PATH)
                copy_ah_attr_from_uverbs(qp->device, &attr-
Quoted 1 line

alt_ah_attr,

                                         &cmd->base.alt_dest);

Doug,

It should be "qp" and not "qp->device" in copy_ah_attr_from_uverbs().

1998 if (cmd->base.attr_mask & IB_QP_AV)
1999 copy_ah_attr_from_uverbs(qp, &attr->ah_attr,
&cmd->base.dest);
2000 if (cmd->base.attr_mask & IB_QP_ALT_PATH)
2001 copy_ah_attr_from_uverbs(qp, &attr->alt_ah_attr,
2002 &cmd->base.alt_dest);
2003 ret = ib_modify_qp_with_udata(qp, attr,
2004 modify_qp_mask(qp-

Quoted 4 lines
Quoted 1 line

qp_type,

2005 cmd-

Quoted 1 line

base.attr_mask),

2006 udata);

I'm pretty sure you need to re-read your own patch Leon ;-). The helper you added uses struct ib_device, so a pointer to the qp would not work. And it wouldn't have even compiled if I had that wrong.

Signature · 8 lines
Doug Ledford <[email protected]>
    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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 1503933314.78641.82.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

#13 lore raw

Re: [rdma-next v3 02/24] IB/uverbs: Introduce and use helper functions to copy ah attributes

Quoted 67 lines · Doug Ledford
Quoted 26 lines · Leon Romanovsky
Quoted 25 lines · Doug Ledford
Quoted 10 lines · Leon Romanovsky

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

This patch introduces two helper functions to copy ah attributes from uverbs to internal ib_ah_attr structure and the other way during modify qp and query qp respectively.

Quoted 5 lines
---
 drivers/infiniband/core/uverbs_cmd.c | 124 ++++++++++++++-------
----
----------
 1 file changed, 49 insertions(+), 75 deletions(-)

I had to fix this patch up considerably...

Quoted 6 lines
drivers/infiniband/core/uverbs_cmd.c +0 -0
index 670176b670a0..515425a50059 100644
Quoted 6 lines
Quoted 6 lines

drivers/infiniband/core/uverbs_cmd.c static int modify_qp_mask(enum ib_qp_type · line 1930

Quoted 6 lines
drivers/infiniband/core/uverbs_cmd.c +1 -0
@@ -1936,6 +1926,28 @@ static int modify_qp_mask(enum ib_qp_type
 qp_type, int mask)
 	}
 }
 
+static void copy_ah_attr_from_uverbs(struct ib_device *dev,
                                             ^^^^^^^^^^^^^^^^^^^^

From your patch...

Quoted 31 lines
Quoted 10 lines

drivers/infiniband/core/uverbs_cmd.c static int modify_qp_mask(enum ib_qp_type · line 1934

Quoted 4 lines
drivers/infiniband/core/uverbs_cmd.c +2 -0
+				     struct rdma_ah_attr
 *rdma_attr,
+				     struct ib_uverbs_qp_dest
 

And we had a fix here where we only copy the ah attributes if the user
set the flag for it, so what I ended up with looks like this:

        if (cmd->base.attr_mask & IB_QP_AV)
                copy_ah_attr_from_uverbs(qp->device, &attr-
Quoted 8 lines
Quoted 1 line

ah_attr,

                                         &cmd->base.dest);

        if (cmd->base.attr_mask & IB_QP_ALT_PATH)
                copy_ah_attr_from_uverbs(qp->device, &attr-
Quoted 1 line

alt_ah_attr,

                                         &cmd->base.alt_dest);

Doug,

It should be "qp" and not "qp->device" in copy_ah_attr_from_uverbs().

1998 if (cmd->base.attr_mask & IB_QP_AV)
1999 copy_ah_attr_from_uverbs(qp, &attr->ah_attr,
&cmd->base.dest);
2000 if (cmd->base.attr_mask & IB_QP_ALT_PATH)
2001 copy_ah_attr_from_uverbs(qp, &attr->alt_ah_attr,
2002 &cmd->base.alt_dest);
2003 ret = ib_modify_qp_with_udata(qp, attr,
2004 modify_qp_mask(qp-

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

qp_type,

2005 cmd-

Quoted 1 line

base.attr_mask),

2006 udata);

I'm pretty sure you need to re-read your own patch Leon ;-). The helper you added uses struct ib_device, so a pointer to the qp would not work. And it wouldn't have even compiled if I had that wrong.

OK, I reread it and it helped me to find the differences.

In rdma-rc, there is the patch from Noa "IB/core: Avoid accessing non-allocated memory when inferring port type", which plays exactly at the same place as this one.

Because, I'm creating my rdma-next for testing, I'm merging -rcX with -next and my merge conflict resolution was to move
+ attr->ah_attr.type = rdma_ah_find_type(qp->device,
+ cmd->base.dest.port_num);
to the copy_ah_attr_from_uverbs and to change function signature to be "static void copy_ah_attr_from_uverbs(struct ib_qp *qp,"

Thanks

Quoted 10 lines

--
Doug Ledford <[email protected]>

    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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

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

#15 lore raw

Re: [rdma-next v3 02/24] IB/uverbs: Introduce and use helper functions to copy ah attributes

Quoted 105 lines · Leon Romanovsky
Quoted 83 lines · Doug Ledford
Quoted 30 lines · Leon Romanovsky
Quoted 29 lines · Doug Ledford
Quoted 13 lines · Leon Romanovsky

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

This patch introduces two helper functions to copy ah attributes
from uverbs to internal ib_ah_attr structure and the other way
during modify qp and query qp respectively.

Quoted 1 line
Quoted 6 lines
---
 drivers/infiniband/core/uverbs_cmd.c | 124 ++++++++++++++---
----
----
----------
 1 file changed, 49 insertions(+), 75 deletions(-)

I had to fix this patch up considerably...

Quoted 6 lines
drivers/infiniband/core/uverbs_cmd.c +0 -0
index 670176b670a0..515425a50059 100644
Quoted 7 lines
Quoted 7 lines

drivers/infiniband/core/uverbs_cmd.c

Quoted 7 lines
drivers/infiniband/core/uverbs_cmd.c +0 -0
@@ -1936,6 +1926,28 @@ static int modify_qp_mask(enum

ib_qp_type
qp_type, int mask)

drivers/infiniband/core/uverbs_cmd.c +1 -0
 	}
 }
 
+static void copy_ah_attr_from_uverbs(struct ib_device *dev,
                                             ^^^^^^^^^^^^^^^^^^^^

From your patch...

Quoted 41 lines
Quoted 15 lines
Quoted 5 lines
drivers/infiniband/core/uverbs_cmd.c +1 -0
+				     struct rdma_ah_attr

*rdma_attr,

drivers/infiniband/core/uverbs_cmd.c +1 -0
+				     struct

ib_uverbs_qp_dest

And we had a fix here where we only copy the ah attributes if the
user
set the flag for it, so what I ended up with looks like this:

        if (cmd->base.attr_mask & IB_QP_AV)
                copy_ah_attr_from_uverbs(qp->device, &attr-
Quoted 1 line

ah_attr,

Quoted 7 lines
                                         &cmd->base.dest);

        if (cmd->base.attr_mask & IB_QP_ALT_PATH)
                copy_ah_attr_from_uverbs(qp->device, &attr-
Quoted 1 line

alt_ah_attr,

                                         &cmd->base.alt_dest);

Doug,

It should be "qp" and not "qp->device" in
copy_ah_attr_from_uverbs().

1998 if (cmd->base.attr_mask & IB_QP_AV)
1999 copy_ah_attr_from_uverbs(qp, &attr->ah_attr,
&cmd->base.dest);
2000 if (cmd->base.attr_mask & IB_QP_ALT_PATH)
2001 copy_ah_attr_from_uverbs(qp, &attr-

Quoted 1 line

alt_ah_attr,

2002 &cmd-

Quoted 1 line

base.alt_dest);

2003 ret = ib_modify_qp_with_udata(qp, attr,
2004 modify_qp_mask(qp-

Quoted 1 line

qp_type,

2005 cmd-

Quoted 8 lines
Quoted 3 lines
Quoted 1 line

base.attr_mask),

2006 udata);

I'm pretty sure you need to re-read your own patch Leon ;-). The helper you added uses struct ib_device, so a pointer to the qp would
not work. And it wouldn't have even compiled if I had that wrong.

OK, I reread it and it helped me to find the differences.

In rdma-rc, there is the patch from Noa "IB/core: Avoid accessing non-allocated memory when inferring port type", which plays exactly at
the same place as this one.

Because, I'm creating my rdma-next for testing, I'm merging -rcX with

(quoted diff) +2 -1
-next and my merge conflict resolution was to move
+               attr->ah_attr.type = rdma_ah_find_type(qp->device,
+                                                      cmd-
Quoted 1 line

base.dest.port_num);

Quoted 3 lines

to the copy_ah_attr_from_uverbs and to change function signature to be
"static void copy_ah_attr_from_uverbs(struct ib_qp *qp,"

It turns out this is kind of a moot point. In uverbs_marshall.c there already exists void ib_copy_ah_attr_to_user(). We should have used that instead of creating new versions of this in this file. So, since I've already taken this patch and pushed it to k.o, please do a new incremental patch that:

1) Merges the differences between the static versions of this routine and the one in uverbs_marshall.c into the one in uverbs_marshall.c
2) Adds a copy from uverbs in uverbs_marshall.c that is coded in the same style as the one already there
3) Converts uverbs_cmd.c routines to use the helpers in uverbs_marshall.c and removes the duplicate helpers in uverbs_cmd.c.

And, if at all possible, I need it quick because I'm making my pull request to Linus for the next merge window tomorrow.

Signature · 8 lines
Doug Ledford <[email protected]>
    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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 1503937924.78641.94.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

#17 lore raw

Re: [rdma-next v3 02/24] IB/uverbs: Introduce and use helper functions to copy ah attributes

Quoted 123 lines · Doug Ledford
Quoted 105 lines · Leon Romanovsky
Quoted 83 lines · Doug Ledford
Quoted 30 lines · Leon Romanovsky
Quoted 29 lines · Doug Ledford
Quoted 13 lines · Leon Romanovsky

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

This patch introduces two helper functions to copy ah attributes
from uverbs to internal ib_ah_attr structure and the other way
during modify qp and query qp respectively.

Quoted 1 line
Quoted 6 lines
---
 drivers/infiniband/core/uverbs_cmd.c | 124 ++++++++++++++---
----
----
----------
 1 file changed, 49 insertions(+), 75 deletions(-)

I had to fix this patch up considerably...

Quoted 6 lines
drivers/infiniband/core/uverbs_cmd.c +0 -0
index 670176b670a0..515425a50059 100644
Quoted 7 lines
Quoted 7 lines

drivers/infiniband/core/uverbs_cmd.c

Quoted 7 lines
drivers/infiniband/core/uverbs_cmd.c +0 -0
@@ -1936,6 +1926,28 @@ static int modify_qp_mask(enum

ib_qp_type
qp_type, int mask)

drivers/infiniband/core/uverbs_cmd.c +1 -0
 	}
 }
 
+static void copy_ah_attr_from_uverbs(struct ib_device *dev,
                                             ^^^^^^^^^^^^^^^^^^^^

From your patch...

Quoted 41 lines
Quoted 15 lines
Quoted 5 lines
drivers/infiniband/core/uverbs_cmd.c +1 -0
+				     struct rdma_ah_attr

*rdma_attr,

drivers/infiniband/core/uverbs_cmd.c +1 -0
+				     struct

ib_uverbs_qp_dest

And we had a fix here where we only copy the ah attributes if the
user
set the flag for it, so what I ended up with looks like this:

        if (cmd->base.attr_mask & IB_QP_AV)
                copy_ah_attr_from_uverbs(qp->device, &attr-
Quoted 1 line

ah_attr,

Quoted 7 lines
                                         &cmd->base.dest);

        if (cmd->base.attr_mask & IB_QP_ALT_PATH)
                copy_ah_attr_from_uverbs(qp->device, &attr-
Quoted 1 line

alt_ah_attr,

                                         &cmd->base.alt_dest);

Doug,

It should be "qp" and not "qp->device" in
copy_ah_attr_from_uverbs().

1998 if (cmd->base.attr_mask & IB_QP_AV)
1999 copy_ah_attr_from_uverbs(qp, &attr->ah_attr,
&cmd->base.dest);
2000 if (cmd->base.attr_mask & IB_QP_ALT_PATH)
2001 copy_ah_attr_from_uverbs(qp, &attr-

Quoted 1 line

alt_ah_attr,

2002 &cmd-

Quoted 1 line

base.alt_dest);

2003 ret = ib_modify_qp_with_udata(qp, attr,
2004 modify_qp_mask(qp-

Quoted 1 line

qp_type,

2005 cmd-

Quoted 8 lines
Quoted 3 lines
Quoted 1 line

base.attr_mask),

2006 udata);

I'm pretty sure you need to re-read your own patch Leon ;-). The helper you added uses struct ib_device, so a pointer to the qp would
not work. And it wouldn't have even compiled if I had that wrong.

OK, I reread it and it helped me to find the differences.

In rdma-rc, there is the patch from Noa "IB/core: Avoid accessing non-allocated memory when inferring port type", which plays exactly at
the same place as this one.

Because, I'm creating my rdma-next for testing, I'm merging -rcX with

(quoted diff) +2 -1
-next and my merge conflict resolution was to move
+               attr->ah_attr.type = rdma_ah_find_type(qp->device,
+                                                      cmd-
Quoted 1 line

base.dest.port_num);

Quoted 3 lines

to the copy_ah_attr_from_uverbs and to change function signature to be
"static void copy_ah_attr_from_uverbs(struct ib_qp *qp,"

It turns out this is kind of a moot point. In uverbs_marshall.c there already exists void ib_copy_ah_attr_to_user(). We should have used that instead of creating new versions of this in this file. So, since I've already taken this patch and pushed it to k.o, please do a new incremental patch that:

1) Merges the differences between the static versions of this routine and the one in uverbs_marshall.c into the one in uverbs_marshall.c
2) Adds a copy from uverbs in uverbs_marshall.c that is coded in the same style as the one already there
3) Converts uverbs_cmd.c routines to use the helpers in uverbs_marshall.c and removes the duplicate helpers in uverbs_cmd.c.

Quoted 2 lines

And, if at all possible, I need it quick because I'm making my pull request to Linus for the next merge window tomorrow.

On which branch do you want me to do it? The k.o/for-next doesn't have if(..) copy_.. parts.

Thanks

Quoted 6 lines

--
Doug Ledford <[email protected]>

    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

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

#19 lore raw

Re: [rdma-next v3 02/24] IB/uverbs: Introduce and use helper functions to copy ah attributes

Quoted 124 lines · Leon Romanovsky
Quoted 123 lines · Doug Ledford
Quoted 105 lines · Leon Romanovsky
Quoted 83 lines · Doug Ledford
Quoted 30 lines · Leon Romanovsky
Quoted 29 lines · Doug Ledford
Quoted 13 lines · Leon Romanovsky

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

This patch introduces two helper functions to copy ah attributes
from uverbs to internal ib_ah_attr structure and the other way
during modify qp and query qp respectively.

Quoted 1 line
Quoted 6 lines
---
 drivers/infiniband/core/uverbs_cmd.c | 124 ++++++++++++++---
----
----
----------
 1 file changed, 49 insertions(+), 75 deletions(-)

I had to fix this patch up considerably...

Quoted 6 lines
drivers/infiniband/core/uverbs_cmd.c +0 -0
index 670176b670a0..515425a50059 100644
Quoted 7 lines
Quoted 7 lines

drivers/infiniband/core/uverbs_cmd.c

Quoted 7 lines
drivers/infiniband/core/uverbs_cmd.c +0 -0
@@ -1936,6 +1926,28 @@ static int modify_qp_mask(enum

ib_qp_type
qp_type, int mask)

drivers/infiniband/core/uverbs_cmd.c +1 -0
 	}
 }
 
+static void copy_ah_attr_from_uverbs(struct ib_device *dev,
                                             ^^^^^^^^^^^^^^^^^^^^

From your patch...

Quoted 41 lines
Quoted 15 lines
Quoted 5 lines
drivers/infiniband/core/uverbs_cmd.c +1 -0
+				     struct rdma_ah_attr

*rdma_attr,

drivers/infiniband/core/uverbs_cmd.c +1 -0
+				     struct

ib_uverbs_qp_dest

And we had a fix here where we only copy the ah attributes if the
user
set the flag for it, so what I ended up with looks like this:

        if (cmd->base.attr_mask & IB_QP_AV)
                copy_ah_attr_from_uverbs(qp->device, &attr-
Quoted 1 line

ah_attr,

Quoted 7 lines
                                         &cmd->base.dest);

        if (cmd->base.attr_mask & IB_QP_ALT_PATH)
                copy_ah_attr_from_uverbs(qp->device, &attr-
Quoted 1 line

alt_ah_attr,

                                         &cmd->base.alt_dest);

Doug,

It should be "qp" and not "qp->device" in
copy_ah_attr_from_uverbs().

1998 if (cmd->base.attr_mask & IB_QP_AV)
1999 copy_ah_attr_from_uverbs(qp, &attr->ah_attr,
&cmd->base.dest);
2000 if (cmd->base.attr_mask & IB_QP_ALT_PATH)
2001 copy_ah_attr_from_uverbs(qp, &attr-

Quoted 1 line

alt_ah_attr,

2002 &cmd-

Quoted 1 line

base.alt_dest);

2003 ret = ib_modify_qp_with_udata(qp, attr,
2004 modify_qp_mask(qp-

Quoted 1 line

qp_type,

2005 cmd-

Quoted 8 lines
Quoted 3 lines
Quoted 1 line

base.attr_mask),

2006 udata);

I'm pretty sure you need to re-read your own patch Leon ;-). The helper you added uses struct ib_device, so a pointer to the qp would
not work. And it wouldn't have even compiled if I had that wrong.

OK, I reread it and it helped me to find the differences.

In rdma-rc, there is the patch from Noa "IB/core: Avoid accessing non-allocated memory when inferring port type", which plays exactly at
the same place as this one.

Because, I'm creating my rdma-next for testing, I'm merging -rcX with

(quoted diff) +2 -1
-next and my merge conflict resolution was to move
+               attr->ah_attr.type = rdma_ah_find_type(qp->device,
+                                                      cmd-
Quoted 1 line

base.dest.port_num);

Quoted 3 lines

to the copy_ah_attr_from_uverbs and to change function signature to be
"static void copy_ah_attr_from_uverbs(struct ib_qp *qp,"

It turns out this is kind of a moot point. In uverbs_marshall.c there already exists void ib_copy_ah_attr_to_user(). We should have used that instead of creating new versions of this in this file. So, since I've already taken this patch and pushed it to k.o, please do a new incremental patch that:

1) Merges the differences between the static versions of this routine and the one in uverbs_marshall.c into the one in uverbs_marshall.c
2) Adds a copy from uverbs in uverbs_marshall.c that is coded in the same style as the one already there
3) Converts uverbs_cmd.c routines to use the helpers in uverbs_marshall.c and removes the duplicate helpers in uverbs_cmd.c.

Quoted 5 lines
Quoted 2 lines

And, if at all possible, I need it quick because I'm making my pull request to Linus for the next merge window tomorrow.

On which branch do you want me to do it? The k.o/for-next doesn't have if(..) copy_.. parts.

Ahh, I found it in k.o/for-next.

Quoted 9 lines

Thanks

Quoted 6 lines

--
Doug Ledford <[email protected]>

    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
#20 lore raw

Re: [rdma-next v3 02/24] IB/uverbs: Introduce and use helper functions to copy ah attributes

In reply to a message not in the archive
Quoted 105 lines · Doug Ledford
Quoted 104 lines · Leon Romanovsky
Quoted 83 lines · Doug Ledford
Quoted 30 lines · Leon Romanovsky
Quoted 29 lines · Doug Ledford
Quoted 13 lines · Leon Romanovsky

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>

This patch introduces two helper functions to copy ah attributes
from uverbs to internal ib_ah_attr structure and the other way
during modify qp and query qp respectively.

Quoted 1 line
Quoted 6 lines
---
 drivers/infiniband/core/uverbs_cmd.c | 124 ++++++++++++++---
----
----
----------
 1 file changed, 49 insertions(+), 75 deletions(-)

I had to fix this patch up considerably...

Quoted 6 lines
drivers/infiniband/core/uverbs_cmd.c +0 -0
index 670176b670a0..515425a50059 100644
Quoted 7 lines
Quoted 7 lines

drivers/infiniband/core/uverbs_cmd.c

Quoted 7 lines
drivers/infiniband/core/uverbs_cmd.c +0 -0
@@ -1936,6 +1926,28 @@ static int modify_qp_mask(enum

ib_qp_type
qp_type, int mask)

drivers/infiniband/core/uverbs_cmd.c +1 -0
 	}
 }
 
+static void copy_ah_attr_from_uverbs(struct ib_device *dev,
                                             ^^^^^^^^^^^^^^^^^^^^

From your patch...

Quoted 41 lines
Quoted 15 lines
Quoted 5 lines
drivers/infiniband/core/uverbs_cmd.c +1 -0
+				     struct rdma_ah_attr

*rdma_attr,

drivers/infiniband/core/uverbs_cmd.c +1 -0
+				     struct

ib_uverbs_qp_dest

And we had a fix here where we only copy the ah attributes if the
user
set the flag for it, so what I ended up with looks like this:

        if (cmd->base.attr_mask & IB_QP_AV)
                copy_ah_attr_from_uverbs(qp->device, &attr-
Quoted 1 line

ah_attr,

Quoted 7 lines
                                         &cmd->base.dest);

        if (cmd->base.attr_mask & IB_QP_ALT_PATH)
                copy_ah_attr_from_uverbs(qp->device, &attr-
Quoted 1 line

alt_ah_attr,

                                         &cmd->base.alt_dest);

Doug,

It should be "qp" and not "qp->device" in
copy_ah_attr_from_uverbs().

1998 if (cmd->base.attr_mask & IB_QP_AV)
1999 copy_ah_attr_from_uverbs(qp, &attr->ah_attr,
&cmd->base.dest);
2000 if (cmd->base.attr_mask & IB_QP_ALT_PATH)
2001 copy_ah_attr_from_uverbs(qp, &attr-

Quoted 1 line

alt_ah_attr,

2002 &cmd-

Quoted 1 line

base.alt_dest);

2003 ret = ib_modify_qp_with_udata(qp, attr,
2004 modify_qp_mask(qp-

Quoted 1 line

qp_type,

2005 cmd-

Quoted 8 lines
Quoted 3 lines
Quoted 1 line

base.attr_mask),

2006 udata);

I'm pretty sure you need to re-read your own patch Leon ;-). The helper you added uses struct ib_device, so a pointer to the qp would
not work. And it wouldn't have even compiled if I had that wrong.

OK, I reread it and it helped me to find the differences.

In rdma-rc, there is the patch from Noa "IB/core: Avoid accessing non-allocated memory when inferring port type", which plays exactly at
the same place as this one.

Because, I'm creating my rdma-next for testing, I'm merging -rcX with

(quoted diff) +2 -1
-next and my merge conflict resolution was to move
+               attr->ah_attr.type = rdma_ah_find_type(qp->device,
+                                                      cmd-
Quoted 8 lines
Quoted 4 lines
Quoted 1 line

base.dest.port_num);

to the copy_ah_attr_from_uverbs and to change function signature to be
"static void copy_ah_attr_from_uverbs(struct ib_qp *qp,"

It turns out this is kind of a moot point. In uverbs_marshall.c there already exists void ib_copy_ah_attr_to_user(). We should have used that instead of creating new versions of this in this file.

I don't see clean way to reuse those functions.

The ib_copy_qp_attr_to_user has second parameter of "struct ib_uverbs_qp_attr", while ib_uverbs_query_qp returns results in "struct ib_uverbs_query_qp_resp" and operates on it.

Thanks

#21 lore raw

PATCH v3 3/24 RDMA/mlx4: Don't use uninitialized variable

In reply to a message not in the archive

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

Avoid usage of uninitialized variable.

Fixes 3078f5f1bd8b ("IB/mlx4: Add support for RSS QP")
---
 drivers/infiniband/hw/mlx4/qp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/infiniband/hw/mlx4/qp.c +1 -1
index e42acfb20588..793d77f29d3e 100644
@@ -2034,8 +2034,8 @@ static u8 gid_type_to_qpc(enum ib_gid_type gid_type)
  */
 static int bringup_rss_rwqs(struct ib_rwq_ind_table *ind_tbl, u8 port_num)
 {
+	int err = 0;
 	int i;
-	int err;
 
 	for (i = 0; i < (1 << ind_tbl->log_ind_tbl_size); i++) {
 		struct ib_wq *ibwq = ind_tbl->ind_tbl[i];
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#22 lore raw

PATCH v3 4/24 RDMA/mlx4: Fix create qp command alignment

In reply to a message not in the archive

From: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/[email protected]>

Avoid extra padding by replacing the order of inl_recv_sz and reserved, otherwise 'mlx4_ib_create_qp' structure might be larger than legacy user input leading to copy of some garbage data from the user space buffer.

Fixes ea30b966f7dd ('IB/mlx4: Add inline-receive support')
---
 include/uapi/rdma/mlx4-abi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/uapi/rdma/mlx4-abi.h +1 -1
index d915cab37ec3..21cce1a4c4dd 100644
@@ -111,8 +111,8 @@ struct mlx4_ib_create_qp {
 	__u8	log_sq_bb_count;
 	__u8	log_sq_stride;
 	__u8	sq_no_prefetch;
-	__u32	inl_recv_sz;
 	__u8	reserved;
+	__u32	inl_recv_sz;
 };
 
 struct mlx4_ib_create_wq {
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#23 lore raw

PATCH v3 5/24 RDMA/(core,ulp): Convert register/unregister event handler to be void

In reply to a message not in the archive

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

The functions ib_register_event_handler() and ib_unregister_event_handler() always returned success and they can't fail.

Let's convert those functions to be void, remove redundant checks and cleanup tons of goto statements.

---
 drivers/infiniband/core/cache.c                 | 23 ++++++++---------------
 drivers/infiniband/core/device.c                |  8 ++------
 drivers/infiniband/core/sa_query.c              |  3 +--
 drivers/infiniband/core/uverbs_main.c           | 13 +------------
 drivers/infiniband/ulp/ipoib/ipoib_main.c       | 10 +---------
 drivers/infiniband/ulp/iser/iser_verbs.c        |  6 ++----
 drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c |  7 +------
 drivers/infiniband/ulp/srpt/ib_srpt.c           |  5 ++---
 include/rdma/ib_verbs.h                         |  4 ++--
 9 files changed, 20 insertions(+), 59 deletions(-)
drivers/infiniband/core/cache.c +8 -15
index efc94304dee3..77515638c55c 100644
@@ -1199,30 +1199,23 @@ int ib_cache_setup_one(struct ib_device *device)
 	device->cache.ports =
 		kzalloc(sizeof(*device->cache.ports) *
 			(rdma_end_port(device) - rdma_start_port(device) + 1), GFP_KERNEL);
-	if (!device->cache.ports) {
-		err = -ENOMEM;
-		goto out;
-	}
+	if (!device->cache.ports)
+		return -ENOMEM;
 
 	err = gid_table_setup_one(device);
-	if (err)
-		goto out;
+	if (err) {
+		kfree(device->cache.ports);
+		device->cache.ports = NULL;
+		return err;
+	}
 
 	for (p = 0; p <= rdma_end_port(device) - rdma_start_port(device); ++p)
 		ib_cache_update(device, p + rdma_start_port(device), true);
 
 	INIT_IB_EVENT_HANDLER(&device->cache.event_handler,
 			      device, ib_cache_event);
-	err = ib_register_event_handler(&device->cache.event_handler);
-	if (err)
-		goto err;
-
+	ib_register_event_handler(&device->cache.event_handler);
 	return 0;
-
-err:
-	gid_table_cleanup_one(device);
-out:
-	return err;
 }
 
 void ib_cache_release_one(struct ib_device *device)
drivers/infiniband/core/device.c +2 -6
index fbc92c649be8..475b93d62748 100644
@@ -746,7 +746,7 @@ EXPORT_SYMBOL(ib_set_client_data);
  * chapter 11 of the InfiniBand Architecture Specification).  This
  * callback may occur in interrupt context.
  */
-int ib_register_event_handler  (struct ib_event_handler *event_handler)
+void ib_register_event_handler(struct ib_event_handler *event_handler)
 {
 	unsigned long flags;
 
@@ -754,8 +754,6 @@ int ib_register_event_handler  (struct ib_event_handler *event_handler)
 	list_add_tail(&event_handler->list,
 		      &event_handler->device->event_handler_list);
 	spin_unlock_irqrestore(&event_handler->device->event_handler_lock, flags);
-
-	return 0;
 }
 EXPORT_SYMBOL(ib_register_event_handler);
 
@@ -766,15 +764,13 @@ EXPORT_SYMBOL(ib_register_event_handler);
  * Unregister an event handler registered with
  * ib_register_event_handler().
  */
-int ib_unregister_event_handler(struct ib_event_handler *event_handler)
+void ib_unregister_event_handler(struct ib_event_handler *event_handler)
 {
 	unsigned long flags;
 
 	spin_lock_irqsave(&event_handler->device->event_handler_lock, flags);
 	list_del(&event_handler->list);
 	spin_unlock_irqrestore(&event_handler->device->event_handler_lock, flags);
-
-	return 0;
 }
 EXPORT_SYMBOL(ib_unregister_event_handler);
 
drivers/infiniband/core/sa_query.c +1 -2
index da29e2863c84..14aef0dd8625 100644
@@ -2408,8 +2408,7 @@ static void ib_sa_add_one(struct ib_device *device)
 	 */
 
 	INIT_IB_EVENT_HANDLER(&sa_dev->event_handler, device, ib_sa_event);
-	if (ib_register_event_handler(&sa_dev->event_handler))
-		goto err;
+	ib_register_event_handler(&sa_dev->event_handler);
 
 	for (i = 0; i <= e - s; ++i) {
 		if (rdma_cap_ib_sa(device, i + 1))
drivers/infiniband/core/uverbs_main.c +1 -12
index 3d2609608f58..1ac99aec3d4e 100644
@@ -594,7 +594,6 @@ struct file *ib_uverbs_alloc_async_event_file(struct ib_uverbs_file *uverbs_file
 {
 	struct ib_uverbs_async_event_file *ev_file;
 	struct file *filp;
-	int ret;
 
 	ev_file = kzalloc(sizeof(*ev_file), GFP_KERNEL);
 	if (!ev_file)
@@ -620,21 +619,11 @@ struct file *ib_uverbs_alloc_async_event_file(struct ib_uverbs_file *uverbs_file
 	INIT_IB_EVENT_HANDLER(&uverbs_file->event_handler,
 			      ib_dev,
 			      ib_uverbs_event_handler);
-	ret = ib_register_event_handler(&uverbs_file->event_handler);
-	if (ret)
-		goto err_put_file;
-
+	ib_register_event_handler(&uverbs_file->event_handler);
 	/* At that point async file stuff was fully set */
 
 	return filp;
 
-err_put_file:
-	fput(filp);
-	kref_put(&uverbs_file->async_file->ref,
-		 ib_uverbs_release_async_event_file);
-	uverbs_file->async_file = NULL;
-	return ERR_PTR(ret);
-
 err_put_refs:
 	kref_put(&ev_file->uverbs_file->ref, ib_uverbs_release_file);
 	kref_put(&ev_file->ref, ib_uverbs_release_async_event_file);
drivers/infiniband/ulp/ipoib/ipoib_main.c +1 -9
index 7dcdbbacbf46..645217232250 100644
@@ -2223,13 +2223,7 @@ static struct net_device *ipoib_add_port(const char *format,
 
 	INIT_IB_EVENT_HANDLER(&priv->event_handler,
 			      priv->ca, ipoib_event);
-	result = ib_register_event_handler(&priv->event_handler);
-	if (result < 0) {
-		printk(KERN_WARNING "%s: ib_register_event_handler failed for "
-		       "port %d (ret = %d)\n",
-		       hca->name, port, result);
-		goto event_failed;
-	}
+	ib_register_event_handler(&priv->event_handler);
 
 	result = register_netdev(priv->dev);
 	if (result) {
@@ -2262,8 +2256,6 @@ static struct net_device *ipoib_add_port(const char *format,
 	set_bit(IPOIB_STOP_NEIGH_GC, &priv->flags);
 	cancel_delayed_work(&priv->neigh_reap_task);
 	flush_workqueue(priv->wq);
-
-event_failed:
 	ipoib_dev_cleanup(priv->dev);
 
 device_init_failed:
drivers/infiniband/ulp/iser/iser_verbs.c +2 -4
index 26a004e97ae0..55a73b0ed4c6 100644
@@ -106,9 +106,7 @@ static int iser_create_device_ib_res(struct iser_device *device)
 
 	INIT_IB_EVENT_HANDLER(&device->event_handler, ib_dev,
 			      iser_event_handler);
-	if (ib_register_event_handler(&device->event_handler))
-		goto cq_err;
-
+	ib_register_event_handler(&device->event_handler);
 	return 0;
 
 cq_err:
@@ -141,7 +139,7 @@ static void iser_free_device_ib_res(struct iser_device *device)
 		comp->cq = NULL;
 	}
 
-	(void)ib_unregister_event_handler(&device->event_handler);
+	ib_unregister_event_handler(&device->event_handler);
 	ib_dealloc_pd(device->pd);
 
 	kfree(device->comps);
drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c +1 -6
index 57b862b94dca..21f0b481edcc 100644
@@ -954,12 +954,7 @@ static int vema_register(struct opa_vnic_ctrl_port *cport)
 
 		INIT_IB_EVENT_HANDLER(&port->event_handler,
 				      cport->ibdev, opa_vnic_event);
-		ret = ib_register_event_handler(&port->event_handler);
-		if (ret) {
-			c_err("port %d: event handler register failed\n", i);
-			vema_unregister(cport);
-			return ret;
-		}
+		ib_register_event_handler(&port->event_handler);
 
 		idr_init(&port->vport_idr);
 		mutex_init(&port->lock);
drivers/infiniband/ulp/srpt/ib_srpt.c +2 -3
index 402275be0931..9e8e9220f816 100644
@@ -2238,7 +2238,7 @@ static int srpt_write_pending(struct se_cmd *se_cmd)
 				cqe, first_wr);
 		cqe = NULL;
 	}
-	
+
 	ret = ib_post_send(ch->qp, first_wr, &bad_wr);
 	if (ret) {
 		pr_err("%s: ib_post_send() returned %d for %d (avail: %d)\n",
@@ -2530,8 +2530,7 @@ static void srpt_add_one(struct ib_device *device)
 
 	INIT_IB_EVENT_HANDLER(&sdev->event_handler, sdev->device,
 			      srpt_event_handler);
-	if (ib_register_event_handler(&sdev->event_handler))
-		goto err_cm;
+	ib_register_event_handler(&sdev->event_handler);
 
 	sdev->ioctx_ring = (struct srpt_recv_ioctx **)
 		srpt_alloc_ioctx_ring(sdev, sdev->srq_size,
include/rdma/ib_verbs.h +2 -2
index 4db4ad56ace6..3c9c514bcd7d 100644
@@ -2411,8 +2411,8 @@ int ib_modify_qp_is_ok(enum ib_qp_state cur_state, enum ib_qp_state next_state,
 		       enum ib_qp_type type, enum ib_qp_attr_mask mask,
 		       enum rdma_link_layer ll);
 
-int ib_register_event_handler  (struct ib_event_handler *event_handler);
-int ib_unregister_event_handler(struct ib_event_handler *event_handler);
+void ib_register_event_handler(struct ib_event_handler *event_handler);
+void ib_unregister_event_handler(struct ib_event_handler *event_handler);
 void ib_dispatch_event(struct ib_event *event);
 
 int ib_query_port(struct ib_device *device,
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#24 lore raw

PATCH v3 6/24 RDMA/core: Cleanup device capability enum

In reply to a message not in the archive

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

Cleanup patch prior exporting the ib_device_cap_flags to the user space. In this patch, we are aligning the indentation, removing IB_DEVICE_INIT_TYPE and IB_DEVICE_RESERVED fields, because it is not used in the kernel.

---
 include/rdma/ib_verbs.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
include/rdma/ib_verbs.h +3 -3
index 3c9c514bcd7d..4ce188128aa9 100644
@@ -170,7 +170,7 @@ enum ib_device_cap_flags {
 	IB_DEVICE_UD_AV_PORT_ENFORCE		= (1 << 6),
 	IB_DEVICE_CURR_QP_STATE_MOD		= (1 << 7),
 	IB_DEVICE_SHUTDOWN_PORT			= (1 << 8),
-	IB_DEVICE_INIT_TYPE			= (1 << 9),
+	/* Not in use, former INIT_TYPE		= (1 << 9),*/
 	IB_DEVICE_PORT_ACTIVE_EVENT		= (1 << 10),
 	IB_DEVICE_SYS_IMAGE_GUID		= (1 << 11),
 	IB_DEVICE_RC_RNR_NAK_GEN		= (1 << 12),
@@ -185,7 +185,7 @@ enum ib_device_cap_flags {
 	 * which will always contain a usable lkey.
 	 */
 	IB_DEVICE_LOCAL_DMA_LKEY		= (1 << 15),
-	IB_DEVICE_RESERVED /* old SEND_W_INV */	= (1 << 16),
+	/* Reserved, old SEND_W_INV		= (1 << 16),*/
 	IB_DEVICE_MEM_WINDOW			= (1 << 17),
 	/*
 	 * Devices should set IB_DEVICE_UD_IP_SUM if they support
@@ -220,7 +220,7 @@ enum ib_device_cap_flags {
 	 * of I/O operations with single completion queue managed
 	 * by hardware.
 	 */
-	IB_DEVICE_CROSS_CHANNEL		= (1 << 27),
+	IB_DEVICE_CROSS_CHANNEL			= (1 << 27),
 	IB_DEVICE_MANAGED_FLOW_STEERING		= (1 << 29),
 	IB_DEVICE_SIGNATURE_HANDOVER		= (1 << 30),
 	IB_DEVICE_ON_DEMAND_PAGING		= (1ULL << 31),
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#25 lore raw

PATCH v3 7/24 RDMA/core: Delete BUG() from unreachable flow

In reply to a message not in the archive

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

Remove call to BUG() in case wrong node_type was provided. This flow is unreachable, because node_types are supplied from specific enum.

---
 drivers/infiniband/core/verbs.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)
drivers/infiniband/core/verbs.c +7 -13
index 3d1de62de839..aee23deba074 100644
@@ -180,21 +180,15 @@ EXPORT_SYMBOL(ib_rate_to_mbps);
 __attribute_const__ enum rdma_transport_type
 rdma_node_get_transport(enum rdma_node_type node_type)
 {
-	switch (node_type) {
-	case RDMA_NODE_IB_CA:
-	case RDMA_NODE_IB_SWITCH:
-	case RDMA_NODE_IB_ROUTER:
-		return RDMA_TRANSPORT_IB;
-	case RDMA_NODE_RNIC:
-		return RDMA_TRANSPORT_IWARP;
-	case RDMA_NODE_USNIC:
+
+	if (node_type == RDMA_NODE_USNIC)
 		return RDMA_TRANSPORT_USNIC;
-	case RDMA_NODE_USNIC_UDP:
+	if (node_type == RDMA_NODE_USNIC_UDP)
 		return RDMA_TRANSPORT_USNIC_UDP;
-	default:
-		BUG();
-		return 0;
-	}
+	if (node_type == RDMA_NODE_RNIC)
+		return RDMA_TRANSPORT_IWARP;
+
+	return RDMA_TRANSPORT_IB;
 }
 EXPORT_SYMBOL(rdma_node_get_transport);
 
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#26 lore raw

PATCH v3 8/24 RDMA/core: Refactor get link layer wrapper

In reply to a message not in the archive

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

The return values from rdma_node_get_transport() are strict and IB_LINK_LAYER_UNSPECIFIED is unreachable in this flow.

---
 drivers/infiniband/core/verbs.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)
drivers/infiniband/core/verbs.c +5 -9
index aee23deba074..7dd13962fc4c 100644
@@ -194,19 +194,15 @@ EXPORT_SYMBOL(rdma_node_get_transport);
 
 enum rdma_link_layer rdma_port_get_link_layer(struct ib_device *device, u8 port_num)
 {
+	enum rdma_transport_type lt;
 	if (device->get_link_layer)
 		return device->get_link_layer(device, port_num);
 
-	switch (rdma_node_get_transport(device->node_type)) {
-	case RDMA_TRANSPORT_IB:
+	lt = rdma_node_get_transport(device->node_type);
+	if (lt == RDMA_TRANSPORT_IB)
 		return IB_LINK_LAYER_INFINIBAND;
-	case RDMA_TRANSPORT_IWARP:
-	case RDMA_TRANSPORT_USNIC:
-	case RDMA_TRANSPORT_USNIC_UDP:
-		return IB_LINK_LAYER_ETHERNET;
-	default:
-		return IB_LINK_LAYER_UNSPECIFIED;
-	}
+
+	return IB_LINK_LAYER_ETHERNET;
 }
 EXPORT_SYMBOL(rdma_port_get_link_layer);
 
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#27 lore raw

PATCH v3 9/24 RDMA/mlx4: Remove gfp_mask argument from acquire_group call

In reply to a message not in the archive

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

All callers of acquire_group() passed the same gfp_mask to it and it is safe to remove it.

---
 drivers/infiniband/hw/mlx4/mcg.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/infiniband/hw/mlx4/mcg.c +4 -5
index b73f89700ef9..70eb9f917303 100644
@@ -808,8 +808,7 @@ static ssize_t sysfs_show_group(struct device *dev,
 		struct device_attribute *attr, char *buf);
 
 static struct mcast_group *acquire_group(struct mlx4_ib_demux_ctx *ctx,
-					 union ib_gid *mgid, int create,
-					 gfp_t gfp_mask)
+					 union ib_gid *mgid, int create)
 {
 	struct mcast_group *group, *cur_group;
 	int is_mgid0;
@@ -825,7 +824,7 @@ static struct mcast_group *acquire_group(struct mlx4_ib_demux_ctx *ctx,
 	if (!create)
 		return ERR_PTR(-ENOENT);
 
-	group = kzalloc(sizeof *group, gfp_mask);
+	group = kzalloc(sizeof(*group), GFP_KERNEL);
 	if (!group)
 		return ERR_PTR(-ENOMEM);
 
@@ -892,7 +891,7 @@ int mlx4_ib_mcg_demux_handler(struct ib_device *ibdev, int port, int slave,
 	case IB_MGMT_METHOD_GET_RESP:
 	case IB_SA_METHOD_DELETE_RESP:
 		mutex_lock(&ctx->mcg_table_lock);
-		group = acquire_group(ctx, &rec->mgid, 0, GFP_KERNEL);
+		group = acquire_group(ctx, &rec->mgid, 0);
 		mutex_unlock(&ctx->mcg_table_lock);
 		if (IS_ERR(group)) {
 			if (mad->mad_hdr.method == IB_MGMT_METHOD_GET_RESP) {
@@ -954,7 +953,7 @@ int mlx4_ib_mcg_multiplex_handler(struct ib_device *ibdev, int port,
 		req->sa_mad = *sa_mad;
 
 		mutex_lock(&ctx->mcg_table_lock);
-		group = acquire_group(ctx, &rec->mgid, may_create, GFP_KERNEL);
+		group = acquire_group(ctx, &rec->mgid, may_create);
 		mutex_unlock(&ctx->mcg_table_lock);
 		if (IS_ERR(group)) {
 			kfree(req);
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#28 lore raw

PATCH v3 10/24 RDMA/usnic: Fix remove address space warning

In reply to a message not in the archive

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

Sparse tool complains with the following error:
drivers/infiniband/hw/usnic/usnic_ib_main.c:445:16: warning: cast removes
address space of expression

Fix it by doing casting on correct field and convert function helper which sets ifaddr to be void, because there are no users who are interested in returned value.

Fixes c7845bcafe4d ("IB/usnic: Add UDP support in u*verbs.c, u*main.c and u*util.h")
---
 drivers/infiniband/hw/usnic/usnic_fwd.c     | 12 ++----------
 drivers/infiniband/hw/usnic/usnic_fwd.h     |  2 +-
 drivers/infiniband/hw/usnic/usnic_ib_main.c | 10 ++++++----
 3 files changed, 9 insertions(+), 15 deletions(-)
drivers/infiniband/hw/usnic/usnic_fwd.c +2 -10
index 3c37dd59c04e..995a26b65156 100644
@@ -110,20 +110,12 @@ void usnic_fwd_set_mac(struct usnic_fwd_dev *ufdev, char mac[ETH_ALEN])
 	spin_unlock(&ufdev->lock);
 }
 
-int usnic_fwd_add_ipaddr(struct usnic_fwd_dev *ufdev, __be32 inaddr)
+void usnic_fwd_add_ipaddr(struct usnic_fwd_dev *ufdev, __be32 inaddr)
 {
-	int status;
-
 	spin_lock(&ufdev->lock);
-	if (ufdev->inaddr == 0) {
+	if (!ufdev->inaddr)
 		ufdev->inaddr = inaddr;
-		status = 0;
-	} else {
-		status = -EFAULT;
-	}
 	spin_unlock(&ufdev->lock);
-
-	return status;
 }
 
 void usnic_fwd_del_ipaddr(struct usnic_fwd_dev *ufdev)
drivers/infiniband/hw/usnic/usnic_fwd.h +1 -1
index b2ac22be0731..0b2cc4e79707 100644
@@ -75,7 +75,7 @@ struct usnic_fwd_dev *usnic_fwd_dev_alloc(struct pci_dev *pdev);
 void usnic_fwd_dev_free(struct usnic_fwd_dev *ufdev);
 
 void usnic_fwd_set_mac(struct usnic_fwd_dev *ufdev, char mac[ETH_ALEN]);
-int usnic_fwd_add_ipaddr(struct usnic_fwd_dev *ufdev, __be32 inaddr);
+void usnic_fwd_add_ipaddr(struct usnic_fwd_dev *ufdev, __be32 inaddr);
 void usnic_fwd_del_ipaddr(struct usnic_fwd_dev *ufdev);
 void usnic_fwd_carrier_up(struct usnic_fwd_dev *ufdev);
 void usnic_fwd_carrier_down(struct usnic_fwd_dev *ufdev);
drivers/infiniband/hw/usnic/usnic_ib_main.c +6 -4
index e86700f994cb..f45e99a938e0 100644
@@ -351,7 +351,7 @@ static void *usnic_ib_device_add(struct pci_dev *dev)
 {
 	struct usnic_ib_dev *us_ibdev;
 	union ib_gid gid;
-	struct in_ifaddr *in;
+	struct in_device *ind;
 	struct net_device *netdev;
 
 	usnic_dbg("\n");
@@ -441,9 +441,11 @@ static void *usnic_ib_device_add(struct pci_dev *dev)
 	if (netif_carrier_ok(us_ibdev->netdev))
 		usnic_fwd_carrier_up(us_ibdev->ufdev);
 
-	in = ((struct in_device *)(netdev->ip_ptr))->ifa_list;
-	if (in != NULL)
-		usnic_fwd_add_ipaddr(us_ibdev->ufdev, in->ifa_address);
+	ind = in_dev_get(netdev);
+	if (ind->ifa_list)
+		usnic_fwd_add_ipaddr(us_ibdev->ufdev,
+				     ind->ifa_list->ifa_address);
+	in_dev_put(ind);
 
 	usnic_mac_ip_to_gid(us_ibdev->netdev->perm_addr,
 				us_ibdev->ufdev->inaddr, &gid.raw[0]);
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#29 lore raw

PATCH v3 11/24 RDMA/mthca: Make explicit conversion to 64bit value

In reply to a message not in the archive

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

The "lg" variable is declared as int so in all places where this variable is used as a shift operand, the output will be int too.

This produces the following smatch warning:
drivers/infiniband/hw/mthca/mthca_cmd.c:701 mthca_map_cmd() warn:
should '1 << lg' be a 64 bit type?

Simple declaration of "1" to be "1ULL" will fix the issue.

---
 drivers/infiniband/hw/mthca/mthca_cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/infiniband/hw/mthca/mthca_cmd.c +1 -1
index e19ae0b9b439..89529d204178 100644
@@ -698,7 +698,7 @@ static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm,
 		for (i = 0; i < mthca_icm_size(&iter) >> lg; ++i) {
 			if (virt != -1) {
 				pages[nent * 2] = cpu_to_be64(virt);
-				virt += 1 << lg;
+				virt += 1ULL << lg;
 			}
 
 			pages[nent * 2 + 1] =
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#30 lore raw

PATCH v3 12/24 IB/mlx4: Fix some spelling mistakes

In reply to a message not in the archive

From: Talat Batheesh <talatb-VPRAkNaXOzVWk0Htik3J/[email protected]>

Fix spelling mistakes in remarks

    "retrun"->"return"
    "cancell"->"cancel"
---
 drivers/infiniband/hw/mlx4/alias_GUID.c | 2 +-
 drivers/infiniband/hw/mlx4/cq.c         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/infiniband/hw/mlx4/alias_GUID.c +1 -1
index 5a897b0106a9..9ed41f56e141 100644
@@ -781,7 +781,7 @@ void mlx4_ib_init_alias_guid_work(struct mlx4_ib_dev *dev, int port)
 	spin_lock_irqsave(&dev->sriov.going_down_lock, flags);
 	spin_lock_irqsave(&dev->sriov.alias_guid.ag_work_lock, flags1);
 	if (!dev->sriov.is_going_down) {
-		/* If there is pending one should cancell then run, otherwise
+		/* If there is pending one should cancel then run, otherwise
 		  * won't run till previous one is ended as same work
 		  * struct is used.
 		  */
drivers/infiniband/hw/mlx4/cq.c +1 -1
index 95382faa7ad1..cab796341697 100644
@@ -637,7 +637,7 @@ static void mlx4_ib_poll_sw_comp(struct mlx4_ib_cq *cq, int num_entries,
 	struct mlx4_ib_qp *qp;
 
 	*npolled = 0;
-	/* Find uncompleted WQEs belonging to that cq and retrun
+	/* Find uncompleted WQEs belonging to that cq and return
 	 * simulated FLUSH_ERR completions
 	 */
 	list_for_each_entry(qp, &cq->send_qp_list, cq_send_list) {
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#31 lore raw

PATCH v3 13/24 IB/mlx5: Fix some spelling mistakes

In reply to a message not in the archive

From: Talat Batheesh <talatb-VPRAkNaXOzVWk0Htik3J/[email protected]>

Fix spelling mistakes in remarks

    "retrun"->"return"
    "Decalring"->"Declaring"
---
 drivers/infiniband/hw/mlx5/cq.c  | 2 +-
 drivers/infiniband/hw/mlx5/mad.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/infiniband/hw/mlx5/cq.c +1 -1
index a384d72ea3cd..f7fcfc8c22bf 100644
@@ -499,7 +499,7 @@ static void mlx5_ib_poll_sw_comp(struct mlx5_ib_cq *cq, int num_entries,
 	struct mlx5_ib_qp *qp;
 
 	*npolled = 0;
-	/* Find uncompleted WQEs belonging to that cq and retrun mmics ones */
+	/* Find uncompleted WQEs belonging to that cq and return mmics ones */
 	list_for_each_entry(qp, &cq->list_send_qp, cq_send_list) {
 		sw_send_comp(qp, num_entries, wc + *npolled, npolled);
 		if (*npolled >= num_entries)
drivers/infiniband/hw/mlx5/mad.c +1 -1
index cd2264ac88ae..98e51d51bb04 100644
@@ -204,7 +204,7 @@ static int process_pma_cmd(struct ib_device *ibdev, u8 port_num,
 	int err;
 	void *out_cnt;
 
-	/* Decalring support of extended counters */
+	/* Declaring support of extended counters */
 	if (in_mad->mad_hdr.attr_id == IB_PMA_CLASS_PORT_INFO) {
 		struct ib_class_port_info cpi = {};
 
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#32 lore raw

PATCH v3 14/24 IB/mlx5: Add necessary delay drop assignment

In reply to a message not in the archive

From: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/[email protected]>

Assign the statistics and configuration structure pointer on success.

Fixes fe248c3a5837 ('IB/mlx5: Add delay drop configuration and statistics')
---
 drivers/infiniband/hw/mlx5/main.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/infiniband/hw/mlx5/main.c +2 -0
index cde7d1ce4a3c..986404827848 100644
@@ -3792,6 +3792,8 @@ static int delay_drop_debugfs_init(struct mlx5_ib_dev *dev)
 	if (!dbg->timeout_debugfs)
 		goto out_debugfs;
 
+	dev->delay_drop.dbg = dbg;
+
 	return 0;
 
 out_debugfs:
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#33 lore raw

PATCH v3 15/24 IB/mlx4: Fix RSS QP type in creation verb

In reply to a message not in the archive

From: Guy Levi <guyle-VPRAkNaXOzVWk0Htik3J/[email protected]>

The mlx4 was designed to support QP type of MLX4_IB_QPT_RAW_PACKET.

Fixes 3078f5f1bd8b ("IB/mlx4: Add support for RSS QP")
---
 drivers/infiniband/hw/mlx4/qp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/infiniband/hw/mlx4/qp.c +1 -1
index 793d77f29d3e..603a0bf49576 100644
@@ -749,7 +749,7 @@ static int create_qp_rss(struct mlx4_ib_dev *dev, struct ib_pd *ibpd,
 	INIT_LIST_HEAD(&qp->gid_list);
 	INIT_LIST_HEAD(&qp->steering_rules);
 
-	qp->mlx4_ib_qp_type = MLX4_IB_QPT_RAW_ETHERTYPE;
+	qp->mlx4_ib_qp_type = MLX4_IB_QPT_RAW_PACKET;
 	qp->state = IB_QPS_RESET;
 
 	/* Set dummy send resources to be compatible with HV and PRM */
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#34 lore raw

PATCH v3 16/24 IB/mlx4: Fix struct mlx4_ib_create_wq alignment

In reply to a message not in the archive

From: Guy Levi <guyle-VPRAkNaXOzVWk0Htik3J/[email protected]>

The mlx4 ABI defines to have structures with alignment of 64B.

Fixes 400b1ebcfe31 ("IB/mlx4: Add support for WQ related verbs")
---
 drivers/infiniband/hw/mlx4/qp.c | 9 ++++-----
 include/uapi/rdma/mlx4-abi.h    | 1 -
 2 files changed, 4 insertions(+), 6 deletions(-)
drivers/infiniband/hw/mlx4/qp.c +4 -5
index 603a0bf49576..44d2d0a2d5e0 100644
@@ -1047,9 +1047,8 @@ static int create_qp_common(struct mlx4_ib_dev *dev, struct ib_pd *pd,
 		}
 
 		if (src == MLX4_IB_RWQ_SRC) {
-			if (ucmd.wq.comp_mask || ucmd.wq.reserved1 ||
-			    ucmd.wq.reserved[0] || ucmd.wq.reserved[1] ||
-			    ucmd.wq.reserved[2]) {
+			if (ucmd.wq.comp_mask || ucmd.wq.reserved[0] ||
+			    ucmd.wq.reserved[1] || ucmd.wq.reserved[2]) {
 				pr_debug("user command isn't supported\n");
 				err = -EOPNOTSUPP;
 				goto err;
@@ -4153,8 +4152,8 @@ struct ib_wq *mlx4_ib_create_wq(struct ib_pd *pd,
 	if (!(udata && pd->uobject))
 		return ERR_PTR(-EINVAL);
 
-	required_cmd_sz = offsetof(typeof(ucmd), reserved) +
-			  sizeof(ucmd.reserved);
+	required_cmd_sz = offsetof(typeof(ucmd), comp_mask) +
+			  sizeof(ucmd.comp_mask);
 	if (udata->inlen < required_cmd_sz) {
 		pr_debug("invalid inlen\n");
 		return ERR_PTR(-EINVAL);
include/uapi/rdma/mlx4-abi.h +0 -1
index 21cce1a4c4dd..0e10102861b5 100644
@@ -121,7 +121,6 @@ struct mlx4_ib_create_wq {
 	__u8	log_range_size;
 	__u8	reserved[3];
 	__u32   comp_mask;
-	__u32   reserved1;
 };
 
 struct mlx4_ib_modify_wq {
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#35 lore raw

PATCH v3 17/24 IB/mlx4: Remove redundant attribute in mlx4_ib_create_qp_rss struct

In reply to a message not in the archive

From: Guy Levi <guyle-VPRAkNaXOzVWk0Htik3J/[email protected]>

rss_key_len is not in use and need to be removed.

Fixes 3078f5f1bd8b ("IB/mlx4: Add support for RSS QP")
---
 include/uapi/rdma/mlx4-abi.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
include/uapi/rdma/mlx4-abi.h +1 -2
index 0e10102861b5..c55f60e05f86 100644
@@ -98,8 +98,7 @@ struct mlx4_ib_create_srq_resp {
 struct mlx4_ib_create_qp_rss {
 	__u64   rx_hash_fields_mask;
 	__u8    rx_hash_function;
-	__u8    rx_key_len;
-	__u8    reserved[6];
+	__u8    reserved[7];
 	__u8    rx_hash_key[40];
 	__u32   comp_mask;
 	__u32   reserved1;
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

#37 lore raw

Re: [rdma-next v3 17/24] IB/mlx4: Remove redundant attribute in mlx4_ib_create_qp_rss struct

Quoted 3 lines · Leon Romanovsky

From: Guy Levi <guyle-VPRAkNaXOzVWk0Htik3J/[email protected]>

rss_key_len is not in use and need to be removed.

I fixed this commit message as I took this in.

Quoted 22 lines
Fixes 3078f5f1bd8b ("IB/mlx4: Add support for RSS QP")
---
 include/uapi/rdma/mlx4-abi.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
include/uapi/rdma/mlx4- renamed from include/uapi/rdma/mlx4-abi.h +0 -0

abi.h
index 0e10102861b5..c55f60e05f86 100644

include/uapi/rdma/mlx4-abi.h +1 -2
@@ -98,8 +98,7 @@ struct mlx4_ib_create_srq_resp {
 struct mlx4_ib_create_qp_rss {
 	__u64   rx_hash_fields_mask;
 	__u8    rx_hash_function;
-	__u8    rx_key_len;
-	__u8    reserved[6];
+	__u8    reserved[7];
 	__u8    rx_hash_key[40];
 	__u32   comp_mask;
 	__u32   reserved1;
Signature · 8 lines
Doug Ledford <[email protected]>
    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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#38 lore raw

PATCH v3 18/24 IB/mlx4: Check that reserved fields in mlx4_ib_create_qp_rss are zero

In reply to a message not in the archive

From: Guy Levi <guyle-VPRAkNaXOzVWk0Htik3J/[email protected]>

According to mlx4 convention, need to fail the command due to a non-zero value in the user data which is expected to be zero.

Fixes 3078f5f1bd8b ("IB/mlx4: Add support for RSS QP")
---
 drivers/infiniband/hw/mlx4/qp.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/infiniband/hw/mlx4/qp.c +3 -0
index 44d2d0a2d5e0..2874d8790570 100644
@@ -813,6 +813,9 @@ static struct ib_qp *_mlx4_ib_create_qp_rss(struct ib_pd *pd,
 		return ERR_PTR(-EFAULT);
 	}
 
+	if (memchr_inv(ucmd.reserved, 0, sizeof(ucmd.reserved)))
+		return ERR_PTR(-EOPNOTSUPP);
+
 	if (ucmd.comp_mask || ucmd.reserved1)
 		return ERR_PTR(-EOPNOTSUPP);
 
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#39 lore raw

PATCH v3 19/24 IB/ipoib: Sync between remove_one to sysfs calls that use rtnl_lock

In reply to a message not in the archive

From: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/[email protected]>

In order to avoid deadlock between sysfs functions (like create/delete child) and remove_one (both of them are using the sysfs lock and rtnl_lock) the driver will use a state mutex for sync.

That will fix traces as the following:

schedule+0x3e/0x90
kernfs_drain+0x75/0xf0
? wait_woken+0x90/0x90
__kernfs_remove+0x12e/0x1c0
kernfs_remove+0x25/0x40
sysfs_remove_dir+0x57/0x90
kobject_del+0x22/0x60
device_del+0x195/0x230
 pm_runtime_set_memalloc_noio+0xac/0xf0
netdev_unregister_kobject+0x71/0x80
rollback_registered_many+0x205/0x2f0
rollback_registered+0x31/0x40
unregister_netdevice_queue+0x58/0xb0
unregister_netdev+0x20/0x30
ipoib_remove_one+0xb7/0x240 [ib_ipoib]
ib_unregister_device+0xbc/0x1b0 [ib_core]
ib_unregister_mad_agent+0x29/0x30 [ib_core]
mlx4_ib_remove+0x67/0x280 [mlx4_ib]
INFO: task echo:24082 blocked for more than 120 seconds.
Tainted: G           OE   4.1.12-37.5.1.el6uek.x86_64 #2
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this
message.
Call Trace:
schedule+0x3e/0x90
schedule_preempt_disabled+0xe/0x10
__mutex_lock_slowpath+0x95/0x110
? _rcu_barrier+0x177/0x220
mutex_lock+0x23/0x40
rtnl_lock+0x15/0x20
netdev_run_todo+0x81/0x1f0
rtnl_unlock+0xe/0x10
ipoib_vlan_delete+0x12f/0x1c0 [ib_ipoib]
delete_child+0x69/0x80 [ib_ipoib]
dev_attr_store+0x20/0x30
sysfs_kf_write+0x41/0x50
---
 drivers/infiniband/ulp/ipoib/ipoib.h      |  1 +
 drivers/infiniband/ulp/ipoib/ipoib_cm.c   |  8 +++++++-
 drivers/infiniband/ulp/ipoib/ipoib_main.c |  5 +++++
 drivers/infiniband/ulp/ipoib/ipoib_vlan.c | 22 ++++++++++++++++++----
 4 files changed, 31 insertions(+), 5 deletions(-)
drivers/infiniband/ulp/ipoib/ipoib.h +1 -0
index 9e738104c2a1..7aa114cf53e3 100644
@@ -336,6 +336,7 @@ struct ipoib_dev_priv {
 	unsigned long flags;
 
 	struct rw_semaphore vlan_rwsem;
+	struct mutex        sysfs_lock; /* Protect sysfs running*/
 
 	struct rb_root  path_tree;
 	struct list_head path_list;
drivers/infiniband/ulp/ipoib/ipoib_cm.c +7 -1
index f87d104837dc..85ef9f10bdee 100644
@@ -1507,9 +1507,14 @@ static ssize_t set_mode(struct device *d, struct device_attribute *attr,
 	if (test_bit(IPOIB_FLAG_GOING_DOWN, &priv->flags))
 		return -EPERM;
 
-	if (!rtnl_trylock())
+	if (!mutex_trylock(&priv->sysfs_lock))
 		return restart_syscall();
 
+	if (!rtnl_trylock()) {
+		mutex_unlock(&priv->sysfs_lock);
+		return restart_syscall();
+	}
+
 	ret = ipoib_set_mode(dev, buf);
 
 	/* The assumption is that the function ipoib_set_mode returned
@@ -1518,6 +1523,7 @@ static ssize_t set_mode(struct device *d, struct device_attribute *attr,
 	 */
 	if (ret != -EBUSY)
 		rtnl_unlock();
+	mutex_unlock(&priv->sysfs_lock);
 
 	return (!ret || ret == -EBUSY) ? count : ret;
 }
drivers/infiniband/ulp/ipoib/ipoib_main.c +5 -0
index 645217232250..627ff8fb59b0 100644
@@ -1876,6 +1876,7 @@ static void ipoib_build_priv(struct net_device *dev)
 	priv->dev = dev;
 	spin_lock_init(&priv->lock);
 	init_rwsem(&priv->vlan_rwsem);
+	mutex_init(&priv->sysfs_lock);
 
 	INIT_LIST_HEAD(&priv->path_list);
 	INIT_LIST_HEAD(&priv->child_intfs);
@@ -2325,7 +2326,11 @@ static void ipoib_remove_one(struct ib_device *device, void *client_data)
 		cancel_delayed_work(&priv->neigh_reap_task);
 		flush_workqueue(priv->wq);
 
+		/* Wrap rtnl_lock/unlock with mutex to protect sysfs calls */
+		mutex_lock(&priv->sysfs_lock);
 		unregister_netdev(priv->dev);
+		mutex_unlock(&priv->sysfs_lock);
+
 		rn->free_rdma_netdev(priv->dev);
 
 		list_for_each_entry_safe(cpriv, tcpriv, &priv->child_intfs, list)
drivers/infiniband/ulp/ipoib/ipoib_vlan.c +18 -4
index 081b33deff1b..dd46abcf53c2 100644
@@ -133,12 +133,20 @@ int ipoib_vlan_add(struct net_device *pdev, unsigned short pkey)
 	snprintf(intf_name, sizeof intf_name, "%s.%04x",
 		 ppriv->dev->name, pkey);
 
-	if (!rtnl_trylock())
+	if (!mutex_trylock(&ppriv->sysfs_lock))
 		return restart_syscall();
 
+	if (!rtnl_trylock()) {
+		mutex_unlock(&ppriv->sysfs_lock);
+		return restart_syscall();
+	}
+
 	priv = ipoib_intf_alloc(ppriv->ca, ppriv->port, intf_name);
-	if (!priv)
+	if (!priv) {
+		rtnl_unlock();
+		mutex_unlock(&ppriv->sysfs_lock);
 		return -ENOMEM;
+	}
 
 	down_write(&ppriv->vlan_rwsem);
 
@@ -164,8 +172,8 @@ int ipoib_vlan_add(struct net_device *pdev, unsigned short pkey)
 
 out:
 	up_write(&ppriv->vlan_rwsem);
-
 	rtnl_unlock();
+	mutex_unlock(&ppriv->sysfs_lock);
 
 	if (result) {
 		free_netdev(priv->dev);
@@ -188,8 +196,13 @@ int ipoib_vlan_delete(struct net_device *pdev, unsigned short pkey)
 	if (test_bit(IPOIB_FLAG_GOING_DOWN, &ppriv->flags))
 		return -EPERM;
 
-	if (!rtnl_trylock())
+	if (!mutex_trylock(&ppriv->sysfs_lock))
+		return restart_syscall();
+
+	if (!rtnl_trylock()) {
+		mutex_unlock(&ppriv->sysfs_lock);
 		return restart_syscall();
+	}
 
 	down_write(&ppriv->vlan_rwsem);
 	list_for_each_entry_safe(priv, tpriv, &ppriv->child_intfs, list) {
@@ -208,6 +221,7 @@ int ipoib_vlan_delete(struct net_device *pdev, unsigned short pkey)
 	}
 
 	rtnl_unlock();
+	mutex_unlock(&ppriv->sysfs_lock);
 
 	if (dev) {
 		free_netdev(dev);
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

#41 lore raw

Re: [rdma-next v3 19/24] IB/ipoib: Sync between remove_one to sysfs calls that use rtnl_lock

Quoted 61 lines · Leon Romanovsky

From: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/[email protected]>

In order to avoid deadlock between sysfs functions (like create/delete
child) and remove_one (both of them are using the sysfs lock and rtnl_lock) the driver will use a state mutex for sync.

That will fix traces as the following:

schedule+0x3e/0x90
kernfs_drain+0x75/0xf0
? wait_woken+0x90/0x90
__kernfs_remove+0x12e/0x1c0
kernfs_remove+0x25/0x40
sysfs_remove_dir+0x57/0x90
kobject_del+0x22/0x60
device_del+0x195/0x230
 pm_runtime_set_memalloc_noio+0xac/0xf0
netdev_unregister_kobject+0x71/0x80
rollback_registered_many+0x205/0x2f0
rollback_registered+0x31/0x40
unregister_netdevice_queue+0x58/0xb0
unregister_netdev+0x20/0x30
ipoib_remove_one+0xb7/0x240 [ib_ipoib]
ib_unregister_device+0xbc/0x1b0 [ib_core]
ib_unregister_mad_agent+0x29/0x30 [ib_core]
mlx4_ib_remove+0x67/0x280 [mlx4_ib]
INFO: task echo:24082 blocked for more than 120 seconds.
Tainted: G           OE   4.1.12-37.5.1.el6uek.x86_64 #2
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this
message.
Call Trace:
schedule+0x3e/0x90
schedule_preempt_disabled+0xe/0x10
__mutex_lock_slowpath+0x95/0x110
? _rcu_barrier+0x177/0x220
mutex_lock+0x23/0x40
rtnl_lock+0x15/0x20
netdev_run_todo+0x81/0x1f0
rtnl_unlock+0xe/0x10
ipoib_vlan_delete+0x12f/0x1c0 [ib_ipoib]
delete_child+0x69/0x80 [ib_ipoib]
dev_attr_store+0x20/0x30
sysfs_kf_write+0x41/0x50
---
 drivers/infiniband/ulp/ipoib/ipoib.h      |  1 +
 drivers/infiniband/ulp/ipoib/ipoib_cm.c   |  8 +++++++-
 drivers/infiniband/ulp/ipoib/ipoib_main.c |  5 +++++
 drivers/infiniband/ulp/ipoib/ipoib_vlan.c | 22 ++++++++++++++++++---
(quoted diff) +0 -1
-
 4 files changed, 31 insertions(+), 5 deletions(-)
drivers/infiniband/ulp/ipoib/ipoib.h +0 -0
index 9e738104c2a1..7aa114cf53e3 100644

drivers/infiniband/ulp/ipoib/ipoib.h struct ipoib_dev_priv { · line 339

Quoted 4 lines
drivers/infiniband/ulp/ipoib/ipoib.h +1 -0
@@ -336,6 +336,7 @@ struct ipoib_dev_priv {
        unsigned long flags;
 
        struct rw_semaphore vlan_rwsem;
+       struct mutex        sysfs_lock; /* Protect sysfs running*/

I detest calling a mutex a lock and vice versa, so I renamed this sysfs_mutex, and then took the patch.

Signature · 8 lines
Doug Ledford <[email protected]>
    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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#42 lore raw

PATCH v3 20/24 IB/ipoib: Add get statistics support to SRIOV VF

In reply to a message not in the archive

From: Feras Daoud <ferasda-VPRAkNaXOzVWk0Htik3J/[email protected]>

Add SRIOV VF support to get traffic statistics.

---
 drivers/infiniband/ulp/ipoib/ipoib_main.c | 1 +
 1 file changed, 1 insertion(+)
drivers/infiniband/ulp/ipoib/ipoib_main.c +1 -0
index 627ff8fb59b0..9e8dce6e9fc2 100644
@@ -1846,6 +1846,7 @@ static const struct net_device_ops ipoib_netdev_ops_vf = {
 	.ndo_tx_timeout		 = ipoib_timeout,
 	.ndo_set_rx_mode	 = ipoib_set_mcast_list,
 	.ndo_get_iflink		 = ipoib_get_iflink,
+	.ndo_get_stats64	 = ipoib_get_stats,
 };
 
 void ipoib_setup_common(struct net_device *dev)
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

#44 lore raw

Re: [rdma-next v3 20/24] IB/ipoib: Add get statistics support to SRIOV VF

Quoted 6 lines · Leon Romanovsky

From: Feras Daoud <ferasda-VPRAkNaXOzVWk0Htik3J/[email protected]>

Add SRIOV VF support to get traffic statistics.

This one was already in my tree and so I had to skip it:

eb54714ddcb2 IB/ipoib: Add get statistics support to SRIOV VF

git tag --contains eb54714ddcb2
for-linus
for-next
v4.13-rc5

So this was in what you submitted for -rc5 in the IPoIB fixes. It shouldn't have made it into this patch series again.

Quoted 18 lines
---
 drivers/infiniband/ulp/ipoib/ipoib_main.c | 1 +
 1 file changed, 1 insertion(+)
drivers/infiniband/ulp/ipoib/ipoib_main.c +1 -0
index 627ff8fb59b0..9e8dce6e9fc2 100644
@@ -1846,6 +1846,7 @@ static const struct net_device_ops
 ipoib_netdev_ops_vf = {
 	.ndo_tx_timeout		 = ipoib_timeout,
 	.ndo_set_rx_mode	 = ipoib_set_mcast_list,
 	.ndo_get_iflink		 = ipoib_get_iflink,
+	.ndo_get_stats64	 = ipoib_get_stats,
 };
 

void ipoib_setup_common(struct net_device *dev)

Signature · 8 lines
Doug Ledford <[email protected]>
    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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 1503607480.78641.51.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

#46 lore raw

Re: [rdma-next v3 20/24] IB/ipoib: Add get statistics support to SRIOV VF

Quoted 12 lines · Doug Ledford
Quoted 6 lines · Leon Romanovsky

From: Feras Daoud <ferasda-VPRAkNaXOzVWk0Htik3J/[email protected]>

Add SRIOV VF support to get traffic statistics.

This one was already in my tree and so I had to skip it:

eb54714ddcb2 IB/ipoib: Add get statistics support to SRIOV VF

Quoted 7 lines

git tag --contains eb54714ddcb2
for-linus
for-next
v4.13-rc5

So this was in what you submitted for -rc5 in the IPoIB fixes. It shouldn't have made it into this patch series again.

You are right, sorry for the mess.

Quoted 24 lines
Quoted 18 lines
---
 drivers/infiniband/ulp/ipoib/ipoib_main.c | 1 +
 1 file changed, 1 insertion(+)
drivers/infiniband/ulp/ipoib/ipoib_main.c +1 -0
index 627ff8fb59b0..9e8dce6e9fc2 100644
@@ -1846,6 +1846,7 @@ static const struct net_device_ops
 ipoib_netdev_ops_vf = {
 	.ndo_tx_timeout		 = ipoib_timeout,
 	.ndo_set_rx_mode	 = ipoib_set_mcast_list,
 	.ndo_get_iflink		 = ipoib_get_iflink,
+	.ndo_get_stats64	 = ipoib_get_stats,
 };
 
Quoted 1 line

void ipoib_setup_common(struct net_device *dev)

--
Doug Ledford <[email protected]>

    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
#47 lore raw

PATCH v3 21/24 IB/rxe: Make rxe_counter_name static

In reply to a message not in the archive

From: Kamal Heib <kamalh-VPRAkNaXOzVWk0Htik3J/[email protected]>

rxe_counter_name is used in rxe_hw_counters.c only. Make it static.

---
 drivers/infiniband/sw/rxe/rxe_hw_counters.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/infiniband/sw/rxe/rxe_hw_counters.c +1 -1
index 7ef90aad7dfd..6aeb7a165e46 100644
@@ -33,7 +33,7 @@
 #include "rxe.h"
 #include "rxe_hw_counters.h"
 
-const char * const rxe_counter_name[] = {
+static const char * const rxe_counter_name[] = {
 	[RXE_CNT_SENT_PKTS]           =  "sent_pkts",
 	[RXE_CNT_RCVD_PKTS]           =  "rcvd_pkts",
 	[RXE_CNT_DUP_REQ]             =  "duplicate_request",
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#48 lore raw

PATCH v3 22/24 RDMA/mlx5: Limit scope of get vector affinity local function

In reply to a message not in the archive

The mlx5_ib_get_vector_affinity() call is local to main.c file and there is no need to be declared globally visible.

Fixes 40b24403f33e ("mlx5: support ->get_vector_affinity")
---
 drivers/infiniband/hw/mlx5/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/infiniband/hw/mlx5/main.c +2 -2
index 986404827848..47cf622c6d24 100644
@@ -3818,8 +3818,8 @@ static void init_delay_drop(struct mlx5_ib_dev *dev)
 		mlx5_ib_warn(dev, "Failed to init delay drop debugfs\n");
 }
 
-const struct cpumask *mlx5_ib_get_vector_affinity(struct ib_device *ibdev,
-		int comp_vector)
+static const struct cpumask *
+mlx5_ib_get_vector_affinity(struct ib_device *ibdev, int comp_vector)
 {
 	struct mlx5_ib_dev *dev = to_mdev(ibdev);
 
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#49 lore raw

PATCH v3 23/24 RDMA/mlx4: Properly annotate link layer variable

In reply to a message not in the archive

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

The rdma_port_get_link_layer() returns enum rdma_link_layer as a return value, hence it is better to store the return value in specially annotated variable and not in int.

---
 drivers/infiniband/hw/mlx4/qp.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/infiniband/hw/mlx4/qp.c +2 -4
index 2874d8790570..8c6ccc392d87 100644
@@ -2732,19 +2732,17 @@ enum {
 static int _mlx4_ib_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
 			      int attr_mask, struct ib_udata *udata)
 {
+	enum rdma_link_layer ll = IB_LINK_LAYER_UNSPECIFIED;
 	struct mlx4_ib_dev *dev = to_mdev(ibqp->device);
 	struct mlx4_ib_qp *qp = to_mqp(ibqp);
 	enum ib_qp_state cur_state, new_state;
 	int err = -EINVAL;
-	int ll;
 	mutex_lock(&qp->mutex);
 
 	cur_state = attr_mask & IB_QP_CUR_STATE ? attr->cur_qp_state : qp->state;
 	new_state = attr_mask & IB_QP_STATE ? attr->qp_state : cur_state;
 
-	if (cur_state == new_state && cur_state == IB_QPS_RESET) {
-		ll = IB_LINK_LAYER_UNSPECIFIED;
-	} else {
+	if (cur_state != new_state || cur_state != IB_QPS_RESET) {
 		int port = attr_mask & IB_QP_PORT ? attr->port_num : qp->port;
 		ll = rdma_port_get_link_layer(&dev->ib_dev, port);
 	}
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#50 lore raw

PATCH v3 24/24 RDMA/nes: Remove zeroed parameter from port query callback

In reply to a message not in the archive

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

There is no need to explicitly zero parameters, because the structure requested to be filled already initialized to zeros.

---
 drivers/infiniband/hw/nes/nes_verbs.c | 5 -----
 1 file changed, 5 deletions(-)
drivers/infiniband/hw/nes/nes_verbs.c +0 -5
index c2943e39d2f9..f0dc5f4aa177 100644
@@ -481,21 +481,16 @@ static int nes_query_port(struct ib_device *ibdev, u8 port, struct ib_port_attr
 	props->active_mtu = ib_mtu_int_to_enum(netdev->mtu);
 
 	props->lid = 1;
-	props->lmc = 0;
-	props->sm_lid = 0;
-	props->sm_sl = 0;
 	if (netif_queue_stopped(netdev))
 		props->state = IB_PORT_DOWN;
 	else if (nesvnic->linkup)
 		props->state = IB_PORT_ACTIVE;
 	else
 		props->state = IB_PORT_DOWN;
-	props->phys_state = 0;
 	props->port_cap_flags = IB_PORT_CM_SUP | IB_PORT_REINIT_SUP |
 			IB_PORT_VENDOR_CLASS_SUP | IB_PORT_BOOT_MGMT_SUP;
 	props->gid_tbl_len = 1;
 	props->pkey_tbl_len = 1;
-	props->qkey_viol_cntr = 0;
 	props->active_width = IB_WIDTH_4X;
 	props->active_speed = IB_SPEED_SDR;
 	props->max_msg_sz = 0x80000000;
Signature · 6 lines
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#51 lore raw
In reply to a message not in the archive
Quoted 9 lines · Leon Romanovsky

Hi,

This is bunch of fixes to the RDMA stack which we found during code review,
static checkers cleanups and by various code inspections.

There is one exception to this batch - patch from Feras to add statistics support to SRIOV VF in IPoIB.

Quoted 6 lines

However, the diff stat of that change is so small and I didn't find other batch to attach it.

 drivers/infiniband/ulp/ipoib/ipoib_main.c | 1 +
 1 file changed, 1 insertion(+)

Thanks

Modulo the notes I sent on the individual patches, I've pulled this series in. But, I would note that our trees were well out of sync on this one and that led to a lot of hand fixes and merges. Not that I can't do it, but it makes me nervous that what I fix up won't be right for the new patch in some way. So please double check the mellanox branch on github to make sure that you're happy with how I fixed everything up.

Signature · 8 lines
Doug Ledford <[email protected]>
    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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html