From 69e73316e9c3c997fe353abd12a2fe1d5a21f5f7 Mon Sep 17 00:00:00 2001
From: Russell Bryant <russell@ovn.org>
Date: Thu, 14 Jan 2016 10:47:18 -0500
Subject: [PATCH] ovn-nbctl: Help catch lport-set-addresses mistakes.

While debugging a broken OVN environment yesterday, the problem turned
out to be invalid entries in the logical port addresses column.  In
particular, the following command had been used:

  $ ovn-nbctl lport-set-addresses lp0 MAC IP

instead of:

  $ ovn-nbctl lport-set-addresses lp0 "MAC IP"

This is really easy to mess up, so add some simple validation to the
lport-set-addresses command.  If the beginning of an argument is ever
an IP address, it's wrong.

In passing, also add a note to the ovn-nb db documentation to note that
the order of "MAC IP" is required, as "IP MAC" is not valid.

Signed-off-by: Russell Bryant <russell@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
---
 ovn/ovn-nb.xml            |  5 +++++
 ovn/utilities/ovn-nbctl.c | 16 ++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index ef34c9b9e..4e414cee8 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -285,6 +285,11 @@
               if any, uses this information to avoid issuing ARP requests for
               logical switch ports.
             </p>
+
+            <p>
+              Note that the order here is important. The Ethernet address must
+              be listed before the IP address.
+            </p>
           </dd>
 
           <dt><code>unknown</code></dt>
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 8522c7e2b..8ac1cd054 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -25,6 +25,7 @@
 #include "fatal-signal.h"
 #include "json.h"
 #include "ovn/lib/ovn-nb-idl.h"
+#include "packets.h"
 #include "poll-loop.h"
 #include "process.h"
 #include "smap.h"
@@ -665,6 +666,21 @@ nbctl_lport_set_addresses(struct ctl_context *ctx)
         return;
     }
 
+    int i;
+    for (i = 2; i < ctx->argc; i++) {
+        struct eth_addr ea;
+
+        if (strcmp(ctx->argv[i], "unknown")
+            && !ovs_scan(ctx->argv[i], ETH_ADDR_SCAN_FMT,
+                         ETH_ADDR_SCAN_ARGS(ea))) {
+            VLOG_ERR("Invalid address format (%s). See ovn-nb(5). "
+                     "Hint: An Ethernet address must be "
+                     "listed before an IP address, together as a single "
+                     "argument.", ctx->argv[i]);
+            return;
+        }
+    }
+
     nbrec_logical_port_set_addresses(lport,
             (const char **) ctx->argv + 2, ctx->argc - 2);
 }
-- 
2.20.1