[PATCH rdma-next 0/2] Introduce an IB device's scatter end padding capability

[PATCH rdma-next 0/2] Introduce an IB device's scatter end padding capability

[Leon Romanovsky]

There are PCIe root complex that are able to optimize their
performance when incoming data is multiple full cache lines.

Expose the device capability to pad the ending of incoming packets
(scatter) to full cache line such that the last upstream write
generated by the incoming packet will be a full cache line.

The patches are available in the git repository at:
  git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git tags/rdma-next-2017-10-17-2

	Thanks
---------------------------------------

Noa Osherovich (2):
  IB/core: Add scatter end padding flags for WQ and QP
  IB/mlx5: Add scatter end padding support

 drivers/infiniband/core/uverbs_cmd.c |  3 ++-
 drivers/infiniband/hw/mlx5/main.c    |  3 +++
 drivers/infiniband/hw/mlx5/mlx5_ib.h |  2 ++
 drivers/infiniband/hw/mlx5/qp.c      | 36 +++++++++++++++++++++++++++++++++---
 include/rdma/ib_verbs.h              |  4 ++++
 5 files changed, 44 insertions(+), 4 deletions(-)

-- 
2.14.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/2] IB/core: Add scatter end padding flags for WQ and QP

[Leon Romanovsky]

From: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

There are root complexes that are able to optimize their
performance when incoming data is multiple full cache lines.

Scatter end padding is the device's ability to pad the ending of
incoming packets (scatter) to full cache line such that the last
upstream write generated by an incoming packet will be a full cache
line.

Add a relevant entry to ib_device_cap_flags to report scatter end
padding capability of an RDMA device.

Add the QP and WQ create flags with an entry for scatter end padding:
 * A QP/WQ created with a scatter end padding flag will cause
   HW to pad the last upstream write generated by a packet to cache line.

User should consider several factors before activating this feature:
- In case of high CPU memory load (which may cause PCI back pressure in
  turn), if a large percent of the writes are partial cache line, this
  feature should be checked as an optional solution.
- This feature might reduce performance if most packets are between one
  and two cache lines and PCIe throughput has reached its maximum
  capacity. E.g. 65B packet from the network port will lead to 128B
  write on PCIe, which may cause traffic on PCIe to reach high
  throughput.

Signed-off-by: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/uverbs_cmd.c | 3 ++-
 include/rdma/ib_verbs.h              | 4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index d31e4bc58e9a..ab29a0327831 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1491,7 +1491,8 @@ static int create_qp(struct ib_uverbs_file *file,
 				IB_QP_CREATE_MANAGED_RECV |
 				IB_QP_CREATE_SCATTER_FCS |
 				IB_QP_CREATE_CVLAN_STRIPPING |
-				IB_QP_CREATE_SOURCE_QPN)) {
+				IB_QP_CREATE_SOURCE_QPN |
+				IB_QP_CREATE_SCATTER_END_PADDING)) {
 		ret = -EINVAL;
 		goto err_put;
 	}
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 9810e4568635..4c0a539cd2a2 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -229,6 +229,8 @@ enum ib_device_cap_flags {
 	/* Deprecated. Please use IB_RAW_PACKET_CAP_SCATTER_FCS. */
 	IB_DEVICE_RAW_SCATTER_FCS		= (1ULL << 34),
 	IB_DEVICE_RDMA_NETDEV_OPA_VNIC		= (1ULL << 35),
+	/* The device supports padding incoming writes to cacheline. */
+	IB_DEVICE_SCATTER_END_PADDING		= (1ULL << 36),
 };
 
 enum ib_signature_prot_cap {
@@ -1098,6 +1100,7 @@ enum ib_qp_create_flags {
 	IB_QP_CREATE_SCATTER_FCS		= 1 << 8,
 	IB_QP_CREATE_CVLAN_STRIPPING		= 1 << 9,
 	IB_QP_CREATE_SOURCE_QPN			= 1 << 10,
+	IB_QP_CREATE_SCATTER_END_PADDING	= 1 << 11,
 	/* reserve bits 26-31 for low level drivers' internal use */
 	IB_QP_CREATE_RESERVED_START		= 1 << 26,
 	IB_QP_CREATE_RESERVED_END		= 1 << 31,
@@ -1621,6 +1624,7 @@ enum ib_wq_flags {
 	IB_WQ_FLAGS_CVLAN_STRIPPING	= 1 << 0,
 	IB_WQ_FLAGS_SCATTER_FCS		= 1 << 1,
 	IB_WQ_FLAGS_DELAY_DROP		= 1 << 2,
+	IB_WQ_FLAGS_SCATTER_END_PADDING = 1 << 3,
 };
 
 struct ib_wq_init_attr {
-- 
2.14.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/2] IB/mlx5: Add scatter end padding support

[Leon Romanovsky]

From: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Add the scatter end padding flag to device_cap_flags enum and set it
during mlx5_ib_query_device so it will be reported to user-space.

During WQ/QP creation, set the scatter end padding capability for
WQ/QP if user requested it and HW supports it.

Scatter end padding modification is not supported for now. There's no
such flag for a QP but for a WQ, create and modify use the same flag.
Return an error if scatter end padding flag is set during modify_wq.

Signed-off-by: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/main.c    |  3 +++
 drivers/infiniband/hw/mlx5/mlx5_ib.h |  2 ++
 drivers/infiniband/hw/mlx5/qp.c      | 36 +++++++++++++++++++++++++++++++++---
 3 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 02da3f58f296..f66f693c058b 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -715,6 +715,9 @@ static int mlx5_ib_query_device(struct ib_device *ibdev,
 	if (mlx5_get_flow_namespace(dev->mdev, MLX5_FLOW_NAMESPACE_BYPASS))
 		props->device_cap_flags |= IB_DEVICE_MANAGED_FLOW_STEERING;
 
+	if (MLX5_CAP_GEN(mdev, end_pad))
+		props->device_cap_flags |= IB_DEVICE_SCATTER_END_PADDING;
+
 	props->vendor_part_id	   = mdev->pdev->device;
 	props->hw_ver		   = mdev->pdev->revision;
 
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index e7deaa08535b..b30945c3a1fb 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -228,6 +228,7 @@ struct wr_list {
 
 enum mlx5_ib_rq_flags {
 	MLX5_IB_RQ_CVLAN_STRIPPING	= 1 << 0,
+	MLX5_IB_RQ_SCATTER_END_PADDING	= 1 << 1,
 };
 
 struct mlx5_ib_wq {
@@ -420,6 +421,7 @@ enum mlx5_ib_qp_flags {
 	MLX5_IB_QP_RSS				= 1 << 8,
 	MLX5_IB_QP_CVLAN_STRIPPING		= 1 << 9,
 	MLX5_IB_QP_UNDERLAY			= 1 << 10,
+	MLX5_IB_QP_SCATTER_END_PADDING		= 1 << 11,
 };
 
 struct mlx5_umr_wr {
diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index d209c684d729..511d4fdcaaae 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -1178,8 +1178,8 @@ static int create_raw_packet_qp_rq(struct mlx5_ib_dev *dev,
 
 	wq = MLX5_ADDR_OF(rqc, rqc, wq);
 	MLX5_SET(wq, wq, wq_type, MLX5_WQ_TYPE_CYCLIC);
-	MLX5_SET(wq, wq, end_padding_mode,
-		 MLX5_GET(qpc, qpc, end_padding_mode));
+	if (rq->flags & MLX5_IB_RQ_SCATTER_END_PADDING)
+		MLX5_SET(wq, wq, end_padding_mode, MLX5_WQ_END_PAD_MODE_ALIGN);
 	MLX5_SET(wq, wq, page_offset, MLX5_GET(qpc, qpc, page_offset));
 	MLX5_SET(wq, wq, pd, MLX5_GET(qpc, qpc, pd));
 	MLX5_SET64(wq, wq, dbr_addr, MLX5_GET64(qpc, qpc, dbr_addr));
@@ -1266,6 +1266,8 @@ static int create_raw_packet_qp(struct mlx5_ib_dev *dev, struct mlx5_ib_qp *qp,
 
 		if (qp->flags & MLX5_IB_QP_CVLAN_STRIPPING)
 			rq->flags |= MLX5_IB_RQ_CVLAN_STRIPPING;
+		if (qp->flags & MLX5_IB_QP_SCATTER_END_PADDING)
+			rq->flags |= MLX5_IB_RQ_SCATTER_END_PADDING;
 		err = create_raw_packet_qp_rq(dev, rq, in);
 		if (err)
 			goto err_destroy_sq;
@@ -1781,6 +1783,19 @@ static int create_qp_common(struct mlx5_ib_dev *dev, struct ib_pd *pd,
 		qp->flags |= MLX5_IB_QP_LSO;
 	}
 
+	if (init_attr->create_flags & IB_QP_CREATE_SCATTER_END_PADDING) {
+		if (!MLX5_CAP_GEN(dev->mdev, end_pad)) {
+			mlx5_ib_dbg(dev, "scatter end padding is not supported\n");
+			err = -EOPNOTSUPP;
+			goto err;
+		} else if (init_attr->qp_type != IB_QPT_RAW_PACKET) {
+			MLX5_SET(qpc, qpc, end_padding_mode,
+				 MLX5_WQ_END_PAD_MODE_ALIGN);
+		} else {
+			qp->flags |= MLX5_IB_QP_SCATTER_END_PADDING;
+		}
+	}
+
 	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;
@@ -1825,6 +1840,7 @@ static int create_qp_common(struct mlx5_ib_dev *dev, struct ib_pd *pd,
 	else if (qp->create_type == MLX5_QP_KERNEL)
 		destroy_qp_kernel(dev, qp);
 
+err:
 	kvfree(in);
 	return err;
 }
@@ -4709,7 +4725,15 @@ static int  create_rq(struct mlx5_ib_rwq *rwq, struct ib_pd *pd,
 	MLX5_SET(wq, wq, wq_type,
 		 rwq->create_flags & MLX5_IB_WQ_FLAGS_STRIDING_RQ ?
 		 MLX5_WQ_TYPE_CYCLIC_STRIDING_RQ : MLX5_WQ_TYPE_CYCLIC);
-	MLX5_SET(wq, wq, end_padding_mode, MLX5_WQ_END_PAD_MODE_ALIGN);
+	if (init_attr->create_flags & IB_WQ_FLAGS_SCATTER_END_PADDING) {
+		if (!MLX5_CAP_GEN(dev->mdev, end_pad)) {
+			mlx5_ib_dbg(dev, "Scatter end padding is not supported\n");
+			err = -EOPNOTSUPP;
+			goto out;
+		} else {
+			MLX5_SET(wq, wq, end_padding_mode, MLX5_WQ_END_PAD_MODE_ALIGN);
+		}
+	}
 	MLX5_SET(wq, wq, log_wq_stride, rwq->log_rq_stride);
 	if (rwq->create_flags & MLX5_IB_WQ_FLAGS_STRIDING_RQ) {
 		MLX5_SET(wq, wq, two_byte_shift_en, rwq->two_byte_shift_en);
@@ -5089,6 +5113,12 @@ int mlx5_ib_modify_wq(struct ib_wq *wq, struct ib_wq_attr *wq_attr,
 			MLX5_SET(rqc, rqc, vsd,
 				 (wq_attr->flags & IB_WQ_FLAGS_CVLAN_STRIPPING) ? 0 : 1);
 		}
+
+		if (wq_attr->flags_mask & IB_WQ_FLAGS_SCATTER_END_PADDING) {
+			mlx5_ib_dbg(dev, "Modifying scatter end padding is not supported\n");
+			err = -EOPNOTSUPP;
+			goto out;
+		}
 	}
 
 	if (curr_wq_state == IB_WQS_RESET && wq_state == IB_WQS_RDY) {
-- 
2.14.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 0/2] Introduce an IB device's scatter end padding capability

[Christoph Hellwig]

On Tue, Oct 17, 2017 at 06:18:55PM +0300, Leon Romanovsky wrote:
> There are PCIe root complex that are able to optimize their
> performance when incoming data is multiple full cache lines.
> 
> Expose the device capability to pad the ending of incoming packets
> (scatter) to full cache line such that the last upstream write
> generated by the incoming packet will be a full cache line.

Any why would this be a user controller option?  Isn't this something
that the kernel should set up automatically instead of needing arcane
tribal knowledge in the ULPs and applications?
--
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/2] IB/mlx5: Add scatter end padding support

[Amrani, Ram]

Hi Leon, Noa,

> @@ -1781,6 +1783,19 @@ static int create_qp_common(struct mlx5_ib_dev *dev, struct ib_pd *pd,
>  		qp->flags |= MLX5_IB_QP_LSO;
>  	}
> 
> +	if (init_attr->create_flags & IB_QP_CREATE_SCATTER_END_PADDING) {
> +		if (!MLX5_CAP_GEN(dev->mdev, end_pad)) {
> +			mlx5_ib_dbg(dev, "scatter end padding is not supported\n");
> +			err = -EOPNOTSUPP;
> +			goto err;
> +		} else if (init_attr->qp_type != IB_QPT_RAW_PACKET) {
> +			MLX5_SET(qpc, qpc, end_padding_mode,
> +				 MLX5_WQ_END_PAD_MODE_ALIGN);
> +		} else {
> +			qp->flags |= MLX5_IB_QP_SCATTER_END_PADDING;
> +		}
> +	}
> +

What's the feature associated with MLX5_WQ_END_PAD_MODE_ALIGN?
It seems it was there before this patch series and bears a similar name.

Thanks,
Ram

--
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 0/2] Introduce an IB device's scatter end padding capability

[Noa Osherovich]

Hi,

On 10/18/2017 10:12 AM, Christoph Hellwig wrote:

> On Tue, Oct 17, 2017 at 06:18:55PM +0300, Leon Romanovsky wrote:
>> There are PCIe root complex that are able to optimize their
>> performance when incoming data is multiple full cache lines.
>>
>> Expose the device capability to pad the ending of incoming packets
>> (scatter) to full cache line such that the last upstream write
>> generated by the incoming packet will be a full cache line.
> Any why would this be a user controller option?  Isn't this something
> that the kernel should set up automatically instead of needing arcane
> tribal knowledge in the ULPs and applications?

User should consider several factors before activating this feature:
- In case of high CPU memory load (which may cause PCI backpressure in
  turn), if a large percent of the writes are partial cache line, this
  feature should be checked as an optional solution.
- This feature might reduce performance if most packets are between
  one and two cache lines and PCIe throughput has reached its maximum
  capacity. E.g. 65B packet from the network port will lead to 128B
  write on PCIe, which may cause trafiic on PCIe to reach high
  throughput.

Userspace series will follow.

Thanks,
Noa

--
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/2] IB/mlx5: Add scatter end padding support

[Noa Osherovich]

Hi Ram,

On 10/18/2017 1:37 PM, Amrani, Ram wrote:

> Hi Leon, Noa,
>
>> @@ -1781,6 +1783,19 @@ static int create_qp_common(struct mlx5_ib_dev *dev, struct ib_pd *pd,
>>  		qp->flags |= MLX5_IB_QP_LSO;
>>  	}
>>
>> +	if (init_attr->create_flags & IB_QP_CREATE_SCATTER_END_PADDING) {
>> +		if (!MLX5_CAP_GEN(dev->mdev, end_pad)) {
>> +			mlx5_ib_dbg(dev, "scatter end padding is not supported\n");
>> +			err = -EOPNOTSUPP;
>> +			goto err;
>> +		} else if (init_attr->qp_type != IB_QPT_RAW_PACKET) {
>> +			MLX5_SET(qpc, qpc, end_padding_mode,
>> +				 MLX5_WQ_END_PAD_MODE_ALIGN);
>> +		} else {
>> +			qp->flags |= MLX5_IB_QP_SCATTER_END_PADDING;
>> +		}
>> +	}
>> +
> What's the feature associated with MLX5_WQ_END_PAD_MODE_ALIGN?
> It seems it was there before this patch series and bears a similar name.

Right, this series exposes this already existing feature to the user.

> Thanks,
> Ram
--
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/2] IB/mlx5: Add scatter end padding support

[Amrani, Ram]

> > What's the feature associated with MLX5_WQ_END_PAD_MODE_ALIGN?
> > It seems it was there before this patch series and bears a similar name.
> 
> Right, this series exposes this already existing feature to the user.

I'm under the impression that they are somewhat different (per QP type??) from this code:

+	if (init_attr->create_flags & IB_QP_CREATE_SCATTER_END_PADDING) {
+		if (!MLX5_CAP_GEN(dev->mdev, end_pad)) {
+			mlx5_ib_dbg(dev, "scatter end padding is not supported\n");
+			err = -EOPNOTSUPP;
+			goto err;
+		} else if (init_attr->qp_type != IB_QPT_RAW_PACKET) {
+			MLX5_SET(qpc, qpc, end_padding_mode,
+				 MLX5_WQ_END_PAD_MODE_ALIGN);
+		} else {
+			qp->flags |= MLX5_IB_QP_SCATTER_END_PADDING;
+		}
+	}
+

But if indeed they are the same then please consider using the same names to increase readability.

Thanks,
Ram

Re: [PATCH rdma-next 2/2] IB/mlx5: Add scatter end padding support

[Noa Osherovich]

Hi Ram,

On 10/19/2017 10:56 AM, Amrani, Ram wrote:

>>> What's the feature associated with MLX5_WQ_END_PAD_MODE_ALIGN?
>>> It seems it was there before this patch series and bears a similar name.
>> Right, this series exposes this already existing feature to the user.
> I'm under the impression that they are somewhat different (per QP type??) from this code:
>
> +	if (init_attr->create_flags & IB_QP_CREATE_SCATTER_END_PADDING) {
> +		if (!MLX5_CAP_GEN(dev->mdev, end_pad)) {
> +			mlx5_ib_dbg(dev, "scatter end padding is not supported\n");
> +			err = -EOPNOTSUPP;
> +			goto err;
> +		} else if (init_attr->qp_type != IB_QPT_RAW_PACKET) {
> +			MLX5_SET(qpc, qpc, end_padding_mode,
> +				 MLX5_WQ_END_PAD_MODE_ALIGN);
> +		} else {
> +			qp->flags |= MLX5_IB_QP_SCATTER_END_PADDING;
> +		}
> +	}
> +
>
> But if indeed they are the same then please consider using the same names to increase readability.

Raw Packet QP is implemented by a WQ, so its flow is different from that of
other QP types.
Eventually, its receive WQ context's field will be set to MLX5_WQ_END_PAD_MODE_ALIGN
as well.

> Thanks,
> Ram
--
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/2] IB/mlx5: Add scatter end padding support

[Amrani, Ram]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 349 bytes --]

> Raw Packet QP is implemented by a WQ, so its flow is different from that of
> other QP types.
> Eventually, its receive WQ context's field will be set to MLX5_WQ_END_PAD_MODE_ALIGN
> as well.

Thanks,
Ram

N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

Re: [PATCH rdma-next 1/2] IB/core: Add scatter end padding flags for WQ and QP

[Doug Ledford]

On Tue, 2017-10-17 at 18:18 +0300, Leon Romanovsky wrote:
> From: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> There are root complexes that are able to optimize their
> performance when incoming data is multiple full cache lines.
> 
> Scatter end padding is the device's ability to pad the ending of
> incoming packets (scatter)

I hate this naming.  I'm sure people inside of Mellanox have gotten
used to it, but this feature really has no bearing on scatter/gather at
all.  This is merely final write padding.  You might have a
scatter/gather list, you might have a single buffer.  Either way, the
PCI root complex couldn't care less about scatter/gather or not, it's
all a byte stream to it.  I would be much happier with a name that
reflected what this really does. 


>  to full cache line such that the last
> upstream write generated by an incoming packet will be a full cache
> line.
> 
> Add a relevant entry to ib_device_cap_flags to report scatter end
> padding capability of an RDMA device.
> 
> Add the QP and WQ create flags with an entry for scatter end padding:
>  * A QP/WQ created with a scatter end padding flag will cause
>    HW to pad the last upstream write generated by a packet to cache
> line.
> 
> User should consider several factors before activating this feature:
> - In case of high CPU memory load (which may cause PCI back pressure
> in
>   turn), if a large percent of the writes are partial cache line,
> this
>   feature should be checked as an optional solution.
> - This feature might reduce performance if most packets are between
> one
>   and two cache lines and PCIe throughput has reached its maximum
>   capacity. E.g. 65B packet from the network port will lead to 128B
>   write on PCIe, which may cause traffic on PCIe to reach high
>   throughput.
> 
> Signed-off-by: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  drivers/infiniband/core/uverbs_cmd.c | 3 ++-
>  include/rdma/ib_verbs.h              | 4 ++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/core/uverbs_cmd.c
> b/drivers/infiniband/core/uverbs_cmd.c
> index d31e4bc58e9a..ab29a0327831 100644
> --- a/drivers/infiniband/core/uverbs_cmd.c
> +++ b/drivers/infiniband/core/uverbs_cmd.c
> @@ -1491,7 +1491,8 @@ static int create_qp(struct ib_uverbs_file
> *file,
>  				IB_QP_CREATE_MANAGED_RECV |
>  				IB_QP_CREATE_SCATTER_FCS |
>  				IB_QP_CREATE_CVLAN_STRIPPING |
> -				IB_QP_CREATE_SOURCE_QPN)) {
> +				IB_QP_CREATE_SOURCE_QPN |
> +				IB_QP_CREATE_SCATTER_END_PADDING)) {

Maybe IB_QP_CREATE_PCI_WRITE_PAD?

>  		ret = -EINVAL;
>  		goto err_put;
>  	}
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 9810e4568635..4c0a539cd2a2 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -229,6 +229,8 @@ enum ib_device_cap_flags {
>  	/* Deprecated. Please use IB_RAW_PACKET_CAP_SCATTER_FCS. */
>  	IB_DEVICE_RAW_SCATTER_FCS		= (1ULL << 34),
>  	IB_DEVICE_RDMA_NETDEV_OPA_VNIC		= (1ULL <<
> 35),
> +	/* The device supports padding incoming writes to cacheline.
> */
/* The device supports padding the final write of a PCI write
transaction to a cacheline boundry so that the PCI root complex can
optimize its memory accesses */
> +	IB_DEVICE_SCATTER_END_PADDING		= (1ULL << 36),
>  };
>  
>  enum ib_signature_prot_cap {
> @@ -1098,6 +1100,7 @@ enum ib_qp_create_flags {
>  	IB_QP_CREATE_SCATTER_FCS		= 1 << 8,
>  	IB_QP_CREATE_CVLAN_STRIPPING		= 1 << 9,
>  	IB_QP_CREATE_SOURCE_QPN			= 1 << 10,
> +	IB_QP_CREATE_SCATTER_END_PADDING	= 1 << 11,
>  	/* reserve bits 26-31 for low level drivers' internal use */
>  	IB_QP_CREATE_RESERVED_START		= 1 << 26,
>  	IB_QP_CREATE_RESERVED_END		= 1 << 31,
> @@ -1621,6 +1624,7 @@ enum ib_wq_flags {
>  	IB_WQ_FLAGS_CVLAN_STRIPPING	= 1 << 0,
>  	IB_WQ_FLAGS_SCATTER_FCS		= 1 << 1,
>  	IB_WQ_FLAGS_DELAY_DROP		= 1 << 2,
> +	IB_WQ_FLAGS_SCATTER_END_PADDING = 1 << 3,
>  };
>  
>  struct ib_wq_init_attr {
-- 
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 1/2] IB/core: Add scatter end padding flags for WQ and QP

[Leon Romanovsky]

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

On Wed, Oct 25, 2017 at 01:58:59PM -0400, Doug Ledford wrote:
> On Tue, 2017-10-17 at 18:18 +0300, Leon Romanovsky wrote:
> > From: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > There are root complexes that are able to optimize their
> > performance when incoming data is multiple full cache lines.
> >
> > Scatter end padding is the device's ability to pad the ending of
> > incoming packets (scatter)
>
> I hate this naming.  I'm sure people inside of Mellanox have gotten
> used to it, but this feature really has no bearing on scatter/gather at
> all.  This is merely final write padding.  You might have a
> scatter/gather list, you might have a single buffer.  Either way, the
> PCI root complex couldn't care less about scatter/gather or not, it's
> all a byte stream to it.  I would be much happier with a name that
> reflected what this really does.

Scatter has more broad meaning than "scatter/gather list". The name
here came from statistical meaning of "scatter", I'll try to come into
something more descriptive.

>
>
> >  to full cache line such that the last
> > upstream write generated by an incoming packet will be a full cache
> > line.
> >
> > Add a relevant entry to ib_device_cap_flags to report scatter end
> > padding capability of an RDMA device.
> >
> > Add the QP and WQ create flags with an entry for scatter end padding:
> >  * A QP/WQ created with a scatter end padding flag will cause
> >    HW to pad the last upstream write generated by a packet to cache
> > line.
> >
> > User should consider several factors before activating this feature:
> > - In case of high CPU memory load (which may cause PCI back pressure
> > in
> >   turn), if a large percent of the writes are partial cache line,
> > this
> >   feature should be checked as an optional solution.
> > - This feature might reduce performance if most packets are between
> > one
> >   and two cache lines and PCIe throughput has reached its maximum
> >   capacity. E.g. 65B packet from the network port will lead to 128B
> >   write on PCIe, which may cause traffic on PCIe to reach high
> >   throughput.
> >
> > Signed-off-by: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Reviewed-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > ---
> >  drivers/infiniband/core/uverbs_cmd.c | 3 ++-
> >  include/rdma/ib_verbs.h              | 4 ++++
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/core/uverbs_cmd.c
> > b/drivers/infiniband/core/uverbs_cmd.c
> > index d31e4bc58e9a..ab29a0327831 100644
> > --- a/drivers/infiniband/core/uverbs_cmd.c
> > +++ b/drivers/infiniband/core/uverbs_cmd.c
> > @@ -1491,7 +1491,8 @@ static int create_qp(struct ib_uverbs_file
> > *file,
> >  				IB_QP_CREATE_MANAGED_RECV |
> >  				IB_QP_CREATE_SCATTER_FCS |
> >  				IB_QP_CREATE_CVLAN_STRIPPING |
> > -				IB_QP_CREATE_SOURCE_QPN)) {
> > +				IB_QP_CREATE_SOURCE_QPN |
> > +				IB_QP_CREATE_SCATTER_END_PADDING)) {
>
> Maybe IB_QP_CREATE_PCI_WRITE_PAD?
>
> >  		ret = -EINVAL;
> >  		goto err_put;
> >  	}
> > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> > index 9810e4568635..4c0a539cd2a2 100644
> > --- a/include/rdma/ib_verbs.h
> > +++ b/include/rdma/ib_verbs.h
> > @@ -229,6 +229,8 @@ enum ib_device_cap_flags {
> >  	/* Deprecated. Please use IB_RAW_PACKET_CAP_SCATTER_FCS. */
> >  	IB_DEVICE_RAW_SCATTER_FCS		= (1ULL << 34),
> >  	IB_DEVICE_RDMA_NETDEV_OPA_VNIC		= (1ULL <<
> > 35),
> > +	/* The device supports padding incoming writes to cacheline.
> > */
> /* The device supports padding the final write of a PCI write
> transaction to a cacheline boundry so that the PCI root complex can
> optimize its memory accesses */
> > +	IB_DEVICE_SCATTER_END_PADDING		= (1ULL << 36),
> >  };
> >
> >  enum ib_signature_prot_cap {
> > @@ -1098,6 +1100,7 @@ enum ib_qp_create_flags {
> >  	IB_QP_CREATE_SCATTER_FCS		= 1 << 8,
> >  	IB_QP_CREATE_CVLAN_STRIPPING		= 1 << 9,
> >  	IB_QP_CREATE_SOURCE_QPN			= 1 << 10,
> > +	IB_QP_CREATE_SCATTER_END_PADDING	= 1 << 11,
> >  	/* reserve bits 26-31 for low level drivers' internal use */
> >  	IB_QP_CREATE_RESERVED_START		= 1 << 26,
> >  	IB_QP_CREATE_RESERVED_END		= 1 << 31,
> > @@ -1621,6 +1624,7 @@ enum ib_wq_flags {
> >  	IB_WQ_FLAGS_CVLAN_STRIPPING	= 1 << 0,
> >  	IB_WQ_FLAGS_SCATTER_FCS		= 1 << 1,
> >  	IB_WQ_FLAGS_DELAY_DROP		= 1 << 2,
> > +	IB_WQ_FLAGS_SCATTER_END_PADDING = 1 << 3,
> >  };
> >
> >  struct ib_wq_init_attr {
> --
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>     GPG KeyID: B826A3330E572FDD
>     Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>

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

Re: [PATCH rdma-next 1/2] IB/core: Add scatter end padding flags for WQ and QP

[Jason Gunthorpe]

On Wed, Oct 25, 2017 at 10:03:52PM +0300, Leon Romanovsky wrote:
> On Wed, Oct 25, 2017 at 01:58:59PM -0400, Doug Ledford wrote:
> > On Tue, 2017-10-17 at 18:18 +0300, Leon Romanovsky wrote:
> > > From: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > >
> > > There are root complexes that are able to optimize their
> > > performance when incoming data is multiple full cache lines.
> > >
> > > Scatter end padding is the device's ability to pad the ending of
> > > incoming packets (scatter)
> >
> > I hate this naming.  I'm sure people inside of Mellanox have gotten
> > used to it, but this feature really has no bearing on scatter/gather at
> > all.  This is merely final write padding.  You might have a
> > scatter/gather list, you might have a single buffer.  Either way, the
> > PCI root complex couldn't care less about scatter/gather or not, it's
> > all a byte stream to it.  I would be much happier with a name that
> > reflected what this really does.
> 
> Scatter has more broad meaning than "scatter/gather list". The name
> here came from statistical meaning of "scatter", I'll try to come into
> something more descriptive.

I don't like it either.. We usually call this 'alignment padding' in CS.

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/2] IB/core: Add scatter end padding flags for WQ and QP

[Leon Romanovsky]

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

On Wed, Oct 25, 2017 at 01:53:11PM -0600, Jason Gunthorpe wrote:
> On Wed, Oct 25, 2017 at 10:03:52PM +0300, Leon Romanovsky wrote:
> > On Wed, Oct 25, 2017 at 01:58:59PM -0400, Doug Ledford wrote:
> > > On Tue, 2017-10-17 at 18:18 +0300, Leon Romanovsky wrote:
> > > > From: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > >
> > > > There are root complexes that are able to optimize their
> > > > performance when incoming data is multiple full cache lines.
> > > >
> > > > Scatter end padding is the device's ability to pad the ending of
> > > > incoming packets (scatter)
> > >
> > > I hate this naming.  I'm sure people inside of Mellanox have gotten
> > > used to it, but this feature really has no bearing on scatter/gather at
> > > all.  This is merely final write padding.  You might have a
> > > scatter/gather list, you might have a single buffer.  Either way, the
> > > PCI root complex couldn't care less about scatter/gather or not, it's
> > > all a byte stream to it.  I would be much happier with a name that
> > > reflected what this really does.
> >
> > Scatter has more broad meaning than "scatter/gather list". The name
> > here came from statistical meaning of "scatter", I'll try to come into
> > something more descriptive.
>
> I don't like it either.. We usually call this 'alignment padding' in CS.

We can return to our initial internal name - "RX end padding".
I'll consult with architecture tomorrow to hear their suggestion.

Thanks

>
> Jason

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