Introduce an IB device's scatter end padding capability

From: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/[email protected]>

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/[email protected]>

Reviewed-by Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---
 drivers/infiniband/core/uverbs_cmd.c | 3 ++-
 include/rdma/ib_verbs.h              | 4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

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

index d31e4bc58e9a..ab29a0327831 100644

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

 		ret = -EINVAL;
 		goto err_put;

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

 	}

include/rdma/ib_verbs.h +0 -0

index 9810e4568635..4c0a539cd2a2 100644

include/rdma/ib_verbs.h +1 -0

+	IB_DEVICE_SCATTER_END_PADDING		= (1ULL << 36),
 };

enum ib_signature_prot_cap {

include/rdma/ib_verbs.h +2 -0

@@ -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 {

Quoted 7 lines · Leon Romanovsky

From: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/[email protected]>

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.

Quoted 41 lines

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/[email protected]>

Reviewed-by Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---
 drivers/infiniband/core/uverbs_cmd.c | 3 ++-
 include/rdma/ib_verbs.h              | 4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

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

index d31e4bc58e9a..ab29a0327831 100644

drivers/infiniband/core/uverbs_cmd.c static int create_qp(struct ib_uverbs_file · line 1496

Quoted 7 lines

drivers/infiniband/core/uverbs_cmd.c +2 -1

@@ -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?

Quoted 8 lines

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

 		ret = -EINVAL;
 		goto err_put;

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

 	}

include/rdma/ib_verbs.h +0 -0

index 9810e4568635..4c0a539cd2a2 100644

include/rdma/ib_verbs.h enum ib_device_cap_flags { · line 234

Quoted 6 lines

include/rdma/ib_verbs.h +1 -0

@@ -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 */

Quoted 13 lines

include/rdma/ib_verbs.h +1 -0

+	IB_DEVICE_SCATTER_END_PADDING		= (1ULL << 36),
 };

enum ib_signature_prot_cap {

include/rdma/ib_verbs.h +1 -0

@@ -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,

include/rdma/ib_verbs.h enum ib_wq_flags { · line 1630

Quoted 7 lines

include/rdma/ib_verbs.h +1 -0

@@ -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 <[email protected]>

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

Quoted 9 lines · Doug Ledford

Quoted 7 lines · Leon Romanovsky

From: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/[email protected]>

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)

Quoted 7 lines

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.

Quoted 18 lines · Leon Romanovsky

Quoted 16 lines · Doug Ledford

Quoted 7 lines · Leon Romanovsky

From: Noa Osherovich <noaos-VPRAkNaXOzVWk0Htik3J/[email protected]>

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.

Quoted 3 lines

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

drivers/infiniband/hw/mlx5/qp.c +3 -0

@@ -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");

Hi Leon, Noa,

Quoted 4 lines

drivers/infiniband/hw/mlx5/qp.c +0 -0

@@ -1781,6 +1783,19 @@ static int create_qp_common(struct mlx5_ib_dev *dev, struct ib_pd *pd,
 		qp->flags |= MLX5_IB_QP_LSO;
 	}
 

drivers/infiniband/hw/mlx5/qp.c static int create_qp_common(struct mlx5_ib_dev *dev, struct ib_pd *pd, · line 1798

Quoted 13 lines

drivers/infiniband/hw/mlx5/qp.c +13 -0

+	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

Quoted 3 lines

Quoted 2 lines

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:

(quoted diff) +13 -0

+	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

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.

Quoted 6 lines · 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.

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?