netdev - [RFC net-next 02/10] skbuff: Add csum_valid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240703224850.1226697-3-tom@herbertland.com>
Date: Wed,  3 Jul 2024 15:48:42 -0700
From: Tom Herbert <tom@...bertland.com>
To: davem@...emloft.net,
	kuba@...nel.org,
	jesse.brandeburg@...el.com,
	anthony.l.nguyen@...el.com,
	cai.huoqing@...ux.dev,
	netdev@...r.kernel.org,
	felipe@...anda.io
Cc: Tom Herbert <tom@...bertland.com>
Subject: [RFC net-next 02/10] skbuff: Add csum_valid_crc32 flag

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@...bertland.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