On Sat, Feb 17, 2024 at 04:07:23PM +0100, Laurent
Vivier wrote:
The TCP and UDP checksums are computed using the
data in the TCP/UDP
payload but also some informations in the IP header (protocol,
length, source and destination addresses).
We add two functions, proto_ipv4_header_psum() and
proto_ipv6_header_psum(), to compute the checksum of the IP
header part.
Signed-off-by: Laurent Vivier <lvivier(a)redhat.com>
---
Notes:
v3:
- function parameters provide tot_len, saddr, daddr and protocol
rather than an iphdr/ipv6hdr
v2:
- move new function to checksum.c
- use _psum rather than _checksum in the name
- replace csum_udp4() and csum_udp6() by the new function
checksum.c | 67 ++++++++++++++++++++++++++++++++++++++++++------------
checksum.h | 4 ++++
tcp.c | 44 ++++++++++++++++-------------------
udp.c | 10 ++++----
4 files changed, 81 insertions(+), 44 deletions(-)
diff --git a/checksum.c b/checksum.c
index 511b296a9a80..55bf1340a257 100644
--- a/checksum.c
+++ b/checksum.c
@@ -134,6 +134,30 @@ uint16_t csum_ip4_header(uint16_t tot_len, uint8_t protocol,
return ~csum_fold(sum);
}
+/**
+ * proto_ipv4_header_psum() - Calculates the partial checksum of an
+ * IPv4 header for UDP or TCP
+ * @tot_len: Payload length
+ * @proto: Protocol number
+ * @saddr: Source address
+ * @daddr: Destination address
+ * @proto: proto Protocol number
+ * Returns: Partial checksum of the IPv4 header
+ */
+uint32_t proto_ipv4_header_psum(uint16_t tot_len, uint8_t protocol,
+ uint32_t saddr, uint32_t daddr)
+{
+ uint32_t psum = htons(protocol);
+
+ psum += (saddr >> 16) & 0xffff;
+ psum += saddr & 0xffff;
+ psum += (daddr >> 16) & 0xffff;
+ psum += daddr & 0xffff;
+ psum += htons(ntohs(tot_len) - 20);
+
+ return psum;
+}
+
/**
* csum_udp4() - Calculate and set checksum for a UDP over IPv4 packet
* @udp4hr: UDP header, initialised apart from checksum
@@ -150,14 +174,10 @@ void csum_udp4(struct udphdr *udp4hr,
udp4hr->check = 0;
if (UDP4_REAL_CHECKSUMS) {
- /* UNTESTED: if we did want real UDPv4 checksums, this
- * is roughly what we'd need */
- uint32_t psum = csum_fold(saddr.s_addr)
- + csum_fold(daddr.s_addr)
- + htons(len + sizeof(*udp4hr))
- + htons(IPPROTO_UDP);
- /* Add in partial checksum for the UDP header alone */
- psum += sum_16b(udp4hr, sizeof(*udp4hr));
+ uint32_t psum = proto_ipv4_header_psum(len, IPPROTO_UDP,
+ saddr.s_addr,
+ daddr.s_addr);
+ psum = csum_unfolded(udp4hr, sizeof(struct udphdr), psum);
udp4hr->check = csum(payload, len, psum);
}
}
@@ -180,6 +200,26 @@ void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t
len)
icmp4hr->checksum = csum(payload, len, psum);
}
+/**
+ * proto_ipv6_header_psum() - Calculates the partial checksum of an
+ * IPv6 header for UDP or TCP
+ * @payload_len: Payload length
+ * @proto: Protocol number
+ * @saddr: Source address
+ * @daddr: Destination address
+ * Returns: Partial checksum of the IPv6 header
+ */
+uint32_t proto_ipv6_header_psum(uint16_t payload_len, uint8_t protocol,
+ struct in6_addr saddr, struct in6_addr daddr)
Hrm, this is passing 2 16-byte IPv6 addresses by value, which might
not be what we want.
The idea here is to avoid the pointer alignment problem (&ip6h->saddr and
&ip6h->daddr can
be misaligned).
Is it a better solution to copy the content of ip6h->saddr and ip6h->daddr to some
local
variables and then provide the pointers of the local variables to
proto_ipv6_header_psum()?
+{
+ uint32_t sum = htons(protocol) + payload_len;
Uh.. doesn't that need to be htons(payload_len + sizeof(struct
ipv6hdr)) rather than simply payload_len?
payload_len is:
- b->ip6h.payload_len (from udp_update_hdr6())
- ip6h->payload_len (from tcp_update_check_tcp6())
and in ip6h payload_len is:
- htons(udp6_l2_mh_sock[n].msg_len + sizeof(b->uh)) (from udp_update_hdr6())
- htons(plen + sizeof(struct tcphdr)) (from tcp_fill_ipv6_header())
So this is correct... but
csum_udp6() uses "len" from tap_udp6_send(), so there is a bug here.
but there is also a problem with proto_ipv4_header_psum() that need host endianness and
tcp_update_check_tcp4() provides network endianness...
The first idea was to use the value from ip6h payload_len as it is already computed.
But mixing network endianness and host endianness appears to be a bad idea...
Thanks,
Laurent