On Thu, May 19, 2022 at 12:09 PM Ihar Hrachyshka <ihrac...@redhat.com> wrote:
>
> In a future patch, there will be a scenario where the same port has
> attachments at multiple (specifically, 2) chassis, so make sure that
> 'up' property is updated by the main chassis only.
>
> Signed-off-by: Ihar Hrachyshka <ihrac...@redhat.com>
The patch LGTM.
Acked-by: Numan Siddique <num...@ovn.org>
Your 2nd patch is not applying cleanly.
Can you please respin another version of the series ?
You can add my Acked-by tag in the v9 patch 1.
Thanks
Numan
> ---
> controller/binding.c | 9 ++++++---
> controller/binding.h | 2 ++
> controller/if-status.c | 15 ++++++++++-----
> controller/if-status.h | 1 +
> controller/ovn-controller.c | 4 ++--
> 5 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index 0b6e83a74..6bdbe9912 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -698,6 +698,7 @@ local_binding_is_down(struct shash *local_bindings, const
> char *pb_name)
>
> void
> local_binding_set_up(struct shash *local_bindings, const char *pb_name,
> + const struct sbrec_chassis *chassis_rec,
> const char *ts_now_str, bool sb_readonly,
> bool ovs_readonly)
> {
> @@ -717,8 +718,8 @@ local_binding_set_up(struct shash *local_bindings, const
> char *pb_name,
> ts_now_str);
> }
>
> - if (!sb_readonly && lbinding && b_lport && b_lport->pb->n_up
> - && !b_lport->pb->up[0]) {
> + if (!sb_readonly && lbinding && b_lport && b_lport->pb->n_up &&
> + !b_lport->pb->up[0] && b_lport->pb->chassis == chassis_rec) {
> VLOG_INFO("Setting lport %s up in Southbound", pb_name);
> binding_lport_set_up(b_lport, sb_readonly);
> LIST_FOR_EACH (b_lport, list_node, &lbinding->binding_lports) {
> @@ -729,6 +730,7 @@ local_binding_set_up(struct shash *local_bindings, const
> char *pb_name,
>
> void
> local_binding_set_down(struct shash *local_bindings, const char *pb_name,
> + const struct sbrec_chassis *chassis_rec,
> bool sb_readonly, bool ovs_readonly)
> {
> struct local_binding *lbinding =
> @@ -743,7 +745,8 @@ local_binding_set_down(struct shash *local_bindings,
> const char *pb_name,
> OVN_INSTALLED_EXT_ID);
> }
>
> - if (!sb_readonly && b_lport && b_lport->pb->n_up && b_lport->pb->up[0]) {
> + if (!sb_readonly && b_lport && b_lport->pb->n_up && b_lport->pb->up[0] &&
> + (!b_lport->pb->chassis || b_lport->pb->chassis == chassis_rec)) {
> VLOG_INFO("Setting lport %s down in Southbound", pb_name);
> binding_lport_set_down(b_lport, sb_readonly);
> LIST_FOR_EACH (b_lport, list_node, &lbinding->binding_lports) {
> diff --git a/controller/binding.h b/controller/binding.h
> index e49e1ebb6..6815d1fd6 100644
> --- a/controller/binding.h
> +++ b/controller/binding.h
> @@ -155,9 +155,11 @@ ofp_port_t local_binding_get_lport_ofport(const struct
> shash *local_bindings,
> bool local_binding_is_up(struct shash *local_bindings, const char *pb_name);
> bool local_binding_is_down(struct shash *local_bindings, const char
> *pb_name);
> void local_binding_set_up(struct shash *local_bindings, const char *pb_name,
> + const struct sbrec_chassis *chassis_rec,
> const char *ts_now_str, bool sb_readonly,
> bool ovs_readonly);
> void local_binding_set_down(struct shash *local_bindings, const char
> *pb_name,
> + const struct sbrec_chassis *chassis_rec,
> bool sb_readonly, bool ovs_readonly);
>
> void binding_register_ovs_idl(struct ovsdb_idl *);
> diff --git a/controller/if-status.c b/controller/if-status.c
> index dbdf8b66f..ad61844d8 100644
> --- a/controller/if-status.c
> +++ b/controller/if-status.c
> @@ -115,6 +115,7 @@ static void ovs_iface_set_state(struct if_status_mgr *,
> struct ovs_iface *,
>
> static void if_status_mgr_update_bindings(
> struct if_status_mgr *mgr, struct local_binding_data *binding_data,
> + const struct sbrec_chassis *,
> bool sb_readonly, bool ovs_readonly);
>
> struct if_status_mgr *
> @@ -306,6 +307,7 @@ if_status_mgr_update(struct if_status_mgr *mgr,
> void
> if_status_mgr_run(struct if_status_mgr *mgr,
> struct local_binding_data *binding_data,
> + const struct sbrec_chassis *chassis_rec,
> bool sb_readonly, bool ovs_readonly)
> {
> struct ofctrl_acked_seqnos *acked_seqnos =
> @@ -328,8 +330,8 @@ if_status_mgr_run(struct if_status_mgr *mgr,
> ofctrl_acked_seqnos_destroy(acked_seqnos);
>
> /* Update binding states. */
> - if_status_mgr_update_bindings(mgr, binding_data, sb_readonly,
> - ovs_readonly);
> + if_status_mgr_update_bindings(mgr, binding_data, chassis_rec,
> + sb_readonly, ovs_readonly);
> }
>
> static void
> @@ -390,6 +392,7 @@ ovs_iface_set_state(struct if_status_mgr *mgr, struct
> ovs_iface *iface,
> static void
> if_status_mgr_update_bindings(struct if_status_mgr *mgr,
> struct local_binding_data *binding_data,
> + const struct sbrec_chassis *chassis_rec,
> bool sb_readonly, bool ovs_readonly)
> {
> if (!binding_data) {
> @@ -405,7 +408,8 @@ if_status_mgr_update_bindings(struct if_status_mgr *mgr,
> HMAPX_FOR_EACH (node, &mgr->ifaces_per_state[OIF_INSTALL_FLOWS]) {
> struct ovs_iface *iface = node->data;
>
> - local_binding_set_down(bindings, iface->id, sb_readonly,
> ovs_readonly);
> + local_binding_set_down(bindings, iface->id, chassis_rec,
> + sb_readonly, ovs_readonly);
> }
>
> /* Notifiy the binding module to set "up" all bindings that have had
> @@ -416,7 +420,7 @@ if_status_mgr_update_bindings(struct if_status_mgr *mgr,
> HMAPX_FOR_EACH (node, &mgr->ifaces_per_state[OIF_MARK_UP]) {
> struct ovs_iface *iface = node->data;
>
> - local_binding_set_up(bindings, iface->id, ts_now_str,
> + local_binding_set_up(bindings, iface->id, chassis_rec, ts_now_str,
> sb_readonly, ovs_readonly);
> }
> free(ts_now_str);
> @@ -427,7 +431,8 @@ if_status_mgr_update_bindings(struct if_status_mgr *mgr,
> HMAPX_FOR_EACH (node, &mgr->ifaces_per_state[OIF_MARK_DOWN]) {
> struct ovs_iface *iface = node->data;
>
> - local_binding_set_down(bindings, iface->id, sb_readonly,
> ovs_readonly);
> + local_binding_set_down(bindings, iface->id, chassis_rec,
> + sb_readonly, ovs_readonly);
> }
> }
>
> diff --git a/controller/if-status.h b/controller/if-status.h
> index ff4aa760e..bb8a3950d 100644
> --- a/controller/if-status.h
> +++ b/controller/if-status.h
> @@ -33,6 +33,7 @@ void if_status_mgr_delete_iface(struct if_status_mgr *,
> const char *iface_id);
>
> void if_status_mgr_update(struct if_status_mgr *, struct local_binding_data
> *);
> void if_status_mgr_run(struct if_status_mgr *mgr, struct local_binding_data
> *,
> + const struct sbrec_chassis *,
> bool sb_readonly, bool ovs_readonly);
> void if_status_mgr_get_memory_usage(struct if_status_mgr *mgr,
> struct simap *usage);
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index dfe30d1d1..2628c887a 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -4034,8 +4034,8 @@ main(int argc, char *argv[])
> time_msec());
> stopwatch_start(IF_STATUS_MGR_RUN_STOPWATCH_NAME,
> time_msec());
> - if_status_mgr_run(if_mgr, binding_data, !ovnsb_idl_txn,
> - !ovs_idl_txn);
> + if_status_mgr_run(if_mgr, binding_data, chassis,
> + !ovnsb_idl_txn, !ovs_idl_txn);
> stopwatch_stop(IF_STATUS_MGR_RUN_STOPWATCH_NAME,
> time_msec());
> }
> --
> 2.34.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
