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]>
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA <https://www.redhat.com>
[email protected] IM: amusil
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
