diff mbox series

[ovs-dev,v3,ovn,2/6] Use normalized IP addresses in `ovn-nbctl lrp-add`

Message ID 20200624161211.1559878-2-mmichels@redhat.com
State New
Headers show
Series [ovs-dev,v3,ovn,1/6] Avoid case-sensitive MAC address comparisons. | expand

Commit Message

Mark Michelson June 24, 2020, 4:12 p.m. UTC
Signed-off-by: Mark Michelson <mmichels@redhat.com>
---
 tests/ovn.at          |  6 +++---
 utilities/ovn-nbctl.c | 44 ++++++++++++++++++++++++++++++++++---------
 2 files changed, 38 insertions(+), 12 deletions(-)

Comments

0-day Robot June 24, 2020, 5:01 p.m. UTC | #1
Bleep bloop.  Greetings Mark Michelson, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 83 characters long (recommended limit is 79)
#71 FILE: utilities/ovn-nbctl.c:4778:
        struct sset *orig_networks = lrp_network_sset((const char **)lrp->networks,

Lines checked: 97, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/tests/ovn.at b/tests/ovn.at
index 9181b8425..d54a4d707 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -19721,11 +19721,11 @@  AT_CHECK([test 2287 = `ovs-ofctl dump-group-stats br-int | grep -o bucket | wc -
 OVN_CLEANUP([hv1])
 AT_CLEANUP
 
-AT_SETUP([ovn -- case-insensitive lrp-add])
+AT_SETUP([ovn -- normalized lrp-add])
 ovn_start
 
 ovn-nbctl lr-add r1
-ovn-nbctl lrp-add r1 rp1 CC:DD:EE:EE:DD:CC
+ovn-nbctl lrp-add r1 rp1 CC:DD:EE:EE:DD:CC AEF0::1/64 BEF0::1/64
 
-AT_CHECK([ovn-nbctl --may-exist lrp-add r1 rp1 cc:dd:ee:ee:dd:cc])
+AT_CHECK([ovn-nbctl --may-exist lrp-add r1 rp1 cc:dd:ee:ee:dd:cc bef0:0000:0000:0000::1/64 aef0::1/64])
 AT_CLEANUP
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 638e6f7c0..708ff2e5b 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -4645,6 +4645,23 @@  nbctl_lrp_get_gateway_chassis(struct ctl_context *ctx)
     free(gcs);
 }
 
+static struct sset *
+lrp_network_sset(const char **networks, int n_networks)
+{
+    struct sset *network_set = xzalloc(sizeof *network_set);
+    sset_init(network_set);
+    for (int i = 0; i < n_networks; i++) {
+        char *norm = normalize_prefix_str(networks[i]);
+        if (!norm) {
+            sset_destroy(network_set);
+            free(network_set);
+            return NULL;
+        }
+        sset_add_and_free(network_set, norm);
+    }
+    return network_set;
+}
+
 static void
 nbctl_lrp_add(struct ctl_context *ctx)
 {
@@ -4718,17 +4735,26 @@  nbctl_lrp_add(struct ctl_context *ctx)
             return;
         }
 
-        struct sset new_networks = SSET_INITIALIZER(&new_networks);
-        for (int i = 0; i < n_networks; i++) {
-            sset_add(&new_networks, networks[i]);
+        struct sset *new_networks = lrp_network_sset(networks, n_networks);
+        if (!new_networks) {
+            ctl_error(ctx, "%s: Invalid networks configured", lrp_name);
+            return;
+        }
+        struct sset *orig_networks = lrp_network_sset((const char **)lrp->networks,
+                                                      lrp->n_networks);
+        if (!orig_networks) {
+            ctl_error(ctx, "%s: Existing port has invalid networks configured",
+                      lrp_name);
+            sset_destroy(new_networks);
+            free(new_networks);
+            return;
         }
 
-        struct sset orig_networks = SSET_INITIALIZER(&orig_networks);
-        sset_add_array(&orig_networks, lrp->networks, lrp->n_networks);
-
-        bool same_networks = sset_equals(&orig_networks, &new_networks);
-        sset_destroy(&orig_networks);
-        sset_destroy(&new_networks);
+        bool same_networks = sset_equals(orig_networks, new_networks);
+        sset_destroy(orig_networks);
+        free(orig_networks);
+        sset_destroy(new_networks);
+        free(new_networks);
         if (!same_networks) {
             ctl_error(ctx, "%s: port already exists with different network",
                       lrp_name);