Re: [PATCH v4 5/8] checksum: use csum_ip4_header() in udp.c and tcp.c

On 3/1/24 00:25, David Gibson wrote:

On Thu, Feb 29, 2024 at 05:59:52PM +0100, Laurent Vivier wrote:

...

We can find the same function to compute the IPv4 header checksum in tcp.c, udp.c and tap.c

Use the function defined for tap.c, csum_ip4_header(), but with the code used in tcp.c and udp.c as it doesn't need a fully initialiazed IPv4 header, only protocol, tot_len, saddr and daddr.

Signed-off-by: Laurent Vivier Reviewed-by: David Gibson ---

Notes: v4: - rebase

v3: - function parameters provide tot_len, saddr, daddr and protocol rather than an iphdr

v2: - use csum_ip4_header() from checksum.c - use code from tcp.c and udp.c in csum_ip4_header() - use "const struct iphfr *", check is not updated by the function but by the caller.

checksum.c | 17 +++++++++++++---- checksum.h | 3 ++- tap.c | 3 ++- tcp.c | 24 +++--------------------- udp.c | 20 ++------------------ 5 files changed, 22 insertions(+), 45 deletions(-)

diff --git a/checksum.c b/checksum.c index 74e3742bc6f6..511b296a9a80 100644 --- a/checksum.c +++ b/checksum.c @@ -57,6 +57,7 @@ #include

#include "util.h" +#include "ip.h" #include "checksum.h"

/* Checksums are optional for UDP over IPv4, so we usually just set @@ -116,13 +117,21 @@ uint16_t csum_fold(uint32_t sum) uint16_t csum(const void *buf, size_t len, uint32_t init);

/** - * csum_ip4_header() - Calculate and set IPv4 header checksum + * csum_ip4_header() - Calculate IPv4 header checksum * @ip4h: IPv4 header

Function comment needs to be updated for the new parameters. In particular it needs to note that tot_len, saddr and daddr are all passed in network order.

As noted elsewhere, I kind of hate passing non-host-endian values in plain integer types, but I can see why doing otherwise here would be very awkward.

Perhaps we can use __be16 and __be32 types to really show the endianness in the code? Thanks, Laurent