On Fri, Aug 31, 2012 at 10:44:20PM +1200, Joe Stringer wrote:
> Signed-off-by: Joe Stringer <j...@wand.net.nz>
I'm so sorry about the delay in review, which really was unreasonable.
I have excuses but you probably aren't interested in hearing them. I
hope you're still interested in this patch series.
"sparse" reports a potential byte-swapping error:
../lib/packets.c:474:23: warning: incorrect type in assignment (different base
types)
../lib/packets.c:474:23: expected restricted __be32 [usertype] sctp_csum
../lib/packets.c:474:23: got unsigned int
../lib/packets.c:569:19: warning: incorrect type in assignment (different base
types)
../lib/packets.c:569:19: expected restricted __be32 [usertype] sctp_csum
../lib/packets.c:569:19: got unsigned int
It looks to me like the SCTP checksum is getting computed across the
entire packet, including the Ethernet header. I doubt that that can
be correct. Can you take a look?
Also, for IP-like checksums we're able to do an incremental update of
the checksum, instead of a wholesale replacement, which means that any
error in the original checksum will be retained, so that the packet
will (correctly) be discarded when it arrives at its destination. To
preserve that property, I think we'd have to check the checksum before
we update it. We could do something like this:
old_csum = sctp_hdr->sctp_csum;
sctp_hdr->sctp_csum = 0;
old_correct_csum = ...calculate checksum...;
...change the header...
new_csum = ...calculate checksum...;
sctp_hdr->sctp_csum = old_csum ^ old_correct_csum ^ new_csum;
so that the behavior is quite similar to that for TCP and UDP,
although much more expensive.
Otherwise this looks good to me.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
