[ovs-dev] [PATCH v6] ovn: Support configuring the BFD params for the tunnel interfaces
Numan Siddique
nusiddiq at redhat.com
Tue Oct 9 08:53:45 UTC 2018
On Tue, Oct 9, 2018 at 2:18 PM <nusiddiq at redhat.com> wrote:
> From: Numan Siddique <nusiddiq at redhat.com>
>
> 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_bfd=false'.
>
If this patch is fine, can you please update from 'enable_bfd=false' to
'enable=false' in the last line of the commit message before applying.
Thanks
Numan
>
> Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
> ---
>
> 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 <em>External IDs</em> at the beginning of this document.
> </column>
> </group>
> +
> + <group title="Common options">
> + <column name="options">
> + This column provides general key/value settings. The supported
> + options are described individually below.
> + </column>
> +
> + <group title="Options for configuring BFD">
> + <p>
> + These options apply when <code>ovn-controller</code> configures
> + BFD on tunnels interfaces.
> + </p>
> +
> + <column name="options" key="bfd-min-rx">
> + BFD option <code>min-rx</code> value to use when configuring
> BFD on
> + tunnel interfaces.
> + </column>
> +
> + <column name="options" key="bfd-decay-min-rx">
> + BFD option <code>decay-min-rx</code> value to use when
> configuring
> + BFD on tunnel interfaces.
> + </column>
> +
> + <column name="options" key="bfd-min-tx">
> + BFD option <code>min-tx</code> value to use when configuring
> BFD on
> + tunnel interfaces.
> + </column>
> +
> + <column name="options" key="bfd-mult">
> + BFD option <code>mult</code> value to use when configuring BFD
> on
> + tunnel interfaces.
> + </column>
> + </group>
> + </group>
> +
> <group title="Connection Options">
> <column name="connections">
> 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 @@
> <column name="external_ids">
> See <em>External IDs</em> at the beginning of this document.
> </column>
> +
> + <column name="options">
> + </column>
> + </group>
> +
> + <group title="Common options">
> + <column name="options">
> + This column provides general key/value settings. The supported
> + options are described individually below.
> + </column>
> +
> + <group title="Options for configuring BFD">
> + <p>
> + These options apply when <code>ovn-controller</code> configures
> + BFD on tunnels interfaces.
> + </p>
> +
> + <column name="options" key="bfd-min-rx">
> + BFD option <code>min-rx</code> value to use when configuring
> BFD on
> + tunnel interfaces.
> + </column>
> +
> + <column name="options" key="bfd-decay-min-rx">
> + BFD option <code>decay-min-rx</code> value to use when
> configuring
> + BFD on tunnel interfaces.
> + </column>
> +
> + <column name="options" key="bfd-min-tx">
> + BFD option <code>min-tx</code> value to use when configuring
> BFD on
> + tunnel interfaces.
> + </column>
> +
> + <column name="options" key="bfd-mult">
> + BFD option <code>mult</code> value to use when configuring BFD
> on
> + tunnel interfaces.
> + </column>
> + </group>
> </group>
> +
> <group title="Connection Options">
> <column name="connections">
> 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
> --
> 2.17.1
>
>
More information about the dev
mailing list