On Mon, Oct 2, 2023 at 1:00 AM Dumitru Ceara <[email protected]> wrote: > > On 10/1/23 21:26, Han Zhou wrote: > > Use conditional monitor for the FDB for local datapaths. > > > > Signed-off-by: Han Zhou <[email protected]> > > --- > > Hi Han, > > This change makes complete sense and I think it should be backported to > all supported stable branches: > > Fixes: 6ec3b12590f9 ("MAC learning: Add a new FDB table in southbound db.") > > There's however one minor issue (also reported by ovsrobot CI at > https://github.com/ovsrobot/ovn/actions/runs/6373049552). > > > v2: remove the accidentally modified OVS commit id of v1. > > > > controller/ovn-controller.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > > index 859d9cab917b..2ff78619a3a7 100644 > > --- a/controller/ovn-controller.c > > +++ b/controller/ovn-controller.c > > @@ -211,8 +211,8 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, > > { > > /* Monitor Port_Bindings rows for local interfaces and local datapaths. > > * > > - * Monitor Logical_Flow, MAC_Binding, Multicast_Group, and DNS tables for > > - * local datapaths. > > + * Monitor Logical_Flow, MAC_Binding, FDB, Multicast_Group, and DNS tables > > + * for local datapaths. > > * > > * Monitor Controller_Event rows for local chassis. > > * > > @@ -230,6 +230,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, > > struct ovsdb_idl_condition lf = OVSDB_IDL_CONDITION_INIT(&lf); > > struct ovsdb_idl_condition ldpg = OVSDB_IDL_CONDITION_INIT(&ldpg); > > struct ovsdb_idl_condition mb = OVSDB_IDL_CONDITION_INIT(&mb); > > + struct ovsdb_idl_condition fdb = OVSDB_IDL_CONDITION_INIT(&fdb); > > struct ovsdb_idl_condition mg = OVSDB_IDL_CONDITION_INIT(&mg); > > struct ovsdb_idl_condition dns = OVSDB_IDL_CONDITION_INIT(&dns); > > struct ovsdb_idl_condition ce = OVSDB_IDL_CONDITION_INIT(&ce); > > @@ -248,6 +249,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, > > ovsdb_idl_condition_add_clause_true(&pb); > > ovsdb_idl_condition_add_clause_true(&lf); > > ovsdb_idl_condition_add_clause_true(&mb); > > + ovsdb_idl_condition_add_clause_true(&fdb); > > ovsdb_idl_condition_add_clause_true(&mg); > > ovsdb_idl_condition_add_clause_true(&dns); > > ovsdb_idl_condition_add_clause_true(&ce); > > @@ -337,6 +339,8 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, > > sbrec_logical_flow_add_clause_logical_datapath(&lf, OVSDB_F_EQ, > > uuid); > > sbrec_mac_binding_add_clause_datapath(&mb, OVSDB_F_EQ, uuid); > > + sbrec_fdb_add_clause_dp_key(&fdb, OVSDB_F_EQ, > > + ld->datapath->tunnel_key); > > sbrec_multicast_group_add_clause_datapath(&mg, OVSDB_F_EQ, uuid); > > sbrec_dns_add_clause_datapaths(&dns, OVSDB_F_INCLUDES, &uuid, 1); > > sbrec_ip_multicast_add_clause_datapath(&ip_mcast, OVSDB_F_EQ, > > @@ -360,6 +364,7 @@ out:; > > 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, fdb, &fdb), > > 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), > > We're leaking the condition at the end of the function. We're > missing: > > ovsdb_idl_condition_destroy(&fdb);
Oops, sorry! Thank you (and the robot) for spotting this. > > With that addressed and assuming all the OVN tests pass with > this applied: > > Acked-by: Dumitru Ceara <[email protected]> Applied to main and backported down to 22.03. Thanks, Han > > Thanks, > Dumitru > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
