On Fri, Aug 11, 2023 at 2:25 PM Dumitru Ceara <dce...@redhat.com> wrote:
> It's safe to assume that tables that existed in the previous LTS branch > first release (currently 22.03.0) can be monitored directly. Do so and > only "optionally" monitor the ones that have been added since. > > This way we avoid the need for the IDL to expose an API to change the > default condition for monitored tables. It also avoids complex code in > ovn-controller because we'd otherwise have to explicitly re-initialize > conditions to a non-default (false) value after every SB reconnect. > > NOTE: In order to make sure that pre-existing L3 and L2 gateways are not > initially considered "non-local" we explicitly request for all port > bindings of this type to be monitored in the startup stage (before we > got the initial contents of the database and our chassis record). > > Reported-at: > https://mail.openvswitch.org/pipermail/ovs-dev/2023-July/406201.html > Reported-by: Vladislav Odintsov <odiv...@gmail.com> > CC: Ales Musil <amu...@redhat.com> > CC: Han Zhou <hz...@ovn.org> > Signed-off-by: Dumitru Ceara <dce...@redhat.com> > --- > controller/ovn-controller.c | 40 > ++++++++++++++++++++++++++-------------- > 1 file changed, 26 insertions(+), 14 deletions(-) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 1bba7715d7..0c975dc5ec 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -190,11 +190,15 @@ static char *get_file_system_id(void) > /* Only set monitor conditions on tables that are available in the > * server schema. > */ > -#define sb_table_set_monitor_condition(idl, table, cond) \ > - (sbrec_server_has_##table##_table(idl) \ > - ? sbrec_##table##_set_condition(idl, cond) \ > +#define sb_table_set_opt_mon_condition(idl, table, cond) \ > + (sbrec_server_has_##table##_table(idl) \ > + ? sbrec_##table##_set_condition(idl, cond) \ > : 0) > > +/* Assume the table exists in the server schema and set its condition. */ > +#define sb_table_set_req_mon_condition(idl, table, cond) \ > + sbrec_##table##_set_condition(idl, cond) > + > static unsigned int > update_sb_monitors(struct ovsdb_idl *ovnsb_idl, > const struct sbrec_chassis *chassis, > @@ -294,6 +298,14 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, > /* During initialization, we monitor all records in > Chassis_Private so > * that we don't try to recreate existing ones. */ > ovsdb_idl_condition_add_clause_true(&chprv); > + /* Also, to avoid traffic disruption (e.g., conntrack flushing for > + * zones that are used by OVN but not yet known due to the SB > initial > + * contents not being available), monitor all port bindings > + * connected to gateways; they might be claimed as soon as the > + * chassis is available. > + */ > + sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ, "l2gateway"); > + sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ, "l3gateway"); > } > > if (local_ifaces) { > @@ -342,17 +354,17 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, > > out:; > unsigned int cond_seqnos[] = { > - sb_table_set_monitor_condition(ovnsb_idl, port_binding, &pb), > - sb_table_set_monitor_condition(ovnsb_idl, logical_flow, &lf), > - sb_table_set_monitor_condition(ovnsb_idl, logical_dp_group, > &ldpg), > - sb_table_set_monitor_condition(ovnsb_idl, mac_binding, &mb), > - sb_table_set_monitor_condition(ovnsb_idl, multicast_group, &mg), > - sb_table_set_monitor_condition(ovnsb_idl, dns, &dns), > - sb_table_set_monitor_condition(ovnsb_idl, controller_event, &ce), > - sb_table_set_monitor_condition(ovnsb_idl, ip_multicast, > &ip_mcast), > - sb_table_set_monitor_condition(ovnsb_idl, igmp_group, &igmp), > - sb_table_set_monitor_condition(ovnsb_idl, chassis_private, > &chprv), > - sb_table_set_monitor_condition(ovnsb_idl, chassis_template_var, > &tv), > + sb_table_set_req_mon_condition(ovnsb_idl, port_binding, &pb), > + sb_table_set_req_mon_condition(ovnsb_idl, logical_flow, &lf), > + sb_table_set_req_mon_condition(ovnsb_idl, logical_dp_group, > &ldpg), > + sb_table_set_req_mon_condition(ovnsb_idl, mac_binding, &mb), > + sb_table_set_req_mon_condition(ovnsb_idl, multicast_group, &mg), > + sb_table_set_req_mon_condition(ovnsb_idl, dns, &dns), > + sb_table_set_req_mon_condition(ovnsb_idl, controller_event, &ce), > + sb_table_set_req_mon_condition(ovnsb_idl, ip_multicast, > &ip_mcast), > + sb_table_set_req_mon_condition(ovnsb_idl, igmp_group, &igmp), > + sb_table_set_req_mon_condition(ovnsb_idl, chassis_private, > &chprv), > + sb_table_set_opt_mon_condition(ovnsb_idl, chassis_template_var, > &tv), > }; > > unsigned int expected_cond_seqno = 0; > > Looks good to me, thanks. Acked-by: Ales Musil <amu...@redhat.com> -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> amu...@redhat.com IM: amusil <https://red.ht/sig> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
