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 a lot,
Miguel Ángel.

On Tue, Sep 25, 2018 at 5:46 PM Anil Venkata <anilvenk...@redhat.com> wrote:

> On Mon, Sep 24, 2018 at 3:12 PM <nusid...@redhat.com> wrote:
>
> > From: Numan Siddique <nusid...@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 <nusid...@redhat.com>
> >
> Acked-by: Venkata Anil <vkomm...@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
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


-- 
Miguel Ángel Ajo
OSP / Networking DFG, OVN Squad Engineering
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to