Skip to contentSkip to conversation

Enable checksum offload capability reporting

PATCH In review Bodong Wang 5 participants 11 messages Sep 16, 2015 – Sep 22, 2015 7 messages not in the archive

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

cover Enable checksum offload capability reporting

The checksum offload capability reporting is enabled based on extended verbs. The capability field has sub-fields for every link layer, and depends on device cap, each link layer will support specific QP types. These will be reported to user space.

I'm new to uverbs extensions and looking forward for review comments on that aspect of the patches.

Bodong Wang (3):
IB/core: Add support of checksum capability reporting in ib verbs IB/uverbs: Add support for checksum capability reporting in user verbs IB/mlx4: Report checksum offload cap when query device

 drivers/infiniband/core/uverbs_cmd.c |  7 +++++++
 drivers/infiniband/hw/mlx4/main.c    |  3 +++
 include/rdma/ib_verbs.h              | 10 ++++++++++
 include/uapi/rdma/ib_user_verbs.h    |  6 ++++++
 4 files changed, 26 insertions(+)
Signature · 6 lines
1.8.3.1

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

A message that is not in the archive (sent off-list or not yet received): cover.1442413048.git.bodong-VPRAkNaXOzVWk0Htik3J/[email protected]

#2 lore raw

PATCH 1/3 IB/core: Add support of checksum capability reporting in ib verbs

A new filed csum_cap is added to both ib_query_device. It contains two members:
eth_csum_cap and ib_csum_cap, indicates checksum capability of Ethernet and Infiniband link layer respectively for different QP types.

Current checksum caps use the following enum members:
- IB_CSUM_SUPPORT_UD: device supports validation/calculation of csum for UD QP.
- IB_CSUM_SUPPORT_RAW: device supports validation/calculation of csum for raw QP.

---
 include/rdma/ib_verbs.h | 10 ++++++++++
 1 file changed, 10 insertions(+)
include/rdma/ib_verbs.h +10 -0
index b0f898e..94dbaee 100644
@@ -183,6 +183,11 @@ struct ib_cq_init_attr {
 	u32		flags;
 };
 
+struct ib_csum_cap_per_link {
+	uint32_t  eth_csum_cap;
+	uint32_t  ib_csum_cap;
+};
+
 struct ib_device_attr {
 	u64			fw_ver;
 	__be64			sys_image_guid;
@@ -229,6 +234,7 @@ struct ib_device_attr {
 	struct ib_odp_caps	odp_caps;
 	uint64_t		timestamp_mask;
 	uint64_t		hca_core_clock; /* in KHZ */
+	struct ib_csum_cap_per_link csum_cap;
 };
 
 enum ib_mtu {
@@ -868,6 +874,10 @@ enum ib_qp_create_flags {
 	IB_QP_CREATE_RESERVED_END		= 1 << 31,
 };
 
+enum ib_csum_cap_flags {
+	IB_CSUM_SUPPORT_UD	= (1 << IB_QPT_UD),
+	IB_CSUM_SUPPORT_RAW	= (1 << IB_QPT_RAW_PACKET),
+};
 
 /*
  * Note: users may not call ib_close_qp or ib_destroy_qp from the event_handler
Signature · 6 lines
1.8.3.1

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

A message that is not in the archive (sent off-list or not yet received): 490a38ab04a96f24500d647392e931c38853ba45.1442413048.git.bodong-VPRAkNaXOzVWk0Htik3J/[email protected]

#4 lore raw

Re: [PATCH 1/3] IB/core: Add support of checksum capability reporting in ib verbs

Quoted 7 lines · Bodong Wang

A new filed csum_cap is added to both ib_query_device. It contains two members:
eth_csum_cap and ib_csum_cap, indicates checksum capability of Ethernet and Infiniband link layer respectively for different QP types.

Current checksum caps use the following enum members:
- IB_CSUM_SUPPORT_UD: device supports validation/calculation of csum for UD QP.
- IB_CSUM_SUPPORT_RAW: device supports validation/calculation of csum for raw QP.

A combination? Is it possible then to also support calculation without validation? Maybe we want to receive packets that do have invalid checksums.

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

A message that is not in the archive (sent off-list or not yet received): alpine.DEB.2.11.1509161201100.22920-wcBtFHqTun5QOdAKl3ChDw@public.gmane.org

#6 lore raw

RE: [PATCH 1/3] IB/core: Add support of checksum capability reporting in ib verbs

For RX: if corresponding QP is not supported, it will not validate the csum, but packets are still received normally. For TX: if corresponding QP is not supported for csum calculation and user application sets the IBV_SEND_IP_CSUM flag, it will return error.

-----Original Message-----
From: Christoph Lameter [mailto:[email protected]]
Sent: Wednesday, September 16, 2015 12:07 PM
To: Bodong Wang
Cc: [email protected]; [email protected]; Bodong Wang; Or Gerlitz; jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org; Moshe Lazer; Haggai Eran; Matan Barak
Subject: Re: [PATCH 1/3] IB/core: Add support of checksum capability reporting in ib verbs

Quoted 7 lines · Bodong Wang

A new filed csum_cap is added to both ib_query_device. It contains two members:
eth_csum_cap and ib_csum_cap, indicates checksum capability of Ethernet and Infiniband link layer respectively for different QP types.

Current checksum caps use the following enum members:
- IB_CSUM_SUPPORT_UD: device supports validation/calculation of csum for UD QP.
- IB_CSUM_SUPPORT_RAW: device supports validation/calculation of csum for raw QP.

A combination? Is it possible then to also support calculation without validation? Maybe we want to receive packets that do have invalid checksums.

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

#7 lore raw

Re: [PATCH 1/3] IB/core: Add support of checksum capability reporting in ib verbs

In reply to a message not in the archive
Quoted 18 lines · Bodong Wang

A new filed csum_cap is added to both ib_query_device. It contains two members:
eth_csum_cap and ib_csum_cap, indicates checksum capability of Ethernet and Infiniband link layer respectively for different QP types.

Current checksum caps use the following enum members:
- IB_CSUM_SUPPORT_UD: device supports validation/calculation of csum for UD QP.
- IB_CSUM_SUPPORT_RAW: device supports validation/calculation of csum for raw QP.

---
 include/rdma/ib_verbs.h | 10 ++++++++++
 1 file changed, 10 insertions(+)
include/rdma/ib_verbs.h +0 -0
index b0f898e..94dbaee 100644

include/rdma/ib_verbs.h struct ib_cq_init_attr { · line 190

Quoted 8 lines
include/rdma/ib_verbs.h +5 -0
@@ -183,6 +183,11 @@ struct ib_cq_init_attr {
 	u32		flags;
 };
 
+struct ib_csum_cap_per_link {
+	uint32_t  eth_csum_cap;
+	uint32_t  ib_csum_cap;
+};
+

I generally don't like to waste this many bits on this little information. 64 bits total for what only uses 4 bits right now, and even on the high side would probably only ever use 8 or 12 bits, is excessive.

That said, it's cleaner and easier to read than something like a double shift where ib is lower 16 bits, eth is upper 16 bits, so I won't request you change it, just register my eyebrow raise over the number of bits used to record so little information. (In fairness, I thought about make you shrink it down, but the area of the struct you are adding this to is currently 64bit aligned and it is reasonably likely that the next item will need 64bit alignment, so saving bits only to possibly loose them to alignment is a exercise in futility)

Quoted 23 lines
include/rdma/ib_verbs.h +5 -0
 struct ib_device_attr {
 	u64			fw_ver;
 	__be64			sys_image_guid;
@@ -229,6 +234,7 @@ struct ib_device_attr {
 	struct ib_odp_caps	odp_caps;
 	uint64_t		timestamp_mask;
 	uint64_t		hca_core_clock; /* in KHZ */
+	struct ib_csum_cap_per_link csum_cap;
 };
 
 enum ib_mtu {
@@ -868,6 +874,10 @@ enum ib_qp_create_flags {
 	IB_QP_CREATE_RESERVED_END		= 1 << 31,
 };
 
+enum ib_csum_cap_flags {
+	IB_CSUM_SUPPORT_UD	= (1 << IB_QPT_UD),
+	IB_CSUM_SUPPORT_RAW	= (1 << IB_QPT_RAW_PACKET),
+};
 
 /*
  * Note: users may not call ib_close_qp or ib_destroy_qp from the event_handler
Signature · 2 lines
Doug Ledford <[email protected]>
              GPG KeyID: 0E572FDD
#8 lore raw

PATCH 2/3 IB/uverbs: Add support for checksum capability reporting in user verbs

In reply to a message not in the archive

New field csum_cap is added to respective uverbs counterpart according to ib_verbs.

---
 drivers/infiniband/core/uverbs_cmd.c | 7 +++++++
 include/uapi/rdma/ib_user_verbs.h    | 6 ++++++
 2 files changed, 13 insertions(+)
drivers/infiniband/core/uverbs_cmd.c +7 -0
index bbb02ff..9d5deec 100644
@@ -3464,6 +3464,13 @@ int ib_uverbs_ex_query_device(struct ib_uverbs_file *file,
 	resp.hca_core_clock = attr.hca_core_clock;
 	resp.response_length += sizeof(resp.hca_core_clock);
 
+	if (ucore->outlen < resp.response_length + sizeof(resp.csum_cap))
+		goto end;
+
+	resp.csum_cap.eth_csum_cap = attr.csum_cap.eth_csum_cap;
+	resp.csum_cap.ib_csum_cap = attr.csum_cap.ib_csum_cap;
+	resp.response_length += sizeof(resp.csum_cap);
+
 end:
 	err = ib_copy_to_udata(ucore, &resp, resp.response_length);
 	if (err)
include/uapi/rdma/ib_user_verbs.h +6 -0
index 978841e..9d69546 100644
@@ -218,6 +218,11 @@ struct ib_uverbs_odp_caps {
 	__u32 reserved;
 };
 
+struct ib_uverbs_csum_cap_per_link {
+	__u32 eth_csum_cap;
+	__u32 ib_csum_cap;
+};
+
 struct ib_uverbs_ex_query_device_resp {
 	struct ib_uverbs_query_device_resp base;
 	__u32 comp_mask;
@@ -225,6 +230,7 @@ struct ib_uverbs_ex_query_device_resp {
 	struct ib_uverbs_odp_caps odp_caps;
 	__u64 timestamp_mask;
 	__u64 hca_core_clock; /* in KHZ */
+	struct ib_uverbs_csum_cap_per_link csum_cap;
 };
 
 struct ib_uverbs_query_port {
Signature · 6 lines
1.8.3.1

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

PATCH 3/3 IB/mlx4: Report checksum offload cap when query device

In reply to a message not in the archive
---
 drivers/infiniband/hw/mlx4/main.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/infiniband/hw/mlx4/main.c +3 -0
index 8be6db8..a70ca6a 100644
@@ -217,6 +217,9 @@ static int mlx4_ib_query_device(struct ib_device *ibdev,
 		props->device_cap_flags |= IB_DEVICE_MANAGED_FLOW_STEERING;
 	}
 
+	props->csum_cap.eth_csum_cap |= IB_CSUM_SUPPORT_RAW;
+	props->csum_cap.ib_csum_cap |= IB_CSUM_SUPPORT_UD;
+
 	props->vendor_id	   = be32_to_cpup((__be32 *) (out_mad->data + 36)) &
 		0xffffff;
 	props->vendor_part_id	   = dev->dev->persist->pdev->device;
Signature · 6 lines
1.8.3.1

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

A message that is not in the archive (sent off-list or not yet received): 68bea4df2c89e5457aaaccf914756bc309d742a7.1442413048.git.bodong-VPRAkNaXOzVWk0Htik3J/[email protected]

#11 lore raw

Re: [PATCH 3/3] IB/mlx4: Report checksum offload cap when query device

Quoted 11 lines · Bodong Wang
---
 drivers/infiniband/hw/mlx4/main.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/infiniband/hw/mlx4/main.c +0 -0
index 8be6db8..a70ca6a 100644

drivers/infiniband/hw/mlx4/main.c static int mlx4_ib_query_device(struct ib_device *ibdev, · line 225

Quoted 9 lines
drivers/infiniband/hw/mlx4/main.c +3 -0
@@ -217,6 +217,9 @@ static int mlx4_ib_query_device(struct ib_device *ibdev,
 		props->device_cap_flags |= IB_DEVICE_MANAGED_FLOW_STEERING;
 	}
 
+	props->csum_cap.eth_csum_cap |= IB_CSUM_SUPPORT_RAW;
+	props->csum_cap.ib_csum_cap |= IB_CSUM_SUPPORT_UD;
+
 	props->vendor_id	   = be32_to_cpup((__be32 *) (out_mad->data + 36)) &
 		0xffffff;
 	props->vendor_part_id	   = dev->dev->persist->pdev->device;

This patch highlights something I didn't think about on the previous patch. Why separate eth/ib if you have per QP flags? The QP denotes the ib/eth relationship without the need to separate it into two different caps. In other words, you can never have an IB qp type on eth because the only eth QP types we support other than RAW are all RDMA and not IP. Really, there's enough spare bits in ib_device_cap_flags that you could do away with the new caps entirely. Right now, we support UD (which we already have a flag for), we can add two flags (for RAW and RC) and that should cover all of the foreseeable options as that would allow us to extend IP CSUM support to cover connected mode and cover all of the current options. I don't see us doing IP traffic in any other situation, so I thing that should suffice. Bits 25 and 26 could be used for the two new bits. Then you just need to extend the bits to user space.

Signature · 2 lines
Doug Ledford <[email protected]>
              GPG KeyID: 0E572FDD

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

#13 lore raw

Re: [PATCH 3/3] IB/mlx4: Report checksum offload cap when query device

Quoted 23 lines · Doug Ledford
Quoted 10 lines · Bodong Wang
---
 drivers/infiniband/hw/mlx4/main.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/infiniband/hw/mlx4/main.c +0 -0
index 8be6db8..a70ca6a 100644

drivers/infiniband/hw/mlx4/main.c static int mlx4_ib_query_device(struct ib_device *ibdev, · line 222

Quoted 6 lines
drivers/infiniband/hw/mlx4/main.c +3 -0
@@ -217,6 +217,9 @@ static int mlx4_ib_query_device(struct ib_device *ibdev,
              props->device_cap_flags |= IB_DEVICE_MANAGED_FLOW_STEERING;
      }
 
+     props->csum_cap.eth_csum_cap |= IB_CSUM_SUPPORT_RAW;
+     props->csum_cap.ib_csum_cap |= IB_CSUM_SUPPORT_UD;
+

This patch highlights something I didn't think about on the previous patch. Why separate eth/ib if you have per QP flags? The QP denotes the ib/eth relationship without the need to separate it into two different caps. In other words, you can never have an IB qp type on eth because the only eth QP types we support other than RAW are all RDMA and

Quoted 8 lines

not IP. Really, there's enough spare bits in ib_device_cap_flags that you could do away with the new caps entirely. Right now, we support UD (which we already have a flag for), we can add two flags (for RAW and RC) and that should cover all of the foreseeable options as that would allow us to extend IP CSUM support to cover connected mode and cover all of the current options. I don't see us doing IP traffic in any other situation, so I thing that should suffice. Bits 25 and 26 could be used for the two new bits. Then you just need to extend the bits to user space.

Doug,

The vendor may support the offload for a certain QP type only over certain link. E.g mlx4 support checksum for UD QPs only over IB but not over Eth, checksum for RC QPs isn't supported, and RAW_PACKET QPs are available anyway only for Eth links. But if this is what you think needs to be done, I guess we can do that.

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

A message that is not in the archive (sent off-list or not yet received): CAJ3xEMg31JSPU2_gW0OGjC9rdTgDRcx=7LnuPncD6+gCE7nRMw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#15 lore raw

Re: [PATCH 3/3] IB/mlx4: Report checksum offload cap when query device

Quoted 33 lines · Or Gerlitz
Quoted 23 lines · Doug Ledford
Quoted 10 lines · Bodong Wang
---
 drivers/infiniband/hw/mlx4/main.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/infiniband/hw/mlx4/main.c +0 -0
index 8be6db8..a70ca6a 100644

drivers/infiniband/hw/mlx4/main.c static int mlx4_ib_query_device(struct ib_device *ibdev, · line 222

Quoted 6 lines
drivers/infiniband/hw/mlx4/main.c +3 -0
@@ -217,6 +217,9 @@ static int mlx4_ib_query_device(struct ib_device *ibdev,
              props->device_cap_flags |= IB_DEVICE_MANAGED_FLOW_STEERING;
      }
 
+     props->csum_cap.eth_csum_cap |= IB_CSUM_SUPPORT_RAW;
+     props->csum_cap.ib_csum_cap |= IB_CSUM_SUPPORT_UD;
+

This patch highlights something I didn't think about on the previous patch. Why separate eth/ib if you have per QP flags? The QP denotes the ib/eth relationship without the need to separate it into two different caps. In other words, you can never have an IB qp type on eth because the only eth QP types we support other than RAW are all RDMA and

Quoted 8 lines

not IP. Really, there's enough spare bits in ib_device_cap_flags that you could do away with the new caps entirely. Right now, we support UD (which we already have a flag for), we can add two flags (for RAW and RC) and that should cover all of the foreseeable options as that would allow us to extend IP CSUM support to cover connected mode and cover all of the current options. I don't see us doing IP traffic in any other situation, so I thing that should suffice. Bits 25 and 26 could be used for the two new bits. Then you just need to extend the bits to user space.

Quoted 4 lines

Doug,

The vendor may support the offload for a certain QP type only over certain link.

Exactly my point.

Quoted 2 lines

E.g mlx4 support checksum for UD QPs only over IB but not over Eth,

As it should be. We are, after all, talking about IP embedded in UD RDMA. Over IB that makes sense. Over Eth it makes no sense. If you are going to do IP on Eth, then just do IP, don't do IP in UD. I see no reason to support this construct.

Quoted 1 line

checksum for RC QPs isn't supported,

But could be for IB. The fact that it isn't is why there is an ongoing effort to work around this issue.

Quoted 2 lines

and RAW_PACKET QPs
are available anyway only for Eth links.

Correct. And this will never be available on IB.

Quoted 2 lines

But if this is what you think
needs to be done, I guess we can do that.

Here's the only matrix of IP checksumming that makes sense:

1) UD over IB (because it is one of the supported IPoIB types)
2) RC over IB (same as above)
3) Raw ETH over Eth (because IP over Eth makes sense and is a common type of packet to send on Raw Eth, but Raw Eth will never be sent on IB as it isn't supported there at all)

Anything else would require adding more Raw ETH QPs elsewhere, or expanding the IPoIB spec to include more connection types.

Signature · 2 lines
Doug Ledford <[email protected]>
              GPG KeyID: 0E572FDD

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

#17 lore raw

Re: [PATCH 3/3] IB/mlx4: Report checksum offload cap when query device

Quoted 10 lines · Doug Ledford

Here's the only matrix of IP checksumming that makes sense:

1) UD over IB (because it is one of the supported IPoIB types)
2) RC over IB (same as above)
3) Raw ETH over Eth (because IP over Eth makes sense and is a common type of packet to send on Raw Eth, but Raw Eth will never be sent on IB as it isn't supported there at all)

Anything else would require adding more Raw ETH QPs elsewhere, or expanding the IPoIB spec to include more connection types.

OK, I'm good with that, so you want two news bits of device caps and not the too fancy
matrix of checksum-caps-for-qp-type-and-link-type.

Quoted 2 lines
Quoted 2 lines

Really, there's enough spare bits in ib_device_cap_flags that you could do away with the new caps entirely.

Yes, Bodong, please go ahead and use bits 26,27 of include/rdma/ib_verbs.h :: enum ib_device_cap_flags for the two new caps Doug asked.

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