On Tue, Sep 25, 2018 at 10:18 PM Miguel Angel Ajo Pelayo < majop...@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 <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