On Fri, Aug 11, 2023 at 5:25 AM 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). >
The commit subject and message had a good introduction about the solution but it would be better to at least add a brief description of the problem (about the memory spike) before the first paragraph. It would also be good to add a Fixes tag for commit 1b0dbde. Otherwise it looks good to me! Acked-by: Han Zhou <hz...@ovn.org> > 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; > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
