On Thu, Mar 14, 2019 at 12:33 PM <nusid...@redhat.com> wrote: > > diff --git a/ovn/controller/bfd.h b/ovn/controller/bfd.h > index e36820afb..789f7b269 100644 > --- a/ovn/controller/bfd.h > +++ b/ovn/controller/bfd.h > @@ -24,16 +24,17 @@ struct ovsrec_interface_table; > struct ovsrec_open_vswitch_table; > struct sbrec_chassis; > struct sbrec_sb_global_table; > +struct sbrec_ha_chassis_group_table; > struct sset; > > void bfd_register_ovs_idl(struct ovsdb_idl *); > -void bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name, > - struct ovsdb_idl_index *sbrec_port_binding_by_datapath, > - const struct ovsrec_interface_table *interface_table, > + > +void bfd_run(const struct ovsrec_interface_table *interface_table, > const struct ovsrec_bridge *br_int, > const struct sbrec_chassis *chassis_rec, > - const struct sbrec_sb_global_table *sb_global_table, > - const struct hmap *local_datapaths); > + const struct sbrec_ha_chassis_group_table *ha_chassis_grp_table, > + const struct sbrec_sb_global_table *sb_global_table); > +
Most parameter names can be omitted in this prototype. > + > + struct ha_chassis_ordered *ordered_ha_ch; > + if (n_ha_ch == 1) { > + if (active_tunnels) { > + /* If n_ha_ch is 1, it means only the local chassis is in the > + * ha_ch_order list. Check if this local chassis has active > + * bfd session with any of the referenced chassis. If so, > + * then the local chassis can be active. Otherwise it can't. > + * This can happen in the following scenario. > + * Lets say we have chassis HA1 (prioirty 20) and HA2 (priority > 10) > + * in the ha_chasis_group and compute chassis C1 and C2 are in the > + * reference chassis list. If HA1 chassis has lost the link and > + * when this function is called for HA2 we need to consider > + * HA2 as active since it has active BFD sessions with C1 and C2. > + * On HA1 chassis, this function won't be called since > + * active_tunnels set will be empty. > + * */ > + bool can_local_chassis_be_active = false; > + for (size_t i = 0; i < ha_ch_grp->n_ref_chassis; i++) { > + if (sset_contains(active_tunnels, > + ha_ch_grp->ref_chassis[i]->name)) { > + can_local_chassis_be_active = true; > + break; > + } > + } > + if (!can_local_chassis_be_active) { > + free(ha_ch_order); > + return NULL; > + } > + } What if, a HA-chassis-group has only one chassis, the n_ha_ch will be 1, and active_tunnels is not NULL but will be empty because bfd won't be setup for HA group with only 1 chassis. However, it will enter this branch and returns NULL, which will lead to ha_chassis_group_is_active() return false, and the GW will not function for the logical router port. Did I miss anything here? _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev