On 7/7/23 14:11, Ales Musil wrote: > On Tue, Jun 27, 2023 at 11:37 AM Xavier Simonart <[email protected]> > wrote: > >> When sb is read-only, the port claim is delayed until sb is rw. >> However, before this patch, this resulted in the chassis always >> claiming the port as main (while it was maybe an additional chassis). >> >> Fixes: a7c7d4519e50 ("controller: avoid recomputes triggered by SBDB >> Port_Binding updates.") >> >> Signed-off-by: Xavier Simonart <[email protected]> >> --- >> controller/binding.c | 14 +++++++++++--- >> controller/binding.h | 3 ++- >> controller/if-status.c | 10 ++++++---- >> 3 files changed, 19 insertions(+), 8 deletions(-) >> >> diff --git a/controller/binding.c b/controller/binding.c >> index 359ad6d66..9fb90cf9f 100644 >> --- a/controller/binding.c >> +++ b/controller/binding.c >> @@ -1164,7 +1164,9 @@ local_bindings_pb_chassis_is_set(struct shash >> *local_bindings, >> local_binding_find(local_bindings, pb_name); >> struct binding_lport *b_lport = >> local_binding_get_primary_lport(lbinding); >> >> - if (b_lport && b_lport->pb && b_lport->pb->chassis == chassis_rec) { >> + if (b_lport && b_lport->pb && >> + ((b_lport->pb->chassis == chassis_rec) || >> + is_additional_chassis(b_lport->pb, chassis_rec))) { >> return true; >> } >> return false; >> @@ -1173,14 +1175,20 @@ local_bindings_pb_chassis_is_set(struct shash >> *local_bindings, >> void >> local_binding_set_pb(struct shash *local_bindings, const char *pb_name, >> const struct sbrec_chassis *chassis_rec, >> - struct hmap *tracked_datapaths, bool is_set) >> + struct hmap *tracked_datapaths, bool is_set, >> + enum can_bind bind_type) >> { >> struct local_binding *lbinding = >> local_binding_find(local_bindings, pb_name); >> struct binding_lport *b_lport = >> local_binding_get_primary_lport(lbinding); >> >> if (b_lport) { >> - set_pb_chassis_in_sbrec(b_lport->pb, chassis_rec, is_set); >> + if (bind_type == CAN_BIND_AS_MAIN) { >> + set_pb_chassis_in_sbrec(b_lport->pb, chassis_rec, is_set); >> + } else if (bind_type == CAN_BIND_AS_ADDITIONAL) { >> + set_pb_additional_chassis_in_sbrec(b_lport->pb, chassis_rec, >> + is_set); >> + } >> if (tracked_datapaths) { >> update_lport_tracking(b_lport->pb, tracked_datapaths, true); >> } >> diff --git a/controller/binding.h b/controller/binding.h >> index 319cbd263..abc3d6117 100644 >> --- a/controller/binding.h >> +++ b/controller/binding.h >> @@ -23,6 +23,7 @@ >> #include "openvswitch/uuid.h" >> #include "openvswitch/list.h" >> #include "sset.h" >> +#include "lport.h" >> >> struct hmap; >> struct ovsdb_idl; >> @@ -177,7 +178,7 @@ void local_binding_set_down(struct shash >> *local_bindings, const char *pb_name, >> void local_binding_set_pb(struct shash *local_bindings, const char >> *pb_name, >> const struct sbrec_chassis *chassis_rec, >> struct hmap *tracked_datapaths, >> - bool is_set); >> + bool is_set, enum can_bind); >> bool local_bindings_pb_chassis_is_set(struct shash *local_bindings, >> const char *pb_name, >> const struct sbrec_chassis >> *chassis_rec); >> diff --git a/controller/if-status.c b/controller/if-status.c >> index 2b2eb1679..b45208746 100644 >> --- a/controller/if-status.c >> +++ b/controller/if-status.c >> @@ -184,6 +184,7 @@ struct ovs_iface { >> * OIF_INSTALL_FLOWS. >> */ >> uint16_t mtu; /* Extracted from OVS interface.mtu field. */ >> + enum can_bind bind_type;/* CAN_BIND_AS_MAIN or CAN_BIND_AS_ADDITIONAL >> */ >> }; >> >> static uint64_t ifaces_usage; >> @@ -285,6 +286,7 @@ if_status_mgr_claim_iface(struct if_status_mgr *mgr, >> if (!iface) { >> iface = ovs_iface_create(mgr, iface_id, iface_rec, OIF_CLAIMED); >> } >> + iface->bind_type = bind_type; >> >> memcpy(&iface->pb_uuid, &pb->header_.uuid, sizeof(iface->pb_uuid)); >> if (!sb_readonly) { >> @@ -406,7 +408,7 @@ if_status_handle_claims(struct if_status_mgr *mgr, >> struct ovs_iface *iface = node->data; >> VLOG_INFO("if_status_handle_claims for %s", iface->id); >> local_binding_set_pb(bindings, iface->id, chassis_rec, >> - tracked_datapath, true); >> + tracked_datapath, true, iface->bind_type); >> rc = true; >> } >> return rc; >> @@ -473,7 +475,7 @@ if_status_mgr_update(struct if_status_mgr *mgr, >> chassis_rec)) { >> if (!sb_readonly) { >> local_binding_set_pb(bindings, iface->id, chassis_rec, >> - NULL, true); >> + NULL, true, iface->bind_type); >> } else { >> continue; >> } >> @@ -495,7 +497,7 @@ if_status_mgr_update(struct if_status_mgr *mgr, >> } >> if (!sb_readonly) { >> local_binding_set_pb(bindings, iface->id, chassis_rec, >> - NULL, false); >> + NULL, false, iface->bind_type); >> } >> if (local_binding_is_down(bindings, iface->id, chassis_rec)) { >> ovs_iface_destroy(mgr, iface); >> @@ -512,7 +514,7 @@ if_status_mgr_update(struct if_status_mgr *mgr, >> if (!local_bindings_pb_chassis_is_set(bindings, iface->id, >> chassis_rec)) { >> local_binding_set_pb(bindings, iface->id, chassis_rec, >> - NULL, true); >> + NULL, true, iface->bind_type); >> } >> } >> } >> -- >> 2.31.1 >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> > Looks good to me, thanks. > > Acked-by: Ales Musil <[email protected]> >
Thanks Xavier and Ales! I applied this to main and backported it to all stable branches down to 22.09. Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
