[ovs-dev] [PATCH 2/4] dpif-netdev: Add SCTP support
joe at wand.net.nz
Thu Oct 11 09:08:56 UTC 2012
Thanks for the review, Ben. This looks like good reference for the next version.
I'm putting this on the shelf for the moment, due to other priorities.
I hope to return early November.
On 2 October 2012 06:15, Ben Pfaff <blp at nicira.com> wrote:
> On Tue, Oct 02, 2012 at 12:44:53AM +1300, Joe Stringer wrote:
>> On 28 September 2012 04:53, Ben Pfaff <blp at 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.
> 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.
More information about the dev