Take 2: new IP Checksum Code from DragonFlyBSD

Hi,

a few month ago I ported DragonFlyBSD's IP checksum code to
FreeBSD.
My first try wasn't complete and I had forgotten it by the
time. Now I
hope I have addressed the former mistakes and it maybe
included in the
tree. ;)

The patch can be found at
h
ttp://generic.0xfce3.net/20060524-ipchecksum.patch

This patch modifies the i386, amd64 and pc98 architectures.
Matt Dillons
new implementation is machine indepement, but there are some
asm
files/code with seems for me i386 centric. I don't know
assembler, so I
can't tell much about it.

I applied the patch to RELENG_6 and have done a simple
netperf
benchmark. The machine was a PIII 900.

The ministat output:
------------------------------------------------------------
----------------
x netperf-localhost-plain.txt
+ netperf-localhost-ncksum.txt
+-----------------------------------------------------------
---------------+
|      x     x     x      x        +             ++         
     +        |
|xx    x     x  xx xxx    x  x x  x+x   *+x x +  +++   ++  +
 + + +++++   +|
|        |__________M_A____________|       
|__________A___________|       |
+-----------------------------------------------------------
---------------+
    N           Min           Max        Median          
Avg        Stddev
x  22        707.31        730.41        717.37    
718.53909     6.7389076
+  22        725.57        746.46       736.535    
736.51727     6.3001188
Difference at 95.0% confidence
	17.9782 +/- 3.96904
	2.50205% +/- 0.552377%
	(Student's t, pooled s = 6.5232)
------------------------------------------------------------
----------------

Any comments, correctures are very appreciated.

best regards,

	Gordon

-- 
Gordon Bergling <GBergling at 0xfce3.net>	      http://www.0xFCE3.net/
PGP Fingerprint:  7732 9BB1 5013 AE8B E42C  28E0 93B9 D32B
C76F 02A0
RIPE-HDL: MDTP-RIPE				  "Minimal Electronic
Music"
_______________________________________________
freebsd-currentfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-curre
nt
To unsubscribe, send any mail to
"freebsd-current-unsubscribefreebsd.org"

Gordon Bergling wrote:
> Hi,
> 
> a few month ago I ported DragonFlyBSD's IP checksum
code to FreeBSD.
> My first try wasn't complete and I had forgotten it by
the time. Now I
> hope I have addressed the former mistakes and it maybe
included in the
> tree. ;)
> 
> The patch can be found at
> h
ttp://generic.0xfce3.net/20060524-ipchecksum.patch
> 
> This patch modifies the i386, amd64 and pc98
architectures. Matt Dillons
> new implementation is machine indepement, but there are
some asm
> files/code with seems for me i386 centric. I don't
know assembler, so I
> can't tell much about it.
> 
> I applied the patch to RELENG_6 and have done a simple
netperf
> benchmark. The machine was a PIII 900.
> 
> The ministat output:
>
------------------------------------------------------------
----------------
> x netperf-localhost-plain.txt
> + netperf-localhost-ncksum.txt
>
+-----------------------------------------------------------
---------------+
> |      x     x     x      x        +             ++    
          +        |
> |xx    x     x  xx xxx    x  x x  x+x   *+x x +  +++  
++  +  + + +++++   +|
> |        |__________M_A____________|       
|__________A___________|       |
>
+-----------------------------------------------------------
---------------+
>     N           Min           Max        Median        
  Avg        Stddev
> x  22        707.31        730.41        717.37    
718.53909     6.7389076
> +  22        725.57        746.46       736.535    
736.51727     6.3001188
> Difference at 95.0% confidence
> 	17.9782 +/- 3.96904
> 	2.50205% +/- 0.552377%
> 	(Student's t, pooled s = 6.5232)
>
------------------------------------------------------------
----------------
> 
> Any comments, correctures are very appreciated.
> 
> best regards,
> 
> 	Gordon
> 

First, it would be nice to know what netstat options you
were using. 
Second, it would be nice to know if hardware checksum
offloading was
enabled at all on either end of the test.

Scott

_______________________________________________
freebsd-currentfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-curre
nt
To unsubscribe, send any mail to
"freebsd-current-unsubscribefreebsd.org"

Hi Scott,

* Thus spake Scott Long (scottlsamsco.org):
> Gordon Bergling wrote:
> >a few month ago I ported DragonFlyBSD's IP
checksum code to FreeBSD.
> >My first try wasn't complete and I had forgotten
it by the time. Now I
> >hope I have addressed the former mistakes and it
maybe included in the
> >tree. ;)
> >
> >The patch can be found at
> >h
ttp://generic.0xfce3.net/20060524-ipchecksum.patch
> >
> >This patch modifies the i386, amd64 and pc98
architectures. Matt Dillons
> >new implementation is machine indepement, but there
are some asm
> >files/code with seems for me i386 centric. I don't
know assembler, so I
> >can't tell much about it.
> >
> >I applied the patch to RELENG_6 and have done a
simple netperf
> >benchmark. The machine was a PIII 900.
> >
> >The ministat output:
>
>--------------------------------------------------------
--------------------
> >x netperf-localhost-plain.txt
> >+ netperf-localhost-ncksum.txt
>
>+-------------------------------------------------------
-------------------+
> >|      x     x     x      x        +             ++
              +        
> >|
> >|xx    x     x  xx xxx    x  x x  x+x   *+x x + 
+++   ++  +  + + +++++   
> >+|
> >|        |__________M_A____________|       
|__________A___________|       
> >|
>
>+-------------------------------------------------------
-------------------+
> >    N           Min           Max        Median    
      Avg        Stddev
> >x  22        707.31        730.41        717.37    
718.53909     6.7389076
> >+  22        725.57        746.46       736.535    
736.51727     6.3001188
> >Difference at 95.0% confidence
> >	17.9782 +/- 3.96904
> >	2.50205% +/- 0.552377%
> >	(Student's t, pooled s = 6.5232)
>
>--------------------------------------------------------
--------------------
> >
> >Any comments, correctures are very appreciated.
> >
> >best regards,
> >
> >	Gordon
> >
> 
> First, it would be nice to know what netstat options
you were using. 
> Second, it would be nice to know if hardware checksum
offloading was
> enabled at all on either end of the test.

I don't know what do you mean with netstat options. If you
mean the
benchmark, that was the following procedure.

1. /usr/local/netperf/netserver
2. 
for i in 0 1 2 3 4 5 6 7 8 9 
do 
 /usr/local/netperf/netperf -P 0 >> ~/netperf-xyz.txt
done

Step 2 was done twice. I cutted the 'Throughput' fields
out of the
netperf-xyz.txt, for use with ministat. The nic I had used
is a simple

| fxp0: <Intel 82550 Pro/100 Ethernet> port ...

I havn't enabled any hardware checksum offloading as far as
I
know.

Please let me know, if there are other questions left...


best regards,

	Gordon

-- 
Gordon Bergling <GBergling at 0xfce3.net>	      http://www.0xFCE3.net/
PGP Fingerprint:  7732 9BB1 5013 AE8B E42C  28E0 93B9 D32B
C76F 02A0
RIPE-HDL: MDTP-RIPE				  "Minimal Electronic
Music"
_______________________________________________
freebsd-currentfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-curre
nt
To unsubscribe, send any mail to
"freebsd-current-unsubscribefreebsd.org"

On Wednesday 24 May 2006 20:08, Gordon Bergling wrote:
> a few month ago I ported DragonFlyBSD's IP checksum
code to FreeBSD.
> My first try wasn't complete and I had forgotten it by
the time. Now I
> hope I have addressed the former mistakes and it maybe
included in the
> tree. ;)
>
> The patch can be found at
> h
ttp://generic.0xfce3.net/20060524-ipchecksum.patch
>
> This patch modifies the i386, amd64 and pc98
architectures. Matt Dillons
> new implementation is machine indepement, but there are
some asm
> files/code with seems for me i386 centric. I don't
know assembler, so I
> can't tell much about it.

I'm a little scared about this.  We have had several
problems in the 
checksumming code that were due to -O2 or -O0 that screwed
up just a little 
of the assembly and *boom* everything went downhill.  I do
appreciate you 
stepping up porting something useful, but I'd be more
comfortable if you 
actually knew what you are doing.

That said, can somebody with real assembly knowledge and
maybe even for 
sparc64 or the like step up and take a look at this?  I
think we should not 
import something unless we fully understand it!

I am confident that Matt knew what he was doing, but I am
not sure that the 
efforts on Dragonfly are compatible with !i386/amd64 ...
FreeBSD, however 
still cares about big endian architectures and others,
don't we?

> I applied the patch to RELENG_6 and have done a simple
netperf
> benchmark. The machine was a PIII 900.
>
> The ministat output:
>
------------------------------------------------------------
---------------
>- x netperf-localhost-plain.txt
> + netperf-localhost-ncksum.txt
>
+-----------------------------------------------------------
---------------
>+
>
> |      x     x     x      x        +             ++    
          +       
> | | xx    x     x  xx xxx    x  x x  x+x   *+x x +  +++
  ++  +  + + +++++ 
> |  +|
> |
> |        |__________M_A____________|       
|__________A___________|      
> |        | |
>
>
+-----------------------------------------------------------
---------------
>+ N           Min           Max        Median          
Avg        Stddev x 
> 22        707.31        730.41        717.37    
718.53909     6.7389076 + 
> 22        725.57        746.46       736.535    
736.51727     6.3001188
> Difference at 95.0% confidence
> 	17.9782 +/- 3.96904
> 	2.50205% +/- 0.552377%
> 	(Student's t, pooled s = 6.5232)
>
------------------------------------------------------------
---------------
>-
>
> Any comments, correctures are very appreciated.
>
> best regards,
>
> 	Gordon

-- 
/"\  Best regards,                      | mlaierfreebsd.org
\ /  Max Laier                          | ICQ #67774661
 X   http://pf4freebsd.l
ove2party.net/  | mlaierEFnet
/ \  ASCII Ribbon Campaign              | Against HTML Mail
and News

* Thus spake Max Laier (maxlove2party.net):
> On Wednesday 24 May 2006 20:08, Gordon Bergling wrote:
> > a few month ago I ported DragonFlyBSD's IP
checksum code to FreeBSD.
> > My first try wasn't complete and I had forgotten
it by the time. Now I
> > hope I have addressed the former mistakes and it
maybe included in the
> > tree. ;)
> >
> > The patch can be found at
> > h
ttp://generic.0xfce3.net/20060524-ipchecksum.patch
> >
> > This patch modifies the i386, amd64 and pc98
architectures. Matt Dillons
> > new implementation is machine indepement, but
there are some asm
> > files/code with seems for me i386 centric. I
don't know assembler, so I
> > can't tell much about it.
> 
> I'm a little scared about this.  We have had several
problems in the 
> checksumming code that were due to -O2 or -O0 that
screwed up just a little 
> of the assembly and *boom* everything went downhill.  I
do appreciate you 
> stepping up porting something useful, but I'd be more
comfortable if you 
> actually knew what you are doing.
> 
> That said, can somebody with real assembly knowledge
and maybe even for 
> sparc64 or the like step up and take a look at this?  I
think we should not 
> import something unless we fully understand it!
> 
> I am confident that Matt knew what he was doing, but I
am not sure that the 
> efforts on Dragonfly are compatible with !i386/amd64
... FreeBSD, however 
> still cares about big endian architectures and others,
don't we?

Even if DragonFly implementation isn't compatible with
archs ! i386 and
derivates isn't it still an improvement on what we have
now?

My intention of porting it was the following comment in
sys/i386/i386/in_cksum.c

 | * This routine is very heavily used in the network
 | * code and should be modified for each CPU to be as fast
as
 | * possible.

So having a better situated implementation is still an
improved. The
patch doesn't touch any arch !i386 and derivates, so I
don't see any reason
why it shouldn't be included.

best regards,

	Gordon

-- 
Gordon Bergling <GBergling at 0xfce3.net>	      http://www.0xFCE3.net/
PGP Fingerprint:  7732 9BB1 5013 AE8B E42C  28E0 93B9 D32B
C76F 02A0
RIPE-HDL: MDTP-RIPE				  "Minimal Electronic
Music"
_______________________________________________
freebsd-currentfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-curre
nt
To unsubscribe, send any mail to
"freebsd-current-unsubscribefreebsd.org"

On Thu, 2006-May-25 12:40:00 +0200, Gordon Bergling wrote:
>* Thus spake Max Laier (maxlove2party.net):
>> I'm a little scared about this.  We have had
several problems in the 
>> checksumming code that were due to -O2 or -O0 that
screwed up just a little 

> | * This routine is very heavily used in the network
> | * code and should be modified for each CPU to be as
fast as
> | * possible.

But _correct_ code is far more important.  And I'm not sure
that comment
is still as relevant as it used to be - most (if not all)
gigabit NICs
have checksum offloading and processors are fast enough that
generic
checksum code should be "good enough" for most
lesser purposes.

>So having a better situated implementation is still an
improved. The
>patch doesn't touch any arch !i386 and derivates, so I
don't see any reason
>why it shouldn't be included.

You stated that you don't understand the code.  IP
checksumming is a
crucial part of the kernel so the Project needs to be very
careful about
making changes.  As Max pointed out, there have been past
situations
where the checksum code revealed gcc optimiser bugs so any
change has
to be checked using a variety of compiler flags.

The justification needs to be along the lines of
"Matt's code is better
than the existing code because it does ... instead of ...
and this means
it makes better use of ...".   I respect Matt's
technical skills so I am
confident that this code works more efficiently in DFBSD but
that doesn't
mean that it's a drop-in replacement for the equivalent
code in FreeBSD.

-- 
Peter Jeremy
_______________________________________________
freebsd-currentfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-curre
nt
To unsubscribe, send any mail to
"freebsd-current-unsubscribefreebsd.org"

Quoting Peter Jeremy <peterjeremyoptushome.com.au> (Thu,
25 May 2006 21:54:48 +1000):

> On Thu, 2006-May-25 12:40:00 +0200, Gordon Bergling
wrote:
> >* Thus spake Max Laier (maxlove2party.net):
> >> I'm a little scared about this.  We have had
several problems in the 
> >> checksumming code that were due to -O2 or -O0
that screwed up just a little 
> 
> > | * This routine is very heavily used in the
network
> > | * code and should be modified for each CPU to be
as fast as
> > | * possible.
> 
> But _correct_ code is far more important.  And I'm not
sure that comment
> is still as relevant as it used to be - most (if not
all) gigabit NICs
> have checksum offloading and processors are fast enough
that generic
> checksum code should be "good enough" for
most lesser purposes.

I don't comment on the GB-NIC part (except for: not
everything is GB
today...), but regarding the "correct" part
above: sorry, but that's
exactly the reason why I put up the entry for porting this
on our ideas
page (it never showed up there, since Gordon took it before
the page
went life). More below.

> >So having a better situated implementation is still
an improved. The
> >patch doesn't touch any arch !i386 and derivates,
so I don't see any reason
> >why it shouldn't be included.
> 
> You stated that you don't understand the code.  IP
checksumming is a
> crucial part of the kernel so the Project needs to be
very careful about
> making changes.  As Max pointed out, there have been
past situations
> where the checksum code revealed gcc optimiser bugs so
any change has
> to be checked using a variety of compiler flags.

These are not bugs in the optimizer, but mostly bugs in our
assembly
code. I had the same problems with the Intel C compiler. The
guys at
Intel had a look at the code and told me that it's it no
the fault of
the compiler, but the code is not done right. Therefore I
switched to
the plain-C version when icc is used to compile the kernel.

Here's a short discussion on our mailinglists where Matt
tells a little
bit more about it:
 http://lists.freebsd.org/pipermail/freebsd-
arch/2004-June/002271.html

Basically most of our "huge assembly one" is
reduced to a faster
"nearly everything is plain-C and only a small part is
done in
assembly" version.

Bye,
Alexander.

-- 
Selling GoodYear Eagle F1 235/40ZR18, 2x 4mm + 2x 5mm, ~150
EUR
you have to pick it up between Germany/Saarland and
Luxembourg/Capellen
http://www.Leidinger.net
   Alexander  Leidinger.net: PGP ID = B0063FE7
http://www.FreeBSD.org 
     netchild  FreeBSD.org  : PGP ID = 72077137
_______________________________________________
freebsd-currentfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-curre
nt
To unsubscribe, send any mail to
"freebsd-current-unsubscribefreebsd.org"

Peter Jeremy writes:
 > On Thu, 2006-May-25 12:40:00 +0200, Gordon Bergling
wrote:
 > >* Thus spake Max Laier (maxlove2party.net):
 > >> I'm a little scared about this.  We have had
several problems in the 
 > >> checksumming code that were due to -O2 or -O0
that screwed up just a little 
 > 
 > > | * This routine is very heavily used in the
network
 > > | * code and should be modified for each CPU to
be as fast as
 > > | * possible.
 > 
 > But _correct_ code is far more important.  And I'm
not sure that comment
 > is still as relevant as it used to be - most (if not
all) gigabit NICs
 > have checksum offloading and processors are fast
enough that generic
 > checksum code should be "good enough" for
most lesser purposes.

The benchmark quoted in the original post is interesting in
that it is
the best example of where improving the checksumming code
would help,
yet we really should not be checksumming packets sent across
lo0
anyway.

If we're going to do anything,  I'd prefer to see us skip
the checksum on everything sent across lo0 and stick with
the slower, yet known to work, existing checksum code for
slow interfaces.

Drew



_______________________________________________
freebsd-currentfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-curre
nt
To unsubscribe, send any mail to
"freebsd-current-unsubscribefreebsd.org"

Quoting Andrew Gallatin <gallatincs.duke.edu> (Thu, 25
May 2006 12:20:17 -0400 (EDT)):

> If we're going to do anything,  I'd prefer to see us
skip
> the checksum on everything sent across lo0 and stick
with
> the slower, yet known to work, existing checksum code
for
> slow interfaces.

The current code is known to work with the current gcc we
use. It is
known to *not* work with the Intel C compiler. It may or may
not work
with an upcomming gcc version.

The current code is a maze of assembly and macros, the new
one is
straight forward C and a little bit of assembly. And the new
one is
also known to work in DragonFlyBSD. Do you expect *this*
code to act
differently between FreeBSD and DragonFlyBSD?

What's the technical backing of your preference to stick
with the
current code? How does the technical backing of your
preference compare
to the technical arguments I presented in this thread
regarding the
priority of the arguments?

Bye,
Alexander.

-- 
Selling GoodYear Eagle F1 235/40ZR18, 2x 4mm + 2x 5mm, ~150
EUR
you have to pick it up between Germany/Saarland and
Luxembourg/Capellen
http://www.Leidinger.net
   Alexander  Leidinger.net: PGP ID = B0063FE7
http://www.FreeBSD.org 
     netchild  FreeBSD.org  : PGP ID = 72077137
_______________________________________________
freebsd-currentfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-curre
nt
To unsubscribe, send any mail to
"freebsd-current-unsubscribefreebsd.org"

On Thu, 2006-May-25 12:40:00 +0200, Gordon Bergling wrote:  
                 
>patch doesn't touch any arch !i386 and derivates, so I
don't see any reason  
>why it shouldn't be included.                          
                     

On Fri, 2006-May-26 13:30:17 +0200, Alexander Leidinger
wrote:
>The current code is a maze of assembly and macros, the
new one is
>straight forward C and a little bit of assembly. And the
new one is
>also known to work in DragonFlyBSD. Do you expect *this*
code to act
>differently between FreeBSD and DragonFlyBSD?

I don't expect the code itself to act differently.  But I
don't know
if FreeBSD and DragonFlyBSD have different expectations of
the code -
probably they don't but someone (the proponent of the
change) needs to
confirm this.

>What's the technical backing of your preference to
stick with the
>current code? How does the technical backing of your
preference compare
>to the technical arguments I presented in this thread
regarding the
>priority of the arguments?

I was responding to Gordon's comments above.  If the code
is better and
there _are_ technical arguments for FreeBSD to use it, then
we should.
"I don't see any reason not to use it" is not
justification for changing
critical code.

-- 
Peter Jeremy
_______________________________________________
freebsd-currentfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-curre
nt
To unsubscribe, send any mail to
"freebsd-current-unsubscribefreebsd.org"

Alexander Leidinger writes:
 > Quoting Andrew Gallatin <gallatincs.duke.edu> (Thu, 25 May 2006 12:20:17 -0400
(EDT)):
 > 
 > > If we're going to do anything,  I'd prefer to
see us skip
 > > the checksum on everything sent across lo0 and
stick with
 > > the slower, yet known to work, existing checksum
code for
 > > slow interfaces.
 > 
 > The current code is known to work with the current gcc
we use. It is
 > known to *not* work with the Intel C compiler. It may
or may not work
 > with an upcomming gcc version.
 > 
 > The current code is a maze of assembly and macros, the
new one is
 > straight forward C and a little bit of assembly. And
the new one is
 > also known to work in DragonFlyBSD. Do you expect
*this* code to act
 > differently between FreeBSD and DragonFlyBSD?

If it is is really known to work and it is higher quality,
then it is
fine with me.  The original poster presented the new code as
a
performance enhancement over the old.  My position is simply
that that
the performance of the software checksumming code is
irrelevant, as it
should never be used in a situation where performace is
important. All
1Gb and 10Gb nics do hardware checksum offload, and we
should not be
checksumming local traffic.

Drew
_______________________________________________
freebsd-currentfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-curre
nt
To unsubscribe, send any mail to
"freebsd-current-unsubscribefreebsd.org"

Quoting Peter Jeremy <peterjeremyoptushome.com.au> (Sat,
27 May 2006 06:14:52 +1000):

> On Thu, 2006-May-25 12:40:00 +0200, Gordon Bergling
wrote:                    
> >patch doesn't touch any arch !i386 and derivates,
so I don't see any reason  
> >why it shouldn't be included.                     
                          
> 
> On Fri, 2006-May-26 13:30:17 +0200, Alexander Leidinger
wrote:
> >The current code is a maze of assembly and macros,
the new one is
> >straight forward C and a little bit of assembly.
And the new one is
> >also known to work in DragonFlyBSD. Do you expect
*this* code to act
> >differently between FreeBSD and DragonFlyBSD?
> 
> I don't expect the code itself to act differently. 
But I don't know
> if FreeBSD and DragonFlyBSD have different expectations
of the code -
> probably they don't but someone (the proponent of the
change) needs to
> confirm this.

They feed the same input to the code and expect the same
output as we
do.

> >What's the technical backing of your preference to
stick with the
> >current code? How does the technical backing of
your preference compare
> >to the technical arguments I presented in this
thread regarding the
> >priority of the arguments?
> 
> I was responding to Gordon's comments above.  If the
code is better and
> there _are_ technical arguments for FreeBSD to use it,
then we should.

It contains less buggy assembly code which may break with
newer gcc
optimizations and already breaks with existing optimizations
in the
Intel C compiler. The folks at Intel investigated it and
told me it's
not because of a bug in icc, but because of the assembly
code. It
doesn't tell the compiler the right things, so the compiler
is using
wrong invariants for some optimizations (the gcc version we
use either
doesn't do those optimizations (yet), or does not make full
use of
those invariants (yet)).

Bye,
Alexander.

-- 
Selling GoodYear Eagle F1 235/40ZR18, 2x 4mm + 2x 5mm, ~150
EUR
you have to pick it up between Germany/Saarland and
Luxembourg/Capellen
http://www.Leidinger.net
   Alexander  Leidinger.net: PGP ID = B0063FE7
http://www.FreeBSD.org 
     netchild  FreeBSD.org  : PGP ID = 72077137
_______________________________________________
freebsd-currentfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-curre
nt
To unsubscribe, send any mail to
"freebsd-current-unsubscribefreebsd.org"