Re: [PATCH 02/14] Add csum_icmp4() helper for calculating ICMPv4 checksums

[Stefano Brivio <sbrivio@redhat.com>]

On Tue, 18 Oct 2022 23:06:11 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Tue, Oct 18, 2022 at 05:01:51AM +0200, Stefano Brivio wrote:
> > On Mon, 17 Oct 2022 19:57:55 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > Although tap_ip_send() is currently the only place calculating ICMPv4
> > > checksums, create a helper function for symmetry with ICMPv6.  For future
> > > flexibility it allows the ICMPv6 header and payload to be in separate
> > > buffers.
> > > 
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > ---
> > >  checksum.c | 15 +++++++++++++++
> > >  checksum.h |  2 ++
> > >  tap.c      |  4 +---
> > >  3 files changed, 18 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/checksum.c b/checksum.c
> > > index 0e207c8..c8b6b42 100644
> > > --- a/checksum.c
> > > +++ b/checksum.c
> > > @@ -52,6 +52,7 @@
> > >  #include <stddef.h>
> > >  #include <stdint.h>
> > >  
> > > +#include <linux/icmp.h>
> > >  #include <linux/icmpv6.h>
> > >  
> > >  /**
> > > @@ -107,6 +108,20 @@ uint16_t csum_unaligned(const void *buf, size_t len, uint32_t init)
> > >  	return (uint16_t)~csum_fold(sum_16b(buf, len) + init);
> > >  }
> > >  
> > > +/**
> > > + * csum_icmp4() - Calculate checksum for an ICMPv4 packet  
> > 
> > "Calculate and set"?  
> 
> Done.
> 
> > By the way, there's no such thing as ICMPv4 --
> > it's ICMP.  
> 
> Technically, yes, but I kind of wanted to make it clear at a glance
> that these are IPv4 specific functions.  I'd also like to avoid the
> implication that v4 is the "normal" sort.  I've changed from "ICMPv4"
> to "ICMP" in the comments, but I've left the '4's in the various names

Ah, yes, sure, makes sense, as long as we don't refer to "ICMPv4" in
the comments I'm fine with it. :)

-- 
Stefano