[ovs-dev] [alpha fix 2/2] csum: Avoid misaligned data access.

Ben Pfaff blp at nicira.com
Tue Nov 16 22:50:05 UTC 2010


Thanks for the review

A "buildd" is Debian slang for a "build daemon" or an automatic build
machine, so that's fine.

On Tue, Nov 16, 2010 at 02:48:57PM -0800, Justin Pettit wrote:
> The series looks fine.  You've got double-Ds in "builds" in the commit comment.
> 
> --Justin
> 
> 
> On Nov 16, 2010, at 1:00 PM, Ben Pfaff wrote:
> 
> > This should fix a checksum test failure observed on Alpha in Debian's
> > buildds.
> > ---
> > lib/csum.c        |    7 ++++---
> > tests/test-csum.c |   13 ++++++++-----
> > 2 files changed, 12 insertions(+), 8 deletions(-)
> > 
> > diff --git a/lib/csum.c b/lib/csum.c
> > index 922f686..33ec28b 100644
> > --- a/lib/csum.c
> > +++ b/lib/csum.c
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2008, 2009 Nicira Networks.
> > + * Copyright (c) 2008, 2009, 2010 Nicira Networks.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> >  * you may not use this file except in compliance with the License.
> > @@ -16,6 +16,7 @@
> > 
> > #include <config.h>
> > #include "csum.h"
> > +#include "unaligned.h"
> > 
> > /* Returns the IP checksum of the 'n' bytes in 'data'.
> >  *
> > @@ -57,8 +58,8 @@ csum_continue(uint32_t partial, const void *data_, size_t n)
> > {
> >     const uint16_t *data = data_;
> > 
> > -    for (; n > 1; n -= 2) {
> > -        partial = csum_add16(partial, *data++);
> > +    for (; n > 1; n -= 2, data++) {
> > +        partial = csum_add16(partial, get_unaligned_u16(data));
> >     }
> >     if (n) {
> >         partial += *(uint8_t *) data;
> > diff --git a/tests/test-csum.c b/tests/test-csum.c
> > index fd16dd3..af6655e 100644
> > --- a/tests/test-csum.c
> > +++ b/tests/test-csum.c
> > @@ -22,6 +22,7 @@
> > #include <stdlib.h>
> > #include <string.h>
> > #include "random.h"
> > +#include "unaligned.h"
> > #include "util.h"
> > 
> > #undef NDEBUG
> > @@ -149,7 +150,7 @@ main(void)
> >         /* Test csum_add16(). */
> >         partial = 0;
> >         for (i = 0; i < tc->size / 2; i++) {
> > -            partial = csum_add16(partial, data16[i]);
> > +            partial = csum_add16(partial, get_unaligned_u16(&data16[i]));
> >         }
> >         assert(ntohs(csum_finish(partial)) == tc->csum);
> >         mark('.');
> > @@ -157,7 +158,7 @@ main(void)
> >         /* Test csum_add32(). */
> >         partial = 0;
> >         for (i = 0; i < tc->size / 4; i++) {
> > -            partial = csum_add32(partial, data32[i]);
> > +            partial = csum_add32(partial, get_unaligned_u32(&data32[i]));
> >         }
> >         assert(ntohs(csum_finish(partial)) == tc->csum);
> >         mark('.');
> > @@ -166,10 +167,12 @@ main(void)
> >         partial = 0;
> >         for (i = 0; i < tc->size / 4; i++) {
> >             if (i % 2) {
> > -                partial = csum_add32(partial, data32[i]);
> > +                partial = csum_add32(partial, get_unaligned_u32(&data32[i]));
> >             } else {
> > -                partial = csum_add16(partial, data16[i * 2]);
> > -                partial = csum_add16(partial, data16[i * 2 + 1]);
> > +                uint16_t u0 = get_unaligned_u16(&data16[i * 2]);
> > +                uint16_t u1 = get_unaligned_u16(&data16[i * 2 + 1]);
> > +                partial = csum_add16(partial, u0);
> > +                partial = csum_add16(partial, u1);
> >             }
> >         }
> >         assert(ntohs(csum_finish(partial)) == tc->csum);
> > -- 
> > 1.7.1
> > 
> > 
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
> 




More information about the dev mailing list