netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC net-next 00/10] crc-offload: Split RX CRC offload from csum offload
@ 2024-07-03 22:48 Tom Herbert
  2024-07-03 22:48 ` [RFC net-next 01/10] skbuff: Rename csum_not_inet to csum_is_crc32 Tom Herbert
                   ` (9 more replies)
  0 siblings, 10 replies; 12+ messages in thread
From: Tom Herbert @ 2024-07-03 22:48 UTC (permalink / raw)
  To: davem, kuba, jesse.brandeburg, anthony.l.nguyen, cai.huoqing,
	netdev, felipe
  Cc: Tom Herbert

In this patch set we create csum_valid_crc32 flag in the skbuff.
This is used by drivers to report a valid offloaded CRC, in lieu
of setting CHECKSUM_UNNECESSARY. The benefits of this are:

1) It's compatible with checksum-complete. We can do checksum-
   complete with a validate CRC at the same time
2) Checksum-unnecessary conversion may erase the indication of
   the offloaded CRC. For instance in a SCTP/UDP packet where the
   driver reports both the non-zero UDP checksum and the CRC
   have been validate (i.e. csum_level is set to 1), then checksum-
   complete conversion erases the indication and the host has to compute
   the CRC again
3) It just seems awkward in general to be mixing fundamentally different
   verifications, and wouldn't be surprising if there are more bugs
   lurking in this area

Additionally, some helper functions are added:
   - skb_csum_crc32_unnecessary
   - skb_reset_csum_crc32_unnecessary
   - skb_set_csum_crc32_unnecessary

Changed FCOE and SCTP input to call skb_csum_crc32_unnecessary and
skb_reset_csum_crc32_unnecessary

Call the helper function skb_set_csum_crc32_unnecessary from drivers
instead of setting CHECKSUM_UNNECESSARY. This includes cavium thunder,
gve, hisilicon, hns3, idpf, ixgbe, wangxun. If I missed any please let
me know. The change was fairly simple, just need to identify that the
SCTP or FCOE CRC was validated and call the function.

Tom Herbert (10):
  skbuff: Rename csum_not_inet to csum_is_crc32
  skbuff: Add csum_valid_crc32 flag
  sctp: Call skb_csum_crc32_unnecessary
  fcoe: Call skb_csum_crc32_unnecessary
  cavium_thunder: Call skb_set_csum_crc32_unnecessary
  gve: Call skb_set_csum_crc32_unnecessary
  hisilicon: Call skb_set_csum_crc32_unnecessary
  idpf: Call skb_set_csum_crc32_unnecessary
  ixgbe: Call skb_set_csum_crc32_unnecessary
  wangxun: Call skb_set_csum_crc32_unnecessary

 .../net/ethernet/cavium/thunder/nicvf_main.c  |  5 +-
 drivers/net/ethernet/google/gve/gve_rx_dqo.c  |  4 +-
 drivers/net/ethernet/hisilicon/hns/hns_enet.c |  5 +-
 .../net/ethernet/hisilicon/hns3/hns3_enet.c   | 18 ++++--
 .../ethernet/intel/idpf/idpf_singleq_txrx.c   |  4 +-
 drivers/net/ethernet/intel/idpf/idpf_txrx.c   |  2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c |  2 +-
 drivers/net/ethernet/wangxun/libwx/wx_lib.c   |  5 +-
 drivers/scsi/fcoe/fcoe.c                      |  6 +-
 include/linux/skbuff.h                        | 58 +++++++++++++++----
 net/core/dev.c                                |  2 +-
 net/sched/act_csum.c                          |  2 +-
 net/sctp/input.c                              |  6 +-
 net/sctp/offload.c                            |  2 +-
 net/sctp/output.c                             |  2 +-
 15 files changed, 89 insertions(+), 34 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC net-next 01/10] skbuff: Rename csum_not_inet to csum_is_crc32
  2024-07-03 22:48 [RFC net-next 00/10] crc-offload: Split RX CRC offload from csum offload Tom Herbert
@ 2024-07-03 22:48 ` Tom Herbert
  2024-07-23 16:23   ` Mina Almasry
  2024-07-03 22:48 ` [RFC net-next 02/10] skbuff: Add csum_valid_crc32 flag Tom Herbert
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 12+ messages in thread
From: Tom Herbert @ 2024-07-03 22:48 UTC (permalink / raw)
  To: davem, kuba, jesse.brandeburg, anthony.l.nguyen, cai.huoqing,
	netdev, felipe
  Cc: Tom Herbert

csum_not_inet really refers to SCTP or FCOE CRC. Rename
to be more precise

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/linux/skbuff.h | 18 +++++++++---------
 net/core/dev.c         |  2 +-
 net/sched/act_csum.c   |  2 +-
 net/sctp/offload.c     |  2 +-
 net/sctp/output.c      |  2 +-
 5 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index f4cda3fbdb75..7fd6ce4df0ec 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -185,7 +185,7 @@
  *   skb_csum_hwoffload_help() can be called to resolve %CHECKSUM_PARTIAL based
  *   on network device checksumming capabilities: if a packet does not match
  *   them, skb_checksum_help() or skb_crc32c_help() (depending on the value of
- *   &sk_buff.csum_not_inet, see :ref:`crc`)
+ *   &sk_buff.csum_is_crc32, see :ref:`crc`)
  *   is called to resolve the checksum.
  *
  * - %CHECKSUM_NONE
@@ -215,12 +215,12 @@
  *     - This feature indicates that a device is capable of
  *	 offloading the SCTP CRC in a packet. To perform this offload the stack
  *	 will set csum_start and csum_offset accordingly, set ip_summed to
- *	 %CHECKSUM_PARTIAL and set csum_not_inet to 1, to provide an indication
+ *	 %CHECKSUM_PARTIAL and set csum_is_crc32 to 1, to provide an indication
  *	 in the skbuff that the %CHECKSUM_PARTIAL refers to CRC32c.
  *	 A driver that supports both IP checksum offload and SCTP CRC32c offload
  *	 must verify which offload is configured for a packet by testing the
- *	 value of &sk_buff.csum_not_inet; skb_crc32c_csum_help() is provided to
- *	 resolve %CHECKSUM_PARTIAL on skbs where csum_not_inet is set to 1.
+ *	 value of &sk_buff.csum_is_crc32; skb_crc32c_csum_help() is provided to
+ *	 resolve %CHECKSUM_PARTIAL on skbs where csum_is_crc32 is set to 1.
  *
  *   * - %NETIF_F_FCOE_CRC
  *     - This feature indicates that a device is capable of offloading the FCOE
@@ -822,7 +822,7 @@ enum skb_tstamp_type {
  *	@encapsulation: indicates the inner headers in the skbuff are valid
  *	@encap_hdr_csum: software checksum is needed
  *	@csum_valid: checksum is already valid
- *	@csum_not_inet: use CRC32c to resolve CHECKSUM_PARTIAL
+ *	@csum_is_crc32: use CRC32c to resolve CHECKSUM_PARTIAL
  *	@csum_complete_sw: checksum was completed by software
  *	@csum_level: indicates the number of consecutive checksums found in
  *		the packet minus one that have been verified as
@@ -1006,7 +1006,7 @@ struct sk_buff {
 #endif
 	__u8			slow_gro:1;
 #if IS_ENABLED(CONFIG_IP_SCTP)
-	__u8			csum_not_inet:1;
+	__u8			csum_is_crc32:1;
 #endif
 
 #if defined(CONFIG_NET_SCHED) || defined(CONFIG_NET_XGRESS)
@@ -5098,17 +5098,17 @@ static inline void skb_set_redirected_noclear(struct sk_buff *skb,
 static inline bool skb_csum_is_sctp(struct sk_buff *skb)
 {
 #if IS_ENABLED(CONFIG_IP_SCTP)
-	return skb->csum_not_inet;
+	return skb->csum_is_crc32;
 #else
 	return 0;
 #endif
 }
 
-static inline void skb_reset_csum_not_inet(struct sk_buff *skb)
+static inline void skb_reset_csum_is_crc32(struct sk_buff *skb)
 {
 	skb->ip_summed = CHECKSUM_NONE;
 #if IS_ENABLED(CONFIG_IP_SCTP)
-	skb->csum_not_inet = 0;
+	skb->csum_is_crc32 = 0;
 #endif
 }
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 385c4091aa77..f6a2b868e561 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3382,7 +3382,7 @@ int skb_crc32c_csum_help(struct sk_buff *skb)
 						  skb->len - start, ~(__u32)0,
 						  crc32c_csum_stub));
 	*(__le32 *)(skb->data + offset) = crc32c_csum;
-	skb_reset_csum_not_inet(skb);
+	skb_reset_csum_is_crc32(skb);
 out:
 	return ret;
 }
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 5cc8e407e791..347622e690be 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -376,7 +376,7 @@ static int tcf_csum_sctp(struct sk_buff *skb, unsigned int ihl,
 
 	sctph->checksum = sctp_compute_cksum(skb,
 					     skb_network_offset(skb) + ihl);
-	skb_reset_csum_not_inet(skb);
+	skb_reset_csum_is_crc32(skb);
 
 	return 1;
 }
diff --git a/net/sctp/offload.c b/net/sctp/offload.c
index 502095173d88..b1a68f43b327 100644
--- a/net/sctp/offload.c
+++ b/net/sctp/offload.c
@@ -27,7 +27,7 @@
 static __le32 sctp_gso_make_checksum(struct sk_buff *skb)
 {
 	skb->ip_summed = CHECKSUM_NONE;
-	skb->csum_not_inet = 0;
+	skb->csum_is_crc32 = 0;
 	/* csum and csum_start in GSO CB may be needed to do the UDP
 	 * checksum when it's a UDP tunneling packet.
 	 */
diff --git a/net/sctp/output.c b/net/sctp/output.c
index a63df055ac57..1d81451cc654 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -553,7 +553,7 @@ static int sctp_packet_pack(struct sctp_packet *packet,
 	} else {
 chksum:
 		head->ip_summed = CHECKSUM_PARTIAL;
-		head->csum_not_inet = 1;
+		head->csum_is_crc32 = 1;
 		head->csum_start = skb_transport_header(head) - head->head;
 		head->csum_offset = offsetof(struct sctphdr, checksum);
 	}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC net-next 02/10] skbuff: Add csum_valid_crc32 flag
  2024-07-03 22:48 [RFC net-next 00/10] crc-offload: Split RX CRC offload from csum offload Tom Herbert
  2024-07-03 22:48 ` [RFC net-next 01/10] skbuff: Rename csum_not_inet to csum_is_crc32 Tom Herbert
@ 2024-07-03 22:48 ` Tom Herbert
  2024-07-03 22:48 ` [RFC net-next 03/10] sctp: Call skb_csum_crc32_unnecessary Tom Herbert
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Tom Herbert @ 2024-07-03 22:48 UTC (permalink / raw)
  To: davem, kuba, jesse.brandeburg, anthony.l.nguyen, cai.huoqing,
	netdev, felipe
  Cc: Tom Herbert

When a device gets notification of that a CRC has been validated
(either for SCTP or FCOE) then this is treated as an instance of
checksum-unnecessary. This creates a few problems:

1) It's incompatible with checksum-complete. We cannot do checksum-
   complete with a validate CRC at the same time
2) Checksum-unnecessary conversion may erase the indication of
   the offloaded CRC. For instance in a SCTP/UDP packet where the
   driver reports both the non-zero UDP checksum and the CRC
   have been validated (i.e. csum_level is set to 1), then checksum-
   complete conversion erases the indication and the host has to compute
   the CRC again
3) It just seems awkward in general to be mixing fundamentally different
   verifications, and wouldn't be surprising if there are bugs lurking
   in this area

This patch introduces csum_valid_crc32 flag in the skbuff. This is
used to inidicate an offloaded CRC. It's independent of the checksum
fields.

Additionally, some helper functions are added:
   - skb_csum_crc32_unnecessary
   - skb_set_csum_crc32_unnecessary
   - skb_reset_csum_crc32_unnecessary

Add comment about new method for offloading SCTP and FCOE RX CRC

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/linux/skbuff.h | 40 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 7fd6ce4df0ec..8706984ea56e 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -119,8 +119,6 @@
  *       zero UDP checksum for either IPv4 or IPv6, the networking stack
  *       may perform further validation in this case.
  *     - GRE: only if the checksum is present in the header.
- *     - SCTP: indicates the CRC in SCTP header has been validated.
- *     - FCOE: indicates the CRC in FC frame has been validated.
  *
  *   &sk_buff.csum_level indicates the number of consecutive checksums found in
  *   the packet minus one that have been verified as %CHECKSUM_UNNECESSARY.
@@ -142,7 +140,6 @@
  *
  *   - Even if device supports only some protocols, but is able to produce
  *     skb->csum, it MUST use CHECKSUM_COMPLETE, not CHECKSUM_UNNECESSARY.
- *   - CHECKSUM_COMPLETE is not applicable to SCTP and FCoE protocols.
  *
  * - %CHECKSUM_PARTIAL
  *
@@ -156,6 +153,15 @@
  *   packet that are after the checksum being offloaded are not considered to
  *   be verified.
  *
+ * SCTP or FCOE CRC in received packets verfied by device
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * An SCTP or FCOE CRC may be verified by device and reported as valid by a
+ * driver. This is done by setting skb->csum_valid_crc32 to 1. The helper
+ * function skb_set_csum_crc32_unnecessary should be called to do that.
+ * The CRC validation can be checked by calling skb_csum_crc32_unnecessary
+ * and cleared by calling skb_reset_csum_crc32_unnecessary
+ *
  * Checksumming on transmit for non-GSO
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  *
@@ -1008,6 +1014,9 @@ struct sk_buff {
 #if IS_ENABLED(CONFIG_IP_SCTP)
 	__u8			csum_is_crc32:1;
 #endif
+#if IS_ENABLED(CONFIG_IP_SCTP) || IS_ENABLED(CONFIG_FCOE)
+	__u8			csum_valid_crc32:1;
+#endif
 
 #if defined(CONFIG_NET_SCHED) || defined(CONFIG_NET_XGRESS)
 	__u16			tc_index;	/* traffic control index */
@@ -4453,6 +4462,31 @@ static inline int skb_csum_unnecessary(const struct sk_buff *skb)
 		 skb_checksum_start_offset(skb) >= 0));
 }
 
+static inline int skb_csum_crc32_unnecessary(const struct sk_buff *skb)
+{
+#if IS_ENABLED(CONFIG_IP_SCTP) || IS_ENABLED(CONFIG_FCOE)
+	return (skb->csum_valid_crc32 ||
+		(skb->ip_summed == CHECKSUM_PARTIAL &&
+		 skb_checksum_start_offset(skb) >= 0));
+#else
+	return 0;
+#endif
+}
+
+static inline void skb_reset_csum_crc32_unnecessary(struct sk_buff *skb)
+{
+#if IS_ENABLED(CONFIG_IP_SCTP) || IS_ENABLED(CONFIG_FCOE)
+	skb->csum_valid_crc32 = 0;
+#endif
+}
+
+static inline void skb_set_csum_crc32_unnecessary(struct sk_buff *skb)
+{
+#if IS_ENABLED(CONFIG_IP_SCTP) || IS_ENABLED(CONFIG_FCOE)
+	skb->csum_valid_crc32 = 1;
+#endif
+}
+
 /**
  *	skb_checksum_complete - Calculate checksum of an entire packet
  *	@skb: packet to process
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC net-next 03/10] sctp: Call skb_csum_crc32_unnecessary
  2024-07-03 22:48 [RFC net-next 00/10] crc-offload: Split RX CRC offload from csum offload Tom Herbert
  2024-07-03 22:48 ` [RFC net-next 01/10] skbuff: Rename csum_not_inet to csum_is_crc32 Tom Herbert
  2024-07-03 22:48 ` [RFC net-next 02/10] skbuff: Add csum_valid_crc32 flag Tom Herbert
@ 2024-07-03 22:48 ` Tom Herbert
  2024-07-03 22:48 ` [RFC net-next 04/10] fcoe: " Tom Herbert
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Tom Herbert @ 2024-07-03 22:48 UTC (permalink / raw)
  To: davem, kuba, jesse.brandeburg, anthony.l.nguyen, cai.huoqing,
	netdev, felipe
  Cc: Tom Herbert

Instead of checking for CHECKSUM_UNNECESSARY, call
skb_csum_crc32_unnecessary to see if the SCTP CRC has been
validated. If it is then call skb_reset_csum_crc32_unnecessary
to clear the flag

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 net/sctp/input.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/sctp/input.c b/net/sctp/input.c
index 17fcaa9b0df9..aefcc3497d27 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -124,14 +124,12 @@ int sctp_rcv(struct sk_buff *skb)
 	/* Pull up the IP header. */
 	__skb_pull(skb, skb_transport_offset(skb));
 
-	skb->csum_valid = 0; /* Previous value not applicable */
-	if (skb_csum_unnecessary(skb))
-		__skb_decr_checksum_unnecessary(skb);
+	if (skb_csum_crc32_unnecessary(skb))
+		skb_reset_csum_crc32_unnecessary(skb);
 	else if (!sctp_checksum_disable &&
 		 !is_gso &&
 		 sctp_rcv_checksum(net, skb) < 0)
 		goto discard_it;
-	skb->csum_valid = 1;
 
 	__skb_pull(skb, sizeof(struct sctphdr));
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC net-next 04/10] fcoe: Call skb_csum_crc32_unnecessary
  2024-07-03 22:48 [RFC net-next 00/10] crc-offload: Split RX CRC offload from csum offload Tom Herbert
                   ` (2 preceding siblings ...)
  2024-07-03 22:48 ` [RFC net-next 03/10] sctp: Call skb_csum_crc32_unnecessary Tom Herbert
@ 2024-07-03 22:48 ` Tom Herbert
  2024-07-03 22:48 ` [RFC net-next 05/10] cavium_thunder: Call skb_set_csum_crc32_unnecessary Tom Herbert
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Tom Herbert @ 2024-07-03 22:48 UTC (permalink / raw)
  To: davem, kuba, jesse.brandeburg, anthony.l.nguyen, cai.huoqing,
	netdev, felipe
  Cc: Tom Herbert

Instead of checking for CHECKSUM_UNNECESSARY, call
skb_csum_crc32_unnecessary to see if the FCOE CRC has been
validated. If it is, then call skb_reset_csum_crc32_unnecessary
to clear the flag

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 drivers/scsi/fcoe/fcoe.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index f1429f270170..9444bf973234 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1612,10 +1612,12 @@ static inline int fcoe_filter_frames(struct fc_lport *lport,
 	 * it's solicited data, in which case, the FCP layer would
 	 * check it during the copy.
 	 */
-	if (lport->crc_offload && skb->ip_summed == CHECKSUM_UNNECESSARY)
+	if (lport->crc_offload && skb_csum_crc32_unnecessary(skb)) {
 		fr_flags(fp) &= ~FCPHF_CRC_UNCHECKED;
-	else
+		skb_reset_csum_crc32_unnecessary(skb);
+	} else {
 		fr_flags(fp) |= FCPHF_CRC_UNCHECKED;
+	}
 
 	fh = fc_frame_header_get(fp);
 	if (fh->fh_r_ctl == FC_RCTL_DD_SOL_DATA && fh->fh_type == FC_TYPE_FCP)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC net-next 05/10] cavium_thunder: Call skb_set_csum_crc32_unnecessary
  2024-07-03 22:48 [RFC net-next 00/10] crc-offload: Split RX CRC offload from csum offload Tom Herbert
                   ` (3 preceding siblings ...)
  2024-07-03 22:48 ` [RFC net-next 04/10] fcoe: " Tom Herbert
@ 2024-07-03 22:48 ` Tom Herbert
  2024-07-03 22:48 ` [RFC net-next 06/10] gve: " Tom Herbert
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Tom Herbert @ 2024-07-03 22:48 UTC (permalink / raw)
  To: davem, kuba, jesse.brandeburg, anthony.l.nguyen, cai.huoqing,
	netdev, felipe
  Cc: Tom Herbert

When a validated offload CRC for SCTP is detected call
skb_set_csum_crc32_unnecessary instead of setting
CHECKSUM_UNNECESSARY

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 drivers/net/ethernet/cavium/thunder/nicvf_main.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index aebb9fef3f6e..72157f9542c5 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -824,7 +824,10 @@ static void nicvf_rcv_pkt_handler(struct net_device *netdev,
 	skb_record_rx_queue(skb, rq_idx);
 	if (netdev->hw_features & NETIF_F_RXCSUM) {
 		/* HW by default verifies TCP/UDP/SCTP checksums */
-		skb->ip_summed = CHECKSUM_UNNECESSARY;
+		if (cqe_rx->l4_type == L4TYPE_SCTP)
+			skb_set_csum_crc32_unnecessary(skb);
+		else
+			skb->ip_summed = CHECKSUM_UNNECESSARY;
 	} else {
 		skb_checksum_none_assert(skb);
 	}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC net-next 06/10] gve: Call skb_set_csum_crc32_unnecessary
  2024-07-03 22:48 [RFC net-next 00/10] crc-offload: Split RX CRC offload from csum offload Tom Herbert
                   ` (4 preceding siblings ...)
  2024-07-03 22:48 ` [RFC net-next 05/10] cavium_thunder: Call skb_set_csum_crc32_unnecessary Tom Herbert
@ 2024-07-03 22:48 ` Tom Herbert
  2024-07-03 22:48 ` [RFC net-next 07/10] hisilicon: " Tom Herbert
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Tom Herbert @ 2024-07-03 22:48 UTC (permalink / raw)
  To: davem, kuba, jesse.brandeburg, anthony.l.nguyen, cai.huoqing,
	netdev, felipe
  Cc: Tom Herbert

When a validated offload CRC for SCTP is detected call
skb_set_csum_crc32_unnessary instead of setting
CHECKSUM_UNNECESSARY

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 drivers/net/ethernet/google/gve/gve_rx_dqo.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/google/gve/gve_rx_dqo.c b/drivers/net/ethernet/google/gve/gve_rx_dqo.c
index 1154c1d8f66f..d3d6d7c6f253 100644
--- a/drivers/net/ethernet/google/gve/gve_rx_dqo.c
+++ b/drivers/net/ethernet/google/gve/gve_rx_dqo.c
@@ -625,9 +625,11 @@ static void gve_rx_skb_csum(struct sk_buff *skb,
 	case GVE_L4_TYPE_TCP:
 	case GVE_L4_TYPE_UDP:
 	case GVE_L4_TYPE_ICMP:
-	case GVE_L4_TYPE_SCTP:
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 		break;
+	case GVE_L4_TYPE_SCTP:
+		skb_set_csum_crc32_unnecessary(skb);
+		break;
 	default:
 		break;
 	}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC net-next 07/10] hisilicon: Call skb_set_csum_crc32_unnecessary
  2024-07-03 22:48 [RFC net-next 00/10] crc-offload: Split RX CRC offload from csum offload Tom Herbert
                   ` (5 preceding siblings ...)
  2024-07-03 22:48 ` [RFC net-next 06/10] gve: " Tom Herbert
@ 2024-07-03 22:48 ` Tom Herbert
  2024-07-03 22:48 ` [RFC net-next 08/10] idpf: " Tom Herbert
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Tom Herbert @ 2024-07-03 22:48 UTC (permalink / raw)
  To: davem, kuba, jesse.brandeburg, anthony.l.nguyen, cai.huoqing,
	netdev, felipe
  Cc: Tom Herbert

When a validated offload CRC for SCTP is detected call
skb_set_csum_crc32_unnessary instead of setting
CHECKSUM_UNNECESSARY

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 drivers/net/ethernet/hisilicon/hns/hns_enet.c  |  5 ++++-
 .../net/ethernet/hisilicon/hns3/hns3_enet.c    | 18 +++++++++++++-----
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index fd32e15cadcb..f3e8b9cb3779 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -556,7 +556,10 @@ static void hns_nic_rx_checksum(struct hns_nic_ring_data *ring_data,
 		return;
 
 	/* now, this has to be a packet with valid RX checksum */
-	skb->ip_summed = CHECKSUM_UNNECESSARY;
+	if (l4id == HNS_RX_FLAG_L4ID_SCTP)
+		skb_set_csum_crc32_unnecessary(skb);
+	else
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
 }
 
 static int hns_nic_poll_rx_skb(struct hns_nic_ring_data *ring_data,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index a5fc0209d628..5fd98854f72d 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -3908,11 +3908,19 @@ static void hns3_rx_handle_csum(struct sk_buff *skb, u32 l234info,
 					  HNS3_RXD_L4ID_S);
 		/* Can checksum ipv4 or ipv6 + UDP/TCP/SCTP packets */
 		if ((l3_type == HNS3_L3_TYPE_IPV4 ||
-		     l3_type == HNS3_L3_TYPE_IPV6) &&
-		    (l4_type == HNS3_L4_TYPE_UDP ||
-		     l4_type == HNS3_L4_TYPE_TCP ||
-		     l4_type == HNS3_L4_TYPE_SCTP))
-			skb->ip_summed = CHECKSUM_UNNECESSARY;
+		     l3_type == HNS3_L3_TYPE_IPV6)) {
+			switch (l4_type) {
+			case HNS3_L4_TYPE_UDP:
+			case HNS3_L4_TYPE_TCP:
+				skb->ip_summed = CHECKSUM_UNNECESSARY;
+				break;
+			case HNS3_L4_TYPE_SCTP:
+				skb_set_csum_crc32_unnecessary(skb);
+				break;
+			default:
+				break;
+			}
+		}
 		break;
 	default:
 		break;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC net-next 08/10] idpf: Call skb_set_csum_crc32_unnecessary
  2024-07-03 22:48 [RFC net-next 00/10] crc-offload: Split RX CRC offload from csum offload Tom Herbert
                   ` (6 preceding siblings ...)
  2024-07-03 22:48 ` [RFC net-next 07/10] hisilicon: " Tom Herbert
@ 2024-07-03 22:48 ` Tom Herbert
  2024-07-03 22:48 ` [RFC net-next 09/10] ixgbe: " Tom Herbert
  2024-07-03 22:48 ` [RFC net-next 10/10] wangxun: " Tom Herbert
  9 siblings, 0 replies; 12+ messages in thread
From: Tom Herbert @ 2024-07-03 22:48 UTC (permalink / raw)
  To: davem, kuba, jesse.brandeburg, anthony.l.nguyen, cai.huoqing,
	netdev, felipe
  Cc: Tom Herbert

When a validated offload CRC for SCTP is detected call
skb_set_csum_crc32_unnecessary instead of setting
CHECKSUM_UNNECESSARY

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c | 4 +++-
 drivers/net/ethernet/intel/idpf/idpf_txrx.c         | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c
index 27b93592c4ba..0ba7abd87d05 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c
@@ -698,9 +698,11 @@ static void idpf_rx_singleq_csum(struct idpf_queue *rxq, struct sk_buff *skb,
 	case IDPF_RX_PTYPE_INNER_PROT_ICMP:
 	case IDPF_RX_PTYPE_INNER_PROT_TCP:
 	case IDPF_RX_PTYPE_INNER_PROT_UDP:
-	case IDPF_RX_PTYPE_INNER_PROT_SCTP:
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 		return;
+	case IDPF_RX_PTYPE_INNER_PROT_SCTP:
+		skb_set_csum_crc32_unnecessary(skb);
+		return;
 	default:
 		return;
 	}
diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
index b023704bbbda..3ff1d181534c 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
@@ -2804,7 +2804,7 @@ static void idpf_rx_csum(struct idpf_queue *rxq, struct sk_buff *skb,
 		}
 		break;
 	case IDPF_RX_PTYPE_INNER_PROT_SCTP:
-		skb->ip_summed = CHECKSUM_UNNECESSARY;
+		skb_set_csum_crc32_unnecessary(skb);
 		break;
 	default:
 		break;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC net-next 09/10] ixgbe: Call skb_set_csum_crc32_unnecessary
  2024-07-03 22:48 [RFC net-next 00/10] crc-offload: Split RX CRC offload from csum offload Tom Herbert
                   ` (7 preceding siblings ...)
  2024-07-03 22:48 ` [RFC net-next 08/10] idpf: " Tom Herbert
@ 2024-07-03 22:48 ` Tom Herbert
  2024-07-03 22:48 ` [RFC net-next 10/10] wangxun: " Tom Herbert
  9 siblings, 0 replies; 12+ messages in thread
From: Tom Herbert @ 2024-07-03 22:48 UTC (permalink / raw)
  To: davem, kuba, jesse.brandeburg, anthony.l.nguyen, cai.huoqing,
	netdev, felipe
  Cc: Tom Herbert

When a validated offload CRC for FCOE is detected call
skb_set_csum_crc32_unnessary instead of setting
CHECKSUM_UNNECESSARY

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
index 18d63c8c2ff4..4596dd85a171 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
@@ -398,7 +398,7 @@ int ixgbe_fcoe_ddp(struct ixgbe_adapter *adapter,
 	if (fcerr == cpu_to_le32(IXGBE_FCERR_BADCRC))
 		skb->ip_summed = CHECKSUM_NONE;
 	else
-		skb->ip_summed = CHECKSUM_UNNECESSARY;
+		skb_set_csum_crc32_unnecessary(skb);
 
 	if (eth_hdr(skb)->h_proto == htons(ETH_P_8021Q))
 		fh = (struct fc_frame_header *)(skb->data +
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC net-next 10/10] wangxun: Call skb_set_csum_crc32_unnecessary
  2024-07-03 22:48 [RFC net-next 00/10] crc-offload: Split RX CRC offload from csum offload Tom Herbert
                   ` (8 preceding siblings ...)
  2024-07-03 22:48 ` [RFC net-next 09/10] ixgbe: " Tom Herbert
@ 2024-07-03 22:48 ` Tom Herbert
  9 siblings, 0 replies; 12+ messages in thread
From: Tom Herbert @ 2024-07-03 22:48 UTC (permalink / raw)
  To: davem, kuba, jesse.brandeburg, anthony.l.nguyen, cai.huoqing,
	netdev, felipe
  Cc: Tom Herbert

When a validated offload CRC for SCTP is detected call
skb_set_csum_crc32_unnessary instead of setting
CHECKSUM_UNNECESSARY

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 drivers/net/ethernet/wangxun/libwx/wx_lib.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
index ac0e1d42fe55..8f4ffc961abf 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
@@ -558,7 +558,10 @@ static void wx_rx_checksum(struct wx_ring *ring,
 	}
 
 	/* It must be a TCP or UDP or SCTP packet with a valid checksum */
-	skb->ip_summed = CHECKSUM_UNNECESSARY;
+	if (dptype.prot == WX_DEC_PTYPE_PROT_SCTP)
+		skb_set_csum_crc32_unnecessary(skb);
+	else
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 	/* If there is an outer header present that might contain a checksum
 	 * we need to bump the checksum level by 1 to reflect the fact that
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC net-next 01/10] skbuff: Rename csum_not_inet to csum_is_crc32
  2024-07-03 22:48 ` [RFC net-next 01/10] skbuff: Rename csum_not_inet to csum_is_crc32 Tom Herbert
@ 2024-07-23 16:23   ` Mina Almasry
  0 siblings, 0 replies; 12+ messages in thread
From: Mina Almasry @ 2024-07-23 16:23 UTC (permalink / raw)
  To: Tom Herbert
  Cc: davem, kuba, jesse.brandeburg, anthony.l.nguyen, cai.huoqing,
	netdev, felipe

On Wed, Jul 03, 2024 at 03:48:41PM -0700, Tom Herbert wrote:
> csum_not_inet really refers to SCTP or FCOE CRC. Rename
> to be more precise
>
> Signed-off-by: Tom Herbert <tom@herbertland.com>

I checked that you haven't missed any instances of csum_not_inet unmodified.

The rename seems straightforward and unobjectionable given that the description
of the csum_not_inet field said that it was a crc32 anway.

The previous name also contained an awkward negation. Removing that seems like
an improvement.

FWIW,

Reviewed-by: Mina Almasry <almasrymina@google.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-07-23 16:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-03 22:48 [RFC net-next 00/10] crc-offload: Split RX CRC offload from csum offload Tom Herbert
2024-07-03 22:48 ` [RFC net-next 01/10] skbuff: Rename csum_not_inet to csum_is_crc32 Tom Herbert
2024-07-23 16:23   ` Mina Almasry
2024-07-03 22:48 ` [RFC net-next 02/10] skbuff: Add csum_valid_crc32 flag Tom Herbert
2024-07-03 22:48 ` [RFC net-next 03/10] sctp: Call skb_csum_crc32_unnecessary Tom Herbert
2024-07-03 22:48 ` [RFC net-next 04/10] fcoe: " Tom Herbert
2024-07-03 22:48 ` [RFC net-next 05/10] cavium_thunder: Call skb_set_csum_crc32_unnecessary Tom Herbert
2024-07-03 22:48 ` [RFC net-next 06/10] gve: " Tom Herbert
2024-07-03 22:48 ` [RFC net-next 07/10] hisilicon: " Tom Herbert
2024-07-03 22:48 ` [RFC net-next 08/10] idpf: " Tom Herbert
2024-07-03 22:48 ` [RFC net-next 09/10] ixgbe: " Tom Herbert
2024-07-03 22:48 ` [RFC net-next 10/10] wangxun: " Tom Herbert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).