[ovs-dev] [PATCH ovn 2/6] ovn-controller: Store the local port bindings in the runtime data I-P state.
Han Zhou
hzhou at ovn.org
Wed Mar 25 06:51:10 UTC 2020
On Mon, Mar 16, 2020 at 4:15 AM <numans at ovn.org> wrote:
>
> From: Numan Siddique <numans at ovn.org>
>
> This patch adds a new data structure - 'struct local_binding' which
represents
> a probable local port binding. A new object of this structure is creared
for
> each interface present in the integration bridge (br-int) with the
> external_ids:iface-id set. This struct has 2 main fields
> - 'struct ovsrec_interface *' and 'struct sbrec_port_binding *'. These
fields
> are updated during port claim and release.
>
> A shash of the local bindings is maintained with the 'iface-id' as the
hash key
> in the runtime data of the incremental processing engine.
>
> This patch helps in the upcoming patch to add incremental processing
support
> for OVS interface and SB port binding changes.
>
> Signed-off-by: Numan Siddique <numans at ovn.org>
> ---
> controller/binding.c | 686 ++++++++++++++++++++++--------------
> controller/binding.h | 16 +-
> controller/ovn-controller.c | 49 ++-
> controller/pinctrl.c | 19 +-
> controller/pinctrl.h | 4 +-
> 5 files changed, 469 insertions(+), 305 deletions(-)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index 1e9f78249..34bd475de 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -69,47 +69,6 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> ovsdb_idl_add_column(ovs_idl, &ovsrec_qos_col_type);
> }
>
> -static void
> -get_local_iface_ids(const struct ovsrec_bridge *br_int,
> - struct shash *lport_to_iface,
> - struct sset *local_lports,
> - struct sset *egress_ifaces)
> -{
> - int i;
> -
> - for (i = 0; i < br_int->n_ports; i++) {
> - const struct ovsrec_port *port_rec = br_int->ports[i];
> - const char *iface_id;
> - int j;
> -
> - if (!strcmp(port_rec->name, br_int->name)) {
> - continue;
> - }
> -
> - for (j = 0; j < port_rec->n_interfaces; j++) {
> - const struct ovsrec_interface *iface_rec;
> -
> - iface_rec = port_rec->interfaces[j];
> - 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) {
> - shash_add(lport_to_iface, iface_id, iface_rec);
> - sset_add(local_lports, iface_id);
> - }
> -
> - /* Check if this is a tunnel interface. */
> - if (smap_get(&iface_rec->options, "remote_ip")) {
> - const char *tunnel_iface
> - = smap_get(&iface_rec->status,
"tunnel_egress_iface");
> - if (tunnel_iface) {
> - sset_add(egress_ifaces, tunnel_iface);
> - }
> - }
> - }
> - }
> -}
> -
> static void
> add_local_datapath__(struct ovsdb_idl_index
*sbrec_datapath_binding_by_key,
> struct ovsdb_idl_index
*sbrec_port_binding_by_datapath,
> @@ -469,171 +428,6 @@ is_our_chassis(const struct sbrec_chassis
*chassis_rec,
> return our_chassis;
> }
>
> -/* Updates 'binding_rec' and if the port binding is local also updates
the
> - * local datapaths and ports.
> - * Updates and returns the array of local virtual ports that will require
> - * additional processing.
> - */
> -static const struct sbrec_port_binding **
> -consider_local_datapath(const struct sbrec_port_binding *binding_rec,
> - struct hmap *qos_map,
> - const struct ovsrec_interface *iface_rec,
> - struct binding_ctx_in *b_ctx_in,
> - struct binding_ctx_out *b_ctx_out,
> - const struct sbrec_port_binding **vpbs,
> - size_t *n_vpbs, size_t *n_allocated_vpbs)
> -{
> - bool our_chassis = is_our_chassis(b_ctx_in->chassis_rec, binding_rec,
> - b_ctx_in->active_tunnels,
iface_rec,
> - b_ctx_out->local_lports);
> - if (iface_rec
> - || (binding_rec->parent_port && binding_rec->parent_port[0] &&
> - sset_contains(b_ctx_out->local_lports,
> - binding_rec->parent_port))) {
> - if (binding_rec->parent_port && binding_rec->parent_port[0]) {
> - /* Add child logical port to the set of all local ports. */
> - sset_add(b_ctx_out->local_lports, binding_rec->logical_port);
> - }
> - add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> - b_ctx_in->sbrec_port_binding_by_datapath,
> - b_ctx_in->sbrec_port_binding_by_name,
> - binding_rec->datapath, false,
> - b_ctx_out->local_datapaths);
> - if (iface_rec && qos_map && b_ctx_in->ovs_idl_txn) {
> - get_qos_params(binding_rec, qos_map);
> - }
> - } else if (!strcmp(binding_rec->type, "l2gateway")) {
> - if (our_chassis) {
> - sset_add(b_ctx_out->local_lports, binding_rec->logical_port);
> - add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> - b_ctx_in->sbrec_port_binding_by_datapath,
> - b_ctx_in->sbrec_port_binding_by_name,
> - binding_rec->datapath, false,
> - b_ctx_out->local_datapaths);
> - }
> - } else if (!strcmp(binding_rec->type, "chassisredirect")) {
> - if (ha_chassis_group_contains(binding_rec->ha_chassis_group,
> - b_ctx_in->chassis_rec)) {
> - add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> - b_ctx_in->sbrec_port_binding_by_datapath,
> - b_ctx_in->sbrec_port_binding_by_name,
> - binding_rec->datapath, false,
> - b_ctx_out->local_datapaths);
> - }
> - } else if (!strcmp(binding_rec->type, "l3gateway")) {
> - if (our_chassis) {
> - add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> - b_ctx_in->sbrec_port_binding_by_datapath,
> - b_ctx_in->sbrec_port_binding_by_name,
> - binding_rec->datapath, true,
> - b_ctx_out->local_datapaths);
> - }
> - } else if (!strcmp(binding_rec->type, "localnet")) {
> - /* Add all localnet ports to local_lports so that we allocate ct
zones
> - * for them. */
> - sset_add(b_ctx_out->local_lports, binding_rec->logical_port);
> - if (qos_map && b_ctx_in->ovs_idl_txn) {
> - get_qos_params(binding_rec, qos_map);
> - }
> - } else if (!strcmp(binding_rec->type, "external")) {
> - if (ha_chassis_group_contains(binding_rec->ha_chassis_group,
> - b_ctx_in->chassis_rec)) {
> - add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> - b_ctx_in->sbrec_port_binding_by_datapath,
> - b_ctx_in->sbrec_port_binding_by_name,
> - binding_rec->datapath, false,
> - b_ctx_out->local_datapaths);
> - }
> - }
> -
> - if (our_chassis
> - || !strcmp(binding_rec->type, "patch")
> - || !strcmp(binding_rec->type, "localport")
> - || !strcmp(binding_rec->type, "vtep")
> - || !strcmp(binding_rec->type, "localnet")) {
> - update_local_lport_ids(b_ctx_out->local_lport_ids, binding_rec);
> - }
> -
> - ovs_assert(b_ctx_in->ovnsb_idl_txn);
> - const char *vif_chassis = smap_get(&binding_rec->options,
> - "requested-chassis");
> - bool can_bind = !vif_chassis || !vif_chassis[0]
> - || !strcmp(vif_chassis, b_ctx_in->chassis_rec->name)
> - || !strcmp(vif_chassis,
b_ctx_in->chassis_rec->hostname);
> -
> - if (can_bind && our_chassis) {
> - if (binding_rec->chassis != b_ctx_in->chassis_rec) {
> - if (binding_rec->chassis) {
> - VLOG_INFO("Changing chassis for lport %s from %s to %s.",
> - binding_rec->logical_port,
> - binding_rec->chassis->name,
> - b_ctx_in->chassis_rec->name);
> - } else {
> - VLOG_INFO("Claiming lport %s for this chassis.",
> - binding_rec->logical_port);
> - }
> - for (int i = 0; i < binding_rec->n_mac; i++) {
> - VLOG_INFO("%s: Claiming %s",
> - binding_rec->logical_port,
binding_rec->mac[i]);
> - }
> - sbrec_port_binding_set_chassis(binding_rec,
b_ctx_in->chassis_rec);
> - }
> - /* Check if the port encap binding, if any, has changed */
> - struct sbrec_encap *encap_rec =
> - sbrec_get_port_encap(b_ctx_in->chassis_rec, iface_rec);
> - if (encap_rec && binding_rec->encap != encap_rec) {
> - sbrec_port_binding_set_encap(binding_rec, encap_rec);
> - }
> - } else if (binding_rec->chassis == b_ctx_in->chassis_rec) {
> - if (!strcmp(binding_rec->type, "virtual")) {
> - if (*n_vpbs == *n_allocated_vpbs) {
> - vpbs = x2nrealloc(vpbs, n_allocated_vpbs, sizeof *vpbs);
> - }
> - vpbs[(*n_vpbs)] = binding_rec;
> - (*n_vpbs)++;
> - } else {
> - VLOG_INFO("Releasing lport %s from this chassis.",
> - binding_rec->logical_port);
> - if (binding_rec->encap) {
> - sbrec_port_binding_set_encap(binding_rec, NULL);
> - }
> - sbrec_port_binding_set_chassis(binding_rec, NULL);
> - }
> - } else if (our_chassis) {
> - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> - VLOG_INFO_RL(&rl,
> - "Not claiming lport %s, chassis %s "
> - "requested-chassis %s",
> - binding_rec->logical_port,
b_ctx_in->chassis_rec->name,
> - vif_chassis);
> - }
> -
> - return vpbs;
> -}
> -
> -static void
> -consider_local_virtual_port(struct ovsdb_idl_index
*sbrec_port_binding_by_name,
> - const struct sbrec_chassis *chassis_rec,
> - const struct sbrec_port_binding *binding_rec)
> -{
> - /* pinctrl module takes care of binding the ports of type 'virtual'.
> - * Release such ports if their virtual parents are no longer claimed
by
> - * this chassis.
> - */
> - const struct sbrec_port_binding *parent =
> - lport_lookup_by_name(sbrec_port_binding_by_name,
> - binding_rec->virtual_parent);
> - if (!parent || parent->chassis != chassis_rec) {
> - VLOG_INFO("Releasing lport %s from this chassis.",
> - binding_rec->logical_port);
> - if (binding_rec->encap) {
> - sbrec_port_binding_set_encap(binding_rec, NULL);
> - }
> - sbrec_port_binding_set_chassis(binding_rec, NULL);
> - sbrec_port_binding_set_virtual_parent(binding_rec, NULL);
> - }
> -}
> -
> static void
> add_localnet_egress_interface_mappings(
> const struct sbrec_port_binding *port_binding,
> @@ -696,6 +490,360 @@ consider_localnet_port(const struct
sbrec_port_binding *binding_rec,
> ld->localnet_port = binding_rec;
> }
>
> +enum local_binding_type {
> + BT_VIF,
> + BT_CHILD,
> + BT_VIRTUAL
> +};
> +
> +struct local_binding {
> + struct ovs_list node; /* In parent if any. */
> + char *name;
> + enum local_binding_type type;
> + const struct ovsrec_interface *iface;
> + const struct sbrec_port_binding *pb;
> +
> + struct ovs_list children;
> +};
> +
> +static struct local_binding *
> +local_binding_create(const char *name, const struct ovsrec_interface
*iface,
> + const struct sbrec_port_binding *pb,
> + enum local_binding_type type)
> +{
> + struct local_binding *lbinding = xzalloc(sizeof *lbinding);
> + lbinding->name = xstrdup(name);
> + lbinding->type = type;
> + lbinding->pb = pb;
> + lbinding->iface = iface;
> + ovs_list_init(&lbinding->children);
> + return lbinding;
> +}
> +
> +static void
> +local_binding_add(struct shash *local_bindings, struct local_binding
*lbinding)
> +{
> + shash_add(local_bindings, lbinding->name, lbinding);
> +}
> +
> +static struct local_binding *
> +local_binding_find(struct shash *local_bindings, const char *name)
> +{
> + return shash_find_data(local_bindings, name);
> +}
> +
> +static void
> +local_binding_destroy(struct local_binding *lbinding)
> +{
> + struct local_binding *c, *next;
> + LIST_FOR_EACH_SAFE (c, next, node, &lbinding->children) {
> + ovs_list_remove(&c->node);
> + free(c->name);
> + free(c);
> + }
> + free(lbinding->name);
> + free(lbinding);
> +}
> +
> +void
> +local_bindings_destroy(struct shash *local_bindings)
> +{
> + struct shash_node *node, *next;
> + SHASH_FOR_EACH_SAFE (node, next, local_bindings) {
> + struct local_binding *lbinding = node->data;
> + struct local_binding *c, *n;
> + LIST_FOR_EACH_SAFE (c, n, node, &lbinding->children) {
> + ovs_list_remove(&c->node);
> + free(c->name);
> + free(c);
> + }
> + }
> +
> + shash_destroy(local_bindings);
> +}
> +
> +static void
> +local_binding_add_child(struct local_binding *lbinding,
> + struct local_binding *child)
> +{
> + struct local_binding *l;
> + LIST_FOR_EACH (l, node, &lbinding->children) {
> + if (l == child) {
> + return;
> + }
> + }
> +
> + ovs_list_push_back(&lbinding->children, &child->node);
> +}
> +
> +static struct local_binding *
> +local_binding_find_child(struct local_binding *lbinding,
> + const char *child_name)
> +{
> + struct local_binding *l;
> + LIST_FOR_EACH (l, node, &lbinding->children) {
> + if (!strcmp(l->name, child_name)) {
> + return l;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +void
> +binding_add_vport_to_local_bindings(struct shash *local_bindings,
> + const struct sbrec_port_binding
*parent,
> + const struct sbrec_port_binding
*vport)
> +{
> + struct local_binding *lbinding = local_binding_find(local_bindings,
> +
parent->logical_port);
> + ovs_assert(lbinding);
> + struct local_binding *vbinding =
> + local_binding_find_child(lbinding, vport->logical_port);
> + if (!vbinding) {
> + vbinding = local_binding_create(vport->logical_port,
lbinding->iface,
> + vport, BT_VIRTUAL);
> + local_binding_add_child(lbinding, vbinding);
> + } else {
> + vbinding->type = BT_VIRTUAL;
> + }
> +}
> +
> +static void
> +claim_lport(const struct sbrec_port_binding *pb,
> + const struct sbrec_chassis *chassis_rec,
> + const struct ovsrec_interface *iface_rec)
> +{
> + if (pb->chassis != chassis_rec) {
> + if (pb->chassis) {
> + VLOG_INFO("Changing chassis for lport %s from %s to %s.",
> + pb->logical_port, pb->chassis->name,
> + chassis_rec->name);
> + } else {
> + VLOG_INFO("Claiming lport %s for this chassis.",
pb->logical_port);
> + }
> + for (int i = 0; i < pb->n_mac; i++) {
> + VLOG_INFO("%s: Claiming %s", pb->logical_port, pb->mac[i]);
> + }
> +
> + sbrec_port_binding_set_chassis(pb, chassis_rec);
> + }
> +
> + /* Check if the port encap binding, if any, has changed */
> + struct sbrec_encap *encap_rec =
> + sbrec_get_port_encap(chassis_rec, iface_rec);
> + if (encap_rec && pb->encap != encap_rec) {
> + sbrec_port_binding_set_encap(pb, encap_rec);
> + }
> +}
> +
> +static void
> +release_lport(const struct sbrec_port_binding *pb)
> +{
> + VLOG_INFO("Releasing lport %s from this chassis.", pb->logical_port);
> + if (pb->encap) {
> + sbrec_port_binding_set_encap(pb, NULL);
> + }
> + sbrec_port_binding_set_chassis(pb, NULL);
> +
> + if (pb->virtual_parent) {
> + sbrec_port_binding_set_virtual_parent(pb, NULL);
> + }
> +}
> +
> +static void
> +consider_port_binding_for_vif(const struct sbrec_port_binding *pb,
> + struct binding_ctx_in *b_ctx_in,
> + enum local_binding_type binding_type,
> + struct local_binding *lbinding,
> + struct binding_ctx_out *b_ctx_out,
> + struct hmap *qos_map)
> +{
> + const char *vif_chassis = smap_get(&pb->options,
"requested-chassis");
> + bool can_bind = !vif_chassis || !vif_chassis[0]
> + || !strcmp(vif_chassis, b_ctx_in->chassis_rec->name)
> + || !strcmp(vif_chassis,
b_ctx_in->chassis_rec->hostname);
> +
> + if (lbinding && lbinding->pb && can_bind) {
> + claim_lport(pb, b_ctx_in->chassis_rec, lbinding->iface);
> +
> + switch (binding_type) {
> + case BT_VIF:
> + lbinding->pb = pb;
> + break;
> + case BT_CHILD:
> + case BT_VIRTUAL:
> + {
> + /* Add child logical port to the set of all local ports. */
> + sset_add(b_ctx_out->local_lports, pb->logical_port);
> + struct local_binding *child =
> + local_binding_find_child(lbinding, pb->logical_port);
> + if (!child) {
> + child = local_binding_create(pb->logical_port,
lbinding->iface,
> + pb, binding_type);
> + local_binding_add_child(lbinding, child);
> + } else {
> + ovs_assert(child->type == BT_CHILD ||
> + child->type == BT_VIRTUAL);
> + child->pb = pb;
> + child->iface = lbinding->iface;
> + }
> + break;
> + default:
> + break;
> + }
> + }
The brace here is strange. Is it a typo or rebase problem?
> +
> + 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,
> + pb->datapath, false,
b_ctx_out->local_datapaths);
> + update_local_lport_ids(b_ctx_out->local_lport_ids, pb);
> + if (lbinding->iface && qos_map && b_ctx_in->ovs_idl_txn) {
> + get_qos_params(pb, qos_map);
> + }
> + } else if (lbinding && lbinding->pb && !can_bind) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> + VLOG_INFO_RL(&rl,
> + "Not claiming lport %s, chassis %s "
> + "requested-chassis %s",
> + pb->logical_port,
> + b_ctx_in->chassis_rec->name,
> + vif_chassis);
> + }
> +
> + if (pb->chassis == b_ctx_in->chassis_rec) {
> + if (!lbinding || !lbinding->pb || !can_bind) {
> + release_lport(pb);
> + }
> + }
> +}
> +
> +static void
> +consider_port_binding(const struct sbrec_port_binding *pb,
> + struct binding_ctx_in *b_ctx_in,
> + struct binding_ctx_out *b_ctx_out,
> + struct hmap *qos_map)
> +{
> +
> + bool our_chassis = is_our_chassis(b_ctx_in->chassis_rec, pb,
> + b_ctx_in->active_tunnels, NULL,
> + b_ctx_out->local_lports);
> +
> + if (!strcmp(pb->type, "l2gateway")) {
> + if (our_chassis) {
> + sset_add(b_ctx_out->local_lports, pb->logical_port);
> + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> + b_ctx_in->sbrec_port_binding_by_datapath,
> + b_ctx_in->sbrec_port_binding_by_name,
> + pb->datapath, false,
> + b_ctx_out->local_datapaths);
> + }
> + } else if (!strcmp(pb->type, "chassisredirect")) {
> + if (ha_chassis_group_contains(pb->ha_chassis_group,
> + b_ctx_in->chassis_rec)) {
> + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> + b_ctx_in->sbrec_port_binding_by_datapath,
> + b_ctx_in->sbrec_port_binding_by_name,
> + pb->datapath, false,
> + b_ctx_out->local_datapaths);
> + }
> + } else if (!strcmp(pb->type, "l3gateway")) {
> + if (our_chassis) {
> + 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,
> + pb->datapath, true,
b_ctx_out->local_datapaths);
> + }
> + } else if (!strcmp(pb->type, "localnet")) {
> + /* Add all localnet ports to local_lports so that we allocate ct
zones
> + * for them. */
> + sset_add(b_ctx_out->local_lports, pb->logical_port);
> + if (qos_map && b_ctx_in->ovs_idl_txn) {
> + get_qos_params(pb, qos_map);
> + }
> + } else if (!strcmp(pb->type, "external")) {
> + if (ha_chassis_group_contains(pb->ha_chassis_group,
> + b_ctx_in->chassis_rec)) {
> + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
> + b_ctx_in->sbrec_port_binding_by_datapath,
> + b_ctx_in->sbrec_port_binding_by_name,
> + pb->datapath, false,
> + b_ctx_out->local_datapaths);
> + }
> + }
> +
> + if (our_chassis || !strcmp(pb->type, "localnet")) {
> + update_local_lport_ids(b_ctx_out->local_lport_ids, pb);
> + }
> +
> + ovs_assert(b_ctx_in->ovnsb_idl_txn);
> + if (our_chassis) {
> + claim_lport(pb, b_ctx_in->chassis_rec, NULL);
> + } else if (pb->chassis == b_ctx_in->chassis_rec) {
> + release_lport(pb);
> + }
> +}
> +
> +static void
> +build_local_bindings_from_local_ifaces(struct binding_ctx_in *b_ctx_in,
> + struct binding_ctx_out *b_ctx_out)
> +{
> + int i;
> + for (i = 0; i < b_ctx_in->br_int->n_ports; i++) {
> + const struct ovsrec_port *port_rec = b_ctx_in->br_int->ports[i];
> + const char *iface_id;
> + int j;
> +
> + if (!strcmp(port_rec->name, b_ctx_in->br_int->name)) {
> + continue;
> + }
> +
> + for (j = 0; j < port_rec->n_interfaces; j++) {
> + const struct ovsrec_interface *iface_rec;
> +
> + iface_rec = port_rec->interfaces[j];
> + 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) {
> + const struct sbrec_port_binding *pb
> + = lport_lookup_by_name(
> + b_ctx_in->sbrec_port_binding_by_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,
pb,
> + BT_VIF);
> + local_binding_add(b_ctx_out->local_bindings,
lbinding);
> + } else {
> + lbinding->type = BT_VIF;
> + lbinding->pb = pb;
> + }
> + sset_add(b_ctx_out->local_lports, iface_id);
> + }
> +
> + /* Check if this is a tunnel interface. */
> + if (smap_get(&iface_rec->options, "remote_ip")) {
> + const char *tunnel_iface
> + = smap_get(&iface_rec->status,
"tunnel_egress_iface");
> + if (tunnel_iface) {
> + sset_add(b_ctx_out->egress_ifaces, tunnel_iface);
It seems a tunnel interface is never removed from egress_ifaces when
deleted?
> + }
> + }
> + }
> + }
> +
> + struct shash_node *node, *next;
> + SHASH_FOR_EACH_SAFE (node, next, b_ctx_out->local_bindings) {
> + struct local_binding *lbinding = node->data;
> + if (!sset_contains(b_ctx_out->local_lports, lbinding->name)) {
> + local_binding_destroy(lbinding);
> + shash_delete(b_ctx_out->local_bindings, node);
> + }
> + }
> +}
> +
> void
> binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out
*b_ctx_out)
> {
> @@ -705,49 +853,62 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct
binding_ctx_out *b_ctx_out)
>
> const struct sbrec_port_binding *binding_rec;
> struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings);
> - struct shash lport_to_iface = SHASH_INITIALIZER(&lport_to_iface);
> - struct sset egress_ifaces = SSET_INITIALIZER(&egress_ifaces);
> struct hmap qos_map;
>
> hmap_init(&qos_map);
> if (b_ctx_in->br_int) {
> - get_local_iface_ids(b_ctx_in->br_int, &lport_to_iface,
> - b_ctx_out->local_lports, &egress_ifaces);
> + build_local_bindings_from_local_ifaces(b_ctx_in, b_ctx_out);
> }
>
> - /* Array to store pointers to local virtual ports. It is populated by
> - * consider_local_datapath.
> - */
> - const struct sbrec_port_binding **vpbs = NULL;
> - size_t n_vpbs = 0;
> - size_t n_allocated_vpbs = 0;
> + struct hmap *qos_map_ptr =
> + !sset_is_empty(b_ctx_out->egress_ifaces) ? &qos_map : NULL;
>
> /* Run through each binding record to see if it is resident on this
> * chassis and update the binding accordingly. This includes both
> - * directly connected logical ports and children of those ports.
> - * Virtual ports are just added to vpbs array and will be processed
> - * later. This is special case for virtual ports is needed in order
to
> - * make sure we update the virtual_parent port bindings first.
> + * directly connected logical ports and children of those ports
> + * (which also includes virtual ports).
> */
> SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec,
> b_ctx_in->port_binding_table) {
> - const struct ovsrec_interface *iface_rec
> - = shash_find_data(&lport_to_iface,
binding_rec->logical_port);
> - vpbs =
> - consider_local_datapath(binding_rec,
> - sset_is_empty(&egress_ifaces) ? NULL
:
> - &qos_map, iface_rec, b_ctx_in,
b_ctx_out,
> - vpbs, &n_vpbs, &n_allocated_vpbs);
> - }
> + if (!strcmp(binding_rec->type, "patch") ||
> + !strcmp(binding_rec->type, "localport") ||
> + !strcmp(binding_rec->type, "vtep")) {
> + update_local_lport_ids(b_ctx_out->local_lport_ids,
binding_rec);
> + continue;
> + }
>
> - /* Now also update the virtual ports in case their parent ports were
> - * updated above.
> - */
> - for (size_t i = 0; i < n_vpbs; i++) {
> - consider_local_virtual_port(b_ctx_in->sbrec_port_binding_by_name,
> - b_ctx_in->chassis_rec, vpbs[i]);
> + bool consider_for_vif = false;
> + struct local_binding *lbinding = NULL;
> + enum local_binding_type binding_type = BT_VIF;
> + if (!binding_rec->type[0]) {
> + if (!binding_rec->parent_port ||
!binding_rec->parent_port[0]) {
> + lbinding = local_binding_find(b_ctx_out->local_bindings,
> + binding_rec->logical_port);
> + } else {
> + lbinding = local_binding_find(b_ctx_out->local_bindings,
> + binding_rec->parent_port);
> + binding_type = BT_CHILD;
> + }
> +
> + consider_for_vif = true;
> + } else if (!strcmp(binding_rec->type, "virtual") &&
> + binding_rec->virtual_parent &&
> + binding_rec->virtual_parent[0]) {
> + lbinding = local_binding_find(b_ctx_out->local_bindings,
> + binding_rec->virtual_parent);
> + consider_for_vif = true;
> + binding_type = BT_VIRTUAL;
> + }
> +
> + if (consider_for_vif) {
> + consider_port_binding_for_vif(binding_rec, b_ctx_in,
> + binding_type, lbinding,
b_ctx_out,
> + qos_map_ptr);
> + } else {
> + consider_port_binding(binding_rec, b_ctx_in, b_ctx_out,
> + qos_map_ptr);
> + }
> }
> - free(vpbs);
>
> add_ovs_bridge_mappings(b_ctx_in->ovs_table, b_ctx_in->bridge_table,
> &bridge_mappings);
> @@ -759,49 +920,39 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct
binding_ctx_out *b_ctx_out)
> b_ctx_in->port_binding_table) {
> if (!strcmp(binding_rec->type, "localnet")) {
> consider_localnet_port(binding_rec, &bridge_mappings,
> - &egress_ifaces,
b_ctx_out->local_datapaths);
> + b_ctx_out->egress_ifaces,
> + b_ctx_out->local_datapaths);
> }
> }
> shash_destroy(&bridge_mappings);
>
> - if (!sset_is_empty(&egress_ifaces)
> + if (!sset_is_empty(b_ctx_out->egress_ifaces)
> && set_noop_qos(b_ctx_in->ovs_idl_txn, b_ctx_in->port_table,
> - b_ctx_in->qos_table, &egress_ifaces)) {
> + b_ctx_in->qos_table, b_ctx_out->egress_ifaces)) {
> const char *entry;
> - SSET_FOR_EACH (entry, &egress_ifaces) {
> + SSET_FOR_EACH (entry, b_ctx_out->egress_ifaces) {
> setup_qos(entry, &qos_map);
> }
> }
>
> - shash_destroy(&lport_to_iface);
> - sset_destroy(&egress_ifaces);
> hmap_destroy(&qos_map);
> }
>
> /* Returns true if port-binding changes potentially require flow changes
on
> * the current chassis. Returns false if we are sure there is no impact.
*/
> bool
> -binding_evaluate_port_binding_changes(
> - const struct sbrec_port_binding_table *pb_table,
> - const struct ovsrec_bridge *br_int,
> - const struct sbrec_chassis *chassis_rec,
> - struct sset *active_tunnels,
> - struct sset *local_lports)
> +binding_evaluate_port_binding_changes(struct binding_ctx_in *b_ctx_in,
> + struct binding_ctx_out *b_ctx_out)
> {
> - if (!chassis_rec) {
> + if (!b_ctx_in->chassis_rec) {
> return true;
> }
>
> bool changed = false;
>
> const struct sbrec_port_binding *binding_rec;
> - struct shash lport_to_iface = SHASH_INITIALIZER(&lport_to_iface);
> - struct sset egress_ifaces = SSET_INITIALIZER(&egress_ifaces);
> - if (br_int) {
> - get_local_iface_ids(br_int, &lport_to_iface, local_lports,
> - &egress_ifaces);
> - }
> - SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding_rec, pb_table) {
> + 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
> @@ -813,22 +964,33 @@ binding_evaluate_port_binding_changes(
> * interface table will be updated, which will trigger
recompute.
> *
> * - If the port is not a regular VIF, always trigger recompute.
*/
> - if (binding_rec->chassis == chassis_rec) {
> + if (binding_rec->chassis == b_ctx_in->chassis_rec) {
> + changed = true;
> + break;
> + }
> +
> + if (strcmp(binding_rec->type, "")) {
> changed = true;
> break;
> }
>
> - const struct ovsrec_interface *iface_rec
> - = shash_find_data(&lport_to_iface,
binding_rec->logical_port);
> - if (is_our_chassis(chassis_rec, binding_rec, active_tunnels,
iface_rec,
> - local_lports) || strcmp(binding_rec->type,
"")) {
> + struct local_binding *lbinding = NULL;
> + if (!binding_rec->type[0]) {
We already break from the loop above "if (strcmp(binding_rec->type, ""))
{", so it is unnecessary to do this check here.
> + if (!binding_rec->parent_port ||
!binding_rec->parent_port[0]) {
> + lbinding = local_binding_find(b_ctx_out->local_bindings,
> + binding_rec->logical_port);
> + } else {
> + lbinding = local_binding_find(b_ctx_out->local_bindings,
> + binding_rec->parent_port);
> + }
> + }
> +
> + if (lbinding) {
> changed = true;
> break;
> }
If a port is just binded to this chassis, since in the
binding_evaluate_port_binding_changes() we don't run the
build_local_bindings_from_local_ifaces(), so there is no chance for the
port-binding to be added to local_bindings. So it will always return
"false" in this case? What's the consideration here?
> }
>
> - shash_destroy(&lport_to_iface);
> - sset_destroy(&egress_ifaces);
> return changed;
> }
>
> diff --git a/controller/binding.h b/controller/binding.h
> index d0b8331af..6bee1d12e 100644
> --- a/controller/binding.h
> +++ b/controller/binding.h
> @@ -31,6 +31,7 @@ struct ovsrec_open_vswitch_table;
> struct sbrec_chassis;
> struct sbrec_port_binding_table;
> struct sset;
> +struct sbrec_port_binding;
>
> struct binding_ctx_in {
> struct ovsdb_idl_txn *ovnsb_idl_txn;
> @@ -51,8 +52,10 @@ struct binding_ctx_in {
>
> struct binding_ctx_out {
> struct hmap *local_datapaths;
> + struct shash *local_bindings;
> struct sset *local_lports;
> struct sset *local_lport_ids;
> + struct sset *egress_ifaces;
> };
>
> void binding_register_ovs_idl(struct ovsdb_idl *);
> @@ -60,11 +63,10 @@ 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(
> - const struct sbrec_port_binding_table *,
> - const struct ovsrec_bridge *br_int,
> - const struct sbrec_chassis *,
> - struct sset *active_tunnels,
> - struct sset *local_lports);
> -
> +bool binding_evaluate_port_binding_changes(struct binding_ctx_in *,
> + struct binding_ctx_out *);
> +void local_bindings_destroy(struct shash *local_bindings);
> +void binding_add_vport_to_local_bindings(
> + struct shash *local_bindings, const struct sbrec_port_binding
*parent,
> + const struct sbrec_port_binding *vport);
> #endif /* controller/binding.h */
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 4930d5ffb..8cc27cebf 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -958,6 +958,9 @@ struct ed_type_runtime_data {
> /* Contains "struct local_datapath" nodes. */
> struct hmap local_datapaths;
>
> + /* Contains "struct local_bindings" nodes. */
> + struct shash local_bindings;
> +
> /* Contains the name of each logical port resident on the local
> * hypervisor. These logical ports include the VIFs (and their child
> * logical ports, if any) that belong to VMs running on the
hypervisor,
> @@ -969,6 +972,8 @@ struct ed_type_runtime_data {
> * <datapath-tunnel-key>_<port-tunnel-key> */
> struct sset local_lport_ids;
> struct sset active_tunnels;
> +
> + struct sset egress_ifaces;
> };
>
> static void *
> @@ -981,6 +986,8 @@ en_runtime_data_init(struct engine_node *node
OVS_UNUSED,
> sset_init(&data->local_lports);
> sset_init(&data->local_lport_ids);
> sset_init(&data->active_tunnels);
> + sset_init(&data->egress_ifaces);
> + shash_init(&data->local_bindings);
> return data;
> }
>
> @@ -992,6 +999,7 @@ en_runtime_data_cleanup(void *data)
> sset_destroy(&rt_data->local_lports);
> sset_destroy(&rt_data->local_lport_ids);
> sset_destroy(&rt_data->active_tunnels);
> + sset_init(&rt_data->egress_ifaces);
> struct local_datapath *cur_node, *next_node;
> HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node,
> &rt_data->local_datapaths) {
> @@ -1000,6 +1008,7 @@ en_runtime_data_cleanup(void *data)
> free(cur_node);
> }
> hmap_destroy(&rt_data->local_datapaths);
> + local_bindings_destroy(&rt_data->local_bindings);
> }
>
> static void
> @@ -1072,6 +1081,8 @@ init_binding_ctx(struct engine_node *node,
> b_ctx_out->local_datapaths = &rt_data->local_datapaths;
> b_ctx_out->local_lports = &rt_data->local_lports;
> b_ctx_out->local_lport_ids = &rt_data->local_lport_ids;
> + b_ctx_out->egress_ifaces = &rt_data->egress_ifaces;
> + b_ctx_out->local_bindings = &rt_data->local_bindings;
> }
>
> static void
> @@ -1098,9 +1109,11 @@ en_runtime_data_run(struct engine_node *node, void
*data)
> sset_destroy(local_lports);
> sset_destroy(local_lport_ids);
> sset_destroy(active_tunnels);
> + sset_destroy(&rt_data->egress_ifaces);
> sset_init(local_lports);
> sset_init(local_lport_ids);
> sset_init(active_tunnels);
> + sset_init(&rt_data->egress_ifaces);
> }
>
> struct binding_ctx_in b_ctx_in;
> @@ -1129,35 +1142,12 @@ static bool
> runtime_data_sb_port_binding_handler(struct engine_node *node, void
*data)
> {
> struct ed_type_runtime_data *rt_data = data;
> - struct sset *local_lports = &rt_data->local_lports;
> - struct sset *active_tunnels = &rt_data->active_tunnels;
> -
> - struct ovsrec_open_vswitch_table *ovs_table =
> - (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET(
> - engine_get_input("OVS_open_vswitch", node));
> - struct ovsrec_bridge_table *bridge_table =
> - (struct ovsrec_bridge_table *)EN_OVSDB_GET(
> - engine_get_input("OVS_bridge", node));
> - const char *chassis_id = chassis_get_id();
> - const struct ovsrec_bridge *br_int = get_br_int(bridge_table,
ovs_table);
> -
> - ovs_assert(br_int && chassis_id);
> -
> - struct ovsdb_idl_index *sbrec_chassis_by_name =
> - engine_ovsdb_node_get_index(
> - engine_get_input("SB_chassis", node),
> - "name");
> -
> - const struct sbrec_chassis *chassis
> - = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
> - ovs_assert(chassis);
> -
> - struct sbrec_port_binding_table *pb_table =
> - (struct sbrec_port_binding_table *)EN_OVSDB_GET(
> - engine_get_input("SB_port_binding", node));
> + struct binding_ctx_in b_ctx_in;
> + struct binding_ctx_out b_ctx_out;
> + init_binding_ctx(node, rt_data, &b_ctx_in, &b_ctx_out);
>
> - bool changed = binding_evaluate_port_binding_changes(
> - pb_table, br_int, chassis, active_tunnels, local_lports);
> + bool changed = binding_evaluate_port_binding_changes(&b_ctx_in,
> + &b_ctx_out);
>
> return !changed;
> }
> @@ -2113,7 +2103,8 @@ main(int argc, char *argv[])
> ovnsb_idl_loop.idl),
> br_int, chassis,
> &runtime_data->local_datapaths,
> - &runtime_data->active_tunnels);
> + &runtime_data->active_tunnels,
> + &runtime_data->local_bindings);
> if (engine_node_changed(&en_runtime_data)) {
> update_sb_monitors(ovnsb_idl_loop.idl,
chassis,
>
&runtime_data->local_lports,
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 8bf19776c..e9f83b26e 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -18,6 +18,7 @@
>
> #include "pinctrl.h"
>
> +#include "binding.h"
> #include "coverage.h"
> #include "csum.h"
> #include "dirs.h"
> @@ -278,7 +279,8 @@ static void run_put_vport_bindings(
> struct ovsdb_idl_txn *ovnsb_idl_txn,
> struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
> struct ovsdb_idl_index *sbrec_port_binding_by_key,
> - const struct sbrec_chassis *chassis)
> + const struct sbrec_chassis *chassis,
> + struct shash *local_bindings)
> OVS_REQUIRES(pinctrl_mutex);
> static void wait_put_vport_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn);
> static void pinctrl_handle_bind_vport(const struct flow *md,
> @@ -2189,7 +2191,8 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> const struct ovsrec_bridge *br_int,
> const struct sbrec_chassis *chassis,
> const struct hmap *local_datapaths,
> - const struct sset *active_tunnels)
> + const struct sset *active_tunnels,
> + struct shash *local_bindings)
> {
> ovs_mutex_lock(&pinctrl_mutex);
> if (br_int && (!pinctrl.br_int_name || strcmp(pinctrl.br_int_name,
> @@ -2206,7 +2209,7 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> sbrec_port_binding_by_key,
> sbrec_mac_binding_by_lport_ip);
> run_put_vport_bindings(ovnsb_idl_txn, sbrec_datapath_binding_by_key,
> - sbrec_port_binding_by_key, chassis);
> + sbrec_port_binding_by_key, chassis,
local_bindings);
> send_garp_rarp_prepare(sbrec_port_binding_by_datapath,
> sbrec_port_binding_by_name, br_int, chassis,
> local_datapaths, active_tunnels);
> @@ -4875,7 +4878,8 @@ run_put_vport_binding(struct ovsdb_idl_txn
*ovnsb_idl_txn OVS_UNUSED,
> struct ovsdb_idl_index
*sbrec_datapath_binding_by_key,
> struct ovsdb_idl_index *sbrec_port_binding_by_key,
> const struct sbrec_chassis *chassis,
> - const struct put_vport_binding *vpb)
> + const struct put_vport_binding *vpb,
> + struct shash *local_bindings)
> {
> /* Convert logical datapath and logical port key into lport. */
> const struct sbrec_port_binding *pb = lport_lookup_by_key(
> @@ -4900,6 +4904,7 @@ run_put_vport_binding(struct ovsdb_idl_txn
*ovnsb_idl_txn OVS_UNUSED,
> pb->logical_port, parent->logical_port);
> sbrec_port_binding_set_chassis(pb, chassis);
> sbrec_port_binding_set_virtual_parent(pb,
parent->logical_port);
> + binding_add_vport_to_local_bindings(local_bindings, parent,
pb);
Updating the engine data here breaks the principle of incremental
processing engine. I think engine data should never be updated directly
other than within the engine or from "input" nodes of the engine.
Otherwise, we can't ensure the correctness. I think we don't need to do
this update here, because the SB update will be reflected as engine input
in next iteration thus calculated by the engine resulted in the runtime
data local_bindings update.
> }
> }
> }
> @@ -4910,7 +4915,8 @@ static void
> run_put_vport_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn,
> struct ovsdb_idl_index
*sbrec_datapath_binding_by_key,
> struct ovsdb_idl_index *sbrec_port_binding_by_key,
> - const struct sbrec_chassis *chassis)
> + const struct sbrec_chassis *chassis,
> + struct shash *local_bindings)
> OVS_REQUIRES(pinctrl_mutex)
> {
> if (!ovnsb_idl_txn) {
> @@ -4920,7 +4926,8 @@ run_put_vport_bindings(struct ovsdb_idl_txn
*ovnsb_idl_txn,
> const struct put_vport_binding *vpb;
> HMAP_FOR_EACH (vpb, hmap_node, &put_vport_bindings) {
> run_put_vport_binding(ovnsb_idl_txn,
sbrec_datapath_binding_by_key,
> - sbrec_port_binding_by_key, chassis, vpb);
> + sbrec_port_binding_by_key, chassis, vpb,
> + local_bindings);
> }
>
> flush_put_vport_bindings();
> diff --git a/controller/pinctrl.h b/controller/pinctrl.h
> index 8fa4baae9..bf2d02455 100644
> --- a/controller/pinctrl.h
> +++ b/controller/pinctrl.h
> @@ -31,6 +31,7 @@ struct sbrec_chassis;
> struct sbrec_dns_table;
> struct sbrec_controller_event_table;
> struct sbrec_service_monitor_table;
> +struct shash;
>
> void pinctrl_init(void);
> void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> @@ -46,7 +47,8 @@ void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> const struct sbrec_service_monitor_table *,
> const struct ovsrec_bridge *, const struct
sbrec_chassis *,
> const struct hmap *local_datapaths,
> - const struct sset *active_tunnels);
> + const struct sset *active_tunnels,
> + struct shash *local_bindings);
> void pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn);
> void pinctrl_destroy(void);
>
> --
> 2.24.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list