diff mbox series

[ovs-dev,v2,1/1] ovn-controller: Ensure br-int is using secure fail-mode

Message ID 20210507174947.1879798-1-flavio@flaviof.com
State Accepted
Headers show
Series [ovs-dev,v2,1/1] ovn-controller: Ensure br-int is using secure fail-mode | expand

Commit Message

Flavio Fernandes May 7, 2021, 5:49 p.m. UTC
By default, OVS bridges use standalone fail-mode, which means it is
configured with a single row with the NORMAL action as its OpenFlow table.
Upon system reboot, an integration bridge with many ports and such a table
could create broadcast storms and duplicate packets. That is why
ovn-controller creates the integration bridge with secure fail-mode.
Under that mode, the OpenFlow table remains empty until the controller
populates it, which could happen many seconds after the bridge is
operational. Unfortunately, the fail-mode setting was not being
done if the bridge was already created by the time ovn-controller
starts. This change fixes that and logs a warning should the fail-mode
ever needed to be corrected.

Reported-at: https://bugzilla.redhat.com/1957025
Signed-off-by: Flavio Fernandes <flavio@flaviof.com>
---
 v1->v2: Changes from code review. Thanks, Frode!
    
 controller/ovn-controller.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Frode Nordahl May 7, 2021, 9:44 p.m. UTC | #1
fre. 7. mai 2021, 19:50 skrev Flavio Fernandes <flavio@flaviof.com>:

> By default, OVS bridges use standalone fail-mode, which means it is
> configured with a single row with the NORMAL action as its OpenFlow table.
> Upon system reboot, an integration bridge with many ports and such a table
> could create broadcast storms and duplicate packets. That is why
> ovn-controller creates the integration bridge with secure fail-mode.
> Under that mode, the OpenFlow table remains empty until the controller
> populates it, which could happen many seconds after the bridge is
> operational. Unfortunately, the fail-mode setting was not being
> done if the bridge was already created by the time ovn-controller
> starts. This change fixes that and logs a warning should the fail-mode
> ever needed to be corrected.
>
> Reported-at: https://bugzilla.redhat.com/1957025
> Signed-off-by: Flavio Fernandes <flavio@flaviof.com>
> ---
>  v1->v2: Changes from code review. Thanks, Frode!
>
>  controller/ovn-controller.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 6106a9661..d925522e1 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -413,6 +413,10 @@ process_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
>          if (datapath_type && strcmp(br_int->datapath_type,
> datapath_type)) {
>              ovsrec_bridge_set_datapath_type(br_int, datapath_type);
>          }
> +        if (!br_int->fail_mode || strcmp(br_int->fail_mode, "secure")) {
> +            ovsrec_bridge_set_fail_mode(br_int, "secure");
> +            VLOG_WARN("Integration bridge fail-mode changed to
> 'secure'.");
> +        }
>      }
>      return br_int;
>  }
> --
> 2.25.1
>

Reviewed-by: Frode Nordahl <frode.nordahl@canonical.com>

(And now I need to go watch that movie where the term Cool beans was
coined).


>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Numan Siddique May 12, 2021, 9:39 p.m. UTC | #2
On Fri, May 7, 2021 at 5:45 PM Frode Nordahl
<frode.nordahl@canonical.com> wrote:
>
> fre. 7. mai 2021, 19:50 skrev Flavio Fernandes <flavio@flaviof.com>:
>
> > By default, OVS bridges use standalone fail-mode, which means it is
> > configured with a single row with the NORMAL action as its OpenFlow table.
> > Upon system reboot, an integration bridge with many ports and such a table
> > could create broadcast storms and duplicate packets. That is why
> > ovn-controller creates the integration bridge with secure fail-mode.
> > Under that mode, the OpenFlow table remains empty until the controller
> > populates it, which could happen many seconds after the bridge is
> > operational. Unfortunately, the fail-mode setting was not being
> > done if the bridge was already created by the time ovn-controller
> > starts. This change fixes that and logs a warning should the fail-mode
> > ever needed to be corrected.
> >
> > Reported-at: https://bugzilla.redhat.com/1957025
> > Signed-off-by: Flavio Fernandes <flavio@flaviof.com>
> > ---
> >  v1->v2: Changes from code review. Thanks, Frode!
> >
> >  controller/ovn-controller.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > index 6106a9661..d925522e1 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -413,6 +413,10 @@ process_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
> >          if (datapath_type && strcmp(br_int->datapath_type,
> > datapath_type)) {
> >              ovsrec_bridge_set_datapath_type(br_int, datapath_type);
> >          }
> > +        if (!br_int->fail_mode || strcmp(br_int->fail_mode, "secure")) {
> > +            ovsrec_bridge_set_fail_mode(br_int, "secure");
> > +            VLOG_WARN("Integration bridge fail-mode changed to
> > 'secure'.");
> > +        }
> >      }
> >      return br_int;
> >  }
> > --
> > 2.25.1
> >
>
> Reviewed-by: Frode Nordahl <frode.nordahl@canonical.com>
>
> (And now I need to go watch that movie where the term Cool beans was
> coined).

Thanks Flavio and Frode for the patch and reviews.  I applied this
patch to the main
branch and to branch-21.03.  Let me know if we need to backport any further ?

Thanks
Numan

>
>
> >
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 6106a9661..d925522e1 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -413,6 +413,10 @@  process_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
         if (datapath_type && strcmp(br_int->datapath_type, datapath_type)) {
             ovsrec_bridge_set_datapath_type(br_int, datapath_type);
         }
+        if (!br_int->fail_mode || strcmp(br_int->fail_mode, "secure")) {
+            ovsrec_bridge_set_fail_mode(br_int, "secure");
+            VLOG_WARN("Integration bridge fail-mode changed to 'secure'.");
+        }
     }
     return br_int;
 }