[ovs-dev] [PATCH v3] ovn: Detect and prevent duplicate address assignments.

Mark Michelson mmichels at redhat.com
Thu Sep 6 20:01:38 UTC 2018


This patch alters the 'ovn-nbctl lsp-set-addresses' command to check if
the IP addresses being added are duplicates of already-set IP addresses.
Test cases have been added for this detection.

This patch also adds a warning message to ovn-northd if duplicate IPv4
addresses are detected on a switch.

Signed-off-by: Mark Michelson <mmichels at redhat.com>
---
v2 -> v3:
 * Removed checks for duplicate MACs from ovn-nbctl. Based on tests
   for ovn-controller-vtep, it is legitimate to add multiple switch
   ports (on separate switches) with duplicate MACs. The duplicate
   address checking code has been updated as well.
 * Dynamic addressing test was updated since it attempted a test that
   now is invalid

v1 -> v2:
 Fixed sparse warning from using htons() instead of htonl()
--- 
 ovn/northd/ovn-northd.c   |  6 ++++
 ovn/utilities/ovn-nbctl.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/ovn.at              | 69 ++++++++++++++++++++++++++++++++++------
 3 files changed, 146 insertions(+), 10 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 72e25181d..31ea5f410 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -944,6 +944,12 @@ ipam_insert_ip(struct ovn_datapath *od, uint32_t ip)
 
     if (ip >= od->ipam_info.start_ipv4 &&
         ip < (od->ipam_info.start_ipv4 + od->ipam_info.total_ipv4s)) {
+        if (bitmap_is_set(od->ipam_info.allocated_ipv4s,
+                          ip - od->ipam_info.start_ipv4)) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+            VLOG_WARN_RL(&rl, "Duplicate IP set on switch %s: "IP_FMT,
+                         od->nbs->name, IP_ARGS(htonl(ip)));
+        }
         bitmap_set1(od->ipam_info.allocated_ipv4s,
                     ip - od->ipam_info.start_ipv4);
     }
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 4f7e7241a..eabd30308 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -1437,6 +1437,74 @@ nbctl_lsp_get_tag(struct ctl_context *ctx)
     }
 }
 
+static char *
+lsp_contains_duplicate_ip(struct lport_addresses *laddrs1,
+                          struct lport_addresses *laddrs2)
+{
+    for (size_t i = 0; i < laddrs1->n_ipv4_addrs; i++) {
+        for (size_t j = 0; j < laddrs2->n_ipv4_addrs; j++) {
+            if (laddrs1->ipv4_addrs[i].addr == laddrs2->ipv4_addrs[j].addr) {
+                return xasprintf("duplicate IPv4 address %s",
+                                 laddrs1->ipv4_addrs[i].addr_s);
+            }
+        }
+    }
+
+    for (size_t i = 0; i < laddrs1->n_ipv6_addrs; i++) {
+        for (size_t j = 0; j < laddrs2->n_ipv6_addrs; j++) {
+            if (IN6_ARE_ADDR_EQUAL(&laddrs1->ipv6_addrs[i].addr,
+                                   &laddrs2->ipv6_addrs[j].addr)) {
+                return xasprintf("duplicate IPv6 address %s",
+                                 laddrs1->ipv6_addrs[i].addr_s);
+            }
+        }
+    }
+
+    return NULL;
+}
+
+static char *
+lsp_contains_duplicates(const struct nbrec_logical_switch *ls,
+                        const struct nbrec_logical_switch_port *lsp,
+                        const char *address)
+{
+    struct lport_addresses laddrs;
+    if (!extract_lsp_addresses(address, &laddrs)) {
+        return NULL;
+    }
+
+    char *sub_error = NULL;
+    for (size_t i = 0; i < ls->n_ports; i++) {
+        struct nbrec_logical_switch_port *lsp_test = ls->ports[i];
+        if (lsp_test == lsp) {
+            continue;
+        }
+        for (size_t j = 0; j < lsp_test->n_addresses; j++) {
+            struct lport_addresses laddrs_test;
+            char *addr = lsp_test->addresses[j];
+            if (is_dynamic_lsp_address(addr)) {
+                addr = lsp_test->dynamic_addresses;
+            }
+            if (extract_lsp_addresses(addr, &laddrs_test)) {
+                sub_error = lsp_contains_duplicate_ip(&laddrs, &laddrs_test);
+                destroy_lport_addresses(&laddrs_test);
+                if (sub_error) {
+                    goto err_out;
+                }
+            }
+        }
+    }
+
+err_out: ;
+    char *error = NULL;
+    if (sub_error) {
+        error = xasprintf("Error on switch %s: %s", ls->name, sub_error);
+        free(sub_error);
+    }
+    destroy_lport_addresses(&laddrs);
+    return error;
+}
+
 static void
 nbctl_lsp_set_addresses(struct ctl_context *ctx)
 {
@@ -1449,6 +1517,13 @@ nbctl_lsp_set_addresses(struct ctl_context *ctx)
         return;
     }
 
+    const struct nbrec_logical_switch *ls;
+    error = lsp_to_ls(ctx->idl, lsp, &ls);
+    if (error) {
+        ctx->error = error;
+        return;
+    }
+
     int i;
     for (i = 2; i < ctx->argc; i++) {
         struct eth_addr ea;
@@ -1463,6 +1538,12 @@ nbctl_lsp_set_addresses(struct ctl_context *ctx)
                       "argument.", ctx->argv[i]);
             return;
         }
+
+        error = lsp_contains_duplicates(ls, lsp, ctx->argv[i]);
+        if (error) {
+            ctl_error(ctx, "%s", error);
+            return;
+        }
     }
 
     nbrec_logical_switch_port_set_addresses(lsp,
diff --git a/tests/ovn.at b/tests/ovn.at
index e10a7f9ba..e4e510728 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -5606,18 +5606,11 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
          ["f0:00:00:00:10:2b 192.168.1.2"
 ])
 
-# Add another port with a conflicting static IPv4 address. p41 should update.
-ovn-nbctl --wait=sb lsp-add sw5 p42 -- lsp-set-addresses p42 \
-"f0:00:00:00:10:2c 192.168.1.2"
-AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
-         ["f0:00:00:00:10:2b 192.168.1.3"
-])
-
 # Add an excluded IP address that conflicts with p41. p41 should update.
 ovn-nbctl --wait=sb add Logical-Switch sw5 other_config \
-exclude_ips="192.168.1.3"
+exclude_ips="192.168.1.2"
 AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
-         ["f0:00:00:00:10:2b 192.168.1.4"
+         ["f0:00:00:00:10:2b 192.168.1.3"
 ])
 
 as ovn-sb
@@ -6297,7 +6290,7 @@ AT_CHECK([ovn-nbctl lsp-add lsw0 localvif1])
 AT_CHECK([ovn-nbctl lsp-set-addresses localvif1 "f0:00:00:00:00:01 192.168.1.1"])
 AT_CHECK([ovn-nbctl lsp-set-port-security localvif1 "f0:00:00:00:00:01"])
 AT_CHECK([ovn-nbctl lsp-add lsw0 localvif2])
-AT_CHECK([ovn-nbctl lsp-set-addresses localvif2 "f0:00:00:00:00:01 192.168.1.2"])
+AT_CHECK([ovn-nbctl lsp-set-addresses localvif2 "f0:00:00:00:00:02 192.168.1.2"])
 AT_CHECK([ovn-nbctl lsp-set-port-security localvif2 "f0:00:00:00:00:02"])
 AT_CHECK([ovn-nbctl lsp-add lsw0 localvif3])
 AT_CHECK([ovn-nbctl lsp-set-addresses localvif3 "f0:00:00:00:00:03 192.168.1.3"])
@@ -11194,3 +11187,59 @@ as hv2 start_daemon ovn-controller
 OVN_CLEANUP([hv1],[hv2])
 
 AT_CLEANUP
+
+AT_SETUP([ovn -- ovn-nbctl duplicate addresses])
+ovn_start
+
+# Set up a switch with some switch ports of varying address types
+ovn-nbctl ls-add sw1
+ovn-nbctl set logical_switch sw1 other_config:subnet=192.168.0.0/24
+
+ovn-nbctl lsp-add sw1 sw1-p1
+ovn-nbctl lsp-add sw1 sw1-p2
+ovn-nbctl lsp-add sw1 sw1-p3
+ovn-nbctl lsp-add sw1 sw1-p4
+
+ovn-nbctl lsp-set-addresses sw1-p1 "00:00:00:00:00:01 10.0.0.1 aef0::1" "00:00:00:00:00:02 10.0.0.2 aef0::2"
+ovn-nbctl lsp-set-addresses sw1-p2 "00:00:00:00:00:03 dynamic"
+ovn-nbctl lsp-set-addresses sw1-p3 "dynamic"
+ovn-nbctl lsp-set-addresses sw1-p4 "router"
+ovn-nbctl lsp-set-addresses sw1-p5 "unknown"
+
+ovn-nbctl list logical_switch_port
+
+# Now try to add duplicate addresses on a new port. These should all fail
+ovn-nbctl lsp-add sw1 sw1-p5
+AT_CHECK([ovn-nbctl lsp-set-addresses sw1-p5 "00:00:00:00:00:04 10.0.0.1"], [1], [],
+[ovn-nbctl: Error on switch sw1: duplicate IPv4 address 10.0.0.1
+])
+AT_CHECK([ovn-nbctl lsp-set-addresses sw1-p5 "00:00:00:00:00:04 10.0.0.2"], [1], [],
+[ovn-nbctl: Error on switch sw1: duplicate IPv4 address 10.0.0.2
+])
+AT_CHECK([ovn-nbctl lsp-set-addresses sw1-p5 "00:00:00:00:00:04 aef0::1"], [1], [],
+[ovn-nbctl: Error on switch sw1: duplicate IPv6 address aef0::1
+])
+AT_CHECK([ovn-nbctl lsp-set-addresses sw1-p5 "00:00:00:00:00:04 aef0::2"], [1], [],
+[ovn-nbctl: Error on switch sw1: duplicate IPv6 address aef0::2
+])
+AT_CHECK([ovn-nbctl lsp-set-addresses sw1-p5 "00:00:00:00:00:04 192.168.0.2"], [1], [],
+[ovn-nbctl: Error on switch sw1: duplicate IPv4 address 192.168.0.2
+])
+AT_CHECK([ovn-nbctl lsp-set-addresses sw1-p5 "00:00:00:00:00:04 192.168.0.3"], [1], [],
+[ovn-nbctl: Error on switch sw1: duplicate IPv4 address 192.168.0.3
+])
+
+# Now try re-setting sw1-p1. This should succeed
+AT_CHECK([ovn-nbctl lsp-set-addresses sw1-p1 "00:00:00:00:00:01 10.0.0.1 aef0::1"])
+
+# Now create a new switch and try setting IP addresses the same as the
+# first switch. This should succeed.
+ovn-nbctl ls-add sw2
+ovn-nbctl lsp-add sw2 sw2-p1
+
+AT_CHECK([ovn-nbctl lsp-set-addresses sw2-p1 "00:00:00:00:00:04 10.0.0.1"])
+AT_CHECK([ovn-nbctl lsp-set-addresses sw2-p1 "00:00:00:00:00:04 192.168.0.2"])
+AT_CHECK([ovn-nbctl lsp-set-addresses sw2-p1 "00:00:00:00:00:04 192.168.0.3"])
+AT_CHECK([ovn-nbctl lsp-set-addresses sw2-p1 "00:00:00:00:00:04 aef0::1"])
+
+AT_CLEANUP
-- 
2.14.4



More information about the dev mailing list