[ovs-dev] [PATCH 3/3] tests: Ignore options order in dhcpv4 ovn test.

[Balazs Nemeth]

>On 12/12/2016 13:14, "Ben Pfaff" <blp at ovn.org<mailto:blp at ovn.org>> wrote:
>
>>On Thu, Dec 08, 2016 at 06:50:32PM -0800, Daniele Di Proietto wrote:
>>> The order of the options in the packet generated by ovs-controller
>>> depends on the hash function.  I believe that murmur hash (our default)
>>> produces different outputs depending on the endianness of the system.
>>>
>>> This commit fixes the test by reordering the options in the packet
>>> before checking them.
>>>
>>> This was reported before as a failure of the test on x86 with sse42
>>> enabled, I'm fixing it now because it affects other targets build by
>>> distros by default.
>>>
>>> Reported-at: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=840770
>>> Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com<mailto:diproiettod at vmware.com>>
>>
>>I agree that this would solve the problem.
>>
>>I think that it might be nicer to instead sort the options in
>>build_dhcpv4_action() and build_dhcpv6_action() in ovn-northd.c.  Then
>>the generated logical actions won't differ from one architecture to
>>another, so that if we one day push a test to a higher level we won't
>>have an underlying source of differences.  There's even a helper
>>smap_sort() that could do most of the work.
>>
>>Are you willing to make that change?
>
>Sure, it's better than parsing TLVs with awk.  I'll send a v2.
>
>Thanks,
>
>Daniele
>
>>
>>Thanks,
>>
>>Ben.

Hi Daniele,

I applied this patch on top of branch-2.6 (which is including 1/3 and 2/3 patch). I built OVS with SSE4.2 flag. UT 2246 is still failing due to only 571st char of 'packets' and 'expected' files are compared and those are different in case of CRC32 hash. The following patch will fix the failing UT in case of build without any extra flag or with SSE4.2 as well:

diff --git a/tests/ovn.at b/tests/ovn.at
index 8b1228f..3f4c6cf 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -3724,8 +3724,8 @@ AT_CHECK([cat 1.packets | cut -c 53-570], [0], [expout])
# The ordering of the options in the packet depends on the hash function used
# and possibly on the endianness of the system.  We parse and sort both the
# expected and the generated packet.
-cat 1.expected | cut -c 571 | sort_dhcpv4opts_tlv > expout
-AT_CHECK([cat 1.packets | cut -c 571 | sort_dhcpv4opts_tlv], [0], [expout])
+cat 1.expected | cut -c 571- | sort_dhcpv4opts_tlv > expout
+AT_CHECK([cat 1.packets | cut -c 571- | sort_dhcpv4opts_tlv], [0], [expout])

# ovs-ofctl also resumes the packets and this causes other ports to receive
# the DHCP request packet. So reset the pcap files so that its easier to test.
@@ -3752,8 +3752,8 @@ AT_CHECK([cat 2.packets | cut -c 53-570], [0], [expout])
# The ordering of the options in the packet depends on the hash function used
# and possibly on the endianness of the system.  We parse and sort both the
# expected and the generated packet.
-cat 2.expected | cut -c 571 | sort_dhcpv4opts_tlv > expout
-AT_CHECK([cat 2.packets | cut -c 571 | sort_dhcpv4opts_tlv], [0], [expout])
+cat 2.expected | cut -c 571- | sort_dhcpv4opts_tlv > expout
+AT_CHECK([cat 2.packets | cut -c 571- | sort_dhcpv4opts_tlv], [0], [expout])

reset_pcap_file hv1-vif1 hv1/vif1
reset_pcap_file hv1-vif2 hv1/vif2