Message ID | 20200316111203.1686979-1-numans@ovn.org |
---|---|
State | Superseded |
Headers | show |
Series | Incremental processing improvements. | expand |
On Mon, Mar 16, 2020 at 4:12 AM <numans@ovn.org> wrote: > > From: Numan Siddique <numans@ovn.org> > > No functional changes are introduced in this patch. > > Signed-off-by: Numan Siddique <numans@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. > 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? > + > + binding_run(&b_ctx_in, &b_ctx_out); > > engine_set_node_state(node, EN_UPDATED); > } > -- > 2.24.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Wed, Mar 25, 2020 at 12:22 PM Han Zhou <zhouhan@gmail.com> wrote: > > On Mon, Mar 16, 2020 at 4:12 AM <numans@ovn.org> wrote: > > > > From: Numan Siddique <numans@ovn.org> > > > > No functional changes are introduced in this patch. > > > > Signed-off-by: Numan Siddique <numans@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 > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
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; } 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); + } + + binding_run(&b_ctx_in, &b_ctx_out); engine_set_node_state(node, EN_UPDATED); }