Re: unused in_cksum_update() ?

From: Ulrich Spörlein <uqs_at_FreeBSD.org>
Date: Mon, 27 Jan 2014 22:09:37 +0100
On Fri, 2014-01-10 at 19:24:48 +0100, Luigi Rizzo wrote:
> On Fri, Jan 10, 2014 at 02:31:40PM +0400, Gleb Smirnoff wrote:
> > On Thu, Jan 09, 2014 at 08:21:14PM +0100, Luigi Rizzo wrote:
> > L> a lot of arch-specific headers (sys/${ARCH}/include/in_cksum.h)
> > L> have a lengthy definition for
> > L> 
> > L> 	in_cksum_update(struct ip *ip)
> > L> 
> > L> which seems completely unused in our source tree.
> > L> Time to remove it perhaps ?
> > L> 
> > L> grep cannot find any use at least since stable/8
> > 
> > I'd prefer not to hurry with its removal. Might be that pf will use it.
> > Since it lives in a header file, it doesn't add a single bit to kernel
> > size.
> 
> we should care more about obfuscation and correcteness, and this is
> a killer in both respects.
> Depending on $arch the function is not even available or wrong:
> 
> In particular, the basic code follows the description in
> http://tools.ietf.org/html/rfc1141A with ntohs/htons to deal
> with endianness (note that the '256' should not be converted):
> 
> 	tmp = ntohs(sum)+256;
> 	tmp = tmp + (tmp >> 16);
> 	sum = htons(tmp); // also truncates high bits
> 
> It is correctly implemented (but in a totally generic way, so no
> point to have it in the arch-specific files) for  amd64, i386,
> ia64, mips, powerpc; it is not implemented for arm, and it is wrong
> for sparc64 (where the 256 is incorrectly replaced by a 1).
> 
> In terms of usage: the svn repo suggests that it was added in r15884
> in 1996 (stable/2.2 is the first branch where it appears):
> 
> http://svnweb.freebsd.org/base/head/sys/i386/include/in_cksum.h?r1=15884&r2=15883&pathrev=15884
> 
> as far as i can tell never used anywhere, and copied from place to
> place when we started to support different architectures.
> 
> Shall we wait until it becomes 18 ? :)
> 
> I am adding Garret to the list as he may have more details.

Git's "pickaxe" is a very good tool for this sort of code archeology.
There's only a handful of commits that touched anything related to
"in_cksum_update".

I'm not going to dump the output of git log -S"in_cksum_update" here,
just the revisions that add/remove that string.

r15884
r36849
r66458
r86144
r99040
r158458
r163022
r178172
r180010

hth
Uli
Received on Mon Jan 27 2014 - 20:09:41 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:46 UTC