From patchwork Wed Sep 6 13:38:40 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Michelson X-Patchwork-Id: 810608 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xnPn26z82z9sNV for ; Wed, 6 Sep 2017 23:38:46 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id BCFA4B6C; Wed, 6 Sep 2017 13:38:43 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id B8EF0ABA for ; Wed, 6 Sep 2017 13:38:42 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 4176FA1 for ; Wed, 6 Sep 2017 13:38:42 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A48318553C for ; Wed, 6 Sep 2017 13:38:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com A48318553C Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=mmichels@redhat.com Received: from monae.redhat.com (ovpn-124-74.rdu2.redhat.com [10.10.124.74]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5C2832CFF3 for ; Wed, 6 Sep 2017 13:38:41 +0000 (UTC) From: Mark Michelson To: dev@openvswitch.org Date: Wed, 6 Sep 2017 08:38:40 -0500 Message-Id: <20170906133840.17572-1-mmichels@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Wed, 06 Sep 2017 13:38:41 +0000 (UTC) X-Spam-Status: No, score=-5.0 required=5.0 tests=RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD autolearn=disabled version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH v4] ovn: Check for known logical switch port types. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org 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 Acked-by: Lance Richardson --- v4: * Fixed failing ovn-controller-vtep test that attempted to set a switch port type to "void" v3: * OVN_NB_LSP_TYPES declaration is static * northd will not copy unknown port types to southbound DB * re-ordered port types in OVN_NB_LSP_TYPES v2: * Used ARRAY_SIZE to calculate length of OVN_NB_LSP_TYPES v1: * Initial patch --- ovn/lib/ovn-util.c | 30 ++++++++++++++++++ ovn/lib/ovn-util.h | 2 ++ ovn/northd/ovn-northd.c | 7 ++++- ovn/utilities/ovn-nbctl.c | 7 ++++- tests/ovn-controller-vtep.at | 2 +- tests/ovn-nbctl.at | 73 ++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 118 insertions(+), 3 deletions(-) diff --git a/ovn/lib/ovn-util.c b/ovn/lib/ovn-util.c index 037d0798a..a554c23f5 100644 --- 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 b3d2125a3..9b456426d 100644 --- 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 49e4ac338..9510a7bdb 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -1962,7 +1962,12 @@ 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 { + VLOG_WARN("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 46ede4e50..d00bd6ec9 100644 --- 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 9b9e42115..0d2711e3a 100644 --- 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 354b8df96..e1c4b4473 100644 --- 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