From patchwork Wed Oct 10 06:08:55 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 981703 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=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com 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 42VNw76Svkz9s8r for ; Wed, 10 Oct 2018 17:09:11 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 046BC74; Wed, 10 Oct 2018 06:09:08 +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 D24F512 for ; Wed, 10 Oct 2018 06:09:06 +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 D268E34F for ; Wed, 10 Oct 2018 06:09:05 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 58C4130821C3 for ; Wed, 10 Oct 2018 06:09:05 +0000 (UTC) Received: from nusiddiq.redhat (ovpn-116-245.sin2.redhat.com [10.67.116.245]) by smtp.corp.redhat.com (Postfix) with ESMTP id BE4EA5B6FA; Wed, 10 Oct 2018 06:09:03 +0000 (UTC) From: nusiddiq@redhat.com To: dev@openvswitch.org Date: Wed, 10 Oct 2018 11:38:55 +0530 Message-Id: <20181010060855.5534-1-nusiddiq@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.47]); Wed, 10 Oct 2018 06:09:05 +0000 (UTC) X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH v7] ovn: Support configuring the BFD params for the tunnel interfaces 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 From: Numan Siddique With this commit the users can override the default values of the BFD params - min_rx, min_tx, decay_min_rx and mult if desired. This can be useful to debug any issues related to BFD (like frequent BFD state changes). A new column 'options' is added in NB_Global and SB_Global tables of OVN_Northbound and OVN_Southbound schemas respectively. CMS can define the options 'bfd-min-rx', 'bfd-min-tx', 'bfd-decay-min-rx' and 'bfd-mult' in the options column of NB_Global table row. ovn-northd copies these options from NB_Global to SB_Global. ovn-controller configures these options to the tunnel interfaces when enabling BFD. When BFD is disabled, this patch now clears the 'bfd' column of the interface row, instead of setting 'enable=false'. Signed-off-by: Numan Siddique --- v6 -> v7 ------- * Corrected a small error in the commit message. v5 -> v6 -------- * Addressed the review comments from Ben - changed the config options from "bfd:min-rx" to "bfd-min-rx" - when disabling the bfd, instead of setting the option "enable_bfd=false", now clears the bfd column of the interface row v4 -> v5 ------- * Addressed a couple of check_patch util warnings - Line exceeding 80 char which was introduced in the v4. v3 -> v4 -------- * As per the suggestion from Ben - provided the option to centrally configuring the BFD params v2 -> v3 -------- * Added the test case for mult option v1 -> v2 ------- * Addressed review comments by Miguel - added the option 'mult' to configure. ovn/controller/bfd.c | 66 +++++++++++++++++++++++---------- ovn/controller/bfd.h | 3 ++ ovn/controller/ovn-controller.c | 4 +- ovn/northd/ovn-northd.c | 2 + ovn/ovn-nb.ovsschema | 9 +++-- ovn/ovn-nb.xml | 35 +++++++++++++++++ ovn/ovn-sb.ovsschema | 9 +++-- ovn/ovn-sb.xml | 38 +++++++++++++++++++ tests/ovn.at | 31 +++++++++++++++- 9 files changed, 168 insertions(+), 29 deletions(-) diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c index 051781f38..94dad236e 100644 --- a/ovn/controller/bfd.c +++ b/ovn/controller/bfd.c @@ -38,24 +38,6 @@ bfd_register_ovs_idl(struct ovsdb_idl *ovs_idl) ovsdb_idl_add_column(ovs_idl, &ovsrec_interface_col_bfd_status); } - -static void -interface_set_bfd(const struct ovsrec_interface *iface, bool bfd_setting) -{ - const char *new_setting = bfd_setting ? "true":"false"; - const char *current_setting = smap_get(&iface->bfd, "enable"); - if (current_setting && !strcmp(current_setting, new_setting)) { - /* If already set to the desired setting we skip setting it again - * to avoid flapping to bfd initialization state */ - return; - } - const struct smap bfd = SMAP_CONST1(&bfd, "enable", new_setting); - ovsrec_interface_verify_bfd(iface); - ovsrec_interface_set_bfd(iface, &bfd); - VLOG_INFO("%s BFD on interface %s", bfd_setting ? "Enabled" : "Disabled", - iface->name); -} - void bfd_calculate_active_tunnels(const struct ovsrec_bridge *br_int, struct sset *active_tunnels) @@ -267,6 +249,7 @@ bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name, const struct ovsrec_interface_table *interface_table, const struct ovsrec_bridge *br_int, const struct sbrec_chassis *chassis_rec, + const struct sbrec_sb_global_table *sb_global_table, const struct hmap *local_datapaths) { @@ -292,15 +275,58 @@ bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name, } } + const struct sbrec_sb_global *sb + = sbrec_sb_global_table_first(sb_global_table); + struct smap bfd = SMAP_INITIALIZER(&bfd); + smap_add(&bfd, "enable", "true"); + + if (sb) { + const char *min_rx = smap_get(&sb->options, "bfd-min-rx"); + const char *decay_min_rx = smap_get(&sb->options, "bfd-decay-min-rx"); + const char *min_tx = smap_get(&sb->options, "bfd-min-tx"); + const char *mult = smap_get(&sb->options, "bfd-mult"); + if (min_rx) { + smap_add(&bfd, "min_rx", min_rx); + } + if (decay_min_rx) { + smap_add(&bfd, "decay_min_rx", decay_min_rx); + } + if (min_tx) { + smap_add(&bfd, "min_tx", min_tx); + } + if (mult) { + smap_add(&bfd, "mult", mult); + } + } + /* Enable or disable bfd */ const struct ovsrec_interface *iface; OVSREC_INTERFACE_TABLE_FOR_EACH (iface, interface_table) { if (sset_contains(&tunnels, iface->name)) { - interface_set_bfd( - iface, sset_contains(&bfd_ifaces, iface->name)); + if (sset_contains(&bfd_ifaces, iface->name)) { + /* We need to enable BFD for this interface. Configure the + * BFD params if + * - If BFD was disabled earlier + * - Or if CMS has updated BFD config options. + */ + if (!smap_equal(&iface->bfd, &bfd)) { + ovsrec_interface_verify_bfd(iface); + ovsrec_interface_set_bfd(iface, &bfd); + VLOG_INFO("Enabled BFD on interface %s", iface->name); + } + } else { + /* We need to disable BFD for this interface if it was enabled + * earlier. */ + if (smap_count(&iface->bfd)) { + ovsrec_interface_verify_bfd(iface); + ovsrec_interface_set_bfd(iface, NULL); + VLOG_INFO("Disabled BFD on interface %s", iface->name); + } + } } } + smap_destroy(&bfd); sset_destroy(&tunnels); sset_destroy(&bfd_ifaces); sset_destroy(&bfd_chassis); diff --git a/ovn/controller/bfd.h b/ovn/controller/bfd.h index 7ea345a3e..e36820afb 100644 --- a/ovn/controller/bfd.h +++ b/ovn/controller/bfd.h @@ -21,7 +21,9 @@ struct ovsdb_idl; struct ovsdb_idl_index; struct ovsrec_bridge; struct ovsrec_interface_table; +struct ovsrec_open_vswitch_table; struct sbrec_chassis; +struct sbrec_sb_global_table; struct sset; void bfd_register_ovs_idl(struct ovsdb_idl *); @@ -30,6 +32,7 @@ void bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name, const struct ovsrec_interface_table *interface_table, const struct ovsrec_bridge *br_int, const struct sbrec_chassis *chassis_rec, + const struct sbrec_sb_global_table *sb_global_table, const struct hmap *local_datapaths); void bfd_calculate_active_tunnels(const struct ovsrec_bridge *br_int, struct sset *active_tunnels); diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index f46156021..2b2779a17 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -756,7 +756,9 @@ main(int argc, char *argv[]) sbrec_chassis_by_name, sbrec_port_binding_by_datapath, ovsrec_interface_table_get(ovs_idl_loop.idl), - br_int, chassis, &local_datapaths); + br_int, chassis, + sbrec_sb_global_table_get(ovnsb_idl_loop.idl), + &local_datapaths); } physical_run( sbrec_chassis_by_name, diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 31ea5f410..439651f80 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -7138,6 +7138,7 @@ ovnnb_db_run(struct northd_context *ctx, sb = sbrec_sb_global_insert(ctx->ovnsb_txn); } sbrec_sb_global_set_nb_cfg(sb, nb->nb_cfg); + sbrec_sb_global_set_options(sb, &nb->options); sb_loop->next_cfg = nb->nb_cfg; cleanup_macam(&macam); @@ -7641,6 +7642,7 @@ main(int argc, char *argv[]) ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_sb_global); add_column_noalert(ovnsb_idl_loop.idl, &sbrec_sb_global_col_nb_cfg); + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_sb_global_col_options); ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_logical_flow); add_column_noalert(ovnsb_idl_loop.idl, diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema index 3e7164baa..705cc2705 100644 --- a/ovn/ovn-nb.ovsschema +++ b/ovn/ovn-nb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Northbound", - "version": "5.13.0", - "cksum": "1278623084 20312", + "version": "5.13.1", + "cksum": "749176366 20467", "tables": { "NB_Global": { "columns": { @@ -19,7 +19,10 @@ "ssl": { "type": {"key": {"type": "uuid", "refTable": "SSL"}, - "min": 0, "max": 1}}}, + "min": 0, "max": 1}}, + "options": { + "type": {"key": "string", "value": "string", + "min": 0, "max": "unlimited"}}}, "maxRows": 1, "isRoot": true}, "Logical_Switch": { diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml index 8564ed39c..c0739fe57 100644 --- a/ovn/ovn-nb.xml +++ b/ovn/ovn-nb.xml @@ -69,6 +69,41 @@ See External IDs at the beginning of this document. + + + + This column provides general key/value settings. The supported + options are described individually below. + + + +

+ These options apply when ovn-controller configures + BFD on tunnels interfaces. +

+ + + BFD option min-rx value to use when configuring BFD on + tunnel interfaces. + + + + BFD option decay-min-rx value to use when configuring + BFD on tunnel interfaces. + + + + BFD option min-tx value to use when configuring BFD on + tunnel interfaces. + + + + BFD option mult value to use when configuring BFD on + tunnel interfaces. + +
+
+ Database clients to which the Open vSwitch database server should diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema index ad6ad3b71..33ccd95a8 100644 --- a/ovn/ovn-sb.ovsschema +++ b/ovn/ovn-sb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Southbound", - "version": "1.16.0", - "cksum": "3046632234 14844", + "version": "1.16.1", + "cksum": "2148542239 14999", "tables": { "SB_Global": { "columns": { @@ -17,7 +17,10 @@ "ssl": { "type": {"key": {"type": "uuid", "refTable": "SSL"}, - "min": 0, "max": 1}}}, + "min": 0, "max": 1}}, + "options": { + "type": {"key": "string", "value": "string", + "min": 0, "max": "unlimited"}}}, "maxRows": 1, "isRoot": true}, "Chassis": { diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml index 68e31db31..a4922bede 100644 --- a/ovn/ovn-sb.xml +++ b/ovn/ovn-sb.xml @@ -162,7 +162,45 @@ See External IDs at the beginning of this document. + + + + + + + + This column provides general key/value settings. The supported + options are described individually below. + + + +

+ These options apply when ovn-controller configures + BFD on tunnels interfaces. +

+ + + BFD option min-rx value to use when configuring BFD on + tunnel interfaces. + + + + BFD option decay-min-rx value to use when configuring + BFD on tunnel interfaces. + + + + BFD option min-tx value to use when configuring BFD on + tunnel interfaces. + + + + BFD option mult value to use when configuring BFD on + tunnel interfaces. + +
+ Database clients to which the Open vSwitch database server should diff --git a/tests/ovn.at b/tests/ovn.at index 44475175d..6e322602a 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -9226,7 +9226,7 @@ for chassis in gw1 gw2; do done # make sure BFD is not enabled to hv2, we don't need it AT_CHECK([ovs-vsctl --bare --columns bfd find Interface name=ovn-hv2-0],[0], - [[enable=false + [[ ]]) @@ -9240,7 +9240,7 @@ for chassis in gw1 gw2; do done # make sure BFD is not enabled to hv1, we don't need it AT_CHECK([ovs-vsctl --bare --columns bfd find Interface name=ovn-hv1-0],[0], - [[enable=false + [[ ]]) sleep 3 # let BFD sessions settle so we get the right flows on the right chassis @@ -9270,6 +9270,33 @@ AT_CHECK([ovn-sbctl --columns chassis --bare find Port_Binding logical_port=cr-o [0],[[1 ]]) +ovn-nbctl --wait=hv set NB_Global . options:"bfd-min-rx"=2000 +as gw2 +for chassis in gw1 hv1 hv2; do + echo "checking gw2 -> $chassis" + OVS_WAIT_UNTIL([ + bfd_cfg=$(ovs-vsctl --bare --columns bfd find Interface name=ovn-$chassis-0) + test "$bfd_cfg" = "enable=true min_rx=2000" +]) +done +ovn-nbctl --wait=hv set NB_Global . options:"bfd-min-tx"=1500 +for chassis in gw1 hv1 hv2; do + echo "checking gw2 -> $chassis" + OVS_WAIT_UNTIL([ + bfd_cfg=$(ovs-vsctl --bare --columns bfd find Interface name=ovn-$chassis-0) + test "$bfd_cfg" = "enable=true min_rx=2000 min_tx=1500" +]) +done +ovn-nbctl remove NB_Global . options "bfd-min-rx" +ovn-nbctl --wait=hv set NB_Global . options:"bfd-mult"=5 +for chassis in gw1 hv1 hv2; do + echo "checking gw2 -> $chassis" + OVS_WAIT_UNTIL([ + bfd_cfg=$(ovs-vsctl --bare --columns bfd find Interface name=ovn-$chassis-0) + test "$bfd_cfg" = "enable=true min_tx=1500 mult=5" +]) +done + OVN_CLEANUP([gw1],[gw2],[hv1],[hv2]) AT_CLEANUP