[ovs-discuss] ipam allows address conflicts?

[Ben Pfaff]

[adding some folks who've worked on IPAM lately]

On Tue, Mar 05, 2019 at 10:26:58AM +0000, James Page wrote:
> Test 2768 is consistently failing on s390x in Ubuntu development.
> 
> Note that this is the only big-endian architecture we build for so I would
> suspect something endian-y as the root cause.

Yes, it's related to big-endian, in particular related to the OVS hash
function, which produces different results depending on endianness.
I can reproduce it on amd64 by changing the hash function:

----------------------------------------------------------------------
diff --git a/lib/hash.h b/lib/hash.h
index eb3776500abe..c7fc6430c71d 100644
--- a/lib/hash.h
+++ b/lib/hash.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2012, 2013, 2014, 2016 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2012, 2013, 2014, 2016, 2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -87,6 +87,7 @@ static inline uint32_t mhash_finish(uint32_t hash)
     hash ^= hash >> 13;
     hash *= 0xc2b2ae35;
     hash ^= hash >> 16;
+    hash = ~hash;
     return hash;
 }
 
@@ -184,7 +185,7 @@ static inline uint32_t hash_finish(uint64_t hash, uint64_t final)
     /* The finishing multiplier 0x805204f3 has been experimentally
      * derived to pass the testsuite hash tests. */
     hash = _mm_crc32_u64(hash, final) * 0x805204f3;
-    return hash ^ (uint32_t)hash >> 16; /* Increase entropy in LSBs. */
+    return ~(hash ^ (uint32_t)hash >> 16); /* Increase entropy in LSBs. */
 }
 
 /* Returns the hash of the 'n' 32-bit words at 'p_', starting from 'basis'.
----------------------------------------------------------------------

The actual failure might be due to a more serious bug though.  When I
apply the following patch:

----------------------------------------------------------------------
diff --git a/tests/ovn.at b/tests/ovn.at
index 2af225a678e1..a53204f544e9 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -12273,6 +12273,8 @@ for i in 2 3 4; do
     ovn-nbctl --wait=sb lsp-set-addresses swp$i "02:00:00:00:00:0$i dynamic"
     cidr=$(ovn-nbctl get logical_switch_port swp$i dynamic_addresses |cut -f2 -d' '|cut -f1 -d\")
     ovn-nbctl lrp-add ro$i rop$i 02:00:00:00:00:0$i $cidr/24 -- set logical_switch_port swp$i type=router options:router-port=rop$i addresses=router;
+    printf "\nadded swp$i\n"
+    ovn-nbctl list logical_switch_port
     AT_CHECK_UNQUOTED([ovn-nbctl get logical_router_port rop$i networks], [0], [[["192.168.1.$i/24"]]
 ])
 done
----------------------------------------------------------------------

the test then reports that ports swp2 and swp3 are both being assigned
dynamic address 192.168.1.2 (instead of 192.168.1.2 and 192.168.1.3,
respectively), as you can see from the "dynamic_addresses" lines
following "added swp3":

----------------------------------------------------------------------
2772. ovn.at:12264: testing ovn -- ipam router ports ...
creating ovn-sb database
creating ovn-nb database
starting ovn-northd
starting backup ovn-northd

added swp2
_uuid               : d78add30-9d8e-4d49-bea6-8f4bc947ab3b
addresses           : [router]
dhcpv4_options      : []
dhcpv6_options      : []
dynamic_addresses   : "02:00:00:00:00:02 192.168.1.2"
enabled             : []
external_ids        : {}
name                : "swp2"
options             : {router-port="rop2"}
parent_name         : []
port_security       : []
tag                 : []
tag_request         : []
type                : router
up                  : true
../../tests/ovn.at:12278: ovn-nbctl get logical_router_port rop$i networks

added swp3
_uuid               : d78add30-9d8e-4d49-bea6-8f4bc947ab3b
addresses           : [router]
dhcpv4_options      : []
dhcpv6_options      : []
dynamic_addresses   : "02:00:00:00:00:02 192.168.1.2"
enabled             : []
external_ids        : {}
name                : "swp2"
options             : {router-port="rop2"}
parent_name         : []
port_security       : []
tag                 : []
tag_request         : []
type                : router
up                  : true

_uuid               : 0542f623-43a8-407d-adbe-0927999c78b1
addresses           : [router]
dhcpv4_options      : []
dhcpv6_options      : []
dynamic_addresses   : "02:00:00:00:00:03 192.168.1.2"
enabled             : []
external_ids        : {}
name                : "swp3"
options             : {router-port="rop3"}
parent_name         : []
port_security       : []
tag                 : []
tag_request         : []
type                : router
up                  : true
../../tests/ovn.at:12278: ovn-nbctl get logical_router_port rop$i networks
--- -   2019-03-05 17:58:01.066306125 -0800
+++ /home/blp/nicira/ovs/_build/tests/testsuite.dir/at-groups/2772/stdout       2019-03-05 17:58:01.060040470 -0800
@@ -1,2 +1,2 @@
-["192.168.1.3/24"]
+["192.168.1.2/24"]
 
----------------------------------------------------------------------

Mark and Lorenzo, this seems like something you should look into?

Thanks,

Ben.