On Tue, Oct 18, 2022 at 05:01:01AM +0200, Stefano Brivio wrote:On Mon, 17 Oct 2022 19:57:54 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:Done.At least two places in passt calculate ICMPv6 checksums, ndp() and tap_ip_send(). Add a helper to handle this calculation in both places. For future flexibility, the new helper takes parameters for the fields in the IPv6 pseudo-header, so an IPv6 header or pseudo-header doesn't need to be explicitly constructed. It also allows the ICMPv6 header and payload to be in separate buffers, although we don't use this yet. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- checksum.c | 27 +++++++++++++++++++++++++++ checksum.h | 7 +++++++ ndp.c | 5 +---- tap.c | 6 ++---- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/checksum.c b/checksum.c index 56ad01e..0e207c8 100644 --- a/checksum.c +++ b/checksum.c @@ -52,6 +52,8 @@ #include <stddef.h> #include <stdint.h> +#include <linux/icmpv6.h> + /** * sum_16b() - Calculate sum of 16-bit words * @buf: Input buffer @@ -105,6 +107,31 @@ uint16_t csum_unaligned(const void *buf, size_t len, uint32_t init) return (uint16_t)~csum_fold(sum_16b(buf, len) + init); } +/** + * csum_icmp6() - Calculate checksum for an ICMPv6 packet"Calculate and set" ...?Done. Not sure why I did it that way in the first place.+ * @icmp6hr: ICMPv6 header, initialized apart from checksum + * @saddr: IPv6 source address + * @daddr: IPv6 destination address + * @payload: ICMP packet payload + * @len: Length of @payload (not including ICMPv6 header) + */ +void csum_icmp6(struct icmp6hdr *icmp6hr, + const struct in6_addr *saddr, + const struct in6_addr *daddr,I think: const struct in6_addr *saddr, const struct in6_addr *daddr, would be easier on eyes.Fair enough, done.+ const void *payload, + size_t len) +{ + /* Partial checksum for the pseudo-IPv6 header */ + uint32_t psum = sum_16b(saddr, sizeof(*saddr)) + + sum_16b(daddr, sizeof(*daddr)) + + htons(len + sizeof(*icmp6hr)) + htons(IPPROTO_ICMPV6);Maybe: uint32_t psum = sum_16b(saddr, sizeof(*saddr)) + sum_16b(daddr, sizeof(*daddr)) + htons(len + sizeof(*icmp6hr)) + htons(IPPROTO_ICMPV6); for me, it turns things from "sum a bunch of things" to "addresses and something else".-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson+ + icmp6hr->icmp6_cksum = 0; + /* Add in partial checksum for the ICMPv6 header alone */ + psum += sum_16b(icmp6hr, sizeof(*icmp6hr)); + icmp6hr->icmp6_cksum = csum_unaligned(payload, len, psum); +} + /** * csum_tcp4() - Calculate TCP checksum for IPv4 and set in place * @iph: Packet buffer, IP header diff --git a/checksum.h b/checksum.h index 5418406..2c72200 100644 --- a/checksum.h +++ b/checksum.h @@ -6,9 +6,16 @@ #ifndef CHECKSUM_H #define CHECKSUM_H +struct icmp6hdr; + uint32_t sum_16b(const void *buf, size_t len); uint16_t csum_fold(uint32_t sum); uint16_t csum_unaligned(const void *buf, size_t len, uint32_t init); +void csum_icmp6(struct icmp6hdr *ih, + const struct in6_addr *saddr, + const struct in6_addr *daddr, + const void *payload, + size_t len);It looks a bit like Haskell. ;) I would really use the horizontal space we have.void csum_tcp4(struct iphdr *iph); uint16_t csum(const void *buf, size_t len, uint32_t init); diff --git a/ndp.c b/ndp.c index dec36a9..03f1d06 100644 --- a/ndp.c +++ b/ndp.c @@ -189,10 +189,7 @@ dns_done: ip6hr->saddr = c->ip6.addr_ll; ip6hr->payload_len = htons(sizeof(*ihr) + len); - ip6hr->hop_limit = IPPROTO_ICMPV6; - ihr->icmp6_cksum = 0; - ihr->icmp6_cksum = csum_unaligned(ip6hr, sizeof(*ip6hr) + - sizeof(*ihr) + len, 0); + csum_icmp6(ihr, &ip6hr->saddr, &ip6hr->daddr, ihr + 1, len);Nice to see this all going away!ip6hr->version = 6; ip6hr->nexthdr = IPPROTO_ICMPV6; diff --git a/tap.c b/tap.c index 8b6d9bc..aafc92b 100644 --- a/tap.c +++ b/tap.c @@ -191,10 +191,8 @@ void tap_ip_send(const struct ctx *c, const struct in6_addr *src, uint8_t proto, } else if (proto == IPPROTO_ICMPV6) { struct icmp6hdr *ih = (struct icmp6hdr *)(ip6h + 1); - ih->icmp6_cksum = 0; - ih->icmp6_cksum = csum_unaligned(ip6h, - len + sizeof(*ip6h), - 0); + csum_icmp6(ih, &ip6h->saddr, &ip6h->daddr, + ih + 1, len - sizeof(*ih)); } ip6h->version = 6; ip6h->nexthdr = proto;