[PATCH rdma-next 0/6] Enable flow steering on IPoIB UD QP

[PATCH rdma-next 0/6] Enable flow steering on IPoIB UD QP

[Leon Romanovsky]

Hi Doug,

---
>From Yishai:

This series comes to allow user space applications to accelerate send
and receive traffic which is typically handled by IPoIB ULP. On the
receive flow, the user space application will use flow steering rules to
direct the ingress IPoIB UD QP traffic to its own user space UD QP. On
the send flow, the user space application will send from its own UD QP
with UD AH, while the packet on the wire will have the source QPN of the
underlay IPoIB UD QP, which is the network interface advertised L2
hardware address.

This will be achieved by extending QP creation of IB UD QP to be
"Associated" with IPoIB UD QP number. In order to create such a UD QP
which can receive and send IPoIB traffic, needs to provide the
associated QPN of the IPoIB network interface. The created QP will
manage the scattering objects as of RQ and SQ, while the transport
attributes will be managed by its underlay QP which is owned by IPoIB
ULP.

The QPN should be achieved by using some networking tool as of ifconfig
,ip link show on the associated interface. For example, when IPoIB is
used, the QPN is the value of 3 bytes from the hardware address starting
after the first byte as defined by rfc4391 [1]

This API enables generic QPN association, current mlx5 driver implements
this API for the IPoIB ULP.

[1] https://tools.ietf.org/html/rfc4391#page-11
---

Available in the "topic/underlay_qp" topic branch of this git repo:
git://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git

Or for browsing:
https://git.kernel.org/cgit/linux/kernel/git/leon/linux-rdma.git/log/?h=topic/underlay_qp

Possible merge conflicts resolution is available in:
https://git.kernel.org/cgit/linux/kernel/git/leon/linux-rdma.git/log/?h=testing/queue-next

Yishai Hadas (6):
  IB/core: Enable QP creation which is associated to underlay QP
  IB/uverbs: Enable QP creation which is associated to underlay QP
  IB/mlx5: Add support for underlay QP managing
  IB/mlx5: Add multicast flow steering support for underlay QP
  net/mlx5: Report enhanced capabilities for IPoIB
  IB/mlx5: Report RX checksum capabilities for IPoIB

 drivers/infiniband/core/uverbs_cmd.c         | 11 +++--
 drivers/infiniband/hw/mlx5/main.c            | 47 +++++++++++++-----
 drivers/infiniband/hw/mlx5/mlx5_ib.h         |  2 +
 drivers/infiniband/hw/mlx5/qp.c              | 72 ++++++++++++++++++++++------
 drivers/net/ethernet/mellanox/mlx5/core/fw.c |  6 +++
 include/linux/mlx5/device.h                  |  6 ++-
 include/rdma/ib_verbs.h                      |  2 +
 include/uapi/rdma/ib_user_verbs.h            |  2 +-
 8 files changed, 116 insertions(+), 32 deletions(-)

--
2.12.2

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

[PATCH rdma-next 1/6] IB/core: Enable QP creation which is associated to underlay QP

[Leon Romanovsky]

From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Enable QP creation which is associated to underlay QP.
The created QP will manage the scattering objects as of RQ and SQ, while
the transport attributes will be managed by its underlay QP

This comes as a pre-patch for downstream patches in this series to
enable flow steering on the underlay IPoIB QP.

Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/rdma/ib_verbs.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index f0cb4906478a..58bea71e54c6 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -1058,6 +1058,7 @@ enum ib_qp_create_flags {
 	IB_QP_CREATE_USE_GFP_NOIO		= 1 << 7,
 	IB_QP_CREATE_SCATTER_FCS		= 1 << 8,
 	IB_QP_CREATE_CVLAN_STRIPPING		= 1 << 9,
+	IB_QP_CREATE_ASSOC_QPN			= 1 << 10,
 	/* reserve bits 26-31 for low level drivers' internal use */
 	IB_QP_CREATE_RESERVED_START		= 1 << 26,
 	IB_QP_CREATE_RESERVED_END		= 1 << 31,
@@ -1085,6 +1086,7 @@ struct ib_qp_init_attr {
 	 */
 	u8			port_num;
 	struct ib_rwq_ind_table *rwq_ind_tbl;
+	u32			associated_qpn;
 };

 struct ib_qp_open_attr {
--
2.12.2

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

[PATCH rdma-next 2/6] IB/uverbs: Enable QP creation which is associated to underlay QP

[Leon Romanovsky]

From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Enable QP creation which is associated to underlay QP. This comes as a
pre-patch for downstream patches in this series to enable flow steering
from user space application on the underlay IPoIB QP.

Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/uverbs_cmd.c | 11 ++++++++---
 include/uapi/rdma/ib_user_verbs.h    |  2 +-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 70b7fb156414..d819aa01da5e 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1385,8 +1385,9 @@ static int create_qp(struct ib_uverbs_file *file,
 		attr.rwq_ind_tbl = ind_tbl;
 	}

-	if ((cmd_sz >= offsetof(typeof(*cmd), reserved1) +
-		       sizeof(cmd->reserved1)) && cmd->reserved1) {
+	if (cmd_sz > sizeof(*cmd) &&
+	    !ib_is_udata_cleared(ucore, sizeof(*cmd),
+				 cmd_sz - sizeof(*cmd))) {
 		ret = -EOPNOTSUPP;
 		goto err_put;
 	}
@@ -1484,11 +1485,15 @@ static int create_qp(struct ib_uverbs_file *file,
 				IB_QP_CREATE_MANAGED_SEND |
 				IB_QP_CREATE_MANAGED_RECV |
 				IB_QP_CREATE_SCATTER_FCS |
-				IB_QP_CREATE_CVLAN_STRIPPING)) {
+				IB_QP_CREATE_CVLAN_STRIPPING |
+				IB_QP_CREATE_ASSOC_QPN)) {
 		ret = -EINVAL;
 		goto err_put;
 	}

+	if (attr.create_flags & IB_QP_CREATE_ASSOC_QPN)
+		attr.associated_qpn = cmd->associated_qpn;
+
 	buf = (void *)cmd + sizeof(*cmd);
 	if (cmd_sz > sizeof(*cmd))
 		if (!(buf[0] == 0 && !memcmp(buf, buf + 1,
diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
index 477d629f539d..422b20456975 100644
--- a/include/uapi/rdma/ib_user_verbs.h
+++ b/include/uapi/rdma/ib_user_verbs.h
@@ -578,7 +578,7 @@ struct ib_uverbs_ex_create_qp {
 	__u32 comp_mask;
 	__u32 create_flags;
 	__u32 rwq_ind_tbl_handle;
-	__u32  reserved1;
+	__u32  associated_qpn;
 };

 struct ib_uverbs_open_qp {
--
2.12.2

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

[PATCH rdma-next 3/6] IB/mlx5: Add support for underlay QP managing

[Leon Romanovsky]

From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Allow user space applications to accelerate send and receive
traffic which is typically handled by IPoIB ULP by creating
a UD QP which is associated with an underlay IPoIB UD QP.

UD QP with underlay QPN should basically be similar to
RAW QP from point of view of its created resources.

However,
- Its TIS should point to the underlay QPN.
- Modify can be done only on its state as the transport attributes
  are managed by its underlay QP.

This patch manages below:
- Creating/destroying UD QP with associated QPN.
- Modifying UD QP which has an underlay QP associated.

Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/mlx5_ib.h |  2 +
 drivers/infiniband/hw/mlx5/qp.c      | 72 ++++++++++++++++++++++++++++--------
 2 files changed, 59 insertions(+), 15 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 38c877bc45e5..f47d25e73ef8 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -378,6 +378,7 @@ struct mlx5_ib_qp {
 	struct list_head	cq_recv_list;
 	struct list_head	cq_send_list;
 	u32			rate_limit;
+	u32                     underlay_qpn;
 };

 struct mlx5_ib_cq_buf {
@@ -399,6 +400,7 @@ enum mlx5_ib_qp_flags {
 	MLX5_IB_QP_CAP_SCATTER_FCS		= 1 << 7,
 	MLX5_IB_QP_RSS				= 1 << 8,
 	MLX5_IB_QP_CVLAN_STRIPPING		= 1 << 9,
+	MLX5_IB_QP_UNDERLAY			= 1 << 10,
 };

 struct mlx5_umr_wr {
diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index 93959e1e43a3..854152a124a5 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -34,6 +34,7 @@
 #include <rdma/ib_umem.h>
 #include <rdma/ib_cache.h>
 #include <rdma/ib_user_verbs.h>
+#include <linux/mlx5/fs.h>
 #include "mlx5_ib.h"

 /* not supported currently */
@@ -453,7 +454,8 @@ static int set_user_buf_size(struct mlx5_ib_dev *dev,
 		return -EINVAL;
 	}

-	if (attr->qp_type == IB_QPT_RAW_PACKET) {
+	if (attr->qp_type == IB_QPT_RAW_PACKET ||
+	    qp->flags & MLX5_IB_QP_UNDERLAY) {
 		base->ubuffer.buf_size = qp->rq.wqe_cnt << qp->rq.wqe_shift;
 		qp->raw_packet_qp.sq.ubuffer.buf_size = qp->sq.wqe_cnt << 6;
 	} else {
@@ -1021,12 +1023,16 @@ static int is_connected(enum ib_qp_type qp_type)
 }

 static int create_raw_packet_qp_tis(struct mlx5_ib_dev *dev,
+				    struct mlx5_ib_qp *qp,
 				    struct mlx5_ib_sq *sq, u32 tdn)
 {
 	u32 in[MLX5_ST_SZ_DW(create_tis_in)] = {0};
 	void *tisc = MLX5_ADDR_OF(create_tis_in, in, ctx);

 	MLX5_SET(tisc, tisc, transport_domain, tdn);
+	if (qp->flags & MLX5_IB_QP_UNDERLAY)
+		MLX5_SET(tisc, tisc, underlay_qpn, qp->underlay_qpn);
+
 	return mlx5_core_create_tis(dev->mdev, in, sizeof(in), &sq->tisn);
 }

@@ -1229,7 +1235,7 @@ static int create_raw_packet_qp(struct mlx5_ib_dev *dev, struct mlx5_ib_qp *qp,
 	u32 tdn = mucontext->tdn;

 	if (qp->sq.wqe_cnt) {
-		err = create_raw_packet_qp_tis(dev, sq, tdn);
+		err = create_raw_packet_qp_tis(dev, qp, sq, tdn);
 		if (err)
 			return err;

@@ -1502,10 +1508,6 @@ static int create_qp_common(struct mlx5_ib_dev *dev, struct ib_pd *pd,
 	u32 *in;
 	int err;

-	base = init_attr->qp_type == IB_QPT_RAW_PACKET ?
-	       &qp->raw_packet_qp.rq.base :
-	       &qp->trans_qp.base;
-
 	mutex_init(&qp->mutex);
 	spin_lock_init(&qp->sq.lock);
 	spin_lock_init(&qp->rq.lock);
@@ -1587,10 +1589,28 @@ static int create_qp_common(struct mlx5_ib_dev *dev, struct ib_pd *pd,

 		qp->wq_sig = !!(ucmd.flags & MLX5_QP_FLAG_SIGNATURE);
 		qp->scat_cqe = !!(ucmd.flags & MLX5_QP_FLAG_SCATTER_CQE);
+
+		if (init_attr->create_flags & IB_QP_CREATE_ASSOC_QPN) {
+			if (init_attr->qp_type != IB_QPT_UD ||
+			    (MLX5_CAP_GEN(dev->mdev, port_type) !=
+			     MLX5_CAP_PORT_TYPE_IB) ||
+			    !mlx5_get_flow_namespace(dev->mdev, MLX5_FLOW_NAMESPACE_BYPASS)) {
+				mlx5_ib_dbg(dev, "Associated QP option isn't supported\n");
+				return -EOPNOTSUPP;
+			}
+
+			qp->flags |= MLX5_IB_QP_UNDERLAY;
+			qp->underlay_qpn = init_attr->associated_qpn;
+		}
 	} else {
 		qp->wq_sig = !!wq_signature;
 	}

+	base = (init_attr->qp_type == IB_QPT_RAW_PACKET ||
+		qp->flags & MLX5_IB_QP_UNDERLAY) ?
+	       &qp->raw_packet_qp.rq.base :
+	       &qp->trans_qp.base;
+
 	qp->has_rq = qp_has_rq(init_attr);
 	err = set_rq_size(dev, &init_attr->cap, qp->has_rq,
 			  qp, (pd && pd->uobject) ? &ucmd : NULL);
@@ -1741,7 +1761,8 @@ static int create_qp_common(struct mlx5_ib_dev *dev, struct ib_pd *pd,
 		qp->flags |= MLX5_IB_QP_LSO;
 	}

-	if (init_attr->qp_type == IB_QPT_RAW_PACKET) {
+	if (init_attr->qp_type == IB_QPT_RAW_PACKET ||
+	    qp->flags & MLX5_IB_QP_UNDERLAY) {
 		qp->raw_packet_qp.sq.ubuffer.buf_addr = ucmd.sq_buf_addr;
 		raw_packet_qp_copy_info(qp, &qp->raw_packet_qp);
 		err = create_raw_packet_qp(dev, qp, in, pd);
@@ -1893,7 +1914,7 @@ static int modify_raw_packet_qp(struct mlx5_ib_dev *dev, struct mlx5_ib_qp *qp,
 static void destroy_qp_common(struct mlx5_ib_dev *dev, struct mlx5_ib_qp *qp)
 {
 	struct mlx5_ib_cq *send_cq, *recv_cq;
-	struct mlx5_ib_qp_base *base = &qp->trans_qp.base;
+	struct mlx5_ib_qp_base *base;
 	unsigned long flags;
 	int err;

@@ -1902,12 +1923,14 @@ static void destroy_qp_common(struct mlx5_ib_dev *dev, struct mlx5_ib_qp *qp)
 		return;
 	}

-	base = qp->ibqp.qp_type == IB_QPT_RAW_PACKET ?
+	base = (qp->ibqp.qp_type == IB_QPT_RAW_PACKET ||
+		qp->flags & MLX5_IB_QP_UNDERLAY) ?
 	       &qp->raw_packet_qp.rq.base :
 	       &qp->trans_qp.base;

 	if (qp->state != IB_QPS_RESET) {
-		if (qp->ibqp.qp_type != IB_QPT_RAW_PACKET) {
+		if (qp->ibqp.qp_type != IB_QPT_RAW_PACKET &&
+		    !(qp->flags & MLX5_IB_QP_UNDERLAY)) {
 			err = mlx5_core_qp_modify(dev->mdev,
 						  MLX5_CMD_OP_2RST_QP, 0,
 						  NULL, &base->mqp);
@@ -1946,7 +1969,8 @@ static void destroy_qp_common(struct mlx5_ib_dev *dev, struct mlx5_ib_qp *qp)
 	mlx5_ib_unlock_cqs(send_cq, recv_cq);
 	spin_unlock_irqrestore(&dev->reset_flow_resource_lock, flags);

-	if (qp->ibqp.qp_type == IB_QPT_RAW_PACKET) {
+	if (qp->ibqp.qp_type == IB_QPT_RAW_PACKET ||
+	    qp->flags & MLX5_IB_QP_UNDERLAY) {
 		destroy_raw_packet_qp(dev, qp);
 	} else {
 		err = mlx5_core_destroy_qp(dev->mdev, &base->mqp);
@@ -2702,7 +2726,8 @@ static int __mlx5_ib_modify_qp(struct ib_qp *ibqp,

 	if (is_sqp(ibqp->qp_type)) {
 		context->mtu_msgmax = (IB_MTU_256 << 5) | 8;
-	} else if (ibqp->qp_type == IB_QPT_UD ||
+	} else if ((ibqp->qp_type == IB_QPT_UD &&
+		    !(qp->flags & MLX5_IB_QP_UNDERLAY)) ||
 		   ibqp->qp_type == MLX5_IB_QPT_REG_UMR) {
 		context->mtu_msgmax = (IB_MTU_4096 << 5) | 12;
 	} else if (attr_mask & IB_QP_PATH_MTU) {
@@ -2799,6 +2824,11 @@ static int __mlx5_ib_modify_qp(struct ib_qp *ibqp,
 	if (cur_state == IB_QPS_RESET && new_state == IB_QPS_INIT) {
 		u8 port_num = (attr_mask & IB_QP_PORT ? attr->port_num :
 			       qp->port) - 1;
+
+		/* Underlay port should be used - index 0 function per port */
+		if (qp->flags & MLX5_IB_QP_UNDERLAY)
+			port_num = 0;
+
 		mibport = &dev->port[port_num];
 		context->qp_counter_set_usr_page |=
 			cpu_to_be32((u32)(mibport->cnts.set_id) << 24);
@@ -2824,7 +2854,8 @@ static int __mlx5_ib_modify_qp(struct ib_qp *ibqp,
 	optpar = ib_mask_to_mlx5_opt(attr_mask);
 	optpar &= opt_mask[mlx5_cur][mlx5_new][mlx5_st];

-	if (qp->ibqp.qp_type == IB_QPT_RAW_PACKET) {
+	if (qp->ibqp.qp_type == IB_QPT_RAW_PACKET ||
+	    qp->flags & MLX5_IB_QP_UNDERLAY) {
 		struct mlx5_modify_raw_qp_param raw_qp_param = {};

 		raw_qp_param.operation = op;
@@ -2913,7 +2944,13 @@ int mlx5_ib_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
 		ll = dev->ib_dev.get_link_layer(&dev->ib_dev, port);
 	}

-	if (qp_type != MLX5_IB_QPT_REG_UMR &&
+	if (qp->flags & MLX5_IB_QP_UNDERLAY) {
+		if (attr_mask & ~(IB_QP_STATE | IB_QP_CUR_STATE)) {
+			mlx5_ib_dbg(dev, "invalid attr_mask 0x%x when underlay QP is used\n",
+				    attr_mask);
+			goto out;
+		}
+	} else if (qp_type != MLX5_IB_QPT_REG_UMR &&
 	    !ib_modify_qp_is_ok(cur_state, new_state, qp_type, attr_mask, ll)) {
 		mlx5_ib_dbg(dev, "invalid QP state transition from %d to %d, qp_type %d, attr_mask 0x%x\n",
 			    cur_state, new_state, ibqp->qp_type, attr_mask);
@@ -4490,9 +4527,14 @@ int mlx5_ib_query_qp(struct ib_qp *ibqp, struct ib_qp_attr *qp_attr,
 		return mlx5_ib_gsi_query_qp(ibqp, qp_attr, qp_attr_mask,
 					    qp_init_attr);

+	/* Not all of output fields are applicable, make sure to zero them */
+	memset(qp_init_attr, 0, sizeof(*qp_init_attr));
+	memset(qp_attr, 0, sizeof(*qp_attr));
+
 	mutex_lock(&qp->mutex);

-	if (qp->ibqp.qp_type == IB_QPT_RAW_PACKET) {
+	if (qp->ibqp.qp_type == IB_QPT_RAW_PACKET ||
+	    qp->flags & MLX5_IB_QP_UNDERLAY) {
 		err = query_raw_packet_qp_state(dev, qp, &raw_packet_qp_state);
 		if (err)
 			goto out;
--
2.12.2

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

[PATCH rdma-next 4/6] IB/mlx5: Add multicast flow steering support for underlay QP

[Leon Romanovsky]

From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

In order to add multicast flow steering support, there is need
to block the attaching of mcg flow for underlay QP, recognize
multicast IB_FLOW_SPEC_IPV4 based on its IP and enable
creating/destroying flow for IB layer.

Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/main.c | 43 ++++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 9f3ba320ce70..d3f1c33a29fb 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -2044,21 +2044,32 @@ static int parse_flow_attr(struct mlx5_core_dev *mdev, u32 *match_c,
  */
 static bool flow_is_multicast_only(struct ib_flow_attr *ib_attr)
 {
-	struct ib_flow_spec_eth *eth_spec;
+	union ib_flow_spec *flow_spec;

 	if (ib_attr->type != IB_FLOW_ATTR_NORMAL ||
-	    ib_attr->size < sizeof(struct ib_flow_attr) +
-	    sizeof(struct ib_flow_spec_eth) ||
 	    ib_attr->num_of_specs < 1)
 		return false;

-	eth_spec = (struct ib_flow_spec_eth *)(ib_attr + 1);
-	if (eth_spec->type != IB_FLOW_SPEC_ETH ||
-	    eth_spec->size != sizeof(*eth_spec))
+	flow_spec = (union ib_flow_spec *)(ib_attr + 1);
+	if (flow_spec->type == IB_FLOW_SPEC_IPV4) {
+		struct ib_flow_spec_ipv4 *ipv4_spec;
+
+		ipv4_spec = (struct ib_flow_spec_ipv4 *)flow_spec;
+		if (ipv4_is_multicast(ipv4_spec->val.dst_ip))
+			return true;
+
 		return false;
+	}
+
+	if (flow_spec->type == IB_FLOW_SPEC_ETH) {
+		struct ib_flow_spec_eth *eth_spec;
+
+		eth_spec = (struct ib_flow_spec_eth *)flow_spec;
+		return is_multicast_ether_addr(eth_spec->mask.dst_mac) &&
+		       is_multicast_ether_addr(eth_spec->val.dst_mac);
+	}

-	return is_multicast_ether_addr(eth_spec->mask.dst_mac) &&
-	       is_multicast_ether_addr(eth_spec->val.dst_mac);
+	return false;
 }

 static bool is_valid_ethertype(struct mlx5_core_dev *mdev,
@@ -2536,8 +2547,14 @@ static struct ib_flow *mlx5_ib_create_flow(struct ib_qp *qp,
 static int mlx5_ib_mcg_attach(struct ib_qp *ibqp, union ib_gid *gid, u16 lid)
 {
 	struct mlx5_ib_dev *dev = to_mdev(ibqp->device);
+	struct mlx5_ib_qp *mqp = to_mqp(ibqp);
 	int err;

+	if (mqp->flags & MLX5_IB_QP_UNDERLAY) {
+		mlx5_ib_dbg(dev, "Attaching a multi cast group to underlay QP is not supported\n");
+		return -EOPNOTSUPP;
+	}
+
 	err = mlx5_core_attach_mcg(dev->mdev, gid, ibqp->qp_num);
 	if (err)
 		mlx5_ib_warn(dev, "failed attaching QPN 0x%x, MGID %pI6\n",
@@ -3692,18 +3709,20 @@ static void *mlx5_ib_add(struct mlx5_core_dev *mdev)
 			(1ull << IB_USER_VERBS_CMD_CLOSE_XRCD);
 	}

+	dev->ib_dev.create_flow	= mlx5_ib_create_flow;
+	dev->ib_dev.destroy_flow = mlx5_ib_destroy_flow;
+	dev->ib_dev.uverbs_ex_cmd_mask |=
+			(1ull << IB_USER_VERBS_EX_CMD_CREATE_FLOW) |
+			(1ull << IB_USER_VERBS_EX_CMD_DESTROY_FLOW);
+
 	if (mlx5_ib_port_link_layer(&dev->ib_dev, 1) ==
 	    IB_LINK_LAYER_ETHERNET) {
-		dev->ib_dev.create_flow	= mlx5_ib_create_flow;
-		dev->ib_dev.destroy_flow = mlx5_ib_destroy_flow;
 		dev->ib_dev.create_wq	 = mlx5_ib_create_wq;
 		dev->ib_dev.modify_wq	 = mlx5_ib_modify_wq;
 		dev->ib_dev.destroy_wq	 = mlx5_ib_destroy_wq;
 		dev->ib_dev.create_rwq_ind_table = mlx5_ib_create_rwq_ind_table;
 		dev->ib_dev.destroy_rwq_ind_table = mlx5_ib_destroy_rwq_ind_table;
 		dev->ib_dev.uverbs_ex_cmd_mask |=
-			(1ull << IB_USER_VERBS_EX_CMD_CREATE_FLOW) |
-			(1ull << IB_USER_VERBS_EX_CMD_DESTROY_FLOW) |
 			(1ull << IB_USER_VERBS_EX_CMD_CREATE_WQ) |
 			(1ull << IB_USER_VERBS_EX_CMD_MODIFY_WQ) |
 			(1ull << IB_USER_VERBS_EX_CMD_DESTROY_WQ) |
--
2.12.2

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

[PATCH rdma-next 5/6] net/mlx5: Report enhanced capabilities for IPoIB

[Leon Romanovsky]

From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Report 'ipoib_enhanced_offloads' capabilities from
the core layer, it will be used in the next patch from this series.

Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/fw.c | 6 ++++++
 include/linux/mlx5/device.h                  | 6 +++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw.c b/drivers/net/ethernet/mellanox/mlx5/core/fw.c
index 1bc14d0fded8..cf468be1479c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fw.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw.c
@@ -119,6 +119,12 @@ int mlx5_query_hca_caps(struct mlx5_core_dev *dev)
 			return err;
 	}

+	if (MLX5_CAP_GEN(dev, ipoib_enhanced_offloads)) {
+		err = mlx5_core_get_caps(dev, MLX5_CAP_IPOIB_ENHANCED_OFFLOADS);
+		if (err)
+			return err;
+	}
+
 	if (MLX5_CAP_GEN(dev, pg)) {
 		err = mlx5_core_get_caps(dev, MLX5_CAP_ODP);
 		if (err)
diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
index dd9a263ed368..1256501959b5 100644
--- a/include/linux/mlx5/device.h
+++ b/include/linux/mlx5/device.h
@@ -960,7 +960,7 @@ enum mlx5_cap_type {
 	MLX5_CAP_ATOMIC,
 	MLX5_CAP_ROCE,
 	MLX5_CAP_IPOIB_OFFLOADS,
-	MLX5_CAP_EOIB_OFFLOADS,
+	MLX5_CAP_IPOIB_ENHANCED_OFFLOADS,
 	MLX5_CAP_FLOW_TABLE,
 	MLX5_CAP_ESWITCH_FLOW_TABLE,
 	MLX5_CAP_ESWITCH,
@@ -1002,6 +1002,10 @@ enum mlx5_mcam_feature_groups {
 	MLX5_GET(per_protocol_networking_offload_caps,\
 		 mdev->caps.hca_max[MLX5_CAP_ETHERNET_OFFLOADS], cap)

+#define MLX5_CAP_IPOIB_ENHANCED(mdev, cap) \
+	MLX5_GET(per_protocol_networking_offload_caps,\
+		 mdev->caps.hca_cur[MLX5_CAP_IPOIB_ENHANCED_OFFLOADS], cap)
+
 #define MLX5_CAP_ROCE(mdev, cap) \
 	MLX5_GET(roce_cap, mdev->caps.hca_cur[MLX5_CAP_ROCE], cap)

--
2.12.2

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

[PATCH rdma-next 6/6] IB/mlx5: Report RX checksum capabilities for IPoIB

[Leon Romanovsky]

From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Report RX checksum capabilities when 'ipoib_enhanced_offloads'
capabilities are set and support checksum.

Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index d3f1c33a29fb..d6ccc00a306b 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -693,6 +693,10 @@ static int mlx5_ib_query_device(struct ib_device *ibdev,
 		props->device_cap_flags |= IB_DEVICE_UD_TSO;
 	}

+	if (MLX5_CAP_GEN(mdev, ipoib_enhanced_offloads) &&
+	    MLX5_CAP_IPOIB_ENHANCED(mdev, csum_cap))
+		props->device_cap_flags |= IB_DEVICE_UD_IP_CSUM;
+
 	if (MLX5_CAP_GEN(dev->mdev, eth_net_offloads) &&
 	    MLX5_CAP_ETH(dev->mdev, scatter_fcs)) {
 		/* Legacy bit to support old userspace libraries */
--
2.12.2

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

Re: [PATCH rdma-next 2/6] IB/uverbs: Enable QP creation which is associated to underlay QP

[Jiri Pirko]

Tue, May 30, 2017 at 09:15:58AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
>From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
>Enable QP creation which is associated to underlay QP. This comes as a
>pre-patch for downstream patches in this series to enable flow steering
>from user space application on the underlay IPoIB QP.
>
>Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
>Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>---

[...]


>diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
>index 477d629f539d..422b20456975 100644
>--- a/include/uapi/rdma/ib_user_verbs.h
>+++ b/include/uapi/rdma/ib_user_verbs.h
>@@ -578,7 +578,7 @@ struct ib_uverbs_ex_create_qp {
> 	__u32 comp_mask;
> 	__u32 create_flags;
> 	__u32 rwq_ind_tbl_handle;
>-	__u32  reserved1;
>+	__u32  associated_qpn;

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

Re: [PATCH rdma-next 2/6] IB/uverbs: Enable QP creation which is associated to underlay QP

[Leon Romanovsky]

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

On Tue, May 30, 2017 at 09:33:52AM +0200, Jiri Pirko wrote:
> Tue, May 30, 2017 at 09:15:58AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
> >From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> >Enable QP creation which is associated to underlay QP. This comes as a
> >pre-patch for downstream patches in this series to enable flow steering
> >from user space application on the underlay IPoIB QP.
> >
> >Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
> >Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >---
>
> [...]
>
>
> >diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
> >index 477d629f539d..422b20456975 100644
> >--- a/include/uapi/rdma/ib_user_verbs.h
> >+++ b/include/uapi/rdma/ib_user_verbs.h
> >@@ -578,7 +578,7 @@ struct ib_uverbs_ex_create_qp {
> > 	__u32 comp_mask;
> > 	__u32 create_flags;
> > 	__u32 rwq_ind_tbl_handle;
> >-	__u32  reserved1;
> >+	__u32  associated_qpn;
>
> This breaks uapi...

Not really, we are using comp_mask scheme to signal changes/predecessor
of these structs. Users of this file and structs check comp_mask and
access new fields only if they are exposed there.

In this case, there are no users of reserved1 and hence it can be
replace to the new field with corresponding comp_mask flag (IB_QP_CREATE_ASSOC_QPN)
without worry of breakage.

Thanks

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

Re: [PATCH rdma-next 2/6] IB/uverbs: Enable QP creation which is associated to underlay QP

[Leon Romanovsky]

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

On Tue, May 30, 2017 at 10:58:45AM +0300, Leon Romanovsky wrote:
> On Tue, May 30, 2017 at 09:33:52AM +0200, Jiri Pirko wrote:
> > Tue, May 30, 2017 at 09:15:58AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
> > >From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > >
> > >Enable QP creation which is associated to underlay QP. This comes as a
> > >pre-patch for downstream patches in this series to enable flow steering
> > >from user space application on the underlay IPoIB QP.
> > >
> > >Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > >Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
> > >Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > >---
> >
> > [...]
> >
> >
> > >diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
> > >index 477d629f539d..422b20456975 100644
> > >--- a/include/uapi/rdma/ib_user_verbs.h
> > >+++ b/include/uapi/rdma/ib_user_verbs.h
> > >@@ -578,7 +578,7 @@ struct ib_uverbs_ex_create_qp {
> > > 	__u32 comp_mask;
> > > 	__u32 create_flags;
> > > 	__u32 rwq_ind_tbl_handle;
> > >-	__u32  reserved1;
> > >+	__u32  associated_qpn;
> >
> > This breaks uapi...
>
> Not really, we are using comp_mask scheme to signal changes/predecessor

s/predecessor/enhancement

> of these structs. Users of this file and structs check comp_mask and
> access new fields only if they are exposed there.
>
> In this case, there are no users of reserved1 and hence it can be
> replace to the new field with corresponding comp_mask flag (IB_QP_CREATE_ASSOC_QPN)
> without worry of breakage.
>
> Thanks



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

Re: [PATCH rdma-next 2/6] IB/uverbs: Enable QP creation which is associated to underlay QP

[Jiri Pirko]

Tue, May 30, 2017 at 09:58:45AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
>On Tue, May 30, 2017 at 09:33:52AM +0200, Jiri Pirko wrote:
>> Tue, May 30, 2017 at 09:15:58AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
>> >From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> >
>> >Enable QP creation which is associated to underlay QP. This comes as a
>> >pre-patch for downstream patches in this series to enable flow steering
>> >from user space application on the underlay IPoIB QP.
>> >
>> >Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> >Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
>> >Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> >---
>>
>> [...]
>>
>>
>> >diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
>> >index 477d629f539d..422b20456975 100644
>> >--- a/include/uapi/rdma/ib_user_verbs.h
>> >+++ b/include/uapi/rdma/ib_user_verbs.h
>> >@@ -578,7 +578,7 @@ struct ib_uverbs_ex_create_qp {
>> > 	__u32 comp_mask;
>> > 	__u32 create_flags;
>> > 	__u32 rwq_ind_tbl_handle;
>> >-	__u32  reserved1;
>> >+	__u32  associated_qpn;
>>
>> This breaks uapi...
>
>Not really, we are using comp_mask scheme to signal changes/predecessor
>of these structs. Users of this file and structs check comp_mask and
>access new fields only if they are exposed there.
>
>In this case, there are no users of reserved1 and hence it can be
>replace to the new field with corresponding comp_mask flag (IB_QP_CREATE_ASSOC_QPN)
>without worry of breakage.

I, as an app could include ib_user_verbs.h and do:

struct ib_uverbs_ex_create_qp something;

something.reserved1 = 0;

Than I won't compile, would I?

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

Re: [PATCH rdma-next 1/6] IB/core: Enable QP creation which is associated to underlay QP

[Jason Gunthorpe]

On Tue, May 30, 2017 at 10:15:57AM +0300, Leon Romanovsky wrote:
> From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Enable QP creation which is associated to underlay QP.
> The created QP will manage the scattering objects as of RQ and SQ, while
> the transport attributes will be managed by its underlay QP
> 
> This comes as a pre-patch for downstream patches in this series to
> enable flow steering on the underlay IPoIB QP.

I think you need to define the semantics here much more precisely.

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

Re: [PATCH rdma-next 2/6] IB/uverbs: Enable QP creation which is associated to underlay QP

[Leon Romanovsky]

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

On Tue, May 30, 2017 at 10:15:14AM +0200, Jiri Pirko wrote:
> Tue, May 30, 2017 at 09:58:45AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
> >On Tue, May 30, 2017 at 09:33:52AM +0200, Jiri Pirko wrote:
> >> Tue, May 30, 2017 at 09:15:58AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
> >> >From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >> >
> >> >Enable QP creation which is associated to underlay QP. This comes as a
> >> >pre-patch for downstream patches in this series to enable flow steering
> >> >from user space application on the underlay IPoIB QP.
> >> >
> >> >Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >> >Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
> >> >Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >> >---
> >>
> >> [...]
> >>
> >>
> >> >diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
> >> >index 477d629f539d..422b20456975 100644
> >> >--- a/include/uapi/rdma/ib_user_verbs.h
> >> >+++ b/include/uapi/rdma/ib_user_verbs.h
> >> >@@ -578,7 +578,7 @@ struct ib_uverbs_ex_create_qp {
> >> > 	__u32 comp_mask;
> >> > 	__u32 create_flags;
> >> > 	__u32 rwq_ind_tbl_handle;
> >> >-	__u32  reserved1;
> >> >+	__u32  associated_qpn;
> >>
> >> This breaks uapi...
> >
> >Not really, we are using comp_mask scheme to signal changes/predecessor
> >of these structs. Users of this file and structs check comp_mask and
> >access new fields only if they are exposed there.
> >
> >In this case, there are no users of reserved1 and hence it can be
> >replace to the new field with corresponding comp_mask flag (IB_QP_CREATE_ASSOC_QPN)
> >without worry of breakage.
>
> I, as an app could include ib_user_verbs.h and do:
>
> struct ib_uverbs_ex_create_qp something;
>
> something.reserved1 = 0;
>
> Than I won't compile, would I?

You are right, the good thing that in IB world, there is no direct usage
of these files and they are needed for rdma-core (libibverbs) library.

Thanks

>

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

Re: [PATCH rdma-next 2/6] IB/uverbs: Enable QP creation which is associated to underlay QP

[Jiri Pirko]

Tue, May 30, 2017 at 07:22:59PM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
>On Tue, May 30, 2017 at 10:15:14AM +0200, Jiri Pirko wrote:
>> Tue, May 30, 2017 at 09:58:45AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
>> >On Tue, May 30, 2017 at 09:33:52AM +0200, Jiri Pirko wrote:
>> >> Tue, May 30, 2017 at 09:15:58AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
>> >> >From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> >> >
>> >> >Enable QP creation which is associated to underlay QP. This comes as a
>> >> >pre-patch for downstream patches in this series to enable flow steering
>> >> >from user space application on the underlay IPoIB QP.
>> >> >
>> >> >Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> >> >Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
>> >> >Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> >> >---
>> >>
>> >> [...]
>> >>
>> >>
>> >> >diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
>> >> >index 477d629f539d..422b20456975 100644
>> >> >--- a/include/uapi/rdma/ib_user_verbs.h
>> >> >+++ b/include/uapi/rdma/ib_user_verbs.h
>> >> >@@ -578,7 +578,7 @@ struct ib_uverbs_ex_create_qp {
>> >> > 	__u32 comp_mask;
>> >> > 	__u32 create_flags;
>> >> > 	__u32 rwq_ind_tbl_handle;
>> >> >-	__u32  reserved1;
>> >> >+	__u32  associated_qpn;
>> >>
>> >> This breaks uapi...
>> >
>> >Not really, we are using comp_mask scheme to signal changes/predecessor
>> >of these structs. Users of this file and structs check comp_mask and
>> >access new fields only if they are exposed there.
>> >
>> >In this case, there are no users of reserved1 and hence it can be
>> >replace to the new field with corresponding comp_mask flag (IB_QP_CREATE_ASSOC_QPN)
>> >without worry of breakage.
>>
>> I, as an app could include ib_user_verbs.h and do:
>>
>> struct ib_uverbs_ex_create_qp something;
>>
>> something.reserved1 = 0;
>>
>> Than I won't compile, would I?
>
>You are right, the good thing that in IB world, there is no direct usage
>of these files and they are needed for rdma-core (libibverbs) library.

Unfortunatelly, that does not make any difference. UAPI is UAPI...


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

Re: [PATCH rdma-next 2/6] IB/uverbs: Enable QP creation which is associated to underlay QP

[Leon Romanovsky]

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

On Tue, May 30, 2017 at 08:03:58PM +0200, Jiri Pirko wrote:
> Tue, May 30, 2017 at 07:22:59PM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
> >On Tue, May 30, 2017 at 10:15:14AM +0200, Jiri Pirko wrote:
> >> Tue, May 30, 2017 at 09:58:45AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
> >> >On Tue, May 30, 2017 at 09:33:52AM +0200, Jiri Pirko wrote:
> >> >> Tue, May 30, 2017 at 09:15:58AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
> >> >> >From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >> >> >
> >> >> >Enable QP creation which is associated to underlay QP. This comes as a
> >> >> >pre-patch for downstream patches in this series to enable flow steering
> >> >> >from user space application on the underlay IPoIB QP.
> >> >> >
> >> >> >Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >> >> >Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
> >> >> >Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >> >> >---
> >> >>
> >> >> [...]
> >> >>
> >> >>
> >> >> >diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
> >> >> >index 477d629f539d..422b20456975 100644
> >> >> >--- a/include/uapi/rdma/ib_user_verbs.h
> >> >> >+++ b/include/uapi/rdma/ib_user_verbs.h
> >> >> >@@ -578,7 +578,7 @@ struct ib_uverbs_ex_create_qp {
> >> >> > 	__u32 comp_mask;
> >> >> > 	__u32 create_flags;
> >> >> > 	__u32 rwq_ind_tbl_handle;
> >> >> >-	__u32  reserved1;
> >> >> >+	__u32  associated_qpn;
> >> >>
> >> >> This breaks uapi...
> >> >
> >> >Not really, we are using comp_mask scheme to signal changes/predecessor
> >> >of these structs. Users of this file and structs check comp_mask and
> >> >access new fields only if they are exposed there.
> >> >
> >> >In this case, there are no users of reserved1 and hence it can be
> >> >replace to the new field with corresponding comp_mask flag (IB_QP_CREATE_ASSOC_QPN)
> >> >without worry of breakage.
> >>
> >> I, as an app could include ib_user_verbs.h and do:
> >>
> >> struct ib_uverbs_ex_create_qp something;
> >>
> >> something.reserved1 = 0;
> >>
> >> Than I won't compile, would I?
> >
> >You are right, the good thing that in IB world, there is no direct usage
> >of these files and they are needed for rdma-core (libibverbs) library.
>
> Unfortunatelly, that does not make any difference. UAPI is UAPI...

Yes, it makes. Changes in UAPI are allowed as long they don't break user
space applications. In this case, there are no applications which uses
this reserved fields directly.

Just as an examples of UAPI changes which can break user-space
applications, but don't:

commit 26202873bb51fafdaa51be3e8de7aab9beb49f70
Author: Hans-Christian Noren Egtvedt <egtvedt-BrfabpQBY5qlHtIdYg32fQ@public.gmane.org>
Date:   Sun Feb 26 12:56:39 2017 +0100

    avr32: remove support for AVR32 architecture

commit 1f4407e2548827e3e6e7b943640a2da90c611306
Author: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Date:   Fri Apr 21 15:59:52 2017 -0400

    net: Remove NET_CORE_BUDGET_USECS from sysctl binary interface.

commit 2611dc1939569718c65ffd59c8fb9ba7474d026c
Author: Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
Date:   Sat Apr 8 14:34:51 2017 -0400

    Remove compat_sys_getdents64()

commit bf74b20d00b13919db7ae5d1015636e76f56f6ae
Author: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Date:   Sun Apr 9 14:45:21 2017 -0700

    Revert "rtnl: Add support for netdev event to link messages"

.....

>
>

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

Re: [PATCH rdma-next 2/6] IB/uverbs: Enable QP creation which is associated to underlay QP

[Jiri Pirko]

Wed, May 31, 2017 at 06:20:31AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
>On Tue, May 30, 2017 at 08:03:58PM +0200, Jiri Pirko wrote:
>> Tue, May 30, 2017 at 07:22:59PM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
>> >On Tue, May 30, 2017 at 10:15:14AM +0200, Jiri Pirko wrote:
>> >> Tue, May 30, 2017 at 09:58:45AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
>> >> >On Tue, May 30, 2017 at 09:33:52AM +0200, Jiri Pirko wrote:
>> >> >> Tue, May 30, 2017 at 09:15:58AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
>> >> >> >From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> >> >> >
>> >> >> >Enable QP creation which is associated to underlay QP. This comes as a
>> >> >> >pre-patch for downstream patches in this series to enable flow steering
>> >> >> >from user space application on the underlay IPoIB QP.
>> >> >> >
>> >> >> >Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> >> >> >Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
>> >> >> >Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> >> >> >---
>> >> >>
>> >> >> [...]
>> >> >>
>> >> >>
>> >> >> >diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
>> >> >> >index 477d629f539d..422b20456975 100644
>> >> >> >--- a/include/uapi/rdma/ib_user_verbs.h
>> >> >> >+++ b/include/uapi/rdma/ib_user_verbs.h
>> >> >> >@@ -578,7 +578,7 @@ struct ib_uverbs_ex_create_qp {
>> >> >> > 	__u32 comp_mask;
>> >> >> > 	__u32 create_flags;
>> >> >> > 	__u32 rwq_ind_tbl_handle;
>> >> >> >-	__u32  reserved1;
>> >> >> >+	__u32  associated_qpn;
>> >> >>
>> >> >> This breaks uapi...
>> >> >
>> >> >Not really, we are using comp_mask scheme to signal changes/predecessor
>> >> >of these structs. Users of this file and structs check comp_mask and
>> >> >access new fields only if they are exposed there.
>> >> >
>> >> >In this case, there are no users of reserved1 and hence it can be
>> >> >replace to the new field with corresponding comp_mask flag (IB_QP_CREATE_ASSOC_QPN)
>> >> >without worry of breakage.
>> >>
>> >> I, as an app could include ib_user_verbs.h and do:
>> >>
>> >> struct ib_uverbs_ex_create_qp something;
>> >>
>> >> something.reserved1 = 0;
>> >>
>> >> Than I won't compile, would I?
>> >
>> >You are right, the good thing that in IB world, there is no direct usage
>> >of these files and they are needed for rdma-core (libibverbs) library.
>>
>> Unfortunatelly, that does not make any difference. UAPI is UAPI...
>
>Yes, it makes. Changes in UAPI are allowed as long they don't break user
>space applications. In this case, there are no applications which uses
>this reserved fields directly.

How do you know? I might have one in my drawer :) 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 2/6] IB/uverbs: Enable QP creation which is associated to underlay QP

[Leon Romanovsky]

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

On Wed, May 31, 2017 at 07:37:11AM +0200, Jiri Pirko wrote:
> Wed, May 31, 2017 at 06:20:31AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
> >On Tue, May 30, 2017 at 08:03:58PM +0200, Jiri Pirko wrote:
> >> Tue, May 30, 2017 at 07:22:59PM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
> >> >On Tue, May 30, 2017 at 10:15:14AM +0200, Jiri Pirko wrote:
> >> >> Tue, May 30, 2017 at 09:58:45AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
> >> >> >On Tue, May 30, 2017 at 09:33:52AM +0200, Jiri Pirko wrote:
> >> >> >> Tue, May 30, 2017 at 09:15:58AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
> >> >> >> >From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >> >> >> >
> >> >> >> >Enable QP creation which is associated to underlay QP. This comes as a
> >> >> >> >pre-patch for downstream patches in this series to enable flow steering
> >> >> >> >from user space application on the underlay IPoIB QP.
> >> >> >> >
> >> >> >> >Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >> >> >> >Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
> >> >> >> >Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >> >> >> >---
> >> >> >>
> >> >> >> [...]
> >> >> >>
> >> >> >>
> >> >> >> >diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
> >> >> >> >index 477d629f539d..422b20456975 100644
> >> >> >> >--- a/include/uapi/rdma/ib_user_verbs.h
> >> >> >> >+++ b/include/uapi/rdma/ib_user_verbs.h
> >> >> >> >@@ -578,7 +578,7 @@ struct ib_uverbs_ex_create_qp {
> >> >> >> > 	__u32 comp_mask;
> >> >> >> > 	__u32 create_flags;
> >> >> >> > 	__u32 rwq_ind_tbl_handle;
> >> >> >> >-	__u32  reserved1;
> >> >> >> >+	__u32  associated_qpn;
> >> >> >>
> >> >> >> This breaks uapi...
> >> >> >
> >> >> >Not really, we are using comp_mask scheme to signal changes/predecessor
> >> >> >of these structs. Users of this file and structs check comp_mask and
> >> >> >access new fields only if they are exposed there.
> >> >> >
> >> >> >In this case, there are no users of reserved1 and hence it can be
> >> >> >replace to the new field with corresponding comp_mask flag (IB_QP_CREATE_ASSOC_QPN)
> >> >> >without worry of breakage.
> >> >>
> >> >> I, as an app could include ib_user_verbs.h and do:
> >> >>
> >> >> struct ib_uverbs_ex_create_qp something;
> >> >>
> >> >> something.reserved1 = 0;
> >> >>
> >> >> Than I won't compile, would I?
> >> >
> >> >You are right, the good thing that in IB world, there is no direct usage
> >> >of these files and they are needed for rdma-core (libibverbs) library.
> >>
> >> Unfortunatelly, that does not make any difference. UAPI is UAPI...
> >
> >Yes, it makes. Changes in UAPI are allowed as long they don't break user
> >space applications. In this case, there are no applications which uses
> >this reserved fields directly.
>
> How do you know? I might have one in my drawer :)

Because, we had this brave man [1] who wanted to implement libibverbs by
himself in GO language and Jason gave a very good explanation [2, 3] why it
is not likely to happen in near future.

[1] http://marc.info/?l=linux-rdma&m=149302796407374&w=2
[2] http://marc.info/?l=linux-rdma&m=149305872017354&w=2
[3] http://marc.info/?l=linux-rdma&m=149314196207011&w=2

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

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

RE: [PATCH rdma-next 2/6] IB/uverbs: Enable QP creation which is associated to underlay QP

[Hefty, Sean]

> > How do you know? I might have one in my drawer :)
> 
> Because, we had this brave man [1] who wanted to implement libibverbs
> by
> himself in GO language and Jason gave a very good explanation [2, 3]
> why it
> is not likely to happen in near future.
> 
> [1] http://marc.info/?l=linux-rdma&m=149302796407374&w=2
> [2] http://marc.info/?l=linux-rdma&m=149305872017354&w=2
> [3] http://marc.info/?l=linux-rdma&m=149314196207011&w=2

I agree with Jiri.  This breaks uapi, and you can't assume that it's not being used, possibly by a forked version of libibverbs.

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

Re: [PATCH rdma-next 1/6] IB/core: Enable QP creation which is associated to underlay QP

[Yishai Hadas]

On 5/30/2017 7:04 PM, Jason Gunthorpe wrote:
> On Tue, May 30, 2017 at 10:15:57AM +0300, Leon Romanovsky wrote:
>> From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>
>> Enable QP creation which is associated to underlay QP.
>> The created QP will manage the scattering objects as of RQ and SQ, while
>> the transport attributes will be managed by its underlay QP
>>
>> This comes as a pre-patch for downstream patches in this series to
>> enable flow steering on the underlay IPoIB QP.
>
> I think you need to define the semantics here much more precisely.

Sure, please find below some clarifications on the semantics and the 
usage, will add as part of V1 to this commit log.

As described above this patch enables QP creation which is associated to 
some underlay QP as of IPoIB QP. It comes to allow in downstream patches 
a user space applications to accelerate send and receive traffic which 
is typically handled by IPoIB ULP.

A successful QP creation will end-up by having an overlay QP (e.g. the 
created one) which is associated to an already existing underlay one
(e.g. IPoIB QP).

The underlay QP is responsible for the transport, the packets on the 
wire will hold its QPN, its pkey will be used, etc.

The overlay QP can steer incoming packets which were target to the 
underlay QP into its own RQ. It can post send WQEs on its own SQ, the 
hardware will use the transport properties of the underlay QP
to format the packets on the wire.
Note: The underlay QP should be in an RTR/RTS state in order to enable 
the overlay QP to work properly.

Each QP should manage its properties and objects via modify, query and 
destroy. Specifically, overlay QP can't affect the underlay QP 
properties and vice versa.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 2/6] IB/uverbs: Enable QP creation which is associated to underlay QP

[Doug Ledford]

On Tue, 2017-05-30 at 09:33 +0200, Jiri Pirko wrote:
> Tue, May 30, 2017 at 09:15:58AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
> > 
> > From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > 
> > Enable QP creation which is associated to underlay QP. This comes
> > as a
> > pre-patch for downstream patches in this series to enable flow
> > steering
> > from user space application on the underlay IPoIB QP.
> > 
> > Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
> > Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > ---
> 
> [...]
> 
> 
> > 
> > diff --git a/include/uapi/rdma/ib_user_verbs.h
> > b/include/uapi/rdma/ib_user_verbs.h
> > index 477d629f539d..422b20456975 100644
> > --- a/include/uapi/rdma/ib_user_verbs.h
> > +++ b/include/uapi/rdma/ib_user_verbs.h
> > @@ -578,7 +578,7 @@ struct ib_uverbs_ex_create_qp {
> > 	__u32 comp_mask;
> > 	__u32 create_flags;
> > 	__u32 rwq_ind_tbl_handle;
> > -	__u32  reserved1;
> > +	__u32  associated_qpn;
> 
> This breaks uapi...

We don't really care about this particular uapi breakage.

First, this is the ib_uverbs_*ex*_create_qp struct, so this is already
part of the "extensible uAPI" we created back when we included XRC
support into the mainline kernel.  A fundamental part of that uAPI was
that we were allowed to extend structs (but not reorganize them) so
that we could add new stuff to the end as we added features, but old
apps would continue to work.  As a result, it was understood that the
uapi structs would continue to grow.

Given that fact, if we put a reserved1 element into a struct to pad it
out to a given size (which we did roughly 1 year ago with
commit c70285f880e88 (IB/uverbs: Extend create QP to get RWQ
indirection table) which made this change to the ex_create_qp struct:

        __u32 create_flags;
+       __u32 rwq_ind_tbl_handle;
+       __u32  reserved1;
 };

that doesn't mean a user of the uapi should directly reference this
element, it means we added 32 bits out of a 64 bit line and we'll add
the other 32 bits later, so don't reference that pad element by name,
use the preferred means of clearing the struct prior to use:
memset(sizeof(struct)) or calloc or equivalent.  Given that only user
space verbs providers that have been updated to support the RWQ
indirection table would possibly ever even have seen this API element,
and given that those should be fairly rare at this point (I doubt that
any exist besides possibly libibverbs or rdma-core, I'm not sure if
this support went into libibverbs before we merged it into rdma-core,
but if it didn't, then only rdma-core would have been updated to know
about these last two struct elements), I'm not going to have Mellanox
put a bunch of ugly cruft in here for probably non-existent, but very
recently updated if it even exists, consumers that don't follow best
practice when accessing/clearing elements of an extensible struct.

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

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

Re: [PATCH rdma-next 2/6] IB/uverbs: Enable QP creation which is associated to underlay QP

[Jiri Pirko]

Sun, Jun 04, 2017 at 03:43:14PM CEST, dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org wrote:
>On Tue, 2017-05-30 at 09:33 +0200, Jiri Pirko wrote:
>> Tue, May 30, 2017 at 09:15:58AM CEST, leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:
>> > 
>> > From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> > 
>> > Enable QP creation which is associated to underlay QP. This comes
>> > as a
>> > pre-patch for downstream patches in this series to enable flow
>> > steering
>> > from user space application on the underlay IPoIB QP.
>> > 
>> > Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> > Reviewed-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
>> > Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> > ---
>> 
>> [...]
>> 
>> 
>> > 
>> > diff --git a/include/uapi/rdma/ib_user_verbs.h
>> > b/include/uapi/rdma/ib_user_verbs.h
>> > index 477d629f539d..422b20456975 100644
>> > --- a/include/uapi/rdma/ib_user_verbs.h
>> > +++ b/include/uapi/rdma/ib_user_verbs.h
>> > @@ -578,7 +578,7 @@ struct ib_uverbs_ex_create_qp {
>> > 	__u32 comp_mask;
>> > 	__u32 create_flags;
>> > 	__u32 rwq_ind_tbl_handle;
>> > -	__u32  reserved1;
>> > +	__u32  associated_qpn;
>> 
>> This breaks uapi...
>
>We don't really care about this particular uapi breakage.
>
>First, this is the ib_uverbs_*ex*_create_qp struct, so this is already
>part of the "extensible uAPI" we created back when we included XRC
>support into the mainline kernel.  A fundamental part of that uAPI was
>that we were allowed to extend structs (but not reorganize them) so
>that we could add new stuff to the end as we added features, but old
>apps would continue to work.  As a result, it was understood that the
>uapi structs would continue to grow.
>
>Given that fact, if we put a reserved1 element into a struct to pad it
>out to a given size (which we did roughly 1 year ago with
>commit c70285f880e88 (IB/uverbs: Extend create QP to get RWQ
>indirection table) which made this change to the ex_create_qp struct:
>
>        __u32 create_flags;
>+       __u32 rwq_ind_tbl_handle;
>+       __u32  reserved1;
> };
>
>that doesn't mean a user of the uapi should directly reference this
>element, it means we added 32 bits out of a 64 bit line and we'll add
>the other 32 bits later, so don't reference that pad element by name,
>use the preferred means of clearing the struct prior to use:
>memset(sizeof(struct)) or calloc or equivalent.  Given that only user
>space verbs providers that have been updated to support the RWQ
>indirection table would possibly ever even have seen this API element,
>and given that those should be fairly rare at this point (I doubt that
>any exist besides possibly libibverbs or rdma-core, I'm not sure if
>this support went into libibverbs before we merged it into rdma-core,
>but if it didn't, then only rdma-core would have been updated to know
>about these last two struct elements), I'm not going to have Mellanox
>put a bunch of ugly cruft in here for probably non-existent, but very
>recently updated if it even exists, consumers that don't follow best
>practice when accessing/clearing elements of an extensible struct.

I understand all the reasons why this breakage should not cause any
harm, don't get me wrong Doug. It's just, it is a breakage, no matter
how pink you paint it :) Either the uapi is guaranteed to be backward
compatible, or not. Nothing in between. Just saying...
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 2/6] IB/uverbs: Enable QP creation which is associated to underlay QP

[Doug Ledford]

On Sun, 2017-06-04 at 15:51 +0200, Jiri Pirko wrote:
> Sun, Jun 04, 2017 at 03:43:14PM CEST, dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org wrote:
> > 
> > > This breaks uapi...
> > 
> > We don't really care about this particular uapi breakage.
> > 
> > First, this is the ib_uverbs_*ex*_create_qp struct, so this is
> > already
> > part of the "extensible uAPI" we created back when we included XRC
> > support into the mainline kernel.  A fundamental part of that uAPI
> > was
> > that we were allowed to extend structs (but not reorganize them) so
> > that we could add new stuff to the end as we added features, but
> > old
> > apps would continue to work.  As a result, it was understood that
> > the
> > uapi structs would continue to grow.
> > 
> > Given that fact, if we put a reserved1 element into a struct to pad
> > it
> > out to a given size (which we did roughly 1 year ago with
> > commit c70285f880e88 (IB/uverbs: Extend create QP to get RWQ
> > indirection table) which made this change to the ex_create_qp
> > struct:
> > 
> >         __u32 create_flags;
> > +       __u32 rwq_ind_tbl_handle;
> > +       __u32  reserved1;
> >  };
> > 
> > that doesn't mean a user of the uapi should directly reference this
> > element, it means we added 32 bits out of a 64 bit line and we'll
> > add
> > the other 32 bits later, so don't reference that pad element by
> > name,
> > use the preferred means of clearing the struct prior to use:
> > memset(sizeof(struct)) or calloc or equivalent.  Given that only
> > user
> > space verbs providers that have been updated to support the RWQ
> > indirection table would possibly ever even have seen this API
> > element,
> > and given that those should be fairly rare at this point (I doubt
> > that
> > any exist besides possibly libibverbs or rdma-core, I'm not sure if
> > this support went into libibverbs before we merged it into rdma-
> > core,
> > but if it didn't, then only rdma-core would have been updated to
> > know
> > about these last two struct elements), I'm not going to have
> > Mellanox
> > put a bunch of ugly cruft in here for probably non-existent, but
> > very
> > recently updated if it even exists, consumers that don't follow
> > best
> > practice when accessing/clearing elements of an extensible struct.
> 
> I understand all the reasons why this breakage should not cause any
> harm, don't get me wrong Doug. It's just, it is a breakage, no matter
> how pink you paint it :) Either the uapi is guaranteed to be backward
> compatible, or not. Nothing in between. Just saying...

We can make it explicit then that the uapi requires the use of
memset/calloc on structs to zero them, and people are not allowed to
access any reserved elements.  Under that understanding, this is *not*
a uapi break and it is an acceptable extension.  This was more or less
understood already, just adding a comment to the uapi header is all we
need.

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

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

Re: [PATCH rdma-next 2/6] IB/uverbs: Enable QP creation which is associated to underlay QP

[Jason Gunthorpe]

On Sun, Jun 04, 2017 at 03:51:22PM +0200, Jiri Pirko wrote:

> I understand all the reasons why this breakage should not cause any
> harm, don't get me wrong Doug. It's just, it is a breakage, no matter
> how pink you paint it :) Either the uapi is guaranteed to be backward
> compatible, or not. Nothing in between. Just saying...

The uapi is *binary* compatible, not necessarily source compatible at
the kernel header level, which is something we have been willing to
accept.

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

Re: [PATCH rdma-next 1/6] IB/core: Enable QP creation which is associated to underlay QP

[Jason Gunthorpe]

On Thu, Jun 01, 2017 at 05:42:25PM +0300, Yishai Hadas wrote:
> On 5/30/2017 7:04 PM, Jason Gunthorpe wrote:
> >On Tue, May 30, 2017 at 10:15:57AM +0300, Leon Romanovsky wrote:
> >>From: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >>
> >>Enable QP creation which is associated to underlay QP.
> >>The created QP will manage the scattering objects as of RQ and SQ, while
> >>the transport attributes will be managed by its underlay QP
> >>
> >>This comes as a pre-patch for downstream patches in this series to
> >>enable flow steering on the underlay IPoIB QP.
> >
> >I think you need to define the semantics here much more precisely.
> 
> Sure, please find below some clarifications on the semantics and the usage,
> will add as part of V1 to this commit log.

I feel like this is really convoluted..

Sounds like the goal here is to allow user space to setup a send-only
'QP' with a fixed QPN, and a receive-only 'QP' associated with the
existing flow rule stuff

Why not do this directly? Why do we need this semanticly odd
association with an 'underlay QP' ?

What happens if thi underlay qp is deleted? Or reconfigured?

How does security work? This is all root-only, right?

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

Re: [PATCH rdma-next 1/6] IB/core: Enable QP creation which is associated to underlay QP

[Jason Gunthorpe]

On Tue, Jun 06, 2017 at 06:49:52PM +0300, Alex Rosenbaum wrote:
>    IPoIB is a tunneled protocol. The underlay part, a UD QP, has a set of
>    properties that define its L2-L4 characteristics. In this suggestion we
>    wanted to make sure the overlay will use the same QP so that there are
>    no mismatches between the IB layer properties. An example for such a
>    mistake can be if the overlay sets a PKey which is different than the
>    IPoIB UD QP. A mistake example on the receive flow; when setting
>    multiple ingress steering on using multiple different PKey's into a
>    single user space UD QP.

That seems like a strange thing to worry about, any user of this
already has to closely track all of the IB parameters as they are
required to issue path records.

So this underlay/overlay complexity really doesn't do much for
safety/usability, IMHO - it just introduces more complexity and more
failure modes.

>    Naming it "source_qpn" instead of associated, as you suggest, sound
>    more appropriate.

Simpler, certainly if you completely eliminate the notion of a linkage
to another QP. This is just creating a TX QP with a specified QPN that
can overlap an existing bidir QP's QPN.

This could then also be potentially used for applications that desire a
fixed QPN.

>    As noted in previous email, the overlay QP can work properly only when
>    the underlay QP is not in RTR or RTS. Meaning no egress or ingress
>    traffic will happen once underlay is not in these states. This is

Yuk, a hidden variable that causes undetectable failure is gross.

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

Re: [PATCH rdma-next 1/6] IB/core: Enable QP creation which is associated to underlay QP

[Alex Rosenbaum]

On Tue, Jun 6, 2017 at 7:33 PM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> On Tue, Jun 06, 2017 at 06:49:52PM +0300, Alex Rosenbaum wrote:
>>    IPoIB is a tunneled protocol. The underlay part, a UD QP, has a set of
>>    properties that define its L2-L4 characteristics. In this suggestion we
>>    wanted to make sure the overlay will use the same QP so that there are
>>    no mismatches between the IB layer properties. An example for such a
>>    mistake can be if the overlay sets a PKey which is different than the
>>    IPoIB UD QP. A mistake example on the receive flow; when setting
>>    multiple ingress steering on using multiple different PKey's into a
>>    single user space UD QP.
>
> That seems like a strange thing to worry about, any user of this
> already has to closely track all of the IB parameters as they are
> required to issue path records.
>
> So this underlay/overlay complexity really doesn't do much for
> safety/usability, IMHO - it just introduces more complexity and more
> failure modes.
>
>>    Naming it "source_qpn" instead of associated, as you suggest, sound
>>    more appropriate.
>
> Simpler, certainly if you completely eliminate the notion of a linkage
> to another QP. This is just creating a TX QP with a specified QPN that
> can overlap an existing bidir QP's QPN.
>
> This could then also be potentially used for applications that desire a
> fixed QPN.
>

We reviewed your suggestion in deeper details and agree with your
suggested semantics to allow user to create a QP with user defined
wire QPN, without mandating association of an overlay with underlay
QP. This will allow each vendor to implement this feature according to
his HW capabilities. Thus the new attribute we will add to
ib_create_qp will be 'source_qpn' and matching flag.
Obviously this will require root privileges from user application, and
thus in v2 we will extend the existing CAP_NET_RAW check on creation
of this kind of QP with source QPN, as done in RAW_PACKET QP.

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

Re: [PATCH rdma-next 1/6] IB/core: Enable QP creation which is associated to underlay QP

[Jason Gunthorpe]

On Wed, Jun 07, 2017 at 03:23:35PM +0300, Alex Rosenbaum wrote:
> We reviewed your suggestion in deeper details and agree with your
> suggested semantics to allow user to create a QP with user defined
> wire QPN, without mandating association of an overlay with underlay
> QP.

It would be excellent if this could be allowed for QP1 as well - the
user space SA could be greatly accelerated if it could send QP1
traffic without going through umad.

This would require multiple QP's with the source_qpn=1 set, for the
range of required PKeys.

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