ovn: Check for known logical switch port types.

OVN is lenient with the types of logical switch ports. Maybe too
lenient. This patch attempts to solve this problem on two fronts:

1) In ovn-nbctl, if you attempt to set the port type to an unknown
type, the command will not end up setting the type.
2) In northd, when copying the port type from the northbound database to
the corresponding port-binding in the southbound database, a warning
will be issued if the port is of an unknown type.

Signed-off-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Lance Richardson <lrichard@redhat.com>

diff --git a/ovn/lib/ovn-util.c b/ovn/lib/ovn-util.c
index 037d0798a9eeaecb743d7b3334da60ef849d77fc..a554c23f5ae8aae18507b14aa9003a702a51ff70 100644 (file)
--- a/ovn/lib/ovn-util.c
+++ b/ovn/lib/ovn-util.c
@@ -299,3 +299,33 @@ default_sb_db(void)
     }
     return def;
 }
+
+/* l3gateway, chassisredirect, and patch
+ * are not in this list since they are
+ * only set in the SB DB by northd
+ */
+static const char *OVN_NB_LSP_TYPES[] = {
+    "l2gateway",
+    "localnet",
+    "localport",
+    "router",
+    "vtep",
+};
+
+bool
+ovn_is_known_nb_lsp_type(const char *type)
+{
+    int i;
+
+    if (!type || !type[0]) {
+        return true;
+    }
+
+    for (i = 0; i < ARRAY_SIZE(OVN_NB_LSP_TYPES); ++i) {
+        if (!strcmp(OVN_NB_LSP_TYPES[i], type)) {
+            return true;
+        }
+    }
+
+    return false;
+}

diff --git a/ovn/lib/ovn-util.h b/ovn/lib/ovn-util.h
index b3d2125a3bcf8a876165998fcf787209a3e8006d..9b456426dc678103ac052dfd758849ce57fef19e 100644 (file)
--- a/ovn/lib/ovn-util.h
+++ b/ovn/lib/ovn-util.h
@@ -67,4 +67,6 @@ char *alloc_nat_zone_key(const struct uuid *key, const char *type);
 const char *default_nb_db(void);
 const char *default_sb_db(void);
 
+bool ovn_is_known_nb_lsp_type(const char *type);
+
 #endif

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index b250eb4fa1fe0e1fce717eac23e6ce14461a5c81..f034ba2c49476cb3b6ed4625b1aa7ed09930bad0 100644 (file)
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1963,7 +1963,14 @@ ovn_port_update_sbrec(struct northd_context *ctx,
             }
             sbrec_port_binding_set_options(op->sb, &options);
             smap_destroy(&options);
-            sbrec_port_binding_set_type(op->sb, op->nbsp->type);
+            if (ovn_is_known_nb_lsp_type(op->nbsp->type)) {
+                sbrec_port_binding_set_type(op->sb, op->nbsp->type);
+            } else {
+                static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+                VLOG_WARN_RL(
+                    &rl, "Unknown port type '%s' set on logical switch '%s'.",
+                    op->nbsp->type, op->nbsp->name);
+            }
         } else {
             const char *chassis = NULL;
             if (op->peer && op->peer->od && op->peer->od->nbr) {

diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 8e5c1a440228140f59cee66f2b6f51f97c6bea76..77889961f0ef90f38bc515d4e2c58140bff6ae00 100644 (file)
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -1173,7 +1173,12 @@ nbctl_lsp_set_type(struct ctl_context *ctx)
     const struct nbrec_logical_switch_port *lsp;
 
     lsp = lsp_by_name_or_uuid(ctx, id, true);
-    nbrec_logical_switch_port_set_type(lsp, type);
+    if (ovn_is_known_nb_lsp_type(type)) {
+        nbrec_logical_switch_port_set_type(lsp, type);
+    } else {
+        ctl_fatal("Logical switch port type '%s' is unrecognized. "
+                  "Not setting type.", type);
+    }
 }
 
 static void

diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at
index 9b9e42115678b37e412535fe34c8dc06b67e250e..0d2711e3a584869919d19b8666192ab5ccdb0693 100644 (file)
--- a/tests/ovn-controller-vtep.at
+++ b/tests/ovn-controller-vtep.at
@@ -327,7 +327,7 @@ ${tunnel_key}
 
 # changes the ovn-nb logical port type so that it is no longer
 # vtep port.
-AT_CHECK([ovn-nbctl lsp-set-type br-vtep_lswitch0 void])
+AT_CHECK([ovn-nbctl lsp-set-type br-vtep_lswitch0 ""])
 OVS_WAIT_UNTIL([test -z "`vtep-ctl --columns=tunnel_key list Logical_Switch | grep 1`"])
 # now should see the tunnel key reset.
 AT_CHECK([vtep-ctl --columns=tunnel_key list Logical_Switch | cut -d ':' -f2 | tr -d ' '], [0], [dnl

diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 354b8df96d01f4391ff000ab8ebd1ada2a468162..e1c4b4473768f6e31e67ea0db8bc97164f850628 100644 (file)
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -951,3 +951,76 @@ IPv6 Routes
 
 OVN_NBCTL_TEST_STOP
 AT_CLEANUP
+
+dnl ---------------------------------------------------------------------
+
+AT_SETUP([ovn-nbctl - lsp types])
+OVN_NBCTL_TEST_START
+
+AT_CHECK([ovn-nbctl ls-add ls0])
+AT_CHECK([ovn-nbctl lsp-add ls0 lp0])
+
+dnl switchport type defaults to empty
+AT_CHECK([ovn-nbctl lsp-get-type lp0], [0], [dnl
+
+])
+
+dnl The following are the valid entries for
+dnl switchport type
+AT_CHECK([ovn-nbctl lsp-set-type lp0 l2gateway])
+AT_CHECK([ovn-nbctl lsp-get-type lp0], [0], [dnl
+l2gateway
+])
+
+AT_CHECK([ovn-nbctl lsp-set-type lp0 router])
+AT_CHECK([ovn-nbctl lsp-get-type lp0], [0], [dnl
+router
+])
+
+AT_CHECK([ovn-nbctl lsp-set-type lp0 localnet])
+AT_CHECK([ovn-nbctl lsp-get-type lp0], [0], [dnl
+localnet
+])
+
+AT_CHECK([ovn-nbctl lsp-set-type lp0 localport])
+AT_CHECK([ovn-nbctl lsp-get-type lp0], [0], [dnl
+localport
+])
+
+AT_CHECK([ovn-nbctl lsp-set-type lp0 vtep])
+AT_CHECK([ovn-nbctl lsp-get-type lp0], [0], [dnl
+vtep
+])
+
+dnl All of these are valid southbound port types but
+dnl should be rejected for northbound logical switch
+dnl ports.
+AT_CHECK([ovn-nbctl lsp-set-type lp0 l3gateway], [1], [], [dnl
+ovn-nbctl: Logical switch port type 'l3gateway' is unrecognized. Not setting type.
+])
+AT_CHECK([ovn-nbctl lsp-set-type lp0 patch], [1], [], [dnl
+ovn-nbctl: Logical switch port type 'patch' is unrecognized. Not setting type.
+])
+AT_CHECK([ovn-nbctl lsp-set-type lp0 chassisredirect], [1], [], [dnl
+ovn-nbctl: Logical switch port type 'chassisredirect' is unrecognized. Not setting type.
+])
+
+dnl switch port type should still be "vtep" since previous
+dnl commands failed.
+AT_CHECK([ovn-nbctl lsp-get-type lp0], [0], [dnl
+vtep
+])
+
+dnl Attempt a nonsense type
+AT_CHECK([ovn-nbctl lsp-set-type lp0 eggs], [1], [], [dnl
+ovn-nbctl: Logical switch port type 'eggs' is unrecognized. Not setting type.
+])
+
+dnl Empty string should work too
+AT_CHECK([ovn-nbctl lsp-set-type lp0 ""])
+AT_CHECK([ovn-nbctl lsp-get-type lp0], [0], [dnl
+
+])
+
+OVN_NBCTL_TEST_STOP
+AT_CLEANUP