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 |
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 >
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 --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; }
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(+)