On Tue, Jun 27, 2023 at 11:37 AM Xavier Simonart <xsimo...@redhat.com> 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 <xsimo...@redhat.com> > --- > 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 > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > 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