diff mbox series

[ovs-dev,4/5] Allow the MAC binding age threshold to be configurable

Message ID 20220614134916.28783-5-amusil@redhat.com
State Superseded
Headers show
Series Add MAC binding aging mechanism | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Ales Musil June 14, 2022, 1:49 p.m. UTC
To allow fine tuning the right value for MAC binding
aging add configuration into NB global table called
"mac_binding_age_threshold" which accept threshold in
seconds. Default value being 60 if not specified.

Reported-at: https://bugzilla.redhat.com/2084668
Signed-off-by: Ales Musil <amusil@redhat.com>
---
 controller/ovn-controller.c | 16 +++++++++++++++-
 ovn-nb.xml                  |  5 +++++
 2 files changed, 20 insertions(+), 1 deletion(-)

Comments

Mark Michelson June 15, 2022, 6:13 p.m. UTC | #1
On 6/14/22 09:49, Ales Musil wrote:
> To allow fine tuning the right value for MAC binding
> aging add configuration into NB global table called
> "mac_binding_age_threshold" which accept threshold in
> seconds. Default value being 60 if not specified.

Would there be any value to allowing users to disable MAC binding aging? 
In other words, do we think anyone is going to want to keep the behavior 
that was present in previous versions of OVN?

> 
> Reported-at: https://bugzilla.redhat.com/2084668
> Signed-off-by: Ales Musil <amusil@redhat.com>
> ---
>   controller/ovn-controller.c | 16 +++++++++++++++-
>   ovn-nb.xml                  |  5 +++++
>   2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index f91efadf5..9c77c5fcb 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -2292,6 +2292,7 @@ non_vif_data_ovs_iface_handler(struct engine_node *node, void *data OVS_UNUSED)
>   
>   struct ed_type_northd_options {
>       bool lb_hairpin_use_ct_mark;
> +    unsigned long long mb_age_threshold_msec;
>   };
>   
>   
> @@ -2322,6 +2323,9 @@ en_northd_options_run(struct engine_node *node, void *data)
>           ? smap_get_bool(&sb_global->options, "lb_hairpin_use_ct_mark",
>                           DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK)
>           : DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK;
> +    n_opts->mb_age_threshold_msec =
> +        smap_get_ullong(&sb_global->options, "mac_binding_age_threshold",
> +                        60) * 1000;
>       engine_set_node_state(node, EN_UPDATED);
>   }
>   
> @@ -2339,11 +2343,19 @@ en_northd_options_sb_sb_global_handler(struct engine_node *node, void *data)
>           ? smap_get_bool(&sb_global->options, "lb_hairpin_use_ct_mark",
>                           DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK)
>           : DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK;
> +    unsigned long long mb_age_threshold_msec =
> +        smap_get_ullong(&sb_global->options, "mac_binding_age_threshold",
> +                        60) * 1000;
>   
>       if (lb_hairpin_use_ct_mark != n_opts->lb_hairpin_use_ct_mark) {
>           n_opts->lb_hairpin_use_ct_mark = lb_hairpin_use_ct_mark;
>           engine_set_node_state(node, EN_UPDATED);
>       }
> +
> +    if (mb_age_threshold_msec != n_opts->mb_age_threshold_msec) {
> +        n_opts->mb_age_threshold_msec = mb_age_threshold_msec;
> +        engine_set_node_state(node, EN_UPDATED);
> +    }
>       return true;
>   }
>   
> @@ -3960,12 +3972,14 @@ main(int argc, char *argv[])
>                           }
>                           stopwatch_start(MAC_BIDNING_AGING_STOPWATCH_NAME,
>                                           time_msec());
> +                        struct ed_type_northd_options *n_opts =
> +                            engine_get_data(&en_northd_options);
>                           mac_binding_aging_run(ovnsb_idl_txn ,br_int->name,
>                                                 chassis,
>                                                 sbrec_mac_binding_table_get
>                                                     (ovnsb_idl_loop.idl),
>                                                 sbrec_mac_biding_by_chassis,
> -                                              60000);
> +                                              n_opts->mb_age_threshold_msec);
>                           stopwatch_stop(MAC_BIDNING_AGING_STOPWATCH_NAME,
>                                          time_msec());
>                           stopwatch_start(PINCTRL_RUN_STOPWATCH_NAME,
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 14a624c16..630f906b5 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -162,6 +162,11 @@
>           dynamically assigned, e.g. <code>00:11:22</code>
>         </column>
>   
> +      <column name="options" key="mac_binding_age_threshold">
> +        MAC binding aging <code>threshold</code> value in secs. MAC binding
> +        exceeding this timeout will be automatically removed.
> +      </column>
> +
>         <column name="options" key="controller_event" type='{"type": "boolean"}'>
>           Value set by the CMS to enable/disable ovn-controller event reporting.
>           Traffic into OVS can raise a 'controller' event that results in a
>
Ales Musil June 17, 2022, 9:08 a.m. UTC | #2
On Wed, Jun 15, 2022 at 8:13 PM Mark Michelson <mmichels@redhat.com> wrote:

> On 6/14/22 09:49, Ales Musil wrote:
> > To allow fine tuning the right value for MAC binding
> > aging add configuration into NB global table called
> > "mac_binding_age_threshold" which accept threshold in
> > seconds. Default value being 60 if not specified.
>
> Would there be any value to allowing users to disable MAC binding aging?
> In other words, do we think anyone is going to want to keep the behavior
> that was present in previous versions of OVN?
>

Hi,

not that much IMO, but we can have special value e.g. 0 to disable it.

Thanks,
Ales


>
> >
> > Reported-at: https://bugzilla.redhat.com/2084668
> > Signed-off-by: Ales Musil <amusil@redhat.com>
> > ---
> >   controller/ovn-controller.c | 16 +++++++++++++++-
> >   ovn-nb.xml                  |  5 +++++
> >   2 files changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > index f91efadf5..9c77c5fcb 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -2292,6 +2292,7 @@ non_vif_data_ovs_iface_handler(struct engine_node
> *node, void *data OVS_UNUSED)
> >
> >   struct ed_type_northd_options {
> >       bool lb_hairpin_use_ct_mark;
> > +    unsigned long long mb_age_threshold_msec;
> >   };
> >
> >
> > @@ -2322,6 +2323,9 @@ en_northd_options_run(struct engine_node *node,
> void *data)
> >           ? smap_get_bool(&sb_global->options, "lb_hairpin_use_ct_mark",
> >                           DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK)
> >           : DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK;
> > +    n_opts->mb_age_threshold_msec =
> > +        smap_get_ullong(&sb_global->options,
> "mac_binding_age_threshold",
> > +                        60) * 1000;
> >       engine_set_node_state(node, EN_UPDATED);
> >   }
> >
> > @@ -2339,11 +2343,19 @@ en_northd_options_sb_sb_global_handler(struct
> engine_node *node, void *data)
> >           ? smap_get_bool(&sb_global->options, "lb_hairpin_use_ct_mark",
> >                           DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK)
> >           : DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK;
> > +    unsigned long long mb_age_threshold_msec =
> > +        smap_get_ullong(&sb_global->options,
> "mac_binding_age_threshold",
> > +                        60) * 1000;
> >
> >       if (lb_hairpin_use_ct_mark != n_opts->lb_hairpin_use_ct_mark) {
> >           n_opts->lb_hairpin_use_ct_mark = lb_hairpin_use_ct_mark;
> >           engine_set_node_state(node, EN_UPDATED);
> >       }
> > +
> > +    if (mb_age_threshold_msec != n_opts->mb_age_threshold_msec) {
> > +        n_opts->mb_age_threshold_msec = mb_age_threshold_msec;
> > +        engine_set_node_state(node, EN_UPDATED);
> > +    }
> >       return true;
> >   }
> >
> > @@ -3960,12 +3972,14 @@ main(int argc, char *argv[])
> >                           }
> >
>  stopwatch_start(MAC_BIDNING_AGING_STOPWATCH_NAME,
> >                                           time_msec());
> > +                        struct ed_type_northd_options *n_opts =
> > +                            engine_get_data(&en_northd_options);
> >                           mac_binding_aging_run(ovnsb_idl_txn
> ,br_int->name,
> >                                                 chassis,
> >
>  sbrec_mac_binding_table_get
> >                                                     (ovnsb_idl_loop.idl),
> >
>  sbrec_mac_biding_by_chassis,
> > -                                              60000);
> > +
> n_opts->mb_age_threshold_msec);
> >
>  stopwatch_stop(MAC_BIDNING_AGING_STOPWATCH_NAME,
> >                                          time_msec());
> >                           stopwatch_start(PINCTRL_RUN_STOPWATCH_NAME,
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index 14a624c16..630f906b5 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -162,6 +162,11 @@
> >           dynamically assigned, e.g. <code>00:11:22</code>
> >         </column>
> >
> > +      <column name="options" key="mac_binding_age_threshold">
> > +        MAC binding aging <code>threshold</code> value in secs. MAC
> binding
> > +        exceeding this timeout will be automatically removed.
> > +      </column>
> > +
> >         <column name="options" key="controller_event" type='{"type":
> "boolean"}'>
> >           Value set by the CMS to enable/disable ovn-controller event
> reporting.
> >           Traffic into OVS can raise a 'controller' event that results
> in a
> >
>
>
diff mbox series

Patch

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index f91efadf5..9c77c5fcb 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2292,6 +2292,7 @@  non_vif_data_ovs_iface_handler(struct engine_node *node, void *data OVS_UNUSED)
 
 struct ed_type_northd_options {
     bool lb_hairpin_use_ct_mark;
+    unsigned long long mb_age_threshold_msec;
 };
 
 
@@ -2322,6 +2323,9 @@  en_northd_options_run(struct engine_node *node, void *data)
         ? smap_get_bool(&sb_global->options, "lb_hairpin_use_ct_mark",
                         DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK)
         : DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK;
+    n_opts->mb_age_threshold_msec =
+        smap_get_ullong(&sb_global->options, "mac_binding_age_threshold",
+                        60) * 1000;
     engine_set_node_state(node, EN_UPDATED);
 }
 
@@ -2339,11 +2343,19 @@  en_northd_options_sb_sb_global_handler(struct engine_node *node, void *data)
         ? smap_get_bool(&sb_global->options, "lb_hairpin_use_ct_mark",
                         DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK)
         : DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK;
+    unsigned long long mb_age_threshold_msec =
+        smap_get_ullong(&sb_global->options, "mac_binding_age_threshold",
+                        60) * 1000;
 
     if (lb_hairpin_use_ct_mark != n_opts->lb_hairpin_use_ct_mark) {
         n_opts->lb_hairpin_use_ct_mark = lb_hairpin_use_ct_mark;
         engine_set_node_state(node, EN_UPDATED);
     }
+
+    if (mb_age_threshold_msec != n_opts->mb_age_threshold_msec) {
+        n_opts->mb_age_threshold_msec = mb_age_threshold_msec;
+        engine_set_node_state(node, EN_UPDATED);
+    }
     return true;
 }
 
@@ -3960,12 +3972,14 @@  main(int argc, char *argv[])
                         }
                         stopwatch_start(MAC_BIDNING_AGING_STOPWATCH_NAME,
                                         time_msec());
+                        struct ed_type_northd_options *n_opts =
+                            engine_get_data(&en_northd_options);
                         mac_binding_aging_run(ovnsb_idl_txn ,br_int->name,
                                               chassis,
                                               sbrec_mac_binding_table_get
                                                   (ovnsb_idl_loop.idl),
                                               sbrec_mac_biding_by_chassis,
-                                              60000);
+                                              n_opts->mb_age_threshold_msec);
                         stopwatch_stop(MAC_BIDNING_AGING_STOPWATCH_NAME,
                                        time_msec());
                         stopwatch_start(PINCTRL_RUN_STOPWATCH_NAME,
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 14a624c16..630f906b5 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -162,6 +162,11 @@ 
         dynamically assigned, e.g. <code>00:11:22</code>
       </column>
 
+      <column name="options" key="mac_binding_age_threshold">
+        MAC binding aging <code>threshold</code> value in secs. MAC binding
+        exceeding this timeout will be automatically removed.
+      </column>
+
       <column name="options" key="controller_event" type='{"type": "boolean"}'>
         Value set by the CMS to enable/disable ovn-controller event reporting.
         Traffic into OVS can raise a 'controller' event that results in a