RE: [PATCH 29/30] IB/mlx4: Create and use another QP1 for RoCEv2

2015-02-19 Thread Shachar Raindel 


 -Original Message-
 From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma-
 ow...@vger.kernel.org] On Behalf Of Somnath Kotur
 Sent: Friday, February 20, 2015 12:03 AM
 To: rol...@kernel.org
 Cc: linux-rdma@vger.kernel.org; Moni Shoua; Somnath Kotur
 Subject: [PATCH 29/30] IB/mlx4: Create and use another QP1 for RoCEv2
 
 From: Moni Shoua mo...@mellanox.com
 
 The mlx4 driver uses a special QP to implement the GSI QP. This kind of
 QP allows to build the InfiniBand headers in SW to be put before the
 payload that comes in with the WR. The mlx4 HW builds the packet,
 calculates the ICRC and puts it at the end of the payload. This ICRC
 calculation however depends on the QP configuration which is determined
 when QP is modified (roce_mode during INIT-RTR). On the other hand,
 ICRC
 verification when packet is received does to depend on this

The grammar in this sentence is broken. Generally speaking, it is hard to 
understand what the patch does and aim to do from this commit message.
Please rephrase.

 configuration.
 Therefore, using 2 GSI QPs for send (one for each RoCE version) and 1
 GSI QP for receive are required.
 
 Signed-off-by: Moni Shoua mo...@mellanox.com
 Signed-off-by: Somnath Kotur somnath.ko...@emulex.com
 ---
  drivers/infiniband/hw/mlx4/mlx4_ib.h |7 ++
  drivers/infiniband/hw/mlx4/qp.c  |  154
 ++
  2 files changed, 143 insertions(+), 18 deletions(-)
 
 diff --git a/drivers/infiniband/hw/mlx4/mlx4_ib.h
 b/drivers/infiniband/hw/mlx4/mlx4_ib.h
 index 018bda6..a853330 100644
 --- a/drivers/infiniband/hw/mlx4/mlx4_ib.h
 +++ b/drivers/infiniband/hw/mlx4/mlx4_ib.h
 @@ -159,11 +159,18 @@ struct mlx4_ib_wq {
   unsignedtail;
  };
 
 +enum {
 + MLX4_IB_QP_CREATE_ROCE_V2_GSI = IB_QP_CREATE_RESERVED_START
 +};
 +
  enum mlx4_ib_qp_flags {
   MLX4_IB_QP_LSO = IB_QP_CREATE_IPOIB_UD_LSO,
   MLX4_IB_QP_BLOCK_MULTICAST_LOOPBACK =
 IB_QP_CREATE_BLOCK_MULTICAST_LOOPBACK,
   MLX4_IB_QP_NETIF = IB_QP_CREATE_NETIF_QP,
   MLX4_IB_QP_CREATE_USE_GFP_NOIO = IB_QP_CREATE_USE_GFP_NOIO,
 +
 + /* Mellanox specific flags start from IB_QP_CREATE_RESERVED_START
 */
 + MLX4_IB_ROCE_V2_GSI_QP = MLX4_IB_QP_CREATE_ROCE_V2_GSI,

Why do you need 2 enums for this flag definition?
Also, the comment refers to IB_QP_CREATE_RESERVED_START, which is not appearing 
here.

   MLX4_IB_SRIOV_TUNNEL_QP = 1  30,
   MLX4_IB_SRIOV_SQP = 1  31,
  };
 diff --git a/drivers/infiniband/hw/mlx4/qp.c
 b/drivers/infiniband/hw/mlx4/qp.c
 index 9996527..161b933 100644
 --- a/drivers/infiniband/hw/mlx4/qp.c
 +++ b/drivers/infiniband/hw/mlx4/qp.c
 @@ -81,6 +81,7 @@ struct mlx4_ib_sqp {
   u32 send_psn;
   struct ib_ud_header ud_header;
   u8  header_buf[MLX4_IB_UD_HEADER_SIZE];
 + struct ib_qp*roce_v2_gsi;
  };
 
  enum {
 @@ -150,7 +151,10 @@ static int is_sqp(struct mlx4_ib_dev *dev, struct
 mlx4_ib_qp *qp)
   }
   }
   }
 - return proxy_sqp;
 + if (proxy_sqp)
 + return 1;
 +
 + return !!(qp-flags  MLX4_IB_ROCE_V2_GSI_QP);

What are the implications of this double-QP scheme in virtualization scenario?

  }
 
  /* used for INIT/CLOSE port logic */
 @@ -672,6 +676,7 @@ static int create_qp_common(struct mlx4_ib_dev *dev,
 struct ib_pd *pd,
   qp = sqp-qp;
   qp-pri.vid = 0x;
   qp-alt.vid = 0x;
 + sqp-roce_v2_gsi = NULL;
   } else {
   qp = kzalloc(sizeof (struct mlx4_ib_qp), gfp);
   if (!qp)
 @@ -1029,9 +1034,17 @@ static void destroy_qp_common(struct mlx4_ib_dev
 *dev, struct mlx4_ib_qp *qp,
   del_gid_entries(qp);
  }
 
 -static u32 get_sqp_num(struct mlx4_ib_dev *dev, struct ib_qp_init_attr
 *attr)
 +static int get_sqp_num(struct mlx4_ib_dev *dev, struct ib_qp_init_attr
 *attr)
  {
   /* Native or PPF */
 + if ((!mlx4_is_mfunc(dev-dev) || mlx4_is_master(dev-dev)) 
 + attr-create_flags  MLX4_IB_QP_CREATE_ROCE_V2_GSI) {
 + int sqpn;
 + int res = mlx4_qp_reserve_range(dev-dev, 1, 1, sqpn, 0);
 +
 + return res ? -abs(res) : sqpn;

This seems wrong. Mlx4_qp_reserve_range returns either 0 (success) or negative 
errno value on error. The check here should therefore say res ? res : sqpn;.

If you want, you can add a WARN_ON(res  0); above it.

 + }
 +
   if (!mlx4_is_mfunc(dev-dev) ||
   (mlx4_is_master(dev-dev) 
attr-create_flags  MLX4_IB_SRIOV_SQP)) {
 @@ -1039,6 +1052,7 @@ static u32 get_sqp_num(struct mlx4_ib_dev *dev,
 struct ib_qp_init_attr *attr)
   (attr-qp_type == IB_QPT_SMI ? 0 : 2) +
   attr-port_num - 1;
   }
 +
   /* PF or VF -- creating proxies */
   if (attr-qp_type == IB_QPT_SMI)
   return dev-dev-caps.qp0_proxy[attr

[PATCH 29/30] IB/mlx4: Create and use another QP1 for RoCEv2

2015-02-18 Thread Somnath Kotur 
From: Moni Shoua mo...@mellanox.com

The mlx4 driver uses a special QP to implement the GSI QP. This kind of
QP allows to build the InfiniBand headers in SW to be put before the
payload that comes in with the WR. The mlx4 HW builds the packet,
calculates the ICRC and puts it at the end of the payload. This ICRC
calculation however depends on the QP configuration which is determined
when QP is modified (roce_mode during INIT-RTR). On the other hand,  ICRC
verification when packet is received does to depend on this
configuration.
Therefore, using 2 GSI QPs for send (one for each RoCE version) and 1
GSI QP for receive are required.

Signed-off-by: Moni Shoua mo...@mellanox.com
Signed-off-by: Somnath Kotur somnath.ko...@emulex.com
---
 drivers/infiniband/hw/mlx4/mlx4_ib.h |7 ++
 drivers/infiniband/hw/mlx4/qp.c  |  154 ++
 2 files changed, 143 insertions(+), 18 deletions(-)

diff --git a/drivers/infiniband/hw/mlx4/mlx4_ib.h 
b/drivers/infiniband/hw/mlx4/mlx4_ib.h
index 018bda6..a853330 100644
--- a/drivers/infiniband/hw/mlx4/mlx4_ib.h
+++ b/drivers/infiniband/hw/mlx4/mlx4_ib.h
@@ -159,11 +159,18 @@ struct mlx4_ib_wq {
unsignedtail;
 };
 
+enum {
+   MLX4_IB_QP_CREATE_ROCE_V2_GSI = IB_QP_CREATE_RESERVED_START
+};
+
 enum mlx4_ib_qp_flags {
MLX4_IB_QP_LSO = IB_QP_CREATE_IPOIB_UD_LSO,
MLX4_IB_QP_BLOCK_MULTICAST_LOOPBACK = 
IB_QP_CREATE_BLOCK_MULTICAST_LOOPBACK,
MLX4_IB_QP_NETIF = IB_QP_CREATE_NETIF_QP,
MLX4_IB_QP_CREATE_USE_GFP_NOIO = IB_QP_CREATE_USE_GFP_NOIO,
+
+   /* Mellanox specific flags start from IB_QP_CREATE_RESERVED_START */
+   MLX4_IB_ROCE_V2_GSI_QP = MLX4_IB_QP_CREATE_ROCE_V2_GSI,
MLX4_IB_SRIOV_TUNNEL_QP = 1  30,
MLX4_IB_SRIOV_SQP = 1  31,
 };
diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index 9996527..161b933 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -81,6 +81,7 @@ struct mlx4_ib_sqp {
u32 send_psn;
struct ib_ud_header ud_header;
u8  header_buf[MLX4_IB_UD_HEADER_SIZE];
+   struct ib_qp*roce_v2_gsi;
 };
 
 enum {
@@ -150,7 +151,10 @@ static int is_sqp(struct mlx4_ib_dev *dev, struct 
mlx4_ib_qp *qp)
}
}
}
-   return proxy_sqp;
+   if (proxy_sqp)
+   return 1;
+
+   return !!(qp-flags  MLX4_IB_ROCE_V2_GSI_QP);
 }
 
 /* used for INIT/CLOSE port logic */
@@ -672,6 +676,7 @@ static int create_qp_common(struct mlx4_ib_dev *dev, struct 
ib_pd *pd,
qp = sqp-qp;
qp-pri.vid = 0x;
qp-alt.vid = 0x;
+   sqp-roce_v2_gsi = NULL;
} else {
qp = kzalloc(sizeof (struct mlx4_ib_qp), gfp);
if (!qp)
@@ -1029,9 +1034,17 @@ static void destroy_qp_common(struct mlx4_ib_dev *dev, 
struct mlx4_ib_qp *qp,
del_gid_entries(qp);
 }
 
-static u32 get_sqp_num(struct mlx4_ib_dev *dev, struct ib_qp_init_attr *attr)
+static int get_sqp_num(struct mlx4_ib_dev *dev, struct ib_qp_init_attr *attr)
 {
/* Native or PPF */
+   if ((!mlx4_is_mfunc(dev-dev) || mlx4_is_master(dev-dev)) 
+   attr-create_flags  MLX4_IB_QP_CREATE_ROCE_V2_GSI) {
+   int sqpn;
+   int res = mlx4_qp_reserve_range(dev-dev, 1, 1, sqpn, 0);
+
+   return res ? -abs(res) : sqpn;
+   }
+
if (!mlx4_is_mfunc(dev-dev) ||
(mlx4_is_master(dev-dev) 
 attr-create_flags  MLX4_IB_SRIOV_SQP)) {
@@ -1039,6 +1052,7 @@ static u32 get_sqp_num(struct mlx4_ib_dev *dev, struct 
ib_qp_init_attr *attr)
(attr-qp_type == IB_QPT_SMI ? 0 : 2) +
attr-port_num - 1;
}
+
/* PF or VF -- creating proxies */
if (attr-qp_type == IB_QPT_SMI)
return dev-dev-caps.qp0_proxy[attr-port_num - 1];
@@ -1046,9 +1060,9 @@ static u32 get_sqp_num(struct mlx4_ib_dev *dev, struct 
ib_qp_init_attr *attr)
return dev-dev-caps.qp1_proxy[attr-port_num - 1];
 }
 
-struct ib_qp *mlx4_ib_create_qp(struct ib_pd *pd,
-   struct ib_qp_init_attr *init_attr,
-   struct ib_udata *udata)
+static struct ib_qp *_mlx4_ib_create_qp(struct ib_pd *pd,
+   struct ib_qp_init_attr *init_attr,
+   struct ib_udata *udata)
 {
struct mlx4_ib_qp *qp = NULL;
int err;
@@ -1066,6 +1080,7 @@ struct ib_qp *mlx4_ib_create_qp(struct ib_pd *pd,
MLX4_IB_SRIOV_TUNNEL_QP |
MLX4_IB_SRIOV_SQP |
MLX4_IB_QP_NETIF |
+   MLX4_IB_QP_CREATE_ROCE_V2_GSI |

[PATCH 29/30] IB/mlx4: Create and use another QP1 for RoCEv2

2015-02-18 Thread Somnath Kotur 
From: Moni Shoua mo...@mellanox.com

The mlx4 driver uses a special QP to implement the GSI QP. This kind of
QP allows to build the InfiniBand headers in SW to be put before the
payload that comes in with the WR. The mlx4 HW builds the packet,
calculates the ICRC and puts it at the end of the payload. This ICRC
calculation however depends on the QP configuration which is determined
when QP is modified (roce_mode during INIT-RTR). On the other hand,  ICRC
verification when packet is received does to depend on this
configuration.
Therefore, using 2 GSI QPs for send (one for each RoCE version) and 1
GSI QP for receive are required.

Signed-off-by: Moni Shoua mo...@mellanox.com
Signed-off-by: Somnath Kotur somnath.ko...@emulex.com
---
 drivers/infiniband/hw/mlx4/mlx4_ib.h |7 ++
 drivers/infiniband/hw/mlx4/qp.c  |  154 ++
 2 files changed, 143 insertions(+), 18 deletions(-)

diff --git a/drivers/infiniband/hw/mlx4/mlx4_ib.h 
b/drivers/infiniband/hw/mlx4/mlx4_ib.h
index 018bda6..a853330 100644
--- a/drivers/infiniband/hw/mlx4/mlx4_ib.h
+++ b/drivers/infiniband/hw/mlx4/mlx4_ib.h
@@ -159,11 +159,18 @@ struct mlx4_ib_wq {
unsignedtail;
 };
 
+enum {
+   MLX4_IB_QP_CREATE_ROCE_V2_GSI = IB_QP_CREATE_RESERVED_START
+};
+
 enum mlx4_ib_qp_flags {
MLX4_IB_QP_LSO = IB_QP_CREATE_IPOIB_UD_LSO,
MLX4_IB_QP_BLOCK_MULTICAST_LOOPBACK = 
IB_QP_CREATE_BLOCK_MULTICAST_LOOPBACK,
MLX4_IB_QP_NETIF = IB_QP_CREATE_NETIF_QP,
MLX4_IB_QP_CREATE_USE_GFP_NOIO = IB_QP_CREATE_USE_GFP_NOIO,
+
+   /* Mellanox specific flags start from IB_QP_CREATE_RESERVED_START */
+   MLX4_IB_ROCE_V2_GSI_QP = MLX4_IB_QP_CREATE_ROCE_V2_GSI,
MLX4_IB_SRIOV_TUNNEL_QP = 1  30,
MLX4_IB_SRIOV_SQP = 1  31,
 };
diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index 9996527..161b933 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -81,6 +81,7 @@ struct mlx4_ib_sqp {
u32 send_psn;
struct ib_ud_header ud_header;
u8  header_buf[MLX4_IB_UD_HEADER_SIZE];
+   struct ib_qp*roce_v2_gsi;
 };
 
 enum {
@@ -150,7 +151,10 @@ static int is_sqp(struct mlx4_ib_dev *dev, struct 
mlx4_ib_qp *qp)
}
}
}
-   return proxy_sqp;
+   if (proxy_sqp)
+   return 1;
+
+   return !!(qp-flags  MLX4_IB_ROCE_V2_GSI_QP);
 }
 
 /* used for INIT/CLOSE port logic */
@@ -672,6 +676,7 @@ static int create_qp_common(struct mlx4_ib_dev *dev, struct 
ib_pd *pd,
qp = sqp-qp;
qp-pri.vid = 0x;
qp-alt.vid = 0x;
+   sqp-roce_v2_gsi = NULL;
} else {
qp = kzalloc(sizeof (struct mlx4_ib_qp), gfp);
if (!qp)
@@ -1029,9 +1034,17 @@ static void destroy_qp_common(struct mlx4_ib_dev *dev, 
struct mlx4_ib_qp *qp,
del_gid_entries(qp);
 }
 
-static u32 get_sqp_num(struct mlx4_ib_dev *dev, struct ib_qp_init_attr *attr)
+static int get_sqp_num(struct mlx4_ib_dev *dev, struct ib_qp_init_attr *attr)
 {
/* Native or PPF */
+   if ((!mlx4_is_mfunc(dev-dev) || mlx4_is_master(dev-dev)) 
+   attr-create_flags  MLX4_IB_QP_CREATE_ROCE_V2_GSI) {
+   int sqpn;
+   int res = mlx4_qp_reserve_range(dev-dev, 1, 1, sqpn, 0);
+
+   return res ? -abs(res) : sqpn;
+   }
+
if (!mlx4_is_mfunc(dev-dev) ||
(mlx4_is_master(dev-dev) 
 attr-create_flags  MLX4_IB_SRIOV_SQP)) {
@@ -1039,6 +1052,7 @@ static u32 get_sqp_num(struct mlx4_ib_dev *dev, struct 
ib_qp_init_attr *attr)
(attr-qp_type == IB_QPT_SMI ? 0 : 2) +
attr-port_num - 1;
}
+
/* PF or VF -- creating proxies */
if (attr-qp_type == IB_QPT_SMI)
return dev-dev-caps.qp0_proxy[attr-port_num - 1];
@@ -1046,9 +1060,9 @@ static u32 get_sqp_num(struct mlx4_ib_dev *dev, struct 
ib_qp_init_attr *attr)
return dev-dev-caps.qp1_proxy[attr-port_num - 1];
 }
 
-struct ib_qp *mlx4_ib_create_qp(struct ib_pd *pd,
-   struct ib_qp_init_attr *init_attr,
-   struct ib_udata *udata)
+static struct ib_qp *_mlx4_ib_create_qp(struct ib_pd *pd,
+   struct ib_qp_init_attr *init_attr,
+   struct ib_udata *udata)
 {
struct mlx4_ib_qp *qp = NULL;
int err;
@@ -1066,6 +1080,7 @@ struct ib_qp *mlx4_ib_create_qp(struct ib_pd *pd,
MLX4_IB_SRIOV_TUNNEL_QP |
MLX4_IB_SRIOV_SQP |
MLX4_IB_QP_NETIF |
+   MLX4_IB_QP_CREATE_ROCE_V2_GSI |