On Wed, Mar 25, 2020 at 12:22 PM Han Zhou <zhou...@gmail.com> wrote:
>
> On Mon, Mar 16, 2020 at 4:12 AM <num...@ovn.org> wrote:
> >
> > From: Numan Siddique <num...@ovn.org>
> >
> > No functional changes are introduced in this patch.
> >
> > Signed-off-by: Numan Siddique <num...@ovn.org>
> > ---
> > controller/binding.c | 261 +++++++++++++++++-------------------
> > controller/binding.h | 39 +++---
> > controller/ovn-controller.c | 120 ++++++++++-------
> > 3 files changed, 215 insertions(+), 205 deletions(-)
> >
> > diff --git a/controller/binding.c b/controller/binding.c
> > index c3376e25e..1e9f78249 100644
> > --- a/controller/binding.c
> > +++ b/controller/binding.c
> > @@ -441,12 +441,9 @@ static bool
> > is_our_chassis(const struct sbrec_chassis *chassis_rec,
> > const struct sbrec_port_binding *binding_rec,
> > const struct sset *active_tunnels,
> > - const struct shash *lport_to_iface,
> > + const struct ovsrec_interface *iface_rec,
> > const struct sset *local_lports)
> > {
> > - const struct ovsrec_interface *iface_rec
> > - = shash_find_data(lport_to_iface, binding_rec->logical_port);
> > -
> > bool our_chassis = false;
> > if (iface_rec
> > || (binding_rec->parent_port && binding_rec->parent_port[0] &&
> > @@ -478,78 +475,74 @@ is_our_chassis(const struct sbrec_chassis
> *chassis_rec,
> > * additional processing.
> > */
> > static const struct sbrec_port_binding **
> > -consider_local_datapath(struct ovsdb_idl_txn *ovnsb_idl_txn,
> > - struct ovsdb_idl_txn *ovs_idl_txn,
> > - struct ovsdb_idl_index
> *sbrec_datapath_binding_by_key,
> > - struct ovsdb_idl_index
> *sbrec_port_binding_by_datapath,
> > - struct ovsdb_idl_index
> *sbrec_port_binding_by_name,
> > - const struct sset *active_tunnels,
> > - const struct sbrec_chassis *chassis_rec,
> > - const struct sbrec_port_binding *binding_rec,
> > +consider_local_datapath(const struct sbrec_port_binding *binding_rec,
> > struct hmap *qos_map,
> > - struct hmap *local_datapaths,
> > - struct shash *lport_to_iface,
> > - struct sset *local_lports,
> > - struct sset *local_lport_ids,
> > + const struct ovsrec_interface *iface_rec,
> > + struct binding_ctx_in *b_ctx_in,
> > + struct binding_ctx_out *b_ctx_out,
> > const struct sbrec_port_binding **vpbs,
> > size_t *n_vpbs, size_t *n_allocated_vpbs)
> > {
> > - const struct ovsrec_interface *iface_rec
> > - = shash_find_data(lport_to_iface, binding_rec->logical_port);
> > -
> > - bool our_chassis = is_our_chassis(chassis_rec, binding_rec,
> active_tunnels,
> > - lport_to_iface, local_lports);
> > + bool our_chassis = is_our_chassis(b_ctx_in->chassis_rec, binding_rec,
> > + b_ctx_in->active_tunnels,
> iface_rec,
> > + b_ctx_out->local_lports);
> > if (iface_rec
> > || (binding_rec->parent_port && binding_rec->parent_port[0] &&
> > - sset_contains(local_lports, binding_rec->parent_port))) {
> > + sset_contains(b_ctx_out->local_lports,
> > + binding_rec->parent_port))) {
> > if (binding_rec->parent_port && binding_rec->parent_port[0]) {
> > /* Add child logical port to the set of all local ports. */
> > - sset_add(local_lports, binding_rec->logical_port);
> > + sset_add(b_ctx_out->local_lports, binding_rec->logical_port);
> > }
> > - add_local_datapath(sbrec_datapath_binding_by_key,
> > - sbrec_port_binding_by_datapath,
> > - sbrec_port_binding_by_name,
> > - binding_rec->datapath, false,
> local_datapaths);
> > - if (iface_rec && qos_map && ovs_idl_txn) {
> > + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> > + b_ctx_in->sbrec_port_binding_by_datapath,
> > + b_ctx_in->sbrec_port_binding_by_name,
> > + binding_rec->datapath, false,
> > + b_ctx_out->local_datapaths);
> > + if (iface_rec && qos_map && b_ctx_in->ovs_idl_txn) {
> > get_qos_params(binding_rec, qos_map);
> > }
> > } else if (!strcmp(binding_rec->type, "l2gateway")) {
> > if (our_chassis) {
> > - sset_add(local_lports, binding_rec->logical_port);
> > - add_local_datapath(sbrec_datapath_binding_by_key,
> > - sbrec_port_binding_by_datapath,
> > - sbrec_port_binding_by_name,
> > - binding_rec->datapath, false,
> local_datapaths);
> > + sset_add(b_ctx_out->local_lports, binding_rec->logical_port);
> > + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> > + b_ctx_in->sbrec_port_binding_by_datapath,
> > + b_ctx_in->sbrec_port_binding_by_name,
> > + binding_rec->datapath, false,
> > + b_ctx_out->local_datapaths);
> > }
> > } else if (!strcmp(binding_rec->type, "chassisredirect")) {
> > if (ha_chassis_group_contains(binding_rec->ha_chassis_group,
> > - chassis_rec)) {
> > - add_local_datapath(sbrec_datapath_binding_by_key,
> > - sbrec_port_binding_by_datapath,
> > - sbrec_port_binding_by_name,
> > - binding_rec->datapath, false,
> local_datapaths);
> > + b_ctx_in->chassis_rec)) {
> > + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> > + b_ctx_in->sbrec_port_binding_by_datapath,
> > + b_ctx_in->sbrec_port_binding_by_name,
> > + binding_rec->datapath, false,
> > + b_ctx_out->local_datapaths);
> > }
> > } else if (!strcmp(binding_rec->type, "l3gateway")) {
> > if (our_chassis) {
> > - add_local_datapath(sbrec_datapath_binding_by_key,
> > - sbrec_port_binding_by_datapath,
> > - sbrec_port_binding_by_name,
> > - binding_rec->datapath, true,
> local_datapaths);
> > + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> > + b_ctx_in->sbrec_port_binding_by_datapath,
> > + b_ctx_in->sbrec_port_binding_by_name,
> > + binding_rec->datapath, true,
> > + b_ctx_out->local_datapaths);
> > }
> > } else if (!strcmp(binding_rec->type, "localnet")) {
> > /* Add all localnet ports to local_lports so that we allocate ct
> zones
> > * for them. */
> > - sset_add(local_lports, binding_rec->logical_port);
> > - if (qos_map && ovs_idl_txn) {
> > + sset_add(b_ctx_out->local_lports, binding_rec->logical_port);
> > + if (qos_map && b_ctx_in->ovs_idl_txn) {
> > get_qos_params(binding_rec, qos_map);
> > }
> > } else if (!strcmp(binding_rec->type, "external")) {
> > if (ha_chassis_group_contains(binding_rec->ha_chassis_group,
> > - chassis_rec)) {
> > - add_local_datapath(sbrec_datapath_binding_by_key,
> > - sbrec_port_binding_by_datapath,
> > - sbrec_port_binding_by_name,
> > - binding_rec->datapath, false,
> local_datapaths);
> > + b_ctx_in->chassis_rec)) {
> > + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> > + b_ctx_in->sbrec_port_binding_by_datapath,
> > + b_ctx_in->sbrec_port_binding_by_name,
> > + binding_rec->datapath, false,
> > + b_ctx_out->local_datapaths);
> > }
> > }
> >
> > @@ -558,65 +551,63 @@ consider_local_datapath(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> > || !strcmp(binding_rec->type, "localport")
> > || !strcmp(binding_rec->type, "vtep")
> > || !strcmp(binding_rec->type, "localnet")) {
> > - update_local_lport_ids(local_lport_ids, binding_rec);
> > + update_local_lport_ids(b_ctx_out->local_lport_ids, binding_rec);
> > }
> >
> > - ovs_assert(ovnsb_idl_txn);
> > - if (ovnsb_idl_txn) {
> > - const char *vif_chassis = smap_get(&binding_rec->options,
> > + ovs_assert(b_ctx_in->ovnsb_idl_txn);
> > + const char *vif_chassis = smap_get(&binding_rec->options,
> > "requested-chassis");
> > - bool can_bind = !vif_chassis || !vif_chassis[0]
> > - || !strcmp(vif_chassis, chassis_rec->name)
> > - || !strcmp(vif_chassis, chassis_rec->hostname);
> > -
> > - if (can_bind && our_chassis) {
> > - if (binding_rec->chassis != chassis_rec) {
> > - if (binding_rec->chassis) {
> > - VLOG_INFO("Changing chassis for lport %s from %s to
> %s.",
> > - binding_rec->logical_port,
> > - binding_rec->chassis->name,
> > - chassis_rec->name);
> > - } else {
> > - VLOG_INFO("Claiming lport %s for this chassis.",
> > - binding_rec->logical_port);
> > - }
> > - for (int i = 0; i < binding_rec->n_mac; i++) {
> > - VLOG_INFO("%s: Claiming %s",
> > - binding_rec->logical_port,
> binding_rec->mac[i]);
> > - }
> > - sbrec_port_binding_set_chassis(binding_rec, chassis_rec);
> > + bool can_bind = !vif_chassis || !vif_chassis[0]
> > + || !strcmp(vif_chassis, b_ctx_in->chassis_rec->name)
> > + || !strcmp(vif_chassis,
> b_ctx_in->chassis_rec->hostname);
> > +
> > + if (can_bind && our_chassis) {
> > + if (binding_rec->chassis != b_ctx_in->chassis_rec) {
> > + if (binding_rec->chassis) {
> > + VLOG_INFO("Changing chassis for lport %s from %s to %s.",
> > + binding_rec->logical_port,
> > + binding_rec->chassis->name,
> > + b_ctx_in->chassis_rec->name);
> > + } else {
> > + VLOG_INFO("Claiming lport %s for this chassis.",
> > + binding_rec->logical_port);
> > }
> > - /* Check if the port encap binding, if any, has changed */
> > - struct sbrec_encap *encap_rec = sbrec_get_port_encap(
> > - chassis_rec, iface_rec);
> > - if (encap_rec && binding_rec->encap != encap_rec) {
> > - sbrec_port_binding_set_encap(binding_rec, encap_rec);
> > + for (int i = 0; i < binding_rec->n_mac; i++) {
> > + VLOG_INFO("%s: Claiming %s",
> > + binding_rec->logical_port,
> binding_rec->mac[i]);
> > }
> > - } else if (binding_rec->chassis == chassis_rec) {
> > - if (!strcmp(binding_rec->type, "virtual")) {
> > - if (*n_vpbs == *n_allocated_vpbs) {
> > - vpbs = x2nrealloc(vpbs, n_allocated_vpbs, sizeof
> *vpbs);
> > - }
> > - vpbs[(*n_vpbs)] = binding_rec;
> > - (*n_vpbs)++;
> > - } else {
> > - VLOG_INFO("Releasing lport %s from this chassis.",
> > - binding_rec->logical_port);
> > - if (binding_rec->encap) {
> > - sbrec_port_binding_set_encap(binding_rec, NULL);
> > - }
> > - sbrec_port_binding_set_chassis(binding_rec, NULL);
> > + sbrec_port_binding_set_chassis(binding_rec,
> b_ctx_in->chassis_rec);
> > + }
> > + /* Check if the port encap binding, if any, has changed */
> > + struct sbrec_encap *encap_rec =
> > + sbrec_get_port_encap(b_ctx_in->chassis_rec, iface_rec);
> > + if (encap_rec && binding_rec->encap != encap_rec) {
> > + sbrec_port_binding_set_encap(binding_rec, encap_rec);
> > + }
> > + } else if (binding_rec->chassis == b_ctx_in->chassis_rec) {
> > + if (!strcmp(binding_rec->type, "virtual")) {
> > + if (*n_vpbs == *n_allocated_vpbs) {
> > + vpbs = x2nrealloc(vpbs, n_allocated_vpbs, sizeof *vpbs);
> > }
> > - } else if (our_chassis) {
> > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
> 1);
> > - VLOG_INFO_RL(&rl,
> > - "Not claiming lport %s, chassis %s "
> > - "requested-chassis %s",
> > - binding_rec->logical_port,
> > - chassis_rec->name,
> > - vif_chassis);
> > + vpbs[(*n_vpbs)] = binding_rec;
> > + (*n_vpbs)++;
> > + } else {
> > + VLOG_INFO("Releasing lport %s from this chassis.",
> > + binding_rec->logical_port);
> > + if (binding_rec->encap) {
> > + sbrec_port_binding_set_encap(binding_rec, NULL);
> > + }
> > + sbrec_port_binding_set_chassis(binding_rec, NULL);
> > }
> > + } else if (our_chassis) {
> > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> > + VLOG_INFO_RL(&rl,
> > + "Not claiming lport %s, chassis %s "
> > + "requested-chassis %s",
> > + binding_rec->logical_port,
> b_ctx_in->chassis_rec->name,
> > + vif_chassis);
> > }
> > +
> > return vpbs;
> > }
> >
> > @@ -706,23 +697,9 @@ consider_localnet_port(const struct
> sbrec_port_binding *binding_rec,
> > }
> >
> > void
> > -binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> > - struct ovsdb_idl_txn *ovs_idl_txn,
> > - struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
> > - struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
> > - struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > - const struct ovsrec_port_table *port_table,
> > - const struct ovsrec_qos_table *qos_table,
> > - const struct sbrec_port_binding_table *port_binding_table,
> > - const struct ovsrec_bridge *br_int,
> > - const struct sbrec_chassis *chassis_rec,
> > - const struct sset *active_tunnels,
> > - const struct ovsrec_bridge_table *bridge_table,
> > - const struct ovsrec_open_vswitch_table *ovs_table,
> > - struct hmap *local_datapaths, struct sset *local_lports,
> > - struct sset *local_lport_ids)
> > +binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out
> *b_ctx_out)
> > {
> > - if (!chassis_rec) {
> > + if (!b_ctx_in->chassis_rec) {
> > return;
> > }
> >
> > @@ -733,9 +710,9 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> > struct hmap qos_map;
> >
> > hmap_init(&qos_map);
> > - if (br_int) {
> > - get_local_iface_ids(br_int, &lport_to_iface, local_lports,
> > - &egress_ifaces);
> > + if (b_ctx_in->br_int) {
> > + get_local_iface_ids(b_ctx_in->br_int, &lport_to_iface,
> > + b_ctx_out->local_lports, &egress_ifaces);
> > }
> >
> > /* Array to store pointers to local virtual ports. It is populated by
> > @@ -752,16 +729,14 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> > * later. This is special case for virtual ports is needed in order
> to
> > * make sure we update the virtual_parent port bindings first.
> > */
> > - SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec, port_binding_table) {
> > + SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec,
> > + b_ctx_in->port_binding_table) {
> > + const struct ovsrec_interface *iface_rec
> > + = shash_find_data(&lport_to_iface,
> binding_rec->logical_port);
> > vpbs =
> > - consider_local_datapath(ovnsb_idl_txn, ovs_idl_txn,
> > - sbrec_datapath_binding_by_key,
> > - sbrec_port_binding_by_datapath,
> > - sbrec_port_binding_by_name,
> > - active_tunnels, chassis_rec,
> binding_rec,
> > + consider_local_datapath(binding_rec,
> > sset_is_empty(&egress_ifaces) ? NULL
> :
> > - &qos_map, local_datapaths,
> &lport_to_iface,
> > - local_lports, local_lport_ids,
> > + &qos_map, iface_rec, b_ctx_in,
> b_ctx_out,
> > vpbs, &n_vpbs, &n_allocated_vpbs);
> > }
> >
> > @@ -769,26 +744,29 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> > * updated above.
> > */
> > for (size_t i = 0; i < n_vpbs; i++) {
> > - consider_local_virtual_port(sbrec_port_binding_by_name,
> chassis_rec,
> > - vpbs[i]);
> > + consider_local_virtual_port(b_ctx_in->sbrec_port_binding_by_name,
> > + b_ctx_in->chassis_rec, vpbs[i]);
> > }
> > free(vpbs);
> >
> > - add_ovs_bridge_mappings(ovs_table, bridge_table, &bridge_mappings);
> > + add_ovs_bridge_mappings(b_ctx_in->ovs_table, b_ctx_in->bridge_table,
> > + &bridge_mappings);
> >
> > /* Run through each binding record to see if it is a localnet port
> > * on local datapaths discovered from above loop, and update the
> > * corresponding local datapath accordingly. */
> > - SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec, port_binding_table) {
> > + SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec,
> > + b_ctx_in->port_binding_table) {
> > if (!strcmp(binding_rec->type, "localnet")) {
> > consider_localnet_port(binding_rec, &bridge_mappings,
> > - &egress_ifaces, local_datapaths);
> > + &egress_ifaces,
> b_ctx_out->local_datapaths);
> > }
> > }
> > shash_destroy(&bridge_mappings);
> >
> > if (!sset_is_empty(&egress_ifaces)
> > - && set_noop_qos(ovs_idl_txn, port_table, qos_table,
> &egress_ifaces)) {
> > + && set_noop_qos(b_ctx_in->ovs_idl_txn, b_ctx_in->port_table,
> > + b_ctx_in->qos_table, &egress_ifaces)) {
> > const char *entry;
> > SSET_FOR_EACH (entry, &egress_ifaces) {
> > setup_qos(entry, &qos_map);
> > @@ -834,13 +812,16 @@ binding_evaluate_port_binding_changes(
> > * - If a regular VIF is unbound from this chassis, the local
> ovsdb
> > * interface table will be updated, which will trigger
> recompute.
> > *
> > - * - If the port is not a regular VIF, and not a "remote" port,
> > - * always trigger recompute. */
> > - if (binding_rec->chassis == chassis_rec
> > - || is_our_chassis(chassis_rec, binding_rec,
> > - active_tunnels, &lport_to_iface,
> local_lports)
> > - || (strcmp(binding_rec->type, "") &&
> strcmp(binding_rec->type,
> > - "remote"))) {
> > + * - If the port is not a regular VIF, always trigger recompute.
> */
> > + if (binding_rec->chassis == chassis_rec) {
> > + changed = true;
> > + break;
> > + }
> > +
> > + const struct ovsrec_interface *iface_rec
> > + = shash_find_data(&lport_to_iface,
> binding_rec->logical_port);
> > + if (is_our_chassis(chassis_rec, binding_rec, active_tunnels,
> iface_rec,
> > + local_lports) || strcmp(binding_rec->type,
> "")) {
> > changed = true;
> > break;
> > }
>
> This seems to be a rebase problem. The condition (and comment) for "remote"
> port shouldn't be dropped.
Oops. Thanks for pointing out. I'll fix it.
>
> > diff --git a/controller/binding.h b/controller/binding.h
> > index 924891c1b..d0b8331af 100644
> > --- a/controller/binding.h
> > +++ b/controller/binding.h
> > @@ -32,22 +32,31 @@ struct sbrec_chassis;
> > struct sbrec_port_binding_table;
> > struct sset;
> >
> > +struct binding_ctx_in {
> > + struct ovsdb_idl_txn *ovnsb_idl_txn;
> > + struct ovsdb_idl_txn *ovs_idl_txn;
> > + struct ovsdb_idl_index *sbrec_datapath_binding_by_key;
> > + struct ovsdb_idl_index *sbrec_port_binding_by_datapath;
> > + struct ovsdb_idl_index *sbrec_port_binding_by_name;
> > + const struct ovsrec_port_table *port_table;
> > + const struct ovsrec_qos_table *qos_table;
> > + const struct sbrec_port_binding_table *port_binding_table;
> > + const struct ovsrec_bridge *br_int;
> > + const struct sbrec_chassis *chassis_rec;
> > + const struct sset *active_tunnels;
> > + const struct ovsrec_bridge_table *bridge_table;
> > + const struct ovsrec_open_vswitch_table *ovs_table;
> > + const struct ovsrec_interface_table *iface_table;
> > +};
> > +
> > +struct binding_ctx_out {
> > + struct hmap *local_datapaths;
> > + struct sset *local_lports;
> > + struct sset *local_lport_ids;
> > +};
> > +
> > void binding_register_ovs_idl(struct ovsdb_idl *);
> > -void binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> > - struct ovsdb_idl_txn *ovs_idl_txn,
> > - struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
> > - struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
> > - struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > - const struct ovsrec_port_table *,
> > - const struct ovsrec_qos_table *,
> > - const struct sbrec_port_binding_table *,
> > - const struct ovsrec_bridge *br_int,
> > - const struct sbrec_chassis *,
> > - const struct sset *active_tunnels,
> > - const struct ovsrec_bridge_table *bridge_table,
> > - const struct ovsrec_open_vswitch_table *ovs_table,
> > - struct hmap *local_datapaths,
> > - struct sset *local_lports, struct sset
> *local_lport_ids);
> > +void binding_run(struct binding_ctx_in *, struct binding_ctx_out *);
> > bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
> > const struct sbrec_port_binding_table *,
> > const struct sbrec_chassis *);
> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > index 2893eaac1..4930d5ffb 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -1003,34 +1003,11 @@ en_runtime_data_cleanup(void *data)
> > }
> >
> > static void
> > -en_runtime_data_run(struct engine_node *node, void *data)
> > +init_binding_ctx(struct engine_node *node,
> > + struct ed_type_runtime_data *rt_data,
> > + struct binding_ctx_in *b_ctx_in,
> > + struct binding_ctx_out *b_ctx_out)
> > {
> > - struct ed_type_runtime_data *rt_data = data;
> > - struct hmap *local_datapaths = &rt_data->local_datapaths;
> > - struct sset *local_lports = &rt_data->local_lports;
> > - struct sset *local_lport_ids = &rt_data->local_lport_ids;
> > - struct sset *active_tunnels = &rt_data->active_tunnels;
> > -
> > - static bool first_run = true;
> > - if (first_run) {
> > - /* don't cleanup since there is no data yet */
> > - first_run = false;
> > - } else {
> > - struct local_datapath *cur_node, *next_node;
> > - HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node,
> local_datapaths) {
> > - free(cur_node->peer_ports);
> > - hmap_remove(local_datapaths, &cur_node->hmap_node);
> > - free(cur_node);
> > - }
> > - hmap_clear(local_datapaths);
> > - sset_destroy(local_lports);
> > - sset_destroy(local_lport_ids);
> > - sset_destroy(active_tunnels);
> > - sset_init(local_lports);
> > - sset_init(local_lport_ids);
> > - sset_init(active_tunnels);
> > - }
> > -
> > struct ovsrec_open_vswitch_table *ovs_table =
> > (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET(
> > engine_get_input("OVS_open_vswitch", node));
> > @@ -1051,19 +1028,6 @@ en_runtime_data_run(struct engine_node *node, void
> *data)
> > = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
> > ovs_assert(chassis);
> >
> > - struct ed_type_ofctrl_is_connected *ed_ofctrl_is_connected =
> > - engine_get_input_data("ofctrl_is_connected", node);
> > - if (ed_ofctrl_is_connected->connected) {
> > - /* Calculate the active tunnels only if have an an active
> > - * OpenFlow connection to br-int.
> > - * If we don't have a connection to br-int, it could mean
> > - * ovs-vswitchd is down for some reason and the BFD status
> > - * in the Interface rows could be stale. So its better to
> > - * consider 'active_tunnels' set to be empty if it's not
> > - * connected. */
> > - bfd_calculate_active_tunnels(br_int, active_tunnels);
> > - }
> > -
> > struct ovsrec_port_table *port_table =
> > (struct ovsrec_port_table *)EN_OVSDB_GET(
> > engine_get_input("OVS_port", node));
> > @@ -1091,16 +1055,72 @@ en_runtime_data_run(struct engine_node *node,
> void *data)
> > engine_get_input("SB_port_binding", node),
> > "datapath");
> >
> > - binding_run(engine_get_context()->ovnsb_idl_txn,
> > - engine_get_context()->ovs_idl_txn,
> > - sbrec_datapath_binding_by_key,
> > - sbrec_port_binding_by_datapath,
> > - sbrec_port_binding_by_name,
> > - port_table, qos_table, pb_table,
> > - br_int, chassis,
> > - active_tunnels, bridge_table,
> > - ovs_table, local_datapaths,
> > - local_lports, local_lport_ids);
> > + b_ctx_in->ovnsb_idl_txn = engine_get_context()->ovnsb_idl_txn;
> > + b_ctx_in->ovs_idl_txn = engine_get_context()->ovs_idl_txn;
> > + b_ctx_in->sbrec_datapath_binding_by_key =
> sbrec_datapath_binding_by_key;
> > + b_ctx_in->sbrec_port_binding_by_datapath =
> sbrec_port_binding_by_datapath;
> > + b_ctx_in->sbrec_port_binding_by_name = sbrec_port_binding_by_name;
> > + b_ctx_in->port_table = port_table;
> > + b_ctx_in->qos_table = qos_table;
> > + b_ctx_in->port_binding_table = pb_table;
> > + b_ctx_in->br_int = br_int;
> > + b_ctx_in->chassis_rec = chassis;
> > + b_ctx_in->active_tunnels = &rt_data->active_tunnels;
> > + b_ctx_in->bridge_table = bridge_table;
> > + b_ctx_in->ovs_table = ovs_table;
> > +
> > + b_ctx_out->local_datapaths = &rt_data->local_datapaths;
> > + b_ctx_out->local_lports = &rt_data->local_lports;
> > + b_ctx_out->local_lport_ids = &rt_data->local_lport_ids;
> > +}
> > +
> > +static void
> > +en_runtime_data_run(struct engine_node *node, void *data)
> > +{
> > + struct ed_type_runtime_data *rt_data = data;
> > + struct hmap *local_datapaths = &rt_data->local_datapaths;
> > + struct sset *local_lports = &rt_data->local_lports;
> > + struct sset *local_lport_ids = &rt_data->local_lport_ids;
> > + struct sset *active_tunnels = &rt_data->active_tunnels;
> > +
> > + static bool first_run = true;
> > + if (first_run) {
> > + /* don't cleanup since there is no data yet */
> > + first_run = false;
> > + } else {
> > + struct local_datapath *cur_node, *next_node;
> > + HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node,
> local_datapaths) {
> > + free(cur_node->peer_ports);
> > + hmap_remove(local_datapaths, &cur_node->hmap_node);
> > + free(cur_node);
> > + }
> > + hmap_clear(local_datapaths);
> > + sset_destroy(local_lports);
> > + sset_destroy(local_lport_ids);
> > + sset_destroy(active_tunnels);
> > + sset_init(local_lports);
> > + sset_init(local_lport_ids);
> > + sset_init(active_tunnels);
> > + }
> > +
> > + struct binding_ctx_in b_ctx_in;
> > + struct binding_ctx_out b_ctx_out;
> > + init_binding_ctx(node, rt_data, &b_ctx_in, &b_ctx_out);
> > +
> > + struct ed_type_ofctrl_is_connected *ed_ofctrl_is_connected =
> > + engine_get_input_data("ofctrl_is_connected", node);
> > + if (ed_ofctrl_is_connected->connected) {
> > + /* Calculate the active tunnels only if have an an active
> > + * OpenFlow connection to br-int.
> > + * If we don't have a connection to br-int, it could mean
> > + * ovs-vswitchd is down for some reason and the BFD status
> > + * in the Interface rows could be stale. So its better to
> > + * consider 'active_tunnels' set to be empty if it's not
> > + * connected. */
> > + bfd_calculate_active_tunnels(b_ctx_in.br_int, active_tunnels);
> > + }
>
> I am curious why not moving this part to init_binding_ctx?
You mean the bfd_calculate_active_tunnels() ? If so, in the later commits,
init_binding_ctx() is called from other functions and this'll not be required.
Thanks
Numan
Numan
>
> > +
> > + binding_run(&b_ctx_in, &b_ctx_out);
> >
> > engine_set_node_state(node, EN_UPDATED);
> > }
> > --
> > 2.24.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
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
