[ovs-dev] [PATCH] ovn: Support configuring the BFD params for the tunnel interfaces
Numan Siddique
nusiddiq at redhat.com
Wed Sep 26 08:30:43 UTC 2018
On Tue, Sep 25, 2018 at 10:18 PM Miguel Angel Ajo Pelayo <
majopela at redhat.com> wrote:
> Nak,
>
> Great work, this will be useful to adjust parameters based on specific
> network conditions, or while debugging network issues (to reduce flapping).
>
> I miss the "mult" parameter to setup the detection multiplier (rx_interval
> * mult).
>
>
Thanks Anil and Miguel for the review. I sent out v2 adding mult option -
https://patchwork.ozlabs.org/patch/974893/
Numan
>
> Thanks a lot,
> Miguel Ángel.
>
> On Tue, Sep 25, 2018 at 5:46 PM Anil Venkata <anilvenkata at redhat.com>
> wrote:
>
>> On Mon, Sep 24, 2018 at 3:12 PM <nusiddiq at redhat.com> wrote:
>>
>> > From: Numan Siddique <nusiddiq at redhat.com>
>> >
>> > With this commit the users can override the default BFD params -
>> > min_rx, min_tx and decay_min_rx if desired. This can be useful
>> > to debug any issues related to BFD, like BFD state changes are
>> > seen frequently.
>> >
>> > ovn-controller checks for the options 'ovn-bfd-min-rx', 'ovn-bfd-min-tx'
>> > and 'ovn-bfd-decay-min-rx' in the external-ids of OpenvSwitch table row
>> > and configures these BFD values to the tunnel interfaces.
>> >
>> > Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
>> >
>> Acked-by: Venkata Anil <vkommadi at redhat.com>
>>
>> > ---
>> > ovn/controller/bfd.c | 45 ++++++++++++++++++++---------
>> > ovn/controller/bfd.h | 2 ++
>> > ovn/controller/ovn-controller.8.xml | 9 ++++++
>> > ovn/controller/ovn-controller.c | 10 ++++---
>> > tests/ovn.at | 26 +++++++++++++++++
>> > 5 files changed, 74 insertions(+), 18 deletions(-)
>> >
>> > diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
>> > index 051781f38..455d7ff1f 100644
>> > --- a/ovn/controller/bfd.c
>> > +++ b/ovn/controller/bfd.c
>> > @@ -40,20 +40,10 @@ bfd_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>> >
>> >
>> > static void
>> > -interface_set_bfd(const struct ovsrec_interface *iface, bool
>> bfd_setting)
>> > +interface_set_bfd(const struct ovsrec_interface *iface, struct smap
>> *bfd)
>> > {
>> > - 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);
>> > + ovsrec_interface_set_bfd(iface, bfd);
>> > }
>> >
>> > void
>> > @@ -265,6 +255,7 @@ void
>> > bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
>> > struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
>> > const struct ovsrec_interface_table *interface_table,
>> > + const struct ovsrec_open_vswitch_table *ovs_table,
>> > const struct ovsrec_bridge *br_int,
>> > const struct sbrec_chassis *chassis_rec,
>> > const struct hmap *local_datapaths)
>> > @@ -292,15 +283,41 @@ bfd_run(struct ovsdb_idl_index
>> > *sbrec_chassis_by_name,
>> > }
>> > }
>> >
>> > + const struct ovsrec_open_vswitch *cfg;
>> > + cfg = ovsrec_open_vswitch_table_first(ovs_table);
>> > + const char *min_rx = NULL;
>> > + const char *decay_min_rx = NULL;
>> > + const char *min_tx = NULL;
>> > + if (cfg) {
>> > + min_rx = smap_get(&cfg->external_ids, "ovn-bfd-min-rx");
>> > + decay_min_rx = smap_get(&cfg->external_ids,
>> > "ovn-bfd-decay-min-rx");
>> > + min_tx = smap_get(&cfg->external_ids, "ovn-bfd-min-tx");
>> > + }
>> > + struct smap bfd = SMAP_INITIALIZER(&bfd);
>> > + 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);
>> > + }
>> > /* 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));
>> > + bool bfd_setting = sset_contains(&bfd_ifaces, iface->name);
>> > + smap_replace(&bfd, "enable", bfd_setting ? "true":"false");
>> > + if (!smap_equal(&iface->bfd, &bfd)) {
>> > + interface_set_bfd(iface, &bfd);
>> > + VLOG_INFO("%s BFD on interface %s",
>> > + bfd_setting ? "Enabled" : "Disabled",
>> > 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..9243bec69 100644
>> > --- a/ovn/controller/bfd.h
>> > +++ b/ovn/controller/bfd.h
>> > @@ -21,6 +21,7 @@ struct ovsdb_idl;
>> > struct ovsdb_idl_index;
>> > struct ovsrec_bridge;
>> > struct ovsrec_interface_table;
>> > +struct ovsrec_open_vswitch_table;
>> > struct sbrec_chassis;
>> > struct sset;
>> >
>> > @@ -28,6 +29,7 @@ void bfd_register_ovs_idl(struct ovsdb_idl *);
>> > void bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
>> > struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
>> > const struct ovsrec_interface_table *interface_table,
>> > + const struct ovsrec_open_vswitch_table *ovs_table,
>> > const struct ovsrec_bridge *br_int,
>> > const struct sbrec_chassis *chassis_rec,
>> > const struct hmap *local_datapaths);
>> > diff --git a/ovn/controller/ovn-controller.8.xml
>> > b/ovn/controller/ovn-controller.8.xml
>> > index 8035638b3..e536f92fa 100644
>> > --- a/ovn/controller/ovn-controller.8.xml
>> > +++ b/ovn/controller/ovn-controller.8.xml
>> > @@ -159,6 +159,15 @@
>> > specific to this particular chassis. An example would be:
>> > <code>cms_option1,cms_option2:foo</code>.
>> > </dd>
>> > +
>> > + <dt><code>external_ids:ovn-bfd-min-rx</code></dt>
>> > + <dt><code>external_ids:ovn-bfd-decay-min-rx</code></dt>
>> > + <dt><code>external_ids:ovn-bfd-min-tx</code></dt>
>> > + <dd>
>> > + These are the BFD parameters to use when
>> > <code>ovn-controller</code>
>> > + enables BFD on the tunnel ports. These can be used to override
>> the
>> > + default BFD parameters.
>> > + </dd>
>> > </dl>
>> >
>> > <p>
>> > diff --git a/ovn/controller/ovn-controller.c
>> > b/ovn/controller/ovn-controller.c
>> > index 85921a03a..bca94e521 100644
>> > --- a/ovn/controller/ovn-controller.c
>> > +++ b/ovn/controller/ovn-controller.c
>> > @@ -765,10 +765,12 @@ main(int argc, char *argv[])
>> > &flow_table, &group_table, &meter_table);
>> >
>> > if (chassis_id) {
>> > - bfd_run(sbrec_chassis_by_name,
>> > - sbrec_port_binding_by_datapath,
>> > -
>> > ovsrec_interface_table_get(ovs_idl_loop.idl),
>> > - br_int, chassis, &local_datapaths);
>> > + bfd_run(
>> > + sbrec_chassis_by_name,
>> > + sbrec_port_binding_by_datapath,
>> > +
>> ovsrec_interface_table_get(ovs_idl_loop.idl),
>> > +
>> > ovsrec_open_vswitch_table_get(ovs_idl_loop.idl),
>> > + br_int, chassis, &local_datapaths);
>> > }
>> > physical_run(
>> > sbrec_chassis_by_name,
>> > diff --git a/tests/ovn.at b/tests/ovn.at
>> > index 769e09f81..5160d74c5 100644
>> > --- a/tests/ovn.at
>> > +++ b/tests/ovn.at
>> > @@ -9271,6 +9271,32 @@ AT_CHECK([ovn-sbctl --columns chassis --bare find
>> > Port_Binding logical_port=cr-o
>> > [0],[[1
>> > ]])
>> >
>> > +as gw2
>> > +ovs-vsctl set Open . external-ids:ovn-bfd-min-rx=2000
>> > +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
>> > +ovs-vsctl set Open . external-ids:ovn-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
>> > +ovs-vsctl remove Open . external-ids ovn-bfd-min-rx
>> > +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"
>> > +])
>> > +done
>> > +
>> > OVN_CLEANUP([gw1],[gw2],[hv1],[hv2])
>> >
>> > AT_CLEANUP
>> > --
>> > 2.17.1
>> >
>> > _______________________________________________
>> > dev mailing list
>> > dev at openvswitch.org
>> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>> >
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
>
> --
> Miguel Ángel Ajo
> OSP / Networking DFG, OVN Squad Engineering
>
More information about the dev
mailing list