Message ID | 20200608134959.1378927-1-numans@ovn.org |
---|---|
State | Accepted |
Headers | show |
Series | Incremental processing improvements. | expand |
On 6/8/20 3:49 PM, 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. > > Acked-by: Mark Michelson <mmichels@redhat.com> > Signed-off-by: Numan Siddique <numans@ovn.org> Hi Numan, I added a few minor comments below. With those fixed the code looks ok to me: Acked-by: Dumitru Ceara <dceara@redhat.com> Regards, Dumitru > --- > controller/binding.c | 1011 ++++++++++++++++++++++++++++++----- > controller/binding.h | 9 +- > controller/ovn-controller.c | 61 ++- > tests/ovn-performance.at | 13 + > 4 files changed, 962 insertions(+), 132 deletions(-) > > diff --git a/controller/binding.c b/controller/binding.c > index 6a13d1a0e..71063fe9a 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -360,16 +360,6 @@ destroy_qos_map(struct hmap *qos_map) > hmap_destroy(qos_map); > } > > -static void > -update_local_lport_ids(struct sset *local_lport_ids, > - const struct sbrec_port_binding *pb) > -{ > - char buf[16]; > - snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, > - pb->datapath->tunnel_key, pb->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 > @@ -448,10 +438,10 @@ is_network_plugged(const struct sbrec_port_binding *binding_rec, > } > > static void > -consider_localnet_port(const struct sbrec_port_binding *binding_rec, > - struct shash *bridge_mappings, > - struct sset *egress_ifaces, > - struct hmap *local_datapaths) > +update_ld_localnet_port(const struct sbrec_port_binding *binding_rec, > + struct shash *bridge_mappings, > + struct sset *egress_ifaces, > + struct hmap *local_datapaths) > { > /* Ignore localnet ports for unplugged networks. */ > if (!is_network_plugged(binding_rec, bridge_mappings)) { > @@ -481,6 +471,27 @@ 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 *pb) > +{ > + char buf[16]; > + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, > + pb->datapath->tunnel_key, pb->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); Nit: indentation. > + sset_find_and_delete(local_lport_ids, buf); > +} > + > /* Local bindings. binding.c module binds the logical port (represented by > * Port_Binding rows) and sets the 'chassis' column when it sees the > * OVS interface row (of type "" or "internal") with the > @@ -520,6 +531,10 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, > * BT_VIF. Its Port_Binding row's 'parent' column is set to > * its parent's Port_Binding. It shares the OVS interface row > * with the parent. > + * Each ovn-controller when it sees a container Port_Binding, > + * it creates 'struct local_binding' for the parent > + * Port_Binding and for its even if the OVS interface row for > + * the parent is not present. > * > * BT_VIRTUAL: Represents a local binding which has a parent of type BT_VIF. > * Its Port_Binding type is "virtual" and it shares the OVS > @@ -527,6 +542,17 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, > * Port_Binding of type "virtual" is claimed by pinctrl module > * when it sees the ARP packet from the parent's VIF. > * > + * > + * An object of 'struct local_binding' is created: > + * - For each interface that has iface-id configured with the type - BT_VIF. > + * > + * - For each container Port Binding (of type BT_CONTAINER) and its > + * parent Port_Binding (of type BT_VIF), no matter if > + * they are bound to this chassis i.e even if OVS interface row for the > + * parent is not present. > + * > + * - For each 'virtual' Port Binding (of type BT_VIRTUAL) provided its parent > + * is bound to this chassis. > */ > enum local_binding_type { > BT_VIF, > @@ -598,6 +624,14 @@ local_bindings_destroy(struct shash *local_bindings) > shash_destroy(local_bindings); > } > > +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); > +} > + > static void > local_binding_add_child(struct local_binding *lbinding, > struct local_binding *child) > @@ -624,6 +658,7 @@ is_lport_container(const struct sbrec_port_binding *pb) > return is_lport_vif(pb) && pb->parent_port && pb->parent_port[0]; > } > > +/* Corresponds to each Port_Binding.type. */ Shouldn't this comment be part of patch 1 of the series? > enum en_lport_type { > LP_UNKNOWN, > LP_VIF, > @@ -669,12 +704,20 @@ get_lport_type(const struct sbrec_port_binding *pb) > return LP_UNKNOWN; > } > > -static void > +/* Returns false if lport is not claimed due to 'sb_readonly'. > + * Returns true otherwise. > + */ > +static bool > claim_lport(const struct sbrec_port_binding *pb, > const struct sbrec_chassis *chassis_rec, > - const struct ovsrec_interface *iface_rec) > + const struct ovsrec_interface *iface_rec, > + bool sb_readonly) > { > if (pb->chassis != chassis_rec) { > + if (sb_readonly) { > + return false; > + } > + > if (pb->chassis) { > VLOG_INFO("Changing chassis for lport %s from %s to %s.", > pb->logical_port, pb->chassis->name, > @@ -693,22 +736,44 @@ claim_lport(const struct sbrec_port_binding *pb, > struct sbrec_encap *encap_rec = > sbrec_get_port_encap(chassis_rec, iface_rec); > if (encap_rec && pb->encap != encap_rec) { > + if (sb_readonly) { > + return false; > + } > sbrec_port_binding_set_encap(pb, encap_rec); > } > + > + return true; > } > > -static void > -release_lport(const struct sbrec_port_binding *pb) > +/* Returns false if lport is not released due to 'sb_readonly'. > + * Returns true otherwise. > + */ > +static bool > +release_lport(const struct sbrec_port_binding *pb, bool sb_readonly) > { > - VLOG_INFO("Releasing lport %s from this chassis.", pb->logical_port); > if (pb->encap) { > + if (sb_readonly) { > + return false; > + } > sbrec_port_binding_set_encap(pb, NULL); > } > - sbrec_port_binding_set_chassis(pb, NULL); > + > + if (pb->chassis) { > + if (sb_readonly) { > + return false; > + } > + sbrec_port_binding_set_chassis(pb, NULL); > + } > > if (pb->virtual_parent) { > + if (sb_readonly) { > + return false; > + } > sbrec_port_binding_set_virtual_parent(pb, NULL); > } > + > + VLOG_INFO("Releasing lport %s from this chassis.", pb->logical_port); > + return true; > } > > static bool > @@ -733,7 +798,64 @@ can_bind_on_this_chassis(const struct sbrec_chassis *chassis_rec, > || !strcmp(requested_chassis, chassis_rec->hostname); > } > > -static void > +/* Returns 'true' if the 'lbinding' has children of type BT_CONTAINER, > + * 'false' otherwise. */ > +static bool > +is_lbinding_container_parent(struct local_binding *lbinding) > +{ > + struct shash_node *node; > + SHASH_FOR_EACH (node, &lbinding->children) { > + struct local_binding *l = node->data; > + if (l->type == BT_CONTAINER) { > + return true; > + } > + } > + > + return false; > +} > + > +static bool > +release_local_binding_children(const struct sbrec_chassis *chassis_rec, > + struct local_binding *lbinding, > + bool sb_readonly) > +{ > + struct shash_node *node; > + SHASH_FOR_EACH (node, &lbinding->children) { > + struct local_binding *l = node->data; > + if (is_lbinding_this_chassis(l, chassis_rec)) { > + if (!release_lport(l->pb, sb_readonly)) { > + return false; > + } > + } > + > + /* Clear the local bindings' 'pb' and 'iface'. */ > + l->pb = NULL; > + l->iface = NULL; > + } > + > + return true; > +} > + > +static bool > +release_local_binding(const struct sbrec_chassis *chassis_rec, > + struct local_binding *lbinding, bool sb_readonly) > +{ > + if (!release_local_binding_children(chassis_rec, lbinding, > + sb_readonly)) { > + return false; > + } > + > + bool retval = true; > + if (is_lbinding_this_chassis(lbinding, chassis_rec)) { > + retval = release_lport(lbinding->pb, sb_readonly); > + } > + > + lbinding->pb = NULL; > + lbinding->iface = NULL; > + return retval; > +} > + > +static bool > consider_vif_lport_(const struct sbrec_port_binding *pb, > bool can_bind, const char *vif_chassis, > struct binding_ctx_in *b_ctx_in, > @@ -745,7 +867,10 @@ consider_vif_lport_(const struct sbrec_port_binding *pb, > if (lbinding_set) { > if (can_bind) { > /* We can claim the lport. */ > - claim_lport(pb, b_ctx_in->chassis_rec, lbinding->iface); > + if (!claim_lport(pb, b_ctx_in->chassis_rec, lbinding->iface, > + !b_ctx_in->ovnsb_idl_txn)){ > + return false; > + } > > add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, > b_ctx_in->sbrec_port_binding_by_datapath, > @@ -771,13 +896,14 @@ consider_vif_lport_(const struct sbrec_port_binding *pb, > if (pb->chassis == b_ctx_in->chassis_rec) { > /* Release the lport if there is no lbinding. */ > if (!lbinding_set || !can_bind) { > - release_lport(pb); > + return release_lport(pb, !b_ctx_in->ovnsb_idl_txn); > } > } > > + return true; > } > > -static void > +static bool > consider_vif_lport(const struct sbrec_port_binding *pb, > struct binding_ctx_in *b_ctx_in, > struct binding_ctx_out *b_ctx_out, > @@ -797,11 +923,11 @@ consider_vif_lport(const struct sbrec_port_binding *pb, > lbinding->pb = pb; > } > > - consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, > - b_ctx_out, lbinding, qos_map); > + return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, > + b_ctx_out, lbinding, qos_map); > } > > -static void > +static bool > consider_container_lport(const struct sbrec_port_binding *pb, > struct binding_ctx_in *b_ctx_in, > struct binding_ctx_out *b_ctx_out, > @@ -811,7 +937,39 @@ consider_container_lport(const struct sbrec_port_binding *pb, > parent_lbinding = local_binding_find(b_ctx_out->local_bindings, > pb->parent_port); > > - if (parent_lbinding && !parent_lbinding->pb) { > + if (!parent_lbinding) { > + /* There is no local_binding for parent port. Create it > + * without OVS interface row. This is the only exception > + * for creating the 'struct local_binding' object without > + * corresponding OVS interface row. > + * > + * This is required for the following reasons: > + * - If a logical port P1 is created and then > + * few container ports - C1, C2, .. are created first by CMS. > + * - And later when OVS interface row is created for P1, then > + * we want the these container ports also be claimed by the > + * chassis. > + * */ > + parent_lbinding = local_binding_create(pb->parent_port, NULL, NULL, > + BT_VIF); > + local_binding_add(b_ctx_out->local_bindings, parent_lbinding); > + } > + > + struct local_binding *container_lbinding = > + local_binding_find_child(parent_lbinding, pb->logical_port); > + > + if (!container_lbinding) { > + container_lbinding = local_binding_create(pb->logical_port, > + parent_lbinding->iface, > + pb, BT_CONTAINER); > + local_binding_add_child(parent_lbinding, container_lbinding); > + } else { > + ovs_assert(container_lbinding->type == BT_CONTAINER); > + container_lbinding->pb = pb; > + container_lbinding->iface = parent_lbinding->iface; > + } > + > + if (!parent_lbinding->pb) { > parent_lbinding->pb = lport_lookup_by_name( > b_ctx_in->sbrec_port_binding_by_name, pb->parent_port); > > @@ -820,37 +978,28 @@ consider_container_lport(const struct sbrec_port_binding *pb, > * So call consider_vif_lport() to process it first. */ > consider_vif_lport(parent_lbinding->pb, b_ctx_in, b_ctx_out, > parent_lbinding, qos_map); > - } > - } > + } else { > + /* The parent lport doesn't exist. Call release_lport() to > + * release the container lport, if it was bound earlier. */ > + if (is_lbinding_this_chassis(container_lbinding, > + b_ctx_in->chassis_rec)) { > + return release_lport(pb, !b_ctx_in->ovnsb_idl_txn); > + } > > - if (!parent_lbinding || !parent_lbinding->pb) { > - /* Call release_lport, to release the container lport, if > - * it was bound earlier. */ > - if (pb->chassis == b_ctx_in->chassis_rec) { > - release_lport(pb); > + return true; > } > - return; > } > > - struct local_binding *container_lbinding = > - local_binding_find_child(parent_lbinding, pb->logical_port); > - ovs_assert(!container_lbinding); > - > - container_lbinding = local_binding_create(pb->logical_port, > - parent_lbinding->iface, > - pb, BT_CONTAINER); > - local_binding_add_child(parent_lbinding, container_lbinding); > - > const char *vif_chassis = smap_get(&parent_lbinding->pb->options, > "requested-chassis"); > bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, > vif_chassis); > > - consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, b_ctx_out, > - container_lbinding, qos_map); > + return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, b_ctx_out, > + container_lbinding, qos_map); > } > > -static void > +static bool > consider_virtual_lport(const struct sbrec_port_binding *pb, > struct binding_ctx_in *b_ctx_in, > struct binding_ctx_out *b_ctx_out, > @@ -873,15 +1022,25 @@ consider_virtual_lport(const struct sbrec_port_binding *pb, > } > } > > + /* Unlike container lports, we don't have to create parent_lbinding if > + * it is NULL. This is because, if parent_lbinding is not present, it > + * means the virtual port can't bind in this chassis. > + * Note: pinctrl module binds the virtual lport when it sees ARP > + * packet from the parent lport. */ > struct local_binding *virtual_lbinding = NULL; > if (is_lbinding_this_chassis(parent_lbinding, b_ctx_in->chassis_rec)) { > virtual_lbinding = > local_binding_find_child(parent_lbinding, pb->logical_port); > - ovs_assert(!virtual_lbinding); > - virtual_lbinding = local_binding_create(pb->logical_port, > - parent_lbinding->iface, > - pb, BT_VIRTUAL); > - local_binding_add_child(parent_lbinding, virtual_lbinding); > + if (!virtual_lbinding) { > + virtual_lbinding = local_binding_create(pb->logical_port, > + parent_lbinding->iface, > + pb, BT_VIRTUAL); > + local_binding_add_child(parent_lbinding, virtual_lbinding); > + } else { > + ovs_assert(virtual_lbinding->type == BT_VIRTUAL); > + virtual_lbinding->pb = pb; > + virtual_lbinding->iface = parent_lbinding->iface; > + } > } > > return consider_vif_lport_(pb, true, NULL, b_ctx_in, b_ctx_out, > @@ -891,14 +1050,13 @@ consider_virtual_lport(const struct sbrec_port_binding *pb, > /* Considers either claiming the lport or releasing the lport > * for non VIF lports. > */ > -static void > +static bool > consider_nonvif_lport_(const struct sbrec_port_binding *pb, > bool our_chassis, > bool has_local_l3gateway, > struct binding_ctx_in *b_ctx_in, > struct binding_ctx_out *b_ctx_out) > { > - ovs_assert(b_ctx_in->ovnsb_idl_txn); > if (our_chassis) { > sset_add(b_ctx_out->local_lports, pb->logical_port); > add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, > @@ -908,13 +1066,16 @@ consider_nonvif_lport_(const struct sbrec_port_binding *pb, > b_ctx_out->local_datapaths); > > update_local_lport_ids(b_ctx_out->local_lport_ids, pb); > - claim_lport(pb, b_ctx_in->chassis_rec, NULL); > + return claim_lport(pb, b_ctx_in->chassis_rec, NULL, > + !b_ctx_in->ovnsb_idl_txn); > } else if (pb->chassis == b_ctx_in->chassis_rec) { > - release_lport(pb); > + return release_lport(pb, !b_ctx_in->ovnsb_idl_txn); > } > + > + return true; > } > > -static void > +static bool > consider_l2gw_lport(const struct sbrec_port_binding *pb, > struct binding_ctx_in *b_ctx_in, > struct binding_ctx_out *b_ctx_out) > @@ -923,10 +1084,10 @@ consider_l2gw_lport(const struct sbrec_port_binding *pb, > bool our_chassis = chassis_id && !strcmp(chassis_id, > b_ctx_in->chassis_rec->name); > > - consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); > + return consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); > } > > -static void > +static bool > consider_l3gw_lport(const struct sbrec_port_binding *pb, > struct binding_ctx_in *b_ctx_in, > struct binding_ctx_out *b_ctx_out) > @@ -935,7 +1096,7 @@ consider_l3gw_lport(const struct sbrec_port_binding *pb, > bool our_chassis = chassis_id && !strcmp(chassis_id, > b_ctx_in->chassis_rec->name); > > - consider_nonvif_lport_(pb, our_chassis, true, b_ctx_in, b_ctx_out); > + return consider_nonvif_lport_(pb, our_chassis, true, b_ctx_in, b_ctx_out); > } > > static void > @@ -954,7 +1115,7 @@ consider_localnet_lport(const struct sbrec_port_binding *pb, > update_local_lport_ids(b_ctx_out->local_lport_ids, pb); > } > > -static void > +static bool > consider_ha_lport(const struct sbrec_port_binding *pb, > struct binding_ctx_in *b_ctx_in, > struct binding_ctx_out *b_ctx_out) > @@ -982,18 +1143,18 @@ consider_ha_lport(const struct sbrec_port_binding *pb, > b_ctx_out->local_datapaths); > } > > - consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); > + return consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); > } > > -static void > +static bool > consider_cr_lport(const struct sbrec_port_binding *pb, > struct binding_ctx_in *b_ctx_in, > struct binding_ctx_out *b_ctx_out) > { > - consider_ha_lport(pb, b_ctx_in, b_ctx_out); > + return consider_ha_lport(pb, b_ctx_in, b_ctx_out); > } > > -static void > +static bool > consider_external_lport(const struct sbrec_port_binding *pb, > struct binding_ctx_in *b_ctx_in, > struct binding_ctx_out *b_ctx_out) > @@ -1046,6 +1207,8 @@ build_local_bindings(struct binding_ctx_in *b_ctx_in, > } > > 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. */ > @@ -1161,9 +1324,9 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) > * corresponding local datapath accordingly. */ > struct localnet_lport *lnet_lport; > LIST_FOR_EACH_POP (lnet_lport, list_node, &localnet_lports) { > - consider_localnet_port(lnet_lport->pb, &bridge_mappings, > - b_ctx_out->egress_ifaces, > - b_ctx_out->local_datapaths); > + update_ld_localnet_port(lnet_lport->pb, &bridge_mappings, > + b_ctx_out->egress_ifaces, > + b_ctx_out->local_datapaths); > free(lnet_lport); > } > > @@ -1181,95 +1344,691 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) > destroy_qos_map(&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. */ > +/* Returns true if the database is all cleaned up, false if more work is > + * required. */ > bool > -binding_evaluate_port_binding_changes(struct binding_ctx_in *b_ctx_in, > - struct binding_ctx_out *b_ctx_out) > +binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, > + const struct sbrec_port_binding_table *port_binding_table, > + const struct sbrec_chassis *chassis_rec) > { > - if (!b_ctx_in->chassis_rec) { > + if (!ovnsb_idl_txn) { > + return false; > + } > + if (!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; > + bool any_changes = false; > + SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec, port_binding_table) { > + if (binding_rec->chassis == chassis_rec) { > + if (binding_rec->encap) { > + sbrec_port_binding_set_encap(binding_rec, NULL); > + } > + sbrec_port_binding_set_chassis(binding_rec, NULL); > + any_changes = true; > + } > + } > + > + if (any_changes) { > + ovsdb_idl_txn_add_comment( > + ovnsb_idl_txn, > + "ovn-controller: removing all port bindings for '%s'", > + chassis_rec->name); > + } > + > + return !any_changes; > +} > + > +/* This function adds the local datapath of the 'peer' of > + * lport 'pb' to the local datapaths if it is not yet added. > + */ > +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) > +{ > + 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; > + } > + > + bool present = false; > + for (size_t i = 0; i < ld->n_peer_ports; i++) { > + if (ld->peer_ports[i].local == pb) { > + present = true; > break; > } > + } > > - if (!strcmp(binding_rec->type, "remote")) { > - continue; > + 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; > + } > > - if (strcmp(binding_rec->type, "")) { > - changed = true; > + 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; > > - struct local_binding *lbinding = NULL; > - if (!binding_rec->parent_port || !binding_rec->parent_port[0]) { > - lbinding = local_binding_find(b_ctx_out->local_bindings, > - binding_rec->logical_port); > + /* Possible improvement: We can shrint the allocated peer ports Typo: s/shrint/shrink. > + * if (ld->n_peer_ports < ld->n_allocated_peer_ports / 2). > + */ > + 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 the peer port from the peer datapath. The peer > + * datapath also tries to remove its peer lport, but that would > + * be no-op. */ > + remove_local_datapath_peer_port(peer, peer_ld, local_datapaths); > + } > +} > + > +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; > + } > + } > +} > + > +/* Considers the ovs iface 'iface_rec' for claiming. > + * This function should be called if the external_ids:iface-id > + * and 'ofport' are set for the 'iface_rec'. > + * > + * If the local_binding for this 'iface_rec' already exists and its > + * already claimed, then this function will be no-op. > + */ > +static bool > +consider_iface_claim(const struct ovsrec_interface *iface_rec, > + const char *iface_id, > + struct binding_ctx_in *b_ctx_in, > + struct binding_ctx_out *b_ctx_out, > + struct hmap *qos_map) > +{ > + sset_add(b_ctx_out->local_lports, iface_id); > + smap_replace(b_ctx_out->local_iface_ids, iface_rec->name, iface_id); > + > + struct local_binding *lbinding = > + local_binding_find(b_ctx_out->local_bindings, iface_id); > + > + if (!lbinding) { > + lbinding = local_binding_create(iface_id, iface_rec, NULL, BT_VIF); > + local_binding_add(b_ctx_out->local_bindings, lbinding); > + } else { > + lbinding->iface = iface_rec; > + } > + > + if (!lbinding->pb || strcmp(lbinding->name, lbinding->pb->logical_port)) { > + lbinding->pb = lport_lookup_by_name( > + b_ctx_in->sbrec_port_binding_by_name, lbinding->name); > + if (lbinding->pb && !strcmp(lbinding->pb->type, "virtual")) { > + lbinding->pb = NULL; > + } > + } > + > + if (lbinding->pb) { > + if (!consider_vif_lport(lbinding->pb, b_ctx_in, b_ctx_out, > + lbinding, qos_map)) { > + return false; > + } > + } > + > + /* Update the child local_binding's iface (if any children) and try to > + * claim the container lbindings. */ > + struct shash_node *node; > + SHASH_FOR_EACH (node, &lbinding->children) { > + struct local_binding *child = node->data; > + child->iface = iface_rec; > + if (child->type == BT_CONTAINER) { > + if (!consider_container_lport(child->pb, b_ctx_in, b_ctx_out, > + qos_map)) { > + return false; > + } > + } > + } > + > + return true; > +} > + > +/* Considers the ovs interface 'iface_rec' for > + * releasing from this chassis if local_binding for this > + * 'iface_rec' (with 'iface_id' as key) already exists and > + * it is claimed by the chassis. > + * > + * The 'iface_id' could be cleared from the 'iface_rec' > + * and hence it is passed separately. > + * > + * This fuction should be called if > + * - OVS interface 'iface_rec' is deleted. > + * - OVS interface 'iface_rec' external_ids:iface-id is updated > + * (with the old value being 'iface_id'.) > + * - OVS interface ofport is reset to 0. > + * */ > +static bool > +consider_iface_release(const struct ovsrec_interface *iface_rec, > + const char *iface_id, > + struct binding_ctx_in *b_ctx_in, > + struct binding_ctx_out *b_ctx_out, bool *changed) > +{ > + struct local_binding *lbinding; > + lbinding = local_binding_find(b_ctx_out->local_bindings, > + iface_id); > + if (is_lbinding_this_chassis(lbinding, b_ctx_in->chassis_rec)) { > + 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); > + } > + > + /* Note: release_local_binding() resets lbinding->pb and > + * lbinding->iface. > + * Cannot access these members of lbinding after this call. */ > + if (!release_local_binding(b_ctx_in->chassis_rec, lbinding, > + !b_ctx_in->ovnsb_idl_txn)) { > + return false; > + } > + > + *changed = true; > + } > + > + /* Check if the lbinding has children of type PB_CONTAINER. > + * If so, don't delete the local_binding. */ Nit: indentation. > + if (lbinding && !is_lbinding_container_parent(lbinding)) { > + local_binding_delete(b_ctx_out->local_bindings, lbinding); > + } > + > + sset_find_and_delete(b_ctx_out->local_lports, iface_id); > + smap_remove(b_ctx_out->local_iface_ids, iface_rec->name); > + > + return true; > +} > + > +static bool > +is_iface_vif(const struct ovsrec_interface *iface_rec) > +{ > + if (iface_rec->type && iface_rec->type[0] && > + strcmp(iface_rec->type, "internal")) { > + return false; > + } > + > + return true; > +} > + > +/* 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, > + bool *changed) > +{ > + if (!b_ctx_in->chassis_rec) { > + return false; > + } > + > + bool handled = true; > + *changed = false; > + > + /* Run the tracked interfaces loop twice. One to handle deleted > + * changes. And another to handle add/update changes. > + * This will ensure correctness. > + * * > + * We consider an OVS interface for release if one of the following > + * happens: > + * 1. OVS interface is deleted. > + * 2. external_ids:iface-id is cleared in which case we need to > + * release the port binding corresponding to the previously set > + * 'old-iface-id' (which is stored in the smap > + * 'b_ctx_out->local_iface_ids'). > + * 3. external_ids:iface-id is updated with a different value > + * in which case we need to release the port binding corresponding > + * to the previously set 'old-iface-id' (which is stored in the smap > + * 'b_ctx_out->local_iface_ids'). > + * 4. ofport of the OVS interface is 0. > + * > + */ > + const struct ovsrec_interface *iface_rec; > + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, > + b_ctx_in->iface_table) { > + if (!is_iface_vif(iface_rec)) { > + /* Right now are not handling ovs_interface changes of > + * other types. This can be enhanced to handle of > + * types - patch and tunnel. */ > + handled = false; > + break; > + } > + > + 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); > + const char *cleared_iface_id = NULL; > + if (!ovsrec_interface_is_deleted(iface_rec)) { > + int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; > + 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 (!ofport) { > + /* If ofport is 0, we need to release the iface if already > + * claimed. */ > + cleared_iface_id = iface_id; > + } > + } else if (old_iface_id) { > + cleared_iface_id = old_iface_id; > + } > } else { > - lbinding = local_binding_find(b_ctx_out->local_bindings, > - binding_rec->parent_port); > + cleared_iface_id = iface_id; > } > > - if (lbinding) { > - changed = true; > + if (cleared_iface_id) { > + handled = consider_iface_release(iface_rec, cleared_iface_id, > + b_ctx_in, b_ctx_out, changed); > + } > + > + if (!handled) { > break; > } > } > > - return changed; > + if (!handled) { > + /* This can happen if any non vif OVS interface is in the tracked > + * list or if consider_iface_release() returned false. > + * There is no need to process further. */ > + return 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; > + > + /* > + * We consider an OVS interface for claiming if the following > + * 2 conditions are met: > + * 1. external_ids:iface-id is set. > + * 2. ofport of the OVS interface is > 0. > + * > + * So when an update of an OVS interface happens we see if these > + * conditions are still true. If so consider this interface for > + * claiming. This would be no-op if the update of the OVS interface > + * didn't change the above two fields. > + */ > + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, > + b_ctx_in->iface_table) { > + /* Loop to handle create and update changes only. */ > + if (ovsrec_interface_is_deleted(iface_rec)) { > + continue; > + } > + > + const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id"); > + int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; > + if (iface_id && ofport > 0) { > + *changed = true; > + handled = consider_iface_claim(iface_rec, iface_id, b_ctx_in, > + b_ctx_out, qos_map_ptr); > + if (!handled) { > + break; > + } > + } > + } > + > + if (handled && 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); > + } > + } > + > + destroy_qos_map(&qos_map); > + return handled; > } > > -/* Returns true if the database is all cleaned up, false if more work is > - * required. */ > -bool > -binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, > - const struct sbrec_port_binding_table *port_binding_table, > - const struct sbrec_chassis *chassis_rec) > +static void > +handle_deleted_lport(const struct sbrec_port_binding *pb, > + struct binding_ctx_in *b_ctx_in, > + struct binding_ctx_out *b_ctx_out) > { > - if (!ovnsb_idl_txn) { > + 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); > + } > +} > + > +static struct local_binding * > +get_lbinding_for_lport(const struct sbrec_port_binding *pb, > + enum en_lport_type lport_type, > + struct binding_ctx_out *b_ctx_out) > +{ > + ovs_assert(lport_type == LP_VIF || lport_type == LP_VIRTUAL); > + > + if (lport_type == LP_VIF && !is_lport_container(pb)) { > + return local_binding_find(b_ctx_out->local_bindings, pb->logical_port); > + } > + > + struct local_binding *parent_lbinding = NULL; > + > + if (lport_type == LP_VIRTUAL) { > + parent_lbinding = local_binding_find(b_ctx_out->local_bindings, > + pb->virtual_parent); > + } else { > + parent_lbinding = local_binding_find(b_ctx_out->local_bindings, > + pb->parent_port); > + } > + > + return parent_lbinding > + ? local_binding_find(&parent_lbinding->children, pb->logical_port) > + : NULL; > +} > + > +static bool > +handle_deleted_vif_lport(const struct sbrec_port_binding *pb, > + enum en_lport_type lport_type, > + struct binding_ctx_in *b_ctx_in, > + struct binding_ctx_out *b_ctx_out, > + bool *changed) > +{ > + struct local_binding *lbinding = > + get_lbinding_for_lport(pb, lport_type, b_ctx_out); > + > + if (lbinding) { > + lbinding->pb = NULL; > + /* The port_binding 'pb' is deleted. So there is no need to > + * clear the 'chassis' column of 'pb'. But we need to do > + * for the local_binding's children. */ > + if (lbinding->type == BT_VIF && > + !release_local_binding_children(b_ctx_in->chassis_rec, > + lbinding, > + !b_ctx_in->ovnsb_idl_txn)) { > + return false; > + } > + > + *changed = true; > + } > + > + handle_deleted_lport(pb, b_ctx_in, b_ctx_out); > + return true; > +} > + > +static bool > +handle_updated_vif_lport(const struct sbrec_port_binding *pb, > + enum en_lport_type lport_type, > + struct binding_ctx_in *b_ctx_in, > + struct binding_ctx_out *b_ctx_out, > + struct hmap *qos_map, > + bool *changed) > +{ > + bool claimed = (pb->chassis == b_ctx_in->chassis_rec); > + bool handled = true; > + > + if (lport_type == LP_VIRTUAL) { > + handled = consider_virtual_lport(pb, b_ctx_in, b_ctx_out, qos_map); > + } else if (lport_type == LP_VIF && is_lport_container(pb)) { > + handled = consider_container_lport(pb, b_ctx_in, b_ctx_out, qos_map); > + } else { > + handled = consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL, qos_map); > + } > + > + if (!handled) { > return false; > } > - if (!chassis_rec) { > + > + bool now_claimed = (pb->chassis == b_ctx_in->chassis_rec); > + bool changed_ = (claimed != now_claimed); > + > + if (changed_) { > + *changed = true; > + } > + > + if (lport_type == LP_VIRTUAL || > + (lport_type == LP_VIF && is_lport_container(pb)) || !changed_) { > return true; > } > > - const struct sbrec_port_binding *binding_rec; > - bool any_changes = false; > - SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec, port_binding_table) { > - if (binding_rec->chassis == chassis_rec) { > - if (binding_rec->encap) > - sbrec_port_binding_set_encap(binding_rec, NULL); > - sbrec_port_binding_set_chassis(binding_rec, NULL); > - any_changes = true; > + struct local_binding *lbinding = > + local_binding_find(b_ctx_out->local_bindings, pb->logical_port); > + > + ovs_assert(lbinding); > + > + struct shash_node *node; > + SHASH_FOR_EACH (node, &lbinding->children) { > + struct local_binding *child = node->data; > + if (child->type == BT_CONTAINER) { > + handled = consider_container_lport(child->pb, b_ctx_in, b_ctx_out, > + qos_map); > + if (!handled) { > + return false; > + } > } > } > > - if (any_changes) { > - ovsdb_idl_txn_add_comment( > - ovnsb_idl_txn, > - "ovn-controller: removing all port bindings for '%s'", > - chassis_rec->name); > + return true; > +} > + > +/* 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 *changed) > +{ > + bool handled = true; > + *changed = false; > + > + const struct sbrec_port_binding *pb; > + > + /* Run the tracked port binding loop twice. One to handle deleted > + * changes. And another to handle add/update changes. > + * This will ensure correctness. > + */ > + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, > + b_ctx_in->port_binding_table) { > + if (!sbrec_port_binding_is_deleted(pb)) { > + continue; > + } > + > + enum en_lport_type lport_type = get_lport_type(pb); > + if (lport_type == LP_VIF || lport_type == LP_VIRTUAL) { > + handled = handle_deleted_vif_lport(pb, lport_type, b_ctx_in, > + b_ctx_out, changed); > + } else { > + handle_deleted_lport(pb, b_ctx_in, b_ctx_out); > + } > + > + if (!handled) { > + break; > + } > } > > - return !any_changes; > + if (!handled) { > + return 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; > + > + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, > + b_ctx_in->port_binding_table) { > + /* Loop to handle create and update changes only. */ > + if (sbrec_port_binding_is_deleted(pb)) { > + continue; > + } > + > + enum en_lport_type lport_type = get_lport_type(pb); > + > + struct local_datapath *ld = > + get_local_datapath(b_ctx_out->local_datapaths, > + pb->datapath->tunnel_key); > + > + switch (lport_type) { > + case LP_VIF: > + case LP_VIRTUAL: > + handled = handle_updated_vif_lport(pb, lport_type, b_ctx_in, > + b_ctx_out, qos_map_ptr, > + changed); > + break; > + > + case LP_PATCH: > + case LP_LOCALPORT: > + case LP_VTEP: > + *changed = true; > + update_local_lport_ids(b_ctx_out->local_lport_ids, pb); > + if (lport_type == LP_PATCH) { > + /* Add the peer datapath to the local datapaths if it's > + * not present yet. > + */ > + if (ld) { > + add_local_datapath_peer_port(pb, b_ctx_in, b_ctx_out, ld); > + } > + } > + break; > + > + case LP_L2GATEWAY: > + *changed = true; > + handled = consider_l2gw_lport(pb, b_ctx_in, b_ctx_out); > + break; > + > + case LP_L3GATEWAY: > + *changed = true; > + handled = consider_l3gw_lport(pb, b_ctx_in, b_ctx_out); > + break; > + > + case LP_CHASSISREDIRECT: > + *changed = true; > + handled = consider_cr_lport(pb, b_ctx_in, b_ctx_out); > + break; > + > + case LP_EXTERNAL: > + *changed = true; > + handled = consider_external_lport(pb, b_ctx_in, b_ctx_out); > + break; > + > + case LP_LOCALNET: { > + *changed = true; > + consider_localnet_lport(pb, b_ctx_in, b_ctx_out, qos_map_ptr); > + > + struct shash bridge_mappings = > + SHASH_INITIALIZER(&bridge_mappings); > + add_ovs_bridge_mappings(b_ctx_in->ovs_table, > + b_ctx_in->bridge_table, > + &bridge_mappings); > + update_ld_localnet_port(pb, &bridge_mappings, > + b_ctx_out->egress_ifaces, > + b_ctx_out->local_datapaths); > + shash_destroy(&bridge_mappings); > + break; > + } > + > + case LP_REMOTE: > + case LP_UNKNOWN: > + break; > + } > + > + if (!handled) { > + break; > + } > + } > + > + if (handled && 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); > + } > + } > + > + destroy_qos_map(&qos_map); > + return handled; > } > diff --git a/controller/binding.h b/controller/binding.h > index 9affc9a96..f7ace6faf 100644 > --- a/controller/binding.h > +++ b/controller/binding.h > @@ -57,6 +57,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 *); > @@ -64,9 +65,13 @@ 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_init(struct shash *local_bindings); > void local_bindings_destroy(struct shash *local_bindings); > +bool binding_handle_ovs_interface_changes(struct binding_ctx_in *, > + struct binding_ctx_out *, > + bool *changed); > +bool binding_handle_port_binding_changes(struct binding_ctx_in *, > + struct binding_ctx_out *, > + bool *changed); > #endif /* controller/binding.h */ > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 61d34157a..871291874 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -752,6 +752,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 { > @@ -973,6 +974,7 @@ struct ed_type_runtime_data { > struct sset active_tunnels; > > struct sset egress_ifaces; > + struct smap local_iface_ids; > }; > > static void * > @@ -986,6 +988,7 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED, > sset_init(&data->local_lport_ids); > sset_init(&data->active_tunnels); > sset_init(&data->egress_ifaces); > + smap_init(&data->local_iface_ids); > local_bindings_init(&data->local_bindings); > return data; > } > @@ -999,6 +1002,7 @@ en_runtime_data_cleanup(void *data) > sset_destroy(&rt_data->local_lport_ids); > sset_destroy(&rt_data->active_tunnels); > sset_destroy(&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) { > @@ -1040,6 +1044,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)); > @@ -1069,6 +1077,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; > @@ -1082,6 +1091,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); > local_bindings_init(&rt_data->local_bindings); > } > > @@ -1139,6 +1151,34 @@ en_runtime_data_run(struct engine_node *node, void *data) > engine_set_node_state(node, EN_UPDATED); > } > > +static bool > +runtime_data_ovs_interface_handler(struct engine_node *node, void *data) > +{ > + 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 = false; > + if (!binding_handle_ovs_interface_changes(&b_ctx_in, &b_ctx_out, > + &changed)) { > + return false; > + } > + > + if (changed) { > + engine_set_node_state(node, EN_UPDATED); > + } > + > + return true; > +} > + > +static bool > +runtime_data_noop_handler(struct engine_node *node OVS_UNUSED, > + void *data OVS_UNUSED) > +{ > + return true; > +} > + Nit: should we move this to inc-proc-eng.c and rename it "engine_noop_handler". There's nothing runtime_data specific in it and maybe in the future other input nodes need a noop change handler too. > static bool > runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) > { > @@ -1146,11 +1186,21 @@ runtime_data_sb_port_binding_handler(struct engine_node *node, void *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 changed = false; > + if (!binding_handle_port_binding_changes(&b_ctx_in, &b_ctx_out, > + &changed)) { > + return false; > + } > > - bool changed = binding_evaluate_port_binding_changes(&b_ctx_in, > - &b_ctx_out); > + if (changed) { > + engine_set_node_state(node, EN_UPDATED); > + } > > - return !changed; > + return true; > } > > /* Connection tracking zones. */ > @@ -1894,7 +1944,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); > diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at > index a8a15f8fe..5dfc6f7ca 100644 > --- a/tests/ovn-performance.at > +++ b/tests/ovn-performance.at > @@ -239,6 +239,19 @@ for i in 1 2; do > ovn_attach n1 br-phys 192.168.0.$i > done > > +# Wait for the tunnel ports to be created and up. > +# Otherwise this may affect the lflow_run count. > + > +OVS_WAIT_UNTIL([ > + test $(as hv1 ovs-vsctl list interface ovn-hv2-0 | \ > +grep tunnel_egress_iface_carrier=up | wc -l) -eq 1 > +]) > + > +OVS_WAIT_UNTIL([ > + test $(as hv2 ovs-vsctl list interface ovn-hv1-0 | \ > +grep tunnel_egress_iface_carrier=up | wc -l) -eq 1 > +]) > + > # Add router lr1 > OVN_CONTROLLER_EXPECT_HIT( > [hv1 hv2], [lflow_run], >
diff --git a/controller/binding.c b/controller/binding.c index 6a13d1a0e..71063fe9a 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -360,16 +360,6 @@ destroy_qos_map(struct hmap *qos_map) hmap_destroy(qos_map); } -static void -update_local_lport_ids(struct sset *local_lport_ids, - const struct sbrec_port_binding *pb) -{ - char buf[16]; - snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, - pb->datapath->tunnel_key, pb->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 @@ -448,10 +438,10 @@ is_network_plugged(const struct sbrec_port_binding *binding_rec, } static void -consider_localnet_port(const struct sbrec_port_binding *binding_rec, - struct shash *bridge_mappings, - struct sset *egress_ifaces, - struct hmap *local_datapaths) +update_ld_localnet_port(const struct sbrec_port_binding *binding_rec, + struct shash *bridge_mappings, + struct sset *egress_ifaces, + struct hmap *local_datapaths) { /* Ignore localnet ports for unplugged networks. */ if (!is_network_plugged(binding_rec, bridge_mappings)) { @@ -481,6 +471,27 @@ 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 *pb) +{ + char buf[16]; + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, + pb->datapath->tunnel_key, pb->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); +} + /* Local bindings. binding.c module binds the logical port (represented by * Port_Binding rows) and sets the 'chassis' column when it sees the * OVS interface row (of type "" or "internal") with the @@ -520,6 +531,10 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, * BT_VIF. Its Port_Binding row's 'parent' column is set to * its parent's Port_Binding. It shares the OVS interface row * with the parent. + * Each ovn-controller when it sees a container Port_Binding, + * it creates 'struct local_binding' for the parent + * Port_Binding and for its even if the OVS interface row for + * the parent is not present. * * BT_VIRTUAL: Represents a local binding which has a parent of type BT_VIF. * Its Port_Binding type is "virtual" and it shares the OVS @@ -527,6 +542,17 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, * Port_Binding of type "virtual" is claimed by pinctrl module * when it sees the ARP packet from the parent's VIF. * + * + * An object of 'struct local_binding' is created: + * - For each interface that has iface-id configured with the type - BT_VIF. + * + * - For each container Port Binding (of type BT_CONTAINER) and its + * parent Port_Binding (of type BT_VIF), no matter if + * they are bound to this chassis i.e even if OVS interface row for the + * parent is not present. + * + * - For each 'virtual' Port Binding (of type BT_VIRTUAL) provided its parent + * is bound to this chassis. */ enum local_binding_type { BT_VIF, @@ -598,6 +624,14 @@ local_bindings_destroy(struct shash *local_bindings) shash_destroy(local_bindings); } +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); +} + static void local_binding_add_child(struct local_binding *lbinding, struct local_binding *child) @@ -624,6 +658,7 @@ is_lport_container(const struct sbrec_port_binding *pb) return is_lport_vif(pb) && pb->parent_port && pb->parent_port[0]; } +/* Corresponds to each Port_Binding.type. */ enum en_lport_type { LP_UNKNOWN, LP_VIF, @@ -669,12 +704,20 @@ get_lport_type(const struct sbrec_port_binding *pb) return LP_UNKNOWN; } -static void +/* Returns false if lport is not claimed due to 'sb_readonly'. + * Returns true otherwise. + */ +static bool claim_lport(const struct sbrec_port_binding *pb, const struct sbrec_chassis *chassis_rec, - const struct ovsrec_interface *iface_rec) + const struct ovsrec_interface *iface_rec, + bool sb_readonly) { if (pb->chassis != chassis_rec) { + if (sb_readonly) { + return false; + } + if (pb->chassis) { VLOG_INFO("Changing chassis for lport %s from %s to %s.", pb->logical_port, pb->chassis->name, @@ -693,22 +736,44 @@ claim_lport(const struct sbrec_port_binding *pb, struct sbrec_encap *encap_rec = sbrec_get_port_encap(chassis_rec, iface_rec); if (encap_rec && pb->encap != encap_rec) { + if (sb_readonly) { + return false; + } sbrec_port_binding_set_encap(pb, encap_rec); } + + return true; } -static void -release_lport(const struct sbrec_port_binding *pb) +/* Returns false if lport is not released due to 'sb_readonly'. + * Returns true otherwise. + */ +static bool +release_lport(const struct sbrec_port_binding *pb, bool sb_readonly) { - VLOG_INFO("Releasing lport %s from this chassis.", pb->logical_port); if (pb->encap) { + if (sb_readonly) { + return false; + } sbrec_port_binding_set_encap(pb, NULL); } - sbrec_port_binding_set_chassis(pb, NULL); + + if (pb->chassis) { + if (sb_readonly) { + return false; + } + sbrec_port_binding_set_chassis(pb, NULL); + } if (pb->virtual_parent) { + if (sb_readonly) { + return false; + } sbrec_port_binding_set_virtual_parent(pb, NULL); } + + VLOG_INFO("Releasing lport %s from this chassis.", pb->logical_port); + return true; } static bool @@ -733,7 +798,64 @@ can_bind_on_this_chassis(const struct sbrec_chassis *chassis_rec, || !strcmp(requested_chassis, chassis_rec->hostname); } -static void +/* Returns 'true' if the 'lbinding' has children of type BT_CONTAINER, + * 'false' otherwise. */ +static bool +is_lbinding_container_parent(struct local_binding *lbinding) +{ + struct shash_node *node; + SHASH_FOR_EACH (node, &lbinding->children) { + struct local_binding *l = node->data; + if (l->type == BT_CONTAINER) { + return true; + } + } + + return false; +} + +static bool +release_local_binding_children(const struct sbrec_chassis *chassis_rec, + struct local_binding *lbinding, + bool sb_readonly) +{ + struct shash_node *node; + SHASH_FOR_EACH (node, &lbinding->children) { + struct local_binding *l = node->data; + if (is_lbinding_this_chassis(l, chassis_rec)) { + if (!release_lport(l->pb, sb_readonly)) { + return false; + } + } + + /* Clear the local bindings' 'pb' and 'iface'. */ + l->pb = NULL; + l->iface = NULL; + } + + return true; +} + +static bool +release_local_binding(const struct sbrec_chassis *chassis_rec, + struct local_binding *lbinding, bool sb_readonly) +{ + if (!release_local_binding_children(chassis_rec, lbinding, + sb_readonly)) { + return false; + } + + bool retval = true; + if (is_lbinding_this_chassis(lbinding, chassis_rec)) { + retval = release_lport(lbinding->pb, sb_readonly); + } + + lbinding->pb = NULL; + lbinding->iface = NULL; + return retval; +} + +static bool consider_vif_lport_(const struct sbrec_port_binding *pb, bool can_bind, const char *vif_chassis, struct binding_ctx_in *b_ctx_in, @@ -745,7 +867,10 @@ consider_vif_lport_(const struct sbrec_port_binding *pb, if (lbinding_set) { if (can_bind) { /* We can claim the lport. */ - claim_lport(pb, b_ctx_in->chassis_rec, lbinding->iface); + if (!claim_lport(pb, b_ctx_in->chassis_rec, lbinding->iface, + !b_ctx_in->ovnsb_idl_txn)){ + return false; + } add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, b_ctx_in->sbrec_port_binding_by_datapath, @@ -771,13 +896,14 @@ consider_vif_lport_(const struct sbrec_port_binding *pb, if (pb->chassis == b_ctx_in->chassis_rec) { /* Release the lport if there is no lbinding. */ if (!lbinding_set || !can_bind) { - release_lport(pb); + return release_lport(pb, !b_ctx_in->ovnsb_idl_txn); } } + return true; } -static void +static bool consider_vif_lport(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out, @@ -797,11 +923,11 @@ consider_vif_lport(const struct sbrec_port_binding *pb, lbinding->pb = pb; } - consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, - b_ctx_out, lbinding, qos_map); + return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, + b_ctx_out, lbinding, qos_map); } -static void +static bool consider_container_lport(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out, @@ -811,7 +937,39 @@ consider_container_lport(const struct sbrec_port_binding *pb, parent_lbinding = local_binding_find(b_ctx_out->local_bindings, pb->parent_port); - if (parent_lbinding && !parent_lbinding->pb) { + if (!parent_lbinding) { + /* There is no local_binding for parent port. Create it + * without OVS interface row. This is the only exception + * for creating the 'struct local_binding' object without + * corresponding OVS interface row. + * + * This is required for the following reasons: + * - If a logical port P1 is created and then + * few container ports - C1, C2, .. are created first by CMS. + * - And later when OVS interface row is created for P1, then + * we want the these container ports also be claimed by the + * chassis. + * */ + parent_lbinding = local_binding_create(pb->parent_port, NULL, NULL, + BT_VIF); + local_binding_add(b_ctx_out->local_bindings, parent_lbinding); + } + + struct local_binding *container_lbinding = + local_binding_find_child(parent_lbinding, pb->logical_port); + + if (!container_lbinding) { + container_lbinding = local_binding_create(pb->logical_port, + parent_lbinding->iface, + pb, BT_CONTAINER); + local_binding_add_child(parent_lbinding, container_lbinding); + } else { + ovs_assert(container_lbinding->type == BT_CONTAINER); + container_lbinding->pb = pb; + container_lbinding->iface = parent_lbinding->iface; + } + + if (!parent_lbinding->pb) { parent_lbinding->pb = lport_lookup_by_name( b_ctx_in->sbrec_port_binding_by_name, pb->parent_port); @@ -820,37 +978,28 @@ consider_container_lport(const struct sbrec_port_binding *pb, * So call consider_vif_lport() to process it first. */ consider_vif_lport(parent_lbinding->pb, b_ctx_in, b_ctx_out, parent_lbinding, qos_map); - } - } + } else { + /* The parent lport doesn't exist. Call release_lport() to + * release the container lport, if it was bound earlier. */ + if (is_lbinding_this_chassis(container_lbinding, + b_ctx_in->chassis_rec)) { + return release_lport(pb, !b_ctx_in->ovnsb_idl_txn); + } - if (!parent_lbinding || !parent_lbinding->pb) { - /* Call release_lport, to release the container lport, if - * it was bound earlier. */ - if (pb->chassis == b_ctx_in->chassis_rec) { - release_lport(pb); + return true; } - return; } - struct local_binding *container_lbinding = - local_binding_find_child(parent_lbinding, pb->logical_port); - ovs_assert(!container_lbinding); - - container_lbinding = local_binding_create(pb->logical_port, - parent_lbinding->iface, - pb, BT_CONTAINER); - local_binding_add_child(parent_lbinding, container_lbinding); - const char *vif_chassis = smap_get(&parent_lbinding->pb->options, "requested-chassis"); bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, vif_chassis); - consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, b_ctx_out, - container_lbinding, qos_map); + return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, b_ctx_out, + container_lbinding, qos_map); } -static void +static bool consider_virtual_lport(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out, @@ -873,15 +1022,25 @@ consider_virtual_lport(const struct sbrec_port_binding *pb, } } + /* Unlike container lports, we don't have to create parent_lbinding if + * it is NULL. This is because, if parent_lbinding is not present, it + * means the virtual port can't bind in this chassis. + * Note: pinctrl module binds the virtual lport when it sees ARP + * packet from the parent lport. */ struct local_binding *virtual_lbinding = NULL; if (is_lbinding_this_chassis(parent_lbinding, b_ctx_in->chassis_rec)) { virtual_lbinding = local_binding_find_child(parent_lbinding, pb->logical_port); - ovs_assert(!virtual_lbinding); - virtual_lbinding = local_binding_create(pb->logical_port, - parent_lbinding->iface, - pb, BT_VIRTUAL); - local_binding_add_child(parent_lbinding, virtual_lbinding); + if (!virtual_lbinding) { + virtual_lbinding = local_binding_create(pb->logical_port, + parent_lbinding->iface, + pb, BT_VIRTUAL); + local_binding_add_child(parent_lbinding, virtual_lbinding); + } else { + ovs_assert(virtual_lbinding->type == BT_VIRTUAL); + virtual_lbinding->pb = pb; + virtual_lbinding->iface = parent_lbinding->iface; + } } return consider_vif_lport_(pb, true, NULL, b_ctx_in, b_ctx_out, @@ -891,14 +1050,13 @@ consider_virtual_lport(const struct sbrec_port_binding *pb, /* Considers either claiming the lport or releasing the lport * for non VIF lports. */ -static void +static bool consider_nonvif_lport_(const struct sbrec_port_binding *pb, bool our_chassis, bool has_local_l3gateway, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) { - ovs_assert(b_ctx_in->ovnsb_idl_txn); if (our_chassis) { sset_add(b_ctx_out->local_lports, pb->logical_port); add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, @@ -908,13 +1066,16 @@ consider_nonvif_lport_(const struct sbrec_port_binding *pb, b_ctx_out->local_datapaths); update_local_lport_ids(b_ctx_out->local_lport_ids, pb); - claim_lport(pb, b_ctx_in->chassis_rec, NULL); + return claim_lport(pb, b_ctx_in->chassis_rec, NULL, + !b_ctx_in->ovnsb_idl_txn); } else if (pb->chassis == b_ctx_in->chassis_rec) { - release_lport(pb); + return release_lport(pb, !b_ctx_in->ovnsb_idl_txn); } + + return true; } -static void +static bool consider_l2gw_lport(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) @@ -923,10 +1084,10 @@ consider_l2gw_lport(const struct sbrec_port_binding *pb, bool our_chassis = chassis_id && !strcmp(chassis_id, b_ctx_in->chassis_rec->name); - consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); + return consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); } -static void +static bool consider_l3gw_lport(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) @@ -935,7 +1096,7 @@ consider_l3gw_lport(const struct sbrec_port_binding *pb, bool our_chassis = chassis_id && !strcmp(chassis_id, b_ctx_in->chassis_rec->name); - consider_nonvif_lport_(pb, our_chassis, true, b_ctx_in, b_ctx_out); + return consider_nonvif_lport_(pb, our_chassis, true, b_ctx_in, b_ctx_out); } static void @@ -954,7 +1115,7 @@ consider_localnet_lport(const struct sbrec_port_binding *pb, update_local_lport_ids(b_ctx_out->local_lport_ids, pb); } -static void +static bool consider_ha_lport(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) @@ -982,18 +1143,18 @@ consider_ha_lport(const struct sbrec_port_binding *pb, b_ctx_out->local_datapaths); } - consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); + return consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); } -static void +static bool consider_cr_lport(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) { - consider_ha_lport(pb, b_ctx_in, b_ctx_out); + return consider_ha_lport(pb, b_ctx_in, b_ctx_out); } -static void +static bool consider_external_lport(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) @@ -1046,6 +1207,8 @@ build_local_bindings(struct binding_ctx_in *b_ctx_in, } 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. */ @@ -1161,9 +1324,9 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) * corresponding local datapath accordingly. */ struct localnet_lport *lnet_lport; LIST_FOR_EACH_POP (lnet_lport, list_node, &localnet_lports) { - consider_localnet_port(lnet_lport->pb, &bridge_mappings, - b_ctx_out->egress_ifaces, - b_ctx_out->local_datapaths); + update_ld_localnet_port(lnet_lport->pb, &bridge_mappings, + b_ctx_out->egress_ifaces, + b_ctx_out->local_datapaths); free(lnet_lport); } @@ -1181,95 +1344,691 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) destroy_qos_map(&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. */ +/* Returns true if the database is all cleaned up, false if more work is + * required. */ bool -binding_evaluate_port_binding_changes(struct binding_ctx_in *b_ctx_in, - struct binding_ctx_out *b_ctx_out) +binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, + const struct sbrec_port_binding_table *port_binding_table, + const struct sbrec_chassis *chassis_rec) { - if (!b_ctx_in->chassis_rec) { + if (!ovnsb_idl_txn) { + return false; + } + if (!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; + bool any_changes = false; + SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec, port_binding_table) { + if (binding_rec->chassis == chassis_rec) { + if (binding_rec->encap) { + sbrec_port_binding_set_encap(binding_rec, NULL); + } + sbrec_port_binding_set_chassis(binding_rec, NULL); + any_changes = true; + } + } + + if (any_changes) { + ovsdb_idl_txn_add_comment( + ovnsb_idl_txn, + "ovn-controller: removing all port bindings for '%s'", + chassis_rec->name); + } + + return !any_changes; +} + +/* This function adds the local datapath of the 'peer' of + * lport 'pb' to the local datapaths if it is not yet added. + */ +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) +{ + 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; + } + + bool present = false; + for (size_t i = 0; i < ld->n_peer_ports; i++) { + if (ld->peer_ports[i].local == pb) { + present = true; break; } + } - if (!strcmp(binding_rec->type, "remote")) { - continue; + 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; + } - if (strcmp(binding_rec->type, "")) { - changed = true; + 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; - struct local_binding *lbinding = NULL; - if (!binding_rec->parent_port || !binding_rec->parent_port[0]) { - lbinding = local_binding_find(b_ctx_out->local_bindings, - binding_rec->logical_port); + /* Possible improvement: We can shrint the allocated peer ports + * if (ld->n_peer_ports < ld->n_allocated_peer_ports / 2). + */ + 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 the peer port from the peer datapath. The peer + * datapath also tries to remove its peer lport, but that would + * be no-op. */ + remove_local_datapath_peer_port(peer, peer_ld, local_datapaths); + } +} + +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; + } + } +} + +/* Considers the ovs iface 'iface_rec' for claiming. + * This function should be called if the external_ids:iface-id + * and 'ofport' are set for the 'iface_rec'. + * + * If the local_binding for this 'iface_rec' already exists and its + * already claimed, then this function will be no-op. + */ +static bool +consider_iface_claim(const struct ovsrec_interface *iface_rec, + const char *iface_id, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + struct hmap *qos_map) +{ + sset_add(b_ctx_out->local_lports, iface_id); + smap_replace(b_ctx_out->local_iface_ids, iface_rec->name, iface_id); + + struct local_binding *lbinding = + local_binding_find(b_ctx_out->local_bindings, iface_id); + + if (!lbinding) { + lbinding = local_binding_create(iface_id, iface_rec, NULL, BT_VIF); + local_binding_add(b_ctx_out->local_bindings, lbinding); + } else { + lbinding->iface = iface_rec; + } + + if (!lbinding->pb || strcmp(lbinding->name, lbinding->pb->logical_port)) { + lbinding->pb = lport_lookup_by_name( + b_ctx_in->sbrec_port_binding_by_name, lbinding->name); + if (lbinding->pb && !strcmp(lbinding->pb->type, "virtual")) { + lbinding->pb = NULL; + } + } + + if (lbinding->pb) { + if (!consider_vif_lport(lbinding->pb, b_ctx_in, b_ctx_out, + lbinding, qos_map)) { + return false; + } + } + + /* Update the child local_binding's iface (if any children) and try to + * claim the container lbindings. */ + struct shash_node *node; + SHASH_FOR_EACH (node, &lbinding->children) { + struct local_binding *child = node->data; + child->iface = iface_rec; + if (child->type == BT_CONTAINER) { + if (!consider_container_lport(child->pb, b_ctx_in, b_ctx_out, + qos_map)) { + return false; + } + } + } + + return true; +} + +/* Considers the ovs interface 'iface_rec' for + * releasing from this chassis if local_binding for this + * 'iface_rec' (with 'iface_id' as key) already exists and + * it is claimed by the chassis. + * + * The 'iface_id' could be cleared from the 'iface_rec' + * and hence it is passed separately. + * + * This fuction should be called if + * - OVS interface 'iface_rec' is deleted. + * - OVS interface 'iface_rec' external_ids:iface-id is updated + * (with the old value being 'iface_id'.) + * - OVS interface ofport is reset to 0. + * */ +static bool +consider_iface_release(const struct ovsrec_interface *iface_rec, + const char *iface_id, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, bool *changed) +{ + struct local_binding *lbinding; + lbinding = local_binding_find(b_ctx_out->local_bindings, + iface_id); + if (is_lbinding_this_chassis(lbinding, b_ctx_in->chassis_rec)) { + 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); + } + + /* Note: release_local_binding() resets lbinding->pb and + * lbinding->iface. + * Cannot access these members of lbinding after this call. */ + if (!release_local_binding(b_ctx_in->chassis_rec, lbinding, + !b_ctx_in->ovnsb_idl_txn)) { + return false; + } + + *changed = true; + } + + /* Check if the lbinding has children of type PB_CONTAINER. + * If so, don't delete the local_binding. */ + if (lbinding && !is_lbinding_container_parent(lbinding)) { + local_binding_delete(b_ctx_out->local_bindings, lbinding); + } + + sset_find_and_delete(b_ctx_out->local_lports, iface_id); + smap_remove(b_ctx_out->local_iface_ids, iface_rec->name); + + return true; +} + +static bool +is_iface_vif(const struct ovsrec_interface *iface_rec) +{ + if (iface_rec->type && iface_rec->type[0] && + strcmp(iface_rec->type, "internal")) { + return false; + } + + return true; +} + +/* 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, + bool *changed) +{ + if (!b_ctx_in->chassis_rec) { + return false; + } + + bool handled = true; + *changed = false; + + /* Run the tracked interfaces loop twice. One to handle deleted + * changes. And another to handle add/update changes. + * This will ensure correctness. + * * + * We consider an OVS interface for release if one of the following + * happens: + * 1. OVS interface is deleted. + * 2. external_ids:iface-id is cleared in which case we need to + * release the port binding corresponding to the previously set + * 'old-iface-id' (which is stored in the smap + * 'b_ctx_out->local_iface_ids'). + * 3. external_ids:iface-id is updated with a different value + * in which case we need to release the port binding corresponding + * to the previously set 'old-iface-id' (which is stored in the smap + * 'b_ctx_out->local_iface_ids'). + * 4. ofport of the OVS interface is 0. + * + */ + const struct ovsrec_interface *iface_rec; + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, + b_ctx_in->iface_table) { + if (!is_iface_vif(iface_rec)) { + /* Right now are not handling ovs_interface changes of + * other types. This can be enhanced to handle of + * types - patch and tunnel. */ + handled = false; + break; + } + + 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); + const char *cleared_iface_id = NULL; + if (!ovsrec_interface_is_deleted(iface_rec)) { + int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; + 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 (!ofport) { + /* If ofport is 0, we need to release the iface if already + * claimed. */ + cleared_iface_id = iface_id; + } + } else if (old_iface_id) { + cleared_iface_id = old_iface_id; + } } else { - lbinding = local_binding_find(b_ctx_out->local_bindings, - binding_rec->parent_port); + cleared_iface_id = iface_id; } - if (lbinding) { - changed = true; + if (cleared_iface_id) { + handled = consider_iface_release(iface_rec, cleared_iface_id, + b_ctx_in, b_ctx_out, changed); + } + + if (!handled) { break; } } - return changed; + if (!handled) { + /* This can happen if any non vif OVS interface is in the tracked + * list or if consider_iface_release() returned false. + * There is no need to process further. */ + return 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; + + /* + * We consider an OVS interface for claiming if the following + * 2 conditions are met: + * 1. external_ids:iface-id is set. + * 2. ofport of the OVS interface is > 0. + * + * So when an update of an OVS interface happens we see if these + * conditions are still true. If so consider this interface for + * claiming. This would be no-op if the update of the OVS interface + * didn't change the above two fields. + */ + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, + b_ctx_in->iface_table) { + /* Loop to handle create and update changes only. */ + if (ovsrec_interface_is_deleted(iface_rec)) { + continue; + } + + const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id"); + int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; + if (iface_id && ofport > 0) { + *changed = true; + handled = consider_iface_claim(iface_rec, iface_id, b_ctx_in, + b_ctx_out, qos_map_ptr); + if (!handled) { + break; + } + } + } + + if (handled && 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); + } + } + + destroy_qos_map(&qos_map); + return handled; } -/* Returns true if the database is all cleaned up, false if more work is - * required. */ -bool -binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, - const struct sbrec_port_binding_table *port_binding_table, - const struct sbrec_chassis *chassis_rec) +static void +handle_deleted_lport(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out) { - if (!ovnsb_idl_txn) { + 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); + } +} + +static struct local_binding * +get_lbinding_for_lport(const struct sbrec_port_binding *pb, + enum en_lport_type lport_type, + struct binding_ctx_out *b_ctx_out) +{ + ovs_assert(lport_type == LP_VIF || lport_type == LP_VIRTUAL); + + if (lport_type == LP_VIF && !is_lport_container(pb)) { + return local_binding_find(b_ctx_out->local_bindings, pb->logical_port); + } + + struct local_binding *parent_lbinding = NULL; + + if (lport_type == LP_VIRTUAL) { + parent_lbinding = local_binding_find(b_ctx_out->local_bindings, + pb->virtual_parent); + } else { + parent_lbinding = local_binding_find(b_ctx_out->local_bindings, + pb->parent_port); + } + + return parent_lbinding + ? local_binding_find(&parent_lbinding->children, pb->logical_port) + : NULL; +} + +static bool +handle_deleted_vif_lport(const struct sbrec_port_binding *pb, + enum en_lport_type lport_type, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + bool *changed) +{ + struct local_binding *lbinding = + get_lbinding_for_lport(pb, lport_type, b_ctx_out); + + if (lbinding) { + lbinding->pb = NULL; + /* The port_binding 'pb' is deleted. So there is no need to + * clear the 'chassis' column of 'pb'. But we need to do + * for the local_binding's children. */ + if (lbinding->type == BT_VIF && + !release_local_binding_children(b_ctx_in->chassis_rec, + lbinding, + !b_ctx_in->ovnsb_idl_txn)) { + return false; + } + + *changed = true; + } + + handle_deleted_lport(pb, b_ctx_in, b_ctx_out); + return true; +} + +static bool +handle_updated_vif_lport(const struct sbrec_port_binding *pb, + enum en_lport_type lport_type, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + struct hmap *qos_map, + bool *changed) +{ + bool claimed = (pb->chassis == b_ctx_in->chassis_rec); + bool handled = true; + + if (lport_type == LP_VIRTUAL) { + handled = consider_virtual_lport(pb, b_ctx_in, b_ctx_out, qos_map); + } else if (lport_type == LP_VIF && is_lport_container(pb)) { + handled = consider_container_lport(pb, b_ctx_in, b_ctx_out, qos_map); + } else { + handled = consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL, qos_map); + } + + if (!handled) { return false; } - if (!chassis_rec) { + + bool now_claimed = (pb->chassis == b_ctx_in->chassis_rec); + bool changed_ = (claimed != now_claimed); + + if (changed_) { + *changed = true; + } + + if (lport_type == LP_VIRTUAL || + (lport_type == LP_VIF && is_lport_container(pb)) || !changed_) { return true; } - const struct sbrec_port_binding *binding_rec; - bool any_changes = false; - SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec, port_binding_table) { - if (binding_rec->chassis == chassis_rec) { - if (binding_rec->encap) - sbrec_port_binding_set_encap(binding_rec, NULL); - sbrec_port_binding_set_chassis(binding_rec, NULL); - any_changes = true; + struct local_binding *lbinding = + local_binding_find(b_ctx_out->local_bindings, pb->logical_port); + + ovs_assert(lbinding); + + struct shash_node *node; + SHASH_FOR_EACH (node, &lbinding->children) { + struct local_binding *child = node->data; + if (child->type == BT_CONTAINER) { + handled = consider_container_lport(child->pb, b_ctx_in, b_ctx_out, + qos_map); + if (!handled) { + return false; + } } } - if (any_changes) { - ovsdb_idl_txn_add_comment( - ovnsb_idl_txn, - "ovn-controller: removing all port bindings for '%s'", - chassis_rec->name); + return true; +} + +/* 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 *changed) +{ + bool handled = true; + *changed = false; + + const struct sbrec_port_binding *pb; + + /* Run the tracked port binding loop twice. One to handle deleted + * changes. And another to handle add/update changes. + * This will ensure correctness. + */ + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, + b_ctx_in->port_binding_table) { + if (!sbrec_port_binding_is_deleted(pb)) { + continue; + } + + enum en_lport_type lport_type = get_lport_type(pb); + if (lport_type == LP_VIF || lport_type == LP_VIRTUAL) { + handled = handle_deleted_vif_lport(pb, lport_type, b_ctx_in, + b_ctx_out, changed); + } else { + handle_deleted_lport(pb, b_ctx_in, b_ctx_out); + } + + if (!handled) { + break; + } } - return !any_changes; + if (!handled) { + return 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; + + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, + b_ctx_in->port_binding_table) { + /* Loop to handle create and update changes only. */ + if (sbrec_port_binding_is_deleted(pb)) { + continue; + } + + enum en_lport_type lport_type = get_lport_type(pb); + + struct local_datapath *ld = + get_local_datapath(b_ctx_out->local_datapaths, + pb->datapath->tunnel_key); + + switch (lport_type) { + case LP_VIF: + case LP_VIRTUAL: + handled = handle_updated_vif_lport(pb, lport_type, b_ctx_in, + b_ctx_out, qos_map_ptr, + changed); + break; + + case LP_PATCH: + case LP_LOCALPORT: + case LP_VTEP: + *changed = true; + update_local_lport_ids(b_ctx_out->local_lport_ids, pb); + if (lport_type == LP_PATCH) { + /* Add the peer datapath to the local datapaths if it's + * not present yet. + */ + if (ld) { + add_local_datapath_peer_port(pb, b_ctx_in, b_ctx_out, ld); + } + } + break; + + case LP_L2GATEWAY: + *changed = true; + handled = consider_l2gw_lport(pb, b_ctx_in, b_ctx_out); + break; + + case LP_L3GATEWAY: + *changed = true; + handled = consider_l3gw_lport(pb, b_ctx_in, b_ctx_out); + break; + + case LP_CHASSISREDIRECT: + *changed = true; + handled = consider_cr_lport(pb, b_ctx_in, b_ctx_out); + break; + + case LP_EXTERNAL: + *changed = true; + handled = consider_external_lport(pb, b_ctx_in, b_ctx_out); + break; + + case LP_LOCALNET: { + *changed = true; + consider_localnet_lport(pb, b_ctx_in, b_ctx_out, qos_map_ptr); + + struct shash bridge_mappings = + SHASH_INITIALIZER(&bridge_mappings); + add_ovs_bridge_mappings(b_ctx_in->ovs_table, + b_ctx_in->bridge_table, + &bridge_mappings); + update_ld_localnet_port(pb, &bridge_mappings, + b_ctx_out->egress_ifaces, + b_ctx_out->local_datapaths); + shash_destroy(&bridge_mappings); + break; + } + + case LP_REMOTE: + case LP_UNKNOWN: + break; + } + + if (!handled) { + break; + } + } + + if (handled && 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); + } + } + + destroy_qos_map(&qos_map); + return handled; } diff --git a/controller/binding.h b/controller/binding.h index 9affc9a96..f7ace6faf 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -57,6 +57,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 *); @@ -64,9 +65,13 @@ 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_init(struct shash *local_bindings); void local_bindings_destroy(struct shash *local_bindings); +bool binding_handle_ovs_interface_changes(struct binding_ctx_in *, + struct binding_ctx_out *, + bool *changed); +bool binding_handle_port_binding_changes(struct binding_ctx_in *, + struct binding_ctx_out *, + bool *changed); #endif /* controller/binding.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 61d34157a..871291874 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -752,6 +752,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 { @@ -973,6 +974,7 @@ struct ed_type_runtime_data { struct sset active_tunnels; struct sset egress_ifaces; + struct smap local_iface_ids; }; static void * @@ -986,6 +988,7 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED, sset_init(&data->local_lport_ids); sset_init(&data->active_tunnels); sset_init(&data->egress_ifaces); + smap_init(&data->local_iface_ids); local_bindings_init(&data->local_bindings); return data; } @@ -999,6 +1002,7 @@ en_runtime_data_cleanup(void *data) sset_destroy(&rt_data->local_lport_ids); sset_destroy(&rt_data->active_tunnels); sset_destroy(&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) { @@ -1040,6 +1044,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)); @@ -1069,6 +1077,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; @@ -1082,6 +1091,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); local_bindings_init(&rt_data->local_bindings); } @@ -1139,6 +1151,34 @@ en_runtime_data_run(struct engine_node *node, void *data) engine_set_node_state(node, EN_UPDATED); } +static bool +runtime_data_ovs_interface_handler(struct engine_node *node, void *data) +{ + 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 = false; + if (!binding_handle_ovs_interface_changes(&b_ctx_in, &b_ctx_out, + &changed)) { + return false; + } + + if (changed) { + engine_set_node_state(node, EN_UPDATED); + } + + return true; +} + +static bool +runtime_data_noop_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + return true; +} + static bool runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) { @@ -1146,11 +1186,21 @@ runtime_data_sb_port_binding_handler(struct engine_node *node, void *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 changed = false; + if (!binding_handle_port_binding_changes(&b_ctx_in, &b_ctx_out, + &changed)) { + return false; + } - bool changed = binding_evaluate_port_binding_changes(&b_ctx_in, - &b_ctx_out); + if (changed) { + engine_set_node_state(node, EN_UPDATED); + } - return !changed; + return true; } /* Connection tracking zones. */ @@ -1894,7 +1944,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); diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at index a8a15f8fe..5dfc6f7ca 100644 --- a/tests/ovn-performance.at +++ b/tests/ovn-performance.at @@ -239,6 +239,19 @@ for i in 1 2; do ovn_attach n1 br-phys 192.168.0.$i done +# Wait for the tunnel ports to be created and up. +# Otherwise this may affect the lflow_run count. + +OVS_WAIT_UNTIL([ + test $(as hv1 ovs-vsctl list interface ovn-hv2-0 | \ +grep tunnel_egress_iface_carrier=up | wc -l) -eq 1 +]) + +OVS_WAIT_UNTIL([ + test $(as hv2 ovs-vsctl list interface ovn-hv1-0 | \ +grep tunnel_egress_iface_carrier=up | wc -l) -eq 1 +]) + # Add router lr1 OVN_CONTROLLER_EXPECT_HIT( [hv1 hv2], [lflow_run],