On Tue, Oct 02, 2012 at 12:44:53AM +1300, Joe Stringer wrote: > On 28 September 2012 04:53, Ben Pfaff <b...@nicira.com> wrote: > > "sparse" reports a potential byte-swapping error: > > I would imagine this is because the CRC32C implementation I introduced > in patch #1 purports to calculate checksums in HBO. Perhaps that > should be modified to use ovs_be32?
If the CRC32C implementation returns a network-byte-order value, then yes it should return it as an ovs_be32. > > 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? > > I wasn't 100% confident that I understood how the function was > interacting with the ofpbuf struct, so it's likely I've simply got it > wrong. With a fresh look it seems like I should actually be using *sh > to calculate the checksum. I'll make the modification for round two. "*sh"? I'd guess that the correct place to start the checksum is the l4 pointer, but I haven't read the SCTP spec. > Do we currently have code that triggers this? I take it that this type > of recalculation is required when we modify the packet, eg set-field > action. What I'd like to know is how I might test that the behaviour > is correct. There's no test code right now that modifies the checksum of an SCTP packet. I'd suggest using set-field actions of the SCTP source and dest ports to trigger it. You could use a tool like Wireshark to verify that the modified packet was correct. Once you have manually constructed a test case that you know is correct, we should add that to the unit tests so that it stays correct. There are existing tests that run packets through the switch; you should be able to follow that model. > One idea perhaps would be to place OVS between two hosts and make it > change the destination port on flows in one direction, then using > lksctp, see whether the hosts continue to communicate correctly. This > has been my general approach to testing this patchset. The downside is > that it's a lot of overhead to set up, unlike the autotests. That's a good thing to try once, as part of constructing an automatic test case. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
