Correct, I'm handling this in the next iteration.

On Wed, May 11, 2022 at 2:23 AM Han Zhou <[email protected]> wrote:
>
>
>
> On Thu, May 5, 2022 at 6:38 AM Ihar Hrachyshka <[email protected]> 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 <[email protected]>
> > ---
> >  controller/binding.c        | 10 +++++++---
> >  controller/binding.h        |  2 ++
> >  controller/if-status.c      | 15 ++++++++++-----
> >  controller/if-status.h      |  1 +
> >  controller/ovn-controller.c |  4 ++--
> >  5 files changed, 22 insertions(+), 10 deletions(-)
> >
> > diff --git a/controller/binding.c b/controller/binding.c
> > index e284704d8..e8c96a64a 100644
> > --- a/controller/binding.c
> > +++ b/controller/binding.c
> > @@ -691,6 +691,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)
> >  {
> > @@ -710,8 +711,9 @@ 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 || b_lport->pb->chassis == 
> > chassis_rec)) {
>
> Why not just add "&& b_lport->pb->chassis == chassis_rec"? It doesn't seem to 
> be reasonable to set "up" when pb->chassis is NULL.
>
> Thanks,
> Han
>
> >          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) {
> > @@ -722,6 +724,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 =
> > @@ -736,7 +739,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 430a8d9b1..46f88aff7 100644
> > --- a/controller/binding.h
> > +++ b/controller/binding.h
> > @@ -154,9 +154,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 22b7fa935..4d51f8657 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -3915,8 +3915,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
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to