| Message ID | 20200316111543.1687608-1-numans@ovn.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Incremental processing improvements. | expand |
On Mon, Mar 16, 2020 at 4:16 AM <numans@ovn.org> wrote: > > From: Numan Siddique <numans@ovn.org> > > This patch handles SB port binding changes and OVS interface changes > incrementally in the runtime_data stage which otherwise would have > resulted in calls to binding_run(). > > Prior to this patch, port binding changes were handled differently. > The changes were only evaluated to see if they need full recompute > or they can be ignored. > > With this patch, the runtime data is updated with the changes (both > SB port binding and OVS interface) and ports are claimed/released in > the handlers itself, avoiding the need to trigger recompute of runtime > data stage. > > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > controller/binding.c | 467 ++++++++++++++++++++++++++++++------ > controller/binding.h | 7 +- > controller/ovn-controller.c | 55 ++++- > 3 files changed, 448 insertions(+), 81 deletions(-) > > diff --git a/controller/binding.c b/controller/binding.c > index 34bd475de..ce4467f31 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -349,17 +349,6 @@ setup_qos(const char *egress_iface, struct hmap *queue_map) > netdev_close(netdev_phy); > } > > -static void > -update_local_lport_ids(struct sset *local_lport_ids, > - const struct sbrec_port_binding *binding_rec) > -{ > - char buf[16]; > - snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, > - binding_rec->datapath->tunnel_key, > - binding_rec->tunnel_key); > - sset_add(local_lport_ids, buf); > -} > - > /* > * Get the encap from the chassis for this port. The interface > * may have an external_ids:encap-ip=<encap-ip> set; if so we > @@ -490,6 +479,28 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, > ld->localnet_port = binding_rec; > } > > +static void > +update_local_lport_ids(struct sset *local_lport_ids, > + const struct sbrec_port_binding *binding_rec) > +{ > + char buf[16]; > + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, > + binding_rec->datapath->tunnel_key, > + binding_rec->tunnel_key); > + sset_add(local_lport_ids, buf); > +} > + > +static void > +remove_local_lport_ids(const struct sbrec_port_binding *binding_rec, > + struct sset *local_lport_ids) > +{ > + char buf[16]; > + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, > + binding_rec->datapath->tunnel_key, > + binding_rec->tunnel_key); > + sset_find_and_delete(local_lport_ids, buf); > +} > + > enum local_binding_type { > BT_VIF, > BT_CHILD, > @@ -545,18 +556,21 @@ local_binding_destroy(struct local_binding *lbinding) > free(lbinding); > } > > +static > +void local_binding_delete(struct shash *local_bindings, > + struct local_binding *lbinding) > +{ > + shash_find_and_delete(local_bindings, lbinding->name); > + local_binding_destroy(lbinding); > +} > + > void > local_bindings_destroy(struct shash *local_bindings) > { > struct shash_node *node, *next; > SHASH_FOR_EACH_SAFE (node, next, local_bindings) { > struct local_binding *lbinding = node->data; > - struct local_binding *c, *n; > - LIST_FOR_EACH_SAFE (c, n, node, &lbinding->children) { > - ovs_list_remove(&c->node); > - free(c->name); > - free(c); > - } > + local_binding_destroy(lbinding); > } > > shash_destroy(local_bindings); > @@ -651,6 +665,22 @@ release_lport(const struct sbrec_port_binding *pb) > } > } > > +static void > +release_local_binding_children(struct local_binding *lbinding) > +{ > + struct local_binding *l; > + LIST_FOR_EACH (l, node, &lbinding->children) { > + release_lport(l->pb); > + } > +} > + > +static void > +release_local_binding(struct local_binding *lbinding) > +{ > + release_local_binding_children(lbinding); > + release_lport(lbinding->pb); > +} > + > static void > consider_port_binding_for_vif(const struct sbrec_port_binding *pb, > struct binding_ctx_in *b_ctx_in, > @@ -725,7 +755,6 @@ consider_port_binding(const struct sbrec_port_binding *pb, > struct binding_ctx_out *b_ctx_out, > struct hmap *qos_map) > { > - > bool our_chassis = is_our_chassis(b_ctx_in->chassis_rec, pb, > b_ctx_in->active_tunnels, NULL, > b_ctx_out->local_lports); > @@ -821,6 +850,8 @@ build_local_bindings_from_local_ifaces(struct binding_ctx_in *b_ctx_in, > lbinding->pb = pb; > } > sset_add(b_ctx_out->local_lports, iface_id); > + smap_replace(b_ctx_out->local_iface_ids, iface_rec->name, > + iface_id); > } > > /* Check if this is a tunnel interface. */ > @@ -938,62 +969,6 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) > hmap_destroy(&qos_map); > } > > -/* Returns true if port-binding changes potentially require flow changes on > - * the current chassis. Returns false if we are sure there is no impact. */ > -bool > -binding_evaluate_port_binding_changes(struct binding_ctx_in *b_ctx_in, > - struct binding_ctx_out *b_ctx_out) > -{ > - if (!b_ctx_in->chassis_rec) { > - return true; > - } > - > - bool changed = false; > - > - const struct sbrec_port_binding *binding_rec; > - SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding_rec, > - b_ctx_in->port_binding_table) { > - /* XXX: currently OVSDB change tracking doesn't support getting old > - * data when the operation is update, so if a port-binding moved from > - * this chassis to another, there is no easy way to find out the > - * change. To workaround this problem, we just makes sure if > - * any port *related to* this chassis has any change, then trigger > - * recompute. > - * > - * - 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, always trigger recompute. */ > - if (binding_rec->chassis == b_ctx_in->chassis_rec) { > - changed = true; > - break; > - } > - > - if (strcmp(binding_rec->type, "")) { > - changed = true; > - break; > - } > - > - struct local_binding *lbinding = NULL; > - if (!binding_rec->type[0]) { > - if (!binding_rec->parent_port || !binding_rec->parent_port[0]) { > - lbinding = local_binding_find(b_ctx_out->local_bindings, > - binding_rec->logical_port); > - } else { > - lbinding = local_binding_find(b_ctx_out->local_bindings, > - binding_rec->parent_port); > - } > - } > - > - if (lbinding) { > - changed = true; > - break; > - } > - } > - > - return changed; > -} > - > /* Returns true if the database is all cleaned up, false if more work is > * required. */ > bool > @@ -1028,3 +1003,347 @@ binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, > > return !any_changes; > } > + > +static void > +add_local_datapath_peer_port(const struct sbrec_port_binding *pb, > + struct binding_ctx_in *b_ctx_in, > + struct binding_ctx_out *b_ctx_out, > + struct local_datapath *ld) > +{ > + bool present = false; > + for (size_t i = 0; i < ld->n_peer_ports; i++) { > + if (ld->peer_ports[i].local == pb) { > + present = true; > + break; > + } > + } > + > + const char *peer_name = smap_get(&pb->options, "peer"); > + if (strcmp(pb->type, "patch") || !peer_name) { > + return; > + } > + > + const struct sbrec_port_binding *peer; > + peer = lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name, > + peer_name); > + > + if (!peer || !peer->datapath) { > + return; > + } > + > + if (!present) { > + ld->n_peer_ports++; > + if (ld->n_peer_ports > ld->n_allocated_peer_ports) { > + ld->peer_ports = > + x2nrealloc(ld->peer_ports, > + &ld->n_allocated_peer_ports, > + sizeof *ld->peer_ports); > + } > + ld->peer_ports[ld->n_peer_ports - 1].local = pb; > + ld->peer_ports[ld->n_peer_ports - 1].remote = peer; > + } > + > + struct local_datapath *peer_ld = > + get_local_datapath(b_ctx_out->local_datapaths, > + peer->datapath->tunnel_key); > + if (!peer_ld) { > + 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, > + peer->datapath, false, > + 1, b_ctx_out->local_datapaths); > + return; > + } > + > + for (size_t i = 0; i < peer_ld->n_peer_ports; i++) { > + if (peer_ld->peer_ports[i].local == peer) { > + return; > + } > + } > + > + peer_ld->n_peer_ports++; > + if (peer_ld->n_peer_ports > peer_ld->n_allocated_peer_ports) { > + peer_ld->peer_ports = > + x2nrealloc(peer_ld->peer_ports, > + &peer_ld->n_allocated_peer_ports, > + sizeof *peer_ld->peer_ports); > + } > + peer_ld->peer_ports[peer_ld->n_peer_ports - 1].local = peer; > + peer_ld->peer_ports[peer_ld->n_peer_ports - 1].remote = pb; > +} > + > +static void > +remove_local_datapath_peer_port(const struct sbrec_port_binding *pb, > + struct local_datapath *ld, > + struct hmap *local_datapaths) > +{ > + size_t i =0; > + for (i = 0; i < ld->n_peer_ports; i++) { > + if (ld->peer_ports[i].local == pb) { > + break; > + } > + } > + > + if (i == ld->n_peer_ports) { > + return; > + } > + > + const struct sbrec_port_binding *peer = ld->peer_ports[i].remote; > + > + ld->peer_ports[i].local = ld->peer_ports[ld->n_peer_ports - 1].local; > + ld->peer_ports[i].remote = ld->peer_ports[ld->n_peer_ports - 1].remote; > + ld->n_peer_ports--; > + > + struct local_datapath *peer_ld = > + get_local_datapath(local_datapaths, peer->datapath->tunnel_key); > + if (peer_ld) { > + remove_local_datapath_peer_port(peer, peer_ld, local_datapaths); > + } > +} > + > +static void > +update_local_datapath_for_pb(const struct sbrec_port_binding *pb, > + struct binding_ctx_in *b_ctx_in, > + struct binding_ctx_out *b_ctx_out, > + struct local_datapath *ld) > +{ > + if (!strcmp(pb->type, "patch")) { > + add_local_datapath_peer_port(pb, b_ctx_in, b_ctx_out, ld); > + } else if (!strcmp(pb->type, "localnet")) { > + struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings); > + add_ovs_bridge_mappings(b_ctx_in->ovs_table, b_ctx_in->bridge_table, > + &bridge_mappings); > + consider_localnet_port(pb, &bridge_mappings, b_ctx_out->egress_ifaces, > + b_ctx_out->local_datapaths); > + shash_destroy(&bridge_mappings); > + } else if (!strcmp(pb->type, "l3gateway")) { > + const char *chassis_id = smap_get(&pb->options, > + "l3gateway-chassis"); > + if (chassis_id && !strcmp(chassis_id, b_ctx_in->chassis_rec->name)) { > + ld->has_local_l3gateway = true; > + } > + } > + > + if (!strcmp(pb->type, "patch") || > + !strcmp(pb->type, "localport") || > + !strcmp(pb->type, "vtep")) { > + update_local_lport_ids(b_ctx_out->local_lport_ids, pb); > + } > +} > + > +static void > +remove_pb_from_local_datapath(const struct sbrec_port_binding *pb, > + const struct sbrec_chassis *chassis_rec, > + struct binding_ctx_out *b_ctx_out, > + struct local_datapath *ld) > +{ > + remove_local_lport_ids(pb, b_ctx_out->local_lport_ids); > + if (!strcmp(pb->type, "patch") || > + !strcmp(pb->type, "l3gateway")) { > + remove_local_datapath_peer_port(pb, ld, b_ctx_out->local_datapaths); > + } else if (!strcmp(pb->type, "localnet")) { > + if (ld->localnet_port && !strcmp(ld->localnet_port->logical_port, > + pb->logical_port)) { > + ld->localnet_port = NULL; > + } > + } else if (!strcmp(pb->type, "l3gateway")) { > + const char *chassis_id = smap_get(&pb->options, > + "l3gateway-chassis"); > + if (chassis_id && !strcmp(chassis_id, chassis_rec->name)) { > + ld->has_local_l3gateway = false; > + } > + } > +} > + > +/* Returns true if the ovs interface changes were handled successfully, > + * false otherwise. > + */ > +bool > +binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, > + struct binding_ctx_out *b_ctx_out) > +{ > + if (!b_ctx_in->chassis_rec) { > + return false; > + } > + > + bool handled = true; > + struct hmap qos_map = HMAP_INITIALIZER(&qos_map); > + struct hmap *qos_map_ptr = > + sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map; > + > + const struct ovsrec_interface *iface_rec; > + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, > + b_ctx_in->iface_table) { > + const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id"); > + const char *old_iface_id = smap_get(b_ctx_out->local_iface_ids, > + iface_rec->name); > + if (iface_rec->type && iface_rec->type[0]) { > + handled = false; > + goto out; > + } > + > + struct local_binding *lbinding = NULL; > + struct local_binding *claim_lbinding = NULL; > + const char *cleared_iface_id = NULL; > + int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; > + if (!ovsrec_interface_is_deleted(iface_rec)) { > + if (iface_id) { > + /* Check if iface_id is changed. If so we need to > + * release the old port binding and associate this > + * inteface to new port binding. */ > + if (old_iface_id && strcmp(iface_id, old_iface_id)) { > + cleared_iface_id = old_iface_id; > + } > + } else if (old_iface_id) { > + cleared_iface_id = old_iface_id; > + } > + } else { > + cleared_iface_id = iface_id; > + iface_id = NULL; > + } > + > + if (cleared_iface_id) { > + lbinding = local_binding_find(b_ctx_out->local_bindings, > + cleared_iface_id); > + if (lbinding && lbinding->pb && > + lbinding->pb->chassis == b_ctx_in->chassis_rec) { > + release_local_binding(lbinding); > + struct local_datapath *ld = > + get_local_datapath(b_ctx_out->local_datapaths, > + lbinding->pb->datapath->tunnel_key); > + if (ld) { > + remove_pb_from_local_datapath(lbinding->pb, > + b_ctx_in->chassis_rec, > + b_ctx_out, ld); > + } > + local_binding_delete(b_ctx_out->local_bindings, lbinding); > + } > + > + sset_find_and_delete(b_ctx_out->local_lports, cleared_iface_id); > + smap_remove(b_ctx_out->local_iface_ids, iface_rec->name); > + } > + > + if (iface_id && ofport > 0) { > + sset_add(b_ctx_out->local_lports, iface_id); > + smap_replace(b_ctx_out->local_iface_ids, iface_rec->name, > + iface_id); > + claim_lbinding = > + local_binding_find(b_ctx_out->local_bindings, iface_id); > + if (!claim_lbinding) { > + claim_lbinding = local_binding_create(iface_id, iface_rec, > + NULL, BT_VIF); > + local_binding_add(b_ctx_out->local_bindings, claim_lbinding); > + } else { > + claim_lbinding->iface = iface_rec; > + } > + > + if (!claim_lbinding->pb || > + strcmp(claim_lbinding->name, > + claim_lbinding->pb->logical_port)) { > + claim_lbinding->pb = > + lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name, > + claim_lbinding->name); > + } > + > + if (claim_lbinding->pb) { > + consider_port_binding_for_vif(claim_lbinding->pb, b_ctx_in, > + BT_VIF, claim_lbinding, > + b_ctx_out, qos_map_ptr); > + } > + } > + } > + > + if (qos_map_ptr && set_noop_qos(b_ctx_in->ovs_idl_txn, > + b_ctx_in->port_table, > + b_ctx_in->qos_table, > + b_ctx_out->egress_ifaces)) { > + const char *entry; > + SSET_FOR_EACH (entry, b_ctx_out->egress_ifaces) { > + setup_qos(entry, &qos_map); > + } > + } > + > + hmap_destroy(&qos_map); > +out: > + return handled; > +} > + > +/* Returns true if the port binding changes resulted in local binding > + * updates, false otherwise. > + */ > +bool > +binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in, > + struct binding_ctx_out *b_ctx_out) > +{ > + bool updated = false; > + struct hmap qos_map = HMAP_INITIALIZER(&qos_map); > + struct hmap *qos_map_ptr = > + sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map; > + > + const struct sbrec_port_binding *pb; > + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, > + b_ctx_in->port_binding_table) { > + bool consider_for_vif = false; > + struct local_binding *lbinding = NULL; > + enum local_binding_type binding_type = BT_VIF; > + bool is_deleted = sbrec_port_binding_is_deleted(pb); > + if (!pb->type[0]) { > + if (!pb->parent_port || !pb->parent_port[0]) { > + lbinding = local_binding_find(b_ctx_out->local_bindings, > + pb->logical_port); > + } else { > + lbinding = local_binding_find(b_ctx_out->local_bindings, > + pb->parent_port); > + binding_type = BT_CHILD; > + } > + > + consider_for_vif = true; > + } else if (!strcmp(pb->type, "virtual") && > + pb->virtual_parent && pb->virtual_parent[0]) { > + lbinding = local_binding_find(b_ctx_out->local_bindings, > + pb->virtual_parent); > + consider_for_vif = true; > + binding_type = BT_VIRTUAL; > + } > + > + if (is_deleted) { > + if (lbinding) { > + updated = true; > + lbinding->pb = NULL; > + > + if (binding_type == BT_VIF) { > + release_local_binding_children(lbinding); > + } > + } > + > + struct local_datapath *ld = > + get_local_datapath(b_ctx_out->local_datapaths, > + pb->datapath->tunnel_key); > + if (ld) { > + remove_pb_from_local_datapath(pb, b_ctx_in->chassis_rec, > + b_ctx_out, ld); > + } > + } else { > + if (consider_for_vif) { > + if (lbinding) { > + updated = true; > + lbinding->pb = pb; > + consider_port_binding_for_vif(pb, b_ctx_in, binding_type, > + lbinding, b_ctx_out, > + qos_map_ptr); > + } > + } else { > + updated = true; > + consider_port_binding(pb, b_ctx_in, b_ctx_out, qos_map_ptr); > + struct local_datapath *ld = > + get_local_datapath(b_ctx_out->local_datapaths, > + pb->datapath->tunnel_key); > + if (ld) { > + update_local_datapath_for_pb(pb, b_ctx_in, b_ctx_out, ld); > + } > + } > + } > + } > + > + return updated; > +} > diff --git a/controller/binding.h b/controller/binding.h > index 6bee1d12e..95ca0367d 100644 > --- a/controller/binding.h > +++ b/controller/binding.h > @@ -56,6 +56,7 @@ struct binding_ctx_out { > struct sset *local_lports; > struct sset *local_lport_ids; > struct sset *egress_ifaces; > + struct smap *local_iface_ids; > }; > > void binding_register_ovs_idl(struct ovsdb_idl *); > @@ -63,10 +64,12 @@ 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 *); > -bool binding_evaluate_port_binding_changes(struct binding_ctx_in *, > - struct binding_ctx_out *); > void local_bindings_destroy(struct shash *local_bindings); > void binding_add_vport_to_local_bindings( > struct shash *local_bindings, const struct sbrec_port_binding *parent, > const struct sbrec_port_binding *vport); > +bool binding_handle_ovs_interface_changes(struct binding_ctx_in *, > + struct binding_ctx_out *); > +bool binding_handle_port_binding_changes(struct binding_ctx_in *, > + struct binding_ctx_out *); > #endif /* controller/binding.h */ > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 8cc27cebf..6841be29d 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -753,6 +753,7 @@ enum sb_engine_node { > OVS_NODE(open_vswitch, "open_vswitch") \ > OVS_NODE(bridge, "bridge") \ > OVS_NODE(port, "port") \ > + OVS_NODE(interface, "interface") \ > OVS_NODE(qos, "qos") > > enum ovs_engine_node { > @@ -974,6 +975,7 @@ struct ed_type_runtime_data { > struct sset active_tunnels; > > struct sset egress_ifaces; > + struct smap local_iface_ids; > }; > > static void * > @@ -988,6 +990,7 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED, > sset_init(&data->active_tunnels); > sset_init(&data->egress_ifaces); > shash_init(&data->local_bindings); > + smap_init(&data->local_iface_ids); > return data; > } > > @@ -1000,6 +1003,7 @@ en_runtime_data_cleanup(void *data) > sset_destroy(&rt_data->local_lport_ids); > sset_destroy(&rt_data->active_tunnels); > sset_init(&rt_data->egress_ifaces); > + smap_destroy(&rt_data->local_iface_ids); > struct local_datapath *cur_node, *next_node; > HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, > &rt_data->local_datapaths) { > @@ -1041,6 +1045,10 @@ init_binding_ctx(struct engine_node *node, > (struct ovsrec_port_table *)EN_OVSDB_GET( > engine_get_input("OVS_port", node)); > > + struct ovsrec_interface_table *iface_table = > + (struct ovsrec_interface_table *)EN_OVSDB_GET( > + engine_get_input("OVS_interface", node)); > + > struct ovsrec_qos_table *qos_table = > (struct ovsrec_qos_table *)EN_OVSDB_GET( > engine_get_input("OVS_qos", node)); > @@ -1070,6 +1078,7 @@ init_binding_ctx(struct engine_node *node, > 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->iface_table = iface_table; > b_ctx_in->qos_table = qos_table; > b_ctx_in->port_binding_table = pb_table; > b_ctx_in->br_int = br_int; > @@ -1083,6 +1092,7 @@ init_binding_ctx(struct engine_node *node, > b_ctx_out->local_lport_ids = &rt_data->local_lport_ids; > b_ctx_out->egress_ifaces = &rt_data->egress_ifaces; > b_ctx_out->local_bindings = &rt_data->local_bindings; > + b_ctx_out->local_iface_ids = &rt_data->local_iface_ids; > } > > static void > @@ -1110,10 +1120,12 @@ en_runtime_data_run(struct engine_node *node, void *data) > sset_destroy(local_lport_ids); > sset_destroy(active_tunnels); > sset_destroy(&rt_data->egress_ifaces); > + smap_destroy(&rt_data->local_iface_ids); > sset_init(local_lports); > sset_init(local_lport_ids); > sset_init(active_tunnels); > sset_init(&rt_data->egress_ifaces); > + smap_init(&rt_data->local_iface_ids); > } > > struct binding_ctx_in b_ctx_in; > @@ -1139,17 +1151,47 @@ en_runtime_data_run(struct engine_node *node, void *data) > } > > static bool > -runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) > +runtime_data_ovs_interface_handler(struct engine_node *node, void *data) > { > + if (!engine_get_context()->ovnsb_idl_txn) { > + return false; > + } > + > struct ed_type_runtime_data *rt_data = data; > 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); > > - bool changed = binding_evaluate_port_binding_changes(&b_ctx_in, > - &b_ctx_out); > + engine_set_node_state(node, EN_UPDATED); > + return binding_handle_ovs_interface_changes(&b_ctx_in, &b_ctx_out); > +} > + > +static bool > +runtime_data_noop_handler(struct engine_node *node OVS_UNUSED, > + void *data OVS_UNUSED) > +{ > + return true; > +} > > - return !changed; > +static bool > +runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) > +{ > + if (!engine_get_context()->ovnsb_idl_txn) { > + return false; > + } > + > + struct ed_type_runtime_data *rt_data = data; > + 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); > + if (!b_ctx_in.chassis_rec) { > + return false; > + } > + > + bool updated = binding_handle_port_binding_changes(&b_ctx_in, &b_ctx_out); > + enum engine_node_state state = updated ? EN_UPDATED : EN_VALID; An input change handler should never set the node's state to EN_VALID, because it doesn't know if any other input handler already set the state to EN_UPDATED. What it should do is just set state to EN_UPDATE if it knows that the data is changed. (Sorry this was not well documented, and there's already existing code like this in address-set handling. This was a miss in a previous review of a refactor of the I-P code.) > + engine_set_node_state(node, state); > + return true; > } > > /* Connection tracking zones. */ > @@ -1891,7 +1933,10 @@ main(int argc, char *argv[]) > > engine_add_input(&en_runtime_data, &en_ovs_open_vswitch, NULL); > engine_add_input(&en_runtime_data, &en_ovs_bridge, NULL); > - engine_add_input(&en_runtime_data, &en_ovs_port, NULL); > + engine_add_input(&en_runtime_data, &en_ovs_port, > + runtime_data_noop_handler); > + engine_add_input(&en_runtime_data, &en_ovs_interface, > + runtime_data_ovs_interface_handler); > engine_add_input(&en_runtime_data, &en_ovs_qos, NULL); > > engine_add_input(&en_runtime_data, &en_sb_chassis, NULL); > -- > 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 <hzhou@ovn.org> wrote: > > On Mon, Mar 16, 2020 at 4:16 AM <numans@ovn.org> wrote: > > > > From: Numan Siddique <numans@ovn.org> > > > > This patch handles SB port binding changes and OVS interface changes > > incrementally in the runtime_data stage which otherwise would have > > resulted in calls to binding_run(). > > > > Prior to this patch, port binding changes were handled differently. > > The changes were only evaluated to see if they need full recompute > > or they can be ignored. > > > > With this patch, the runtime data is updated with the changes (both > > SB port binding and OVS interface) and ports are claimed/released in > > the handlers itself, avoiding the need to trigger recompute of runtime > > data stage. > > > > Signed-off-by: Numan Siddique <numans@ovn.org> > > --- > > controller/binding.c | 467 ++++++++++++++++++++++++++++++------ > > controller/binding.h | 7 +- > > controller/ovn-controller.c | 55 ++++- > > 3 files changed, 448 insertions(+), 81 deletions(-) > > > > diff --git a/controller/binding.c b/controller/binding.c > > index 34bd475de..ce4467f31 100644 > > --- a/controller/binding.c > > +++ b/controller/binding.c > > @@ -349,17 +349,6 @@ setup_qos(const char *egress_iface, struct hmap > *queue_map) > > netdev_close(netdev_phy); > > } > > > > -static void > > -update_local_lport_ids(struct sset *local_lport_ids, > > - const struct sbrec_port_binding *binding_rec) > > -{ > > - char buf[16]; > > - snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, > > - binding_rec->datapath->tunnel_key, > > - binding_rec->tunnel_key); > > - sset_add(local_lport_ids, buf); > > -} > > - > > /* > > * Get the encap from the chassis for this port. The interface > > * may have an external_ids:encap-ip=<encap-ip> set; if so we > > @@ -490,6 +479,28 @@ consider_localnet_port(const struct > sbrec_port_binding *binding_rec, > > ld->localnet_port = binding_rec; > > } > > > > +static void > > +update_local_lport_ids(struct sset *local_lport_ids, > > + const struct sbrec_port_binding *binding_rec) > > +{ > > + char buf[16]; > > + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, > > + binding_rec->datapath->tunnel_key, > > + binding_rec->tunnel_key); > > + sset_add(local_lport_ids, buf); > > +} > > + > > +static void > > +remove_local_lport_ids(const struct sbrec_port_binding *binding_rec, > > + struct sset *local_lport_ids) > > +{ > > + char buf[16]; > > + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, > > + binding_rec->datapath->tunnel_key, > > + binding_rec->tunnel_key); > > + sset_find_and_delete(local_lport_ids, buf); > > +} > > + > > enum local_binding_type { > > BT_VIF, > > BT_CHILD, > > @@ -545,18 +556,21 @@ local_binding_destroy(struct local_binding > *lbinding) > > free(lbinding); > > } > > > > +static > > +void local_binding_delete(struct shash *local_bindings, > > + struct local_binding *lbinding) > > +{ > > + shash_find_and_delete(local_bindings, lbinding->name); > > + local_binding_destroy(lbinding); > > +} > > + > > void > > local_bindings_destroy(struct shash *local_bindings) > > { > > struct shash_node *node, *next; > > SHASH_FOR_EACH_SAFE (node, next, local_bindings) { > > struct local_binding *lbinding = node->data; > > - struct local_binding *c, *n; > > - LIST_FOR_EACH_SAFE (c, n, node, &lbinding->children) { > > - ovs_list_remove(&c->node); > > - free(c->name); > > - free(c); > > - } > > + local_binding_destroy(lbinding); > > } > > > > shash_destroy(local_bindings); > > @@ -651,6 +665,22 @@ release_lport(const struct sbrec_port_binding *pb) > > } > > } > > > > +static void > > +release_local_binding_children(struct local_binding *lbinding) > > +{ > > + struct local_binding *l; > > + LIST_FOR_EACH (l, node, &lbinding->children) { > > + release_lport(l->pb); > > + } > > +} > > + > > +static void > > +release_local_binding(struct local_binding *lbinding) > > +{ > > + release_local_binding_children(lbinding); > > + release_lport(lbinding->pb); > > +} > > + > > static void > > consider_port_binding_for_vif(const struct sbrec_port_binding *pb, > > struct binding_ctx_in *b_ctx_in, > > @@ -725,7 +755,6 @@ consider_port_binding(const struct sbrec_port_binding > *pb, > > struct binding_ctx_out *b_ctx_out, > > struct hmap *qos_map) > > { > > - > > bool our_chassis = is_our_chassis(b_ctx_in->chassis_rec, pb, > > b_ctx_in->active_tunnels, NULL, > > b_ctx_out->local_lports); > > @@ -821,6 +850,8 @@ build_local_bindings_from_local_ifaces(struct > binding_ctx_in *b_ctx_in, > > lbinding->pb = pb; > > } > > sset_add(b_ctx_out->local_lports, iface_id); > > + smap_replace(b_ctx_out->local_iface_ids, iface_rec->name, > > + iface_id); > > } > > > > /* Check if this is a tunnel interface. */ > > @@ -938,62 +969,6 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct > binding_ctx_out *b_ctx_out) > > hmap_destroy(&qos_map); > > } > > > > -/* Returns true if port-binding changes potentially require flow changes > on > > - * the current chassis. Returns false if we are sure there is no impact. > */ > > -bool > > -binding_evaluate_port_binding_changes(struct binding_ctx_in *b_ctx_in, > > - struct binding_ctx_out *b_ctx_out) > > -{ > > - if (!b_ctx_in->chassis_rec) { > > - return true; > > - } > > - > > - bool changed = false; > > - > > - const struct sbrec_port_binding *binding_rec; > > - SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding_rec, > > - > b_ctx_in->port_binding_table) { > > - /* XXX: currently OVSDB change tracking doesn't support getting > old > > - * data when the operation is update, so if a port-binding moved > from > > - * this chassis to another, there is no easy way to find out the > > - * change. To workaround this problem, we just makes sure if > > - * any port *related to* this chassis has any change, then > trigger > > - * recompute. > > - * > > - * - 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, always trigger recompute. > */ > > - if (binding_rec->chassis == b_ctx_in->chassis_rec) { > > - changed = true; > > - break; > > - } > > - > > - if (strcmp(binding_rec->type, "")) { > > - changed = true; > > - break; > > - } > > - > > - struct local_binding *lbinding = NULL; > > - if (!binding_rec->type[0]) { > > - if (!binding_rec->parent_port || > !binding_rec->parent_port[0]) { > > - lbinding = local_binding_find(b_ctx_out->local_bindings, > > - binding_rec->logical_port); > > - } else { > > - lbinding = local_binding_find(b_ctx_out->local_bindings, > > - binding_rec->parent_port); > > - } > > - } > > - > > - if (lbinding) { > > - changed = true; > > - break; > > - } > > - } > > - > > - return changed; > > -} > > - > > /* Returns true if the database is all cleaned up, false if more work is > > * required. */ > > bool > > @@ -1028,3 +1003,347 @@ binding_cleanup(struct ovsdb_idl_txn > *ovnsb_idl_txn, > > > > return !any_changes; > > } > > + > > +static void > > +add_local_datapath_peer_port(const struct sbrec_port_binding *pb, > > + struct binding_ctx_in *b_ctx_in, > > + struct binding_ctx_out *b_ctx_out, > > + struct local_datapath *ld) > > +{ > > + bool present = false; > > + for (size_t i = 0; i < ld->n_peer_ports; i++) { > > + if (ld->peer_ports[i].local == pb) { > > + present = true; > > + break; > > + } > > + } > > + > > + const char *peer_name = smap_get(&pb->options, "peer"); > > + if (strcmp(pb->type, "patch") || !peer_name) { > > + return; > > + } > > + > > + const struct sbrec_port_binding *peer; > > + peer = lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name, > > + peer_name); > > + > > + if (!peer || !peer->datapath) { > > + return; > > + } > > + > > + if (!present) { > > + ld->n_peer_ports++; > > + if (ld->n_peer_ports > ld->n_allocated_peer_ports) { > > + ld->peer_ports = > > + x2nrealloc(ld->peer_ports, > > + &ld->n_allocated_peer_ports, > > + sizeof *ld->peer_ports); > > + } > > + ld->peer_ports[ld->n_peer_ports - 1].local = pb; > > + ld->peer_ports[ld->n_peer_ports - 1].remote = peer; > > + } > > + > > + struct local_datapath *peer_ld = > > + get_local_datapath(b_ctx_out->local_datapaths, > > + peer->datapath->tunnel_key); > > + if (!peer_ld) { > > + 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, > > + peer->datapath, false, > > + 1, b_ctx_out->local_datapaths); > > + return; > > + } > > + > > + for (size_t i = 0; i < peer_ld->n_peer_ports; i++) { > > + if (peer_ld->peer_ports[i].local == peer) { > > + return; > > + } > > + } > > + > > + peer_ld->n_peer_ports++; > > + if (peer_ld->n_peer_ports > peer_ld->n_allocated_peer_ports) { > > + peer_ld->peer_ports = > > + x2nrealloc(peer_ld->peer_ports, > > + &peer_ld->n_allocated_peer_ports, > > + sizeof *peer_ld->peer_ports); > > + } > > + peer_ld->peer_ports[peer_ld->n_peer_ports - 1].local = peer; > > + peer_ld->peer_ports[peer_ld->n_peer_ports - 1].remote = pb; > > +} > > + > > +static void > > +remove_local_datapath_peer_port(const struct sbrec_port_binding *pb, > > + struct local_datapath *ld, > > + struct hmap *local_datapaths) > > +{ > > + size_t i =0; > > + for (i = 0; i < ld->n_peer_ports; i++) { > > + if (ld->peer_ports[i].local == pb) { > > + break; > > + } > > + } > > + > > + if (i == ld->n_peer_ports) { > > + return; > > + } > > + > > + const struct sbrec_port_binding *peer = ld->peer_ports[i].remote; > > + > > + ld->peer_ports[i].local = ld->peer_ports[ld->n_peer_ports - 1].local; > > + ld->peer_ports[i].remote = ld->peer_ports[ld->n_peer_ports - > 1].remote; > > + ld->n_peer_ports--; > > + > > + struct local_datapath *peer_ld = > > + get_local_datapath(local_datapaths, peer->datapath->tunnel_key); > > + if (peer_ld) { > > + remove_local_datapath_peer_port(peer, peer_ld, local_datapaths); > > + } > > +} > > + > > +static void > > +update_local_datapath_for_pb(const struct sbrec_port_binding *pb, > > + struct binding_ctx_in *b_ctx_in, > > + struct binding_ctx_out *b_ctx_out, > > + struct local_datapath *ld) > > +{ > > + if (!strcmp(pb->type, "patch")) { > > + add_local_datapath_peer_port(pb, b_ctx_in, b_ctx_out, ld); > > + } else if (!strcmp(pb->type, "localnet")) { > > + struct shash bridge_mappings = > SHASH_INITIALIZER(&bridge_mappings); > > + add_ovs_bridge_mappings(b_ctx_in->ovs_table, > b_ctx_in->bridge_table, > > + &bridge_mappings); > > + consider_localnet_port(pb, &bridge_mappings, > b_ctx_out->egress_ifaces, > > + b_ctx_out->local_datapaths); > > + shash_destroy(&bridge_mappings); > > + } else if (!strcmp(pb->type, "l3gateway")) { > > + const char *chassis_id = smap_get(&pb->options, > > + "l3gateway-chassis"); > > + if (chassis_id && !strcmp(chassis_id, > b_ctx_in->chassis_rec->name)) { > > + ld->has_local_l3gateway = true; > > + } > > + } > > + > > + if (!strcmp(pb->type, "patch") || > > + !strcmp(pb->type, "localport") || > > + !strcmp(pb->type, "vtep")) { > > + update_local_lport_ids(b_ctx_out->local_lport_ids, pb); > > + } > > +} > > + > > +static void > > +remove_pb_from_local_datapath(const struct sbrec_port_binding *pb, > > + const struct sbrec_chassis *chassis_rec, > > + struct binding_ctx_out *b_ctx_out, > > + struct local_datapath *ld) > > +{ > > + remove_local_lport_ids(pb, b_ctx_out->local_lport_ids); > > + if (!strcmp(pb->type, "patch") || > > + !strcmp(pb->type, "l3gateway")) { > > + remove_local_datapath_peer_port(pb, ld, > b_ctx_out->local_datapaths); > > + } else if (!strcmp(pb->type, "localnet")) { > > + if (ld->localnet_port && !strcmp(ld->localnet_port->logical_port, > > + pb->logical_port)) { > > + ld->localnet_port = NULL; > > + } > > + } else if (!strcmp(pb->type, "l3gateway")) { > > + const char *chassis_id = smap_get(&pb->options, > > + "l3gateway-chassis"); > > + if (chassis_id && !strcmp(chassis_id, chassis_rec->name)) { > > + ld->has_local_l3gateway = false; > > + } > > + } > > +} > > + > > +/* Returns true if the ovs interface changes were handled successfully, > > + * false otherwise. > > + */ > > +bool > > +binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, > > + struct binding_ctx_out *b_ctx_out) > > +{ > > + if (!b_ctx_in->chassis_rec) { > > + return false; > > + } > > + > > + bool handled = true; > > + struct hmap qos_map = HMAP_INITIALIZER(&qos_map); > > + struct hmap *qos_map_ptr = > > + sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map; > > + > > + const struct ovsrec_interface *iface_rec; > > + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, > > + b_ctx_in->iface_table) { > > + const char *iface_id = smap_get(&iface_rec->external_ids, > "iface-id"); > > + const char *old_iface_id = smap_get(b_ctx_out->local_iface_ids, > > + iface_rec->name); > > + if (iface_rec->type && iface_rec->type[0]) { > > + handled = false; > > + goto out; > > + } > > + > > + struct local_binding *lbinding = NULL; > > + struct local_binding *claim_lbinding = NULL; > > + const char *cleared_iface_id = NULL; > > + int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; > > + if (!ovsrec_interface_is_deleted(iface_rec)) { > > + if (iface_id) { > > + /* Check if iface_id is changed. If so we need to > > + * release the old port binding and associate this > > + * inteface to new port binding. */ > > + if (old_iface_id && strcmp(iface_id, old_iface_id)) { > > + cleared_iface_id = old_iface_id; > > + } > > + } else if (old_iface_id) { > > + cleared_iface_id = old_iface_id; > > + } > > + } else { > > + cleared_iface_id = iface_id; > > + iface_id = NULL; > > + } > > + > > + if (cleared_iface_id) { > > + lbinding = local_binding_find(b_ctx_out->local_bindings, > > + cleared_iface_id); > > + if (lbinding && lbinding->pb && > > + lbinding->pb->chassis == b_ctx_in->chassis_rec) { > > + release_local_binding(lbinding); > > + struct local_datapath *ld = > > + get_local_datapath(b_ctx_out->local_datapaths, > > + > lbinding->pb->datapath->tunnel_key); > > + if (ld) { > > + remove_pb_from_local_datapath(lbinding->pb, > > + b_ctx_in->chassis_rec, > > + b_ctx_out, ld); > > + } > > + local_binding_delete(b_ctx_out->local_bindings, > lbinding); > > + } > > + > > + sset_find_and_delete(b_ctx_out->local_lports, > cleared_iface_id); > > + smap_remove(b_ctx_out->local_iface_ids, iface_rec->name); > > + } > > + > > + if (iface_id && ofport > 0) { > > + sset_add(b_ctx_out->local_lports, iface_id); > > + smap_replace(b_ctx_out->local_iface_ids, iface_rec->name, > > + iface_id); > > + claim_lbinding = > > + local_binding_find(b_ctx_out->local_bindings, iface_id); > > + if (!claim_lbinding) { > > + claim_lbinding = local_binding_create(iface_id, > iface_rec, > > + NULL, BT_VIF); > > + local_binding_add(b_ctx_out->local_bindings, > claim_lbinding); > > + } else { > > + claim_lbinding->iface = iface_rec; > > + } > > + > > + if (!claim_lbinding->pb || > > + strcmp(claim_lbinding->name, > > + claim_lbinding->pb->logical_port)) { > > + claim_lbinding->pb = > > + > lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name, > > + claim_lbinding->name); > > + } > > + > > + if (claim_lbinding->pb) { > > + consider_port_binding_for_vif(claim_lbinding->pb, > b_ctx_in, > > + BT_VIF, claim_lbinding, > > + b_ctx_out, qos_map_ptr); > > + } > > + } > > + } > > + > > + if (qos_map_ptr && set_noop_qos(b_ctx_in->ovs_idl_txn, > > + b_ctx_in->port_table, > > + b_ctx_in->qos_table, > > + b_ctx_out->egress_ifaces)) { > > + const char *entry; > > + SSET_FOR_EACH (entry, b_ctx_out->egress_ifaces) { > > + setup_qos(entry, &qos_map); > > + } > > + } > > + > > + hmap_destroy(&qos_map); > > +out: > > + return handled; > > +} > > + > > +/* Returns true if the port binding changes resulted in local binding > > + * updates, false otherwise. > > + */ > > +bool > > +binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in, > > + struct binding_ctx_out *b_ctx_out) > > +{ > > + bool updated = false; > > + struct hmap qos_map = HMAP_INITIALIZER(&qos_map); > > + struct hmap *qos_map_ptr = > > + sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map; > > + > > + const struct sbrec_port_binding *pb; > > + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, > > + > b_ctx_in->port_binding_table) { > > + bool consider_for_vif = false; > > + struct local_binding *lbinding = NULL; > > + enum local_binding_type binding_type = BT_VIF; > > + bool is_deleted = sbrec_port_binding_is_deleted(pb); > > + if (!pb->type[0]) { > > + if (!pb->parent_port || !pb->parent_port[0]) { > > + lbinding = local_binding_find(b_ctx_out->local_bindings, > > + pb->logical_port); > > + } else { > > + lbinding = local_binding_find(b_ctx_out->local_bindings, > > + pb->parent_port); > > + binding_type = BT_CHILD; > > + } > > + > > + consider_for_vif = true; > > + } else if (!strcmp(pb->type, "virtual") && > > + pb->virtual_parent && pb->virtual_parent[0]) { > > + lbinding = local_binding_find(b_ctx_out->local_bindings, > > + pb->virtual_parent); > > + consider_for_vif = true; > > + binding_type = BT_VIRTUAL; > > + } > > + > > + if (is_deleted) { > > + if (lbinding) { > > + updated = true; > > + lbinding->pb = NULL; > > + > > + if (binding_type == BT_VIF) { > > + release_local_binding_children(lbinding); > > + } > > + } > > + > > + struct local_datapath *ld = > > + get_local_datapath(b_ctx_out->local_datapaths, > > + pb->datapath->tunnel_key); > > + if (ld) { > > + remove_pb_from_local_datapath(pb, b_ctx_in->chassis_rec, > > + b_ctx_out, ld); > > + } > > + } else { > > + if (consider_for_vif) { > > + if (lbinding) { > > + updated = true; > > + lbinding->pb = pb; > > + consider_port_binding_for_vif(pb, b_ctx_in, > binding_type, > > + lbinding, b_ctx_out, > > + qos_map_ptr); > > + } > > + } else { > > + updated = true; > > + consider_port_binding(pb, b_ctx_in, b_ctx_out, > qos_map_ptr); > > + struct local_datapath *ld = > > + get_local_datapath(b_ctx_out->local_datapaths, > > + pb->datapath->tunnel_key); > > + if (ld) { > > + update_local_datapath_for_pb(pb, b_ctx_in, > b_ctx_out, ld); > > + } > > + } > > + } > > + } > > + > > + return updated; > > +} > > diff --git a/controller/binding.h b/controller/binding.h > > index 6bee1d12e..95ca0367d 100644 > > --- a/controller/binding.h > > +++ b/controller/binding.h > > @@ -56,6 +56,7 @@ struct binding_ctx_out { > > struct sset *local_lports; > > struct sset *local_lport_ids; > > struct sset *egress_ifaces; > > + struct smap *local_iface_ids; > > }; > > > > void binding_register_ovs_idl(struct ovsdb_idl *); > > @@ -63,10 +64,12 @@ 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 *); > > -bool binding_evaluate_port_binding_changes(struct binding_ctx_in *, > > - struct binding_ctx_out *); > > void local_bindings_destroy(struct shash *local_bindings); > > void binding_add_vport_to_local_bindings( > > struct shash *local_bindings, const struct sbrec_port_binding > *parent, > > const struct sbrec_port_binding *vport); > > +bool binding_handle_ovs_interface_changes(struct binding_ctx_in *, > > + struct binding_ctx_out *); > > +bool binding_handle_port_binding_changes(struct binding_ctx_in *, > > + struct binding_ctx_out *); > > #endif /* controller/binding.h */ > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > > index 8cc27cebf..6841be29d 100644 > > --- a/controller/ovn-controller.c > > +++ b/controller/ovn-controller.c > > @@ -753,6 +753,7 @@ enum sb_engine_node { > > OVS_NODE(open_vswitch, "open_vswitch") \ > > OVS_NODE(bridge, "bridge") \ > > OVS_NODE(port, "port") \ > > + OVS_NODE(interface, "interface") \ > > OVS_NODE(qos, "qos") > > > > enum ovs_engine_node { > > @@ -974,6 +975,7 @@ struct ed_type_runtime_data { > > struct sset active_tunnels; > > > > struct sset egress_ifaces; > > + struct smap local_iface_ids; > > }; > > > > static void * > > @@ -988,6 +990,7 @@ en_runtime_data_init(struct engine_node *node > OVS_UNUSED, > > sset_init(&data->active_tunnels); > > sset_init(&data->egress_ifaces); > > shash_init(&data->local_bindings); > > + smap_init(&data->local_iface_ids); > > return data; > > } > > > > @@ -1000,6 +1003,7 @@ en_runtime_data_cleanup(void *data) > > sset_destroy(&rt_data->local_lport_ids); > > sset_destroy(&rt_data->active_tunnels); > > sset_init(&rt_data->egress_ifaces); > > + smap_destroy(&rt_data->local_iface_ids); > > struct local_datapath *cur_node, *next_node; > > HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, > > &rt_data->local_datapaths) { > > @@ -1041,6 +1045,10 @@ init_binding_ctx(struct engine_node *node, > > (struct ovsrec_port_table *)EN_OVSDB_GET( > > engine_get_input("OVS_port", node)); > > > > + struct ovsrec_interface_table *iface_table = > > + (struct ovsrec_interface_table *)EN_OVSDB_GET( > > + engine_get_input("OVS_interface", node)); > > + > > struct ovsrec_qos_table *qos_table = > > (struct ovsrec_qos_table *)EN_OVSDB_GET( > > engine_get_input("OVS_qos", node)); > > @@ -1070,6 +1078,7 @@ init_binding_ctx(struct engine_node *node, > > 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->iface_table = iface_table; > > b_ctx_in->qos_table = qos_table; > > b_ctx_in->port_binding_table = pb_table; > > b_ctx_in->br_int = br_int; > > @@ -1083,6 +1092,7 @@ init_binding_ctx(struct engine_node *node, > > b_ctx_out->local_lport_ids = &rt_data->local_lport_ids; > > b_ctx_out->egress_ifaces = &rt_data->egress_ifaces; > > b_ctx_out->local_bindings = &rt_data->local_bindings; > > + b_ctx_out->local_iface_ids = &rt_data->local_iface_ids; > > } > > > > static void > > @@ -1110,10 +1120,12 @@ en_runtime_data_run(struct engine_node *node, > void *data) > > sset_destroy(local_lport_ids); > > sset_destroy(active_tunnels); > > sset_destroy(&rt_data->egress_ifaces); > > + smap_destroy(&rt_data->local_iface_ids); > > sset_init(local_lports); > > sset_init(local_lport_ids); > > sset_init(active_tunnels); > > sset_init(&rt_data->egress_ifaces); > > + smap_init(&rt_data->local_iface_ids); > > } > > > > struct binding_ctx_in b_ctx_in; > > @@ -1139,17 +1151,47 @@ en_runtime_data_run(struct engine_node *node, > void *data) > > } > > > > static bool > > -runtime_data_sb_port_binding_handler(struct engine_node *node, void > *data) > > +runtime_data_ovs_interface_handler(struct engine_node *node, void *data) > > { > > + if (!engine_get_context()->ovnsb_idl_txn) { > > + return false; > > + } > > + > > struct ed_type_runtime_data *rt_data = data; > > 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); > > > > - bool changed = binding_evaluate_port_binding_changes(&b_ctx_in, > > - &b_ctx_out); > > + engine_set_node_state(node, EN_UPDATED); > > + return binding_handle_ovs_interface_changes(&b_ctx_in, &b_ctx_out); > > +} > > + > > +static bool > > +runtime_data_noop_handler(struct engine_node *node OVS_UNUSED, > > + void *data OVS_UNUSED) > > +{ > > + return true; > > +} > > > > - return !changed; > > +static bool > > +runtime_data_sb_port_binding_handler(struct engine_node *node, void > *data) > > +{ > > + if (!engine_get_context()->ovnsb_idl_txn) { > > + return false; > > + } > > + > > + struct ed_type_runtime_data *rt_data = data; > > + 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); > > + if (!b_ctx_in.chassis_rec) { > > + return false; > > + } > > + > > + bool updated = binding_handle_port_binding_changes(&b_ctx_in, > &b_ctx_out); > > + enum engine_node_state state = updated ? EN_UPDATED : EN_VALID; > > An input change handler should never set the node's state to EN_VALID, > because it doesn't know if any other input handler already set the state to > EN_UPDATED. What it should do is just set state to EN_UPDATE if it knows > that the data is changed. (Sorry this was not well documented, and there's > already existing code like this in address-set handling. This was a miss in > a previous review of a refactor of the I-P code.) Makes sense to me. I'll update it in v2. Thanks Numan Numan > > > + engine_set_node_state(node, state); > > + return true; > > } > > > > /* Connection tracking zones. */ > > @@ -1891,7 +1933,10 @@ main(int argc, char *argv[]) > > > > engine_add_input(&en_runtime_data, &en_ovs_open_vswitch, NULL); > > engine_add_input(&en_runtime_data, &en_ovs_bridge, NULL); > > - engine_add_input(&en_runtime_data, &en_ovs_port, NULL); > > + engine_add_input(&en_runtime_data, &en_ovs_port, > > + runtime_data_noop_handler); > > + engine_add_input(&en_runtime_data, &en_ovs_interface, > > + runtime_data_ovs_interface_handler); > > engine_add_input(&en_runtime_data, &en_ovs_qos, NULL); > > > > engine_add_input(&en_runtime_data, &en_sb_chassis, NULL); > > -- > > 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 34bd475de..ce4467f31 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -349,17 +349,6 @@ setup_qos(const char *egress_iface, struct hmap *queue_map) netdev_close(netdev_phy); } -static void -update_local_lport_ids(struct sset *local_lport_ids, - const struct sbrec_port_binding *binding_rec) -{ - char buf[16]; - snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, - binding_rec->datapath->tunnel_key, - binding_rec->tunnel_key); - sset_add(local_lport_ids, buf); -} - /* * Get the encap from the chassis for this port. The interface * may have an external_ids:encap-ip=<encap-ip> set; if so we @@ -490,6 +479,28 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, ld->localnet_port = binding_rec; } +static void +update_local_lport_ids(struct sset *local_lport_ids, + const struct sbrec_port_binding *binding_rec) +{ + char buf[16]; + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, + binding_rec->datapath->tunnel_key, + binding_rec->tunnel_key); + sset_add(local_lport_ids, buf); +} + +static void +remove_local_lport_ids(const struct sbrec_port_binding *binding_rec, + struct sset *local_lport_ids) +{ + char buf[16]; + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, + binding_rec->datapath->tunnel_key, + binding_rec->tunnel_key); + sset_find_and_delete(local_lport_ids, buf); +} + enum local_binding_type { BT_VIF, BT_CHILD, @@ -545,18 +556,21 @@ local_binding_destroy(struct local_binding *lbinding) free(lbinding); } +static +void local_binding_delete(struct shash *local_bindings, + struct local_binding *lbinding) +{ + shash_find_and_delete(local_bindings, lbinding->name); + local_binding_destroy(lbinding); +} + void local_bindings_destroy(struct shash *local_bindings) { struct shash_node *node, *next; SHASH_FOR_EACH_SAFE (node, next, local_bindings) { struct local_binding *lbinding = node->data; - struct local_binding *c, *n; - LIST_FOR_EACH_SAFE (c, n, node, &lbinding->children) { - ovs_list_remove(&c->node); - free(c->name); - free(c); - } + local_binding_destroy(lbinding); } shash_destroy(local_bindings); @@ -651,6 +665,22 @@ release_lport(const struct sbrec_port_binding *pb) } } +static void +release_local_binding_children(struct local_binding *lbinding) +{ + struct local_binding *l; + LIST_FOR_EACH (l, node, &lbinding->children) { + release_lport(l->pb); + } +} + +static void +release_local_binding(struct local_binding *lbinding) +{ + release_local_binding_children(lbinding); + release_lport(lbinding->pb); +} + static void consider_port_binding_for_vif(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, @@ -725,7 +755,6 @@ consider_port_binding(const struct sbrec_port_binding *pb, struct binding_ctx_out *b_ctx_out, struct hmap *qos_map) { - bool our_chassis = is_our_chassis(b_ctx_in->chassis_rec, pb, b_ctx_in->active_tunnels, NULL, b_ctx_out->local_lports); @@ -821,6 +850,8 @@ build_local_bindings_from_local_ifaces(struct binding_ctx_in *b_ctx_in, lbinding->pb = pb; } sset_add(b_ctx_out->local_lports, iface_id); + smap_replace(b_ctx_out->local_iface_ids, iface_rec->name, + iface_id); } /* Check if this is a tunnel interface. */ @@ -938,62 +969,6 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) hmap_destroy(&qos_map); } -/* Returns true if port-binding changes potentially require flow changes on - * the current chassis. Returns false if we are sure there is no impact. */ -bool -binding_evaluate_port_binding_changes(struct binding_ctx_in *b_ctx_in, - struct binding_ctx_out *b_ctx_out) -{ - if (!b_ctx_in->chassis_rec) { - return true; - } - - bool changed = false; - - const struct sbrec_port_binding *binding_rec; - SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding_rec, - b_ctx_in->port_binding_table) { - /* XXX: currently OVSDB change tracking doesn't support getting old - * data when the operation is update, so if a port-binding moved from - * this chassis to another, there is no easy way to find out the - * change. To workaround this problem, we just makes sure if - * any port *related to* this chassis has any change, then trigger - * recompute. - * - * - 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, always trigger recompute. */ - if (binding_rec->chassis == b_ctx_in->chassis_rec) { - changed = true; - break; - } - - if (strcmp(binding_rec->type, "")) { - changed = true; - break; - } - - struct local_binding *lbinding = NULL; - if (!binding_rec->type[0]) { - if (!binding_rec->parent_port || !binding_rec->parent_port[0]) { - lbinding = local_binding_find(b_ctx_out->local_bindings, - binding_rec->logical_port); - } else { - lbinding = local_binding_find(b_ctx_out->local_bindings, - binding_rec->parent_port); - } - } - - if (lbinding) { - changed = true; - break; - } - } - - return changed; -} - /* Returns true if the database is all cleaned up, false if more work is * required. */ bool @@ -1028,3 +1003,347 @@ binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, return !any_changes; } + +static void +add_local_datapath_peer_port(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + struct local_datapath *ld) +{ + bool present = false; + for (size_t i = 0; i < ld->n_peer_ports; i++) { + if (ld->peer_ports[i].local == pb) { + present = true; + break; + } + } + + const char *peer_name = smap_get(&pb->options, "peer"); + if (strcmp(pb->type, "patch") || !peer_name) { + return; + } + + const struct sbrec_port_binding *peer; + peer = lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name, + peer_name); + + if (!peer || !peer->datapath) { + return; + } + + if (!present) { + ld->n_peer_ports++; + if (ld->n_peer_ports > ld->n_allocated_peer_ports) { + ld->peer_ports = + x2nrealloc(ld->peer_ports, + &ld->n_allocated_peer_ports, + sizeof *ld->peer_ports); + } + ld->peer_ports[ld->n_peer_ports - 1].local = pb; + ld->peer_ports[ld->n_peer_ports - 1].remote = peer; + } + + struct local_datapath *peer_ld = + get_local_datapath(b_ctx_out->local_datapaths, + peer->datapath->tunnel_key); + if (!peer_ld) { + 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, + peer->datapath, false, + 1, b_ctx_out->local_datapaths); + return; + } + + for (size_t i = 0; i < peer_ld->n_peer_ports; i++) { + if (peer_ld->peer_ports[i].local == peer) { + return; + } + } + + peer_ld->n_peer_ports++; + if (peer_ld->n_peer_ports > peer_ld->n_allocated_peer_ports) { + peer_ld->peer_ports = + x2nrealloc(peer_ld->peer_ports, + &peer_ld->n_allocated_peer_ports, + sizeof *peer_ld->peer_ports); + } + peer_ld->peer_ports[peer_ld->n_peer_ports - 1].local = peer; + peer_ld->peer_ports[peer_ld->n_peer_ports - 1].remote = pb; +} + +static void +remove_local_datapath_peer_port(const struct sbrec_port_binding *pb, + struct local_datapath *ld, + struct hmap *local_datapaths) +{ + size_t i =0; + for (i = 0; i < ld->n_peer_ports; i++) { + if (ld->peer_ports[i].local == pb) { + break; + } + } + + if (i == ld->n_peer_ports) { + return; + } + + const struct sbrec_port_binding *peer = ld->peer_ports[i].remote; + + ld->peer_ports[i].local = ld->peer_ports[ld->n_peer_ports - 1].local; + ld->peer_ports[i].remote = ld->peer_ports[ld->n_peer_ports - 1].remote; + ld->n_peer_ports--; + + struct local_datapath *peer_ld = + get_local_datapath(local_datapaths, peer->datapath->tunnel_key); + if (peer_ld) { + remove_local_datapath_peer_port(peer, peer_ld, local_datapaths); + } +} + +static void +update_local_datapath_for_pb(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + struct local_datapath *ld) +{ + if (!strcmp(pb->type, "patch")) { + add_local_datapath_peer_port(pb, b_ctx_in, b_ctx_out, ld); + } else if (!strcmp(pb->type, "localnet")) { + struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings); + add_ovs_bridge_mappings(b_ctx_in->ovs_table, b_ctx_in->bridge_table, + &bridge_mappings); + consider_localnet_port(pb, &bridge_mappings, b_ctx_out->egress_ifaces, + b_ctx_out->local_datapaths); + shash_destroy(&bridge_mappings); + } else if (!strcmp(pb->type, "l3gateway")) { + const char *chassis_id = smap_get(&pb->options, + "l3gateway-chassis"); + if (chassis_id && !strcmp(chassis_id, b_ctx_in->chassis_rec->name)) { + ld->has_local_l3gateway = true; + } + } + + if (!strcmp(pb->type, "patch") || + !strcmp(pb->type, "localport") || + !strcmp(pb->type, "vtep")) { + update_local_lport_ids(b_ctx_out->local_lport_ids, pb); + } +} + +static void +remove_pb_from_local_datapath(const struct sbrec_port_binding *pb, + const struct sbrec_chassis *chassis_rec, + struct binding_ctx_out *b_ctx_out, + struct local_datapath *ld) +{ + remove_local_lport_ids(pb, b_ctx_out->local_lport_ids); + if (!strcmp(pb->type, "patch") || + !strcmp(pb->type, "l3gateway")) { + remove_local_datapath_peer_port(pb, ld, b_ctx_out->local_datapaths); + } else if (!strcmp(pb->type, "localnet")) { + if (ld->localnet_port && !strcmp(ld->localnet_port->logical_port, + pb->logical_port)) { + ld->localnet_port = NULL; + } + } else if (!strcmp(pb->type, "l3gateway")) { + const char *chassis_id = smap_get(&pb->options, + "l3gateway-chassis"); + if (chassis_id && !strcmp(chassis_id, chassis_rec->name)) { + ld->has_local_l3gateway = false; + } + } +} + +/* Returns true if the ovs interface changes were handled successfully, + * false otherwise. + */ +bool +binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out) +{ + if (!b_ctx_in->chassis_rec) { + return false; + } + + bool handled = true; + struct hmap qos_map = HMAP_INITIALIZER(&qos_map); + struct hmap *qos_map_ptr = + sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map; + + const struct ovsrec_interface *iface_rec; + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, + b_ctx_in->iface_table) { + const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id"); + const char *old_iface_id = smap_get(b_ctx_out->local_iface_ids, + iface_rec->name); + if (iface_rec->type && iface_rec->type[0]) { + handled = false; + goto out; + } + + struct local_binding *lbinding = NULL; + struct local_binding *claim_lbinding = NULL; + const char *cleared_iface_id = NULL; + int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; + if (!ovsrec_interface_is_deleted(iface_rec)) { + if (iface_id) { + /* Check if iface_id is changed. If so we need to + * release the old port binding and associate this + * inteface to new port binding. */ + if (old_iface_id && strcmp(iface_id, old_iface_id)) { + cleared_iface_id = old_iface_id; + } + } else if (old_iface_id) { + cleared_iface_id = old_iface_id; + } + } else { + cleared_iface_id = iface_id; + iface_id = NULL; + } + + if (cleared_iface_id) { + lbinding = local_binding_find(b_ctx_out->local_bindings, + cleared_iface_id); + if (lbinding && lbinding->pb && + lbinding->pb->chassis == b_ctx_in->chassis_rec) { + release_local_binding(lbinding); + struct local_datapath *ld = + get_local_datapath(b_ctx_out->local_datapaths, + lbinding->pb->datapath->tunnel_key); + if (ld) { + remove_pb_from_local_datapath(lbinding->pb, + b_ctx_in->chassis_rec, + b_ctx_out, ld); + } + local_binding_delete(b_ctx_out->local_bindings, lbinding); + } + + sset_find_and_delete(b_ctx_out->local_lports, cleared_iface_id); + smap_remove(b_ctx_out->local_iface_ids, iface_rec->name); + } + + if (iface_id && ofport > 0) { + sset_add(b_ctx_out->local_lports, iface_id); + smap_replace(b_ctx_out->local_iface_ids, iface_rec->name, + iface_id); + claim_lbinding = + local_binding_find(b_ctx_out->local_bindings, iface_id); + if (!claim_lbinding) { + claim_lbinding = local_binding_create(iface_id, iface_rec, + NULL, BT_VIF); + local_binding_add(b_ctx_out->local_bindings, claim_lbinding); + } else { + claim_lbinding->iface = iface_rec; + } + + if (!claim_lbinding->pb || + strcmp(claim_lbinding->name, + claim_lbinding->pb->logical_port)) { + claim_lbinding->pb = + lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name, + claim_lbinding->name); + } + + if (claim_lbinding->pb) { + consider_port_binding_for_vif(claim_lbinding->pb, b_ctx_in, + BT_VIF, claim_lbinding, + b_ctx_out, qos_map_ptr); + } + } + } + + if (qos_map_ptr && set_noop_qos(b_ctx_in->ovs_idl_txn, + b_ctx_in->port_table, + b_ctx_in->qos_table, + b_ctx_out->egress_ifaces)) { + const char *entry; + SSET_FOR_EACH (entry, b_ctx_out->egress_ifaces) { + setup_qos(entry, &qos_map); + } + } + + hmap_destroy(&qos_map); +out: + return handled; +} + +/* Returns true if the port binding changes resulted in local binding + * updates, false otherwise. + */ +bool +binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out) +{ + bool updated = false; + struct hmap qos_map = HMAP_INITIALIZER(&qos_map); + struct hmap *qos_map_ptr = + sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map; + + const struct sbrec_port_binding *pb; + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, + b_ctx_in->port_binding_table) { + bool consider_for_vif = false; + struct local_binding *lbinding = NULL; + enum local_binding_type binding_type = BT_VIF; + bool is_deleted = sbrec_port_binding_is_deleted(pb); + if (!pb->type[0]) { + if (!pb->parent_port || !pb->parent_port[0]) { + lbinding = local_binding_find(b_ctx_out->local_bindings, + pb->logical_port); + } else { + lbinding = local_binding_find(b_ctx_out->local_bindings, + pb->parent_port); + binding_type = BT_CHILD; + } + + consider_for_vif = true; + } else if (!strcmp(pb->type, "virtual") && + pb->virtual_parent && pb->virtual_parent[0]) { + lbinding = local_binding_find(b_ctx_out->local_bindings, + pb->virtual_parent); + consider_for_vif = true; + binding_type = BT_VIRTUAL; + } + + if (is_deleted) { + if (lbinding) { + updated = true; + lbinding->pb = NULL; + + if (binding_type == BT_VIF) { + release_local_binding_children(lbinding); + } + } + + struct local_datapath *ld = + get_local_datapath(b_ctx_out->local_datapaths, + pb->datapath->tunnel_key); + if (ld) { + remove_pb_from_local_datapath(pb, b_ctx_in->chassis_rec, + b_ctx_out, ld); + } + } else { + if (consider_for_vif) { + if (lbinding) { + updated = true; + lbinding->pb = pb; + consider_port_binding_for_vif(pb, b_ctx_in, binding_type, + lbinding, b_ctx_out, + qos_map_ptr); + } + } else { + updated = true; + consider_port_binding(pb, b_ctx_in, b_ctx_out, qos_map_ptr); + struct local_datapath *ld = + get_local_datapath(b_ctx_out->local_datapaths, + pb->datapath->tunnel_key); + if (ld) { + update_local_datapath_for_pb(pb, b_ctx_in, b_ctx_out, ld); + } + } + } + } + + return updated; +} diff --git a/controller/binding.h b/controller/binding.h index 6bee1d12e..95ca0367d 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -56,6 +56,7 @@ struct binding_ctx_out { struct sset *local_lports; struct sset *local_lport_ids; struct sset *egress_ifaces; + struct smap *local_iface_ids; }; void binding_register_ovs_idl(struct ovsdb_idl *); @@ -63,10 +64,12 @@ 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 *); -bool binding_evaluate_port_binding_changes(struct binding_ctx_in *, - struct binding_ctx_out *); void local_bindings_destroy(struct shash *local_bindings); void binding_add_vport_to_local_bindings( struct shash *local_bindings, const struct sbrec_port_binding *parent, const struct sbrec_port_binding *vport); +bool binding_handle_ovs_interface_changes(struct binding_ctx_in *, + struct binding_ctx_out *); +bool binding_handle_port_binding_changes(struct binding_ctx_in *, + struct binding_ctx_out *); #endif /* controller/binding.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 8cc27cebf..6841be29d 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -753,6 +753,7 @@ enum sb_engine_node { OVS_NODE(open_vswitch, "open_vswitch") \ OVS_NODE(bridge, "bridge") \ OVS_NODE(port, "port") \ + OVS_NODE(interface, "interface") \ OVS_NODE(qos, "qos") enum ovs_engine_node { @@ -974,6 +975,7 @@ struct ed_type_runtime_data { struct sset active_tunnels; struct sset egress_ifaces; + struct smap local_iface_ids; }; static void * @@ -988,6 +990,7 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED, sset_init(&data->active_tunnels); sset_init(&data->egress_ifaces); shash_init(&data->local_bindings); + smap_init(&data->local_iface_ids); return data; } @@ -1000,6 +1003,7 @@ en_runtime_data_cleanup(void *data) sset_destroy(&rt_data->local_lport_ids); sset_destroy(&rt_data->active_tunnels); sset_init(&rt_data->egress_ifaces); + smap_destroy(&rt_data->local_iface_ids); struct local_datapath *cur_node, *next_node; HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, &rt_data->local_datapaths) { @@ -1041,6 +1045,10 @@ init_binding_ctx(struct engine_node *node, (struct ovsrec_port_table *)EN_OVSDB_GET( engine_get_input("OVS_port", node)); + struct ovsrec_interface_table *iface_table = + (struct ovsrec_interface_table *)EN_OVSDB_GET( + engine_get_input("OVS_interface", node)); + struct ovsrec_qos_table *qos_table = (struct ovsrec_qos_table *)EN_OVSDB_GET( engine_get_input("OVS_qos", node)); @@ -1070,6 +1078,7 @@ init_binding_ctx(struct engine_node *node, 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->iface_table = iface_table; b_ctx_in->qos_table = qos_table; b_ctx_in->port_binding_table = pb_table; b_ctx_in->br_int = br_int; @@ -1083,6 +1092,7 @@ init_binding_ctx(struct engine_node *node, b_ctx_out->local_lport_ids = &rt_data->local_lport_ids; b_ctx_out->egress_ifaces = &rt_data->egress_ifaces; b_ctx_out->local_bindings = &rt_data->local_bindings; + b_ctx_out->local_iface_ids = &rt_data->local_iface_ids; } static void @@ -1110,10 +1120,12 @@ en_runtime_data_run(struct engine_node *node, void *data) sset_destroy(local_lport_ids); sset_destroy(active_tunnels); sset_destroy(&rt_data->egress_ifaces); + smap_destroy(&rt_data->local_iface_ids); sset_init(local_lports); sset_init(local_lport_ids); sset_init(active_tunnels); sset_init(&rt_data->egress_ifaces); + smap_init(&rt_data->local_iface_ids); } struct binding_ctx_in b_ctx_in; @@ -1139,17 +1151,47 @@ en_runtime_data_run(struct engine_node *node, void *data) } static bool -runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) +runtime_data_ovs_interface_handler(struct engine_node *node, void *data) { + if (!engine_get_context()->ovnsb_idl_txn) { + return false; + } + struct ed_type_runtime_data *rt_data = data; 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); - bool changed = binding_evaluate_port_binding_changes(&b_ctx_in, - &b_ctx_out); + engine_set_node_state(node, EN_UPDATED); + return binding_handle_ovs_interface_changes(&b_ctx_in, &b_ctx_out); +} + +static bool +runtime_data_noop_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + return true; +} - return !changed; +static bool +runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) +{ + if (!engine_get_context()->ovnsb_idl_txn) { + return false; + } + + struct ed_type_runtime_data *rt_data = data; + 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); + if (!b_ctx_in.chassis_rec) { + return false; + } + + bool updated = binding_handle_port_binding_changes(&b_ctx_in, &b_ctx_out); + enum engine_node_state state = updated ? EN_UPDATED : EN_VALID; + engine_set_node_state(node, state); + return true; } /* Connection tracking zones. */ @@ -1891,7 +1933,10 @@ main(int argc, char *argv[]) engine_add_input(&en_runtime_data, &en_ovs_open_vswitch, NULL); engine_add_input(&en_runtime_data, &en_ovs_bridge, NULL); - engine_add_input(&en_runtime_data, &en_ovs_port, NULL); + engine_add_input(&en_runtime_data, &en_ovs_port, + runtime_data_noop_handler); + engine_add_input(&en_runtime_data, &en_ovs_interface, + runtime_data_ovs_interface_handler); engine_add_input(&en_runtime_data, &en_ovs_qos, NULL); engine_add_input(&en_runtime_data, &en_sb_chassis, NULL);