Re: [ovs-dev] [PATCH v6] ovn: Support configuring the BFD params for the tunnel interfaces

2019-02-01 Thread Miguel Angel Ajo Pelayo
Oops thank you I don’t know how I looked exactly... :)

On Fri, 1 Feb 2019 at 18:14, Numan Siddique  wrote:

>
>
> On Fri, Feb 1, 2019, 7:55 PM Miguel Angel Ajo Pelayo 
> wrote:
>
>> Hmm, numan I see an older version of your patch on patchworks [1], but
>> not v6
>> May be there was an issue with mail?
>>
>> [1] https://patchwork.ozlabs.org/patch/973888/
>>
>
> Hi Miguel,
> The patch is merged long back :) -
> https://github.com/openvswitch/ovs/commit/2d661a2733010d7e94560c624edbe7f192952b3d
>
> Thanks
> Numan
>
>
>
>> On Fri, Feb 1, 2019 at 3:17 PM Miguel Angel Ajo Pelayo <
>> majop...@redhat.com> wrote:
>>
>>> Hey,
>>>
>>>Did we get this merged in the end?,
>>>
>>>We're having customers facing issues related to this and we may
>>> need to throttle the BFD settings.y
>>>
>>> On Tue, Oct 9, 2018 at 10:54 AM Numan Siddique 
>>> wrote:
>>>
 On Tue, Oct 9, 2018 at 2:18 PM  wrote:

 > 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_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 
 > ---
 >
 > 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, _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(>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(, "enable", new_setting);
 > -ovsrec_interface_verify_bfd(iface);
 > -ovsrec_interface_set_bfd(iface, );
 > -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 

Re: [ovs-dev] [PATCH v6] ovn: Support configuring the BFD params for the tunnel interfaces

2019-02-01 Thread Numan Siddique
On Fri, Feb 1, 2019, 7:55 PM Miguel Angel Ajo Pelayo 
wrote:

> Hmm, numan I see an older version of your patch on patchworks [1], but not
> v6
> May be there was an issue with mail?
>
> [1] https://patchwork.ozlabs.org/patch/973888/
>

Hi Miguel,
The patch is merged long back :) -
https://github.com/openvswitch/ovs/commit/2d661a2733010d7e94560c624edbe7f192952b3d

Thanks
Numan



> On Fri, Feb 1, 2019 at 3:17 PM Miguel Angel Ajo Pelayo <
> majop...@redhat.com> wrote:
>
>> Hey,
>>
>>Did we get this merged in the end?,
>>
>>We're having customers facing issues related to this and we may
>> need to throttle the BFD settings.y
>>
>> On Tue, Oct 9, 2018 at 10:54 AM Numan Siddique 
>> wrote:
>>
>>> On Tue, Oct 9, 2018 at 2:18 PM  wrote:
>>>
>>> > 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_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 
>>> > ---
>>> >
>>> > 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, _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(>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(, "enable", new_setting);
>>> > -ovsrec_interface_verify_bfd(iface);
>>> > -ovsrec_interface_set_bfd(iface, );
>>> > -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)
>>> >  {
>>> >

Re: [ovs-dev] [PATCH v6] ovn: Support configuring the BFD params for the tunnel interfaces

2019-02-01 Thread Miguel Angel Ajo Pelayo
Hmm, numan I see an older version of your patch on patchworks [1], but not
v6
May be there was an issue with mail?

[1] https://patchwork.ozlabs.org/patch/973888/

On Fri, Feb 1, 2019 at 3:17 PM Miguel Angel Ajo Pelayo 
wrote:

> Hey,
>
>Did we get this merged in the end?,
>
>We're having customers facing issues related to this and we may
> need to throttle the BFD settings.y
>
> On Tue, Oct 9, 2018 at 10:54 AM Numan Siddique 
> wrote:
>
>> On Tue, Oct 9, 2018 at 2:18 PM  wrote:
>>
>> > 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_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 
>> > ---
>> >
>> > 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, _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(>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(, "enable", new_setting);
>> > -ovsrec_interface_verify_bfd(iface);
>> > -ovsrec_interface_set_bfd(iface, );
>> > -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();
>> > +smap_add(, "enable", "true");
>> > +
>> > +if (sb) {
>> > +const char 

Re: [ovs-dev] [PATCH v6] ovn: Support configuring the BFD params for the tunnel interfaces

2019-02-01 Thread Miguel Angel Ajo Pelayo
Hey,

   Did we get this merged in the end?,

   We're having customers facing issues related to this and we may
need to throttle the BFD settings.y

On Tue, Oct 9, 2018 at 10:54 AM Numan Siddique  wrote:

> On Tue, Oct 9, 2018 at 2:18 PM  wrote:
>
> > 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_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 
> > ---
> >
> > 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, _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(>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(, "enable", new_setting);
> > -ovsrec_interface_verify_bfd(iface);
> > -ovsrec_interface_set_bfd(iface, );
> > -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();
> > +smap_add(, "enable", "true");
> > +
> > +if (sb) {
> > +const char *min_rx = smap_get(>options, "bfd-min-rx");
> > +const char *decay_min_rx = smap_get(>options,
> > "bfd-decay-min-rx");
> > +const char *min_tx = smap_get(>options, "bfd-min-tx");
> > +const char *mult = smap_get(>options, "bfd-mult");
> > +if (min_rx) {
> > +smap_add(, "min_rx", min_rx);
> > +}
> > +if 

Re: [ovs-dev] [PATCH v6] ovn: Support configuring the BFD params for the tunnel interfaces

2018-10-09 Thread Numan Siddique
On Tue, Oct 9, 2018 at 2:18 PM  wrote:

> 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_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 
> ---
>
> 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, _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(>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(, "enable", new_setting);
> -ovsrec_interface_verify_bfd(iface);
> -ovsrec_interface_set_bfd(iface, );
> -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();
> +smap_add(, "enable", "true");
> +
> +if (sb) {
> +const char *min_rx = smap_get(>options, "bfd-min-rx");
> +const char *decay_min_rx = smap_get(>options,
> "bfd-decay-min-rx");
> +const char *min_tx = smap_get(>options, "bfd-min-tx");
> +const char *mult = smap_get(>options, "bfd-mult");
> +if (min_rx) {
> +smap_add(, "min_rx", min_rx);
> +}
> +if (decay_min_rx) {
> +smap_add(, "decay_min_rx", decay_min_rx);
> +}
> +if (min_tx) {
> +smap_add(, "min_tx", min_tx);
> +}
> +if (mult) {
> +smap_add(, "mult", mult);
> +}
> +}
> +
>  /* Enable or disable bfd */
>  const struct ovsrec_interface *iface;
>  OVSREC_INTERFACE_TABLE_FOR_EACH (iface, interface_table) {
>  if (sset_contains(, iface->name)) {
> -   

[ovs-dev] [PATCH v6] ovn: Support configuring the BFD params for the tunnel interfaces

2018-10-09 Thread nusiddiq
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_bfd=false'.

Signed-off-by: Numan Siddique 
---

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, _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(>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(, "enable", new_setting);
-ovsrec_interface_verify_bfd(iface);
-ovsrec_interface_set_bfd(iface, );
-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();
+smap_add(, "enable", "true");
+
+if (sb) {
+const char *min_rx = smap_get(>options, "bfd-min-rx");
+const char *decay_min_rx = smap_get(>options, "bfd-decay-min-rx");
+const char *min_tx = smap_get(>options, "bfd-min-tx");
+const char *mult = smap_get(>options, "bfd-mult");
+if (min_rx) {
+smap_add(, "min_rx", min_rx);
+}
+if (decay_min_rx) {
+smap_add(, "decay_min_rx", decay_min_rx);
+}
+if (min_tx) {
+smap_add(, "min_tx", min_tx);
+}
+if (mult) {
+smap_add(, "mult", mult);
+}
+}
+
 /* Enable or disable bfd */
 const struct ovsrec_interface *iface;
 OVSREC_INTERFACE_TABLE_FOR_EACH (iface, interface_table) {
 if (sset_contains(, iface->name)) {
-interface_set_bfd(
-iface, sset_contains(_ifaces, iface->name));
+if (sset_contains(_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(>bfd, )) {
+