[ovs-dev] [PATCH v3 2/2] ovn: Fix localnet ports on the same chassis.

Han Zhou zhouhan at gmail.com
Mon Jan 25 07:26:40 UTC 2016


On Thu, Jan 21, 2016 at 1:11 PM, Russell Bryant <russell at ovn.org> wrote:
>
> Multiple logical ports on the same chassis that were connected to the
> same physical network via localnet ports were not able to send packets
> to each other.  This was because ovn-controller created a single patch
> port between br-int and the physical network access bridge and used it
> for all localnet ports.
>
> The fix implemented here is to create a separate patch port for every
> logical port of type=localnet.  An optimization is included where these
> ports are only created if the localnet port is on a logical switch with
> another logical port with an associated local VIF.
>
> A nice side effect of this fix is that the code in physical.c got a lot
> simpler, as localnet ports are now handled mostly like local VIFs.
>
> Fixes: c02819293d52 ("ovn: Add "localnet" logical port type.")
> Reported-by: Han Zhou <zhouhan at gmail.com>
> Reported-at: http://openvswitch.org/pipermail/dev/2016-January/064413.html
> Signed-off-by: Russell Bryant <russell at ovn.org>
> Tested-by: Kyle Mestery <mestery at mestery.com
> Acked-By: Kyle Mestery <mestery at mestery.com>
> ---
>
>
> v2->v3:
>  - tweak localnet port input flow handling as suggested by ben
>
>
>  ovn/controller/ovn-controller.8.xml |   7 +-
>  ovn/controller/ovn-controller.c     |  10 +-
>  ovn/controller/patch.c              |  80 ++++++++-----
>  ovn/controller/patch.h              |   4 +-
>  ovn/controller/physical.c           | 229
++++++++----------------------------
>  ovn/controller/physical.h           |   3 +-
>  ovn/ovn-architecture.7.xml          |   7 --
>  tests/ovn-controller.at             |  39 +++---
>  tests/ovn.at                        |  12 +-
>  9 files changed, 139 insertions(+), 252 deletions(-)
>
> diff --git a/ovn/controller/ovn-controller.8.xml
b/ovn/controller/ovn-controller.8.xml
> index 6dcc579..b261af9 100644
> --- a/ovn/controller/ovn-controller.8.xml
> +++ b/ovn/controller/ovn-controller.8.xml
> @@ -167,9 +167,10 @@
>            The presence of this key identifies a patch port as one
created by
>            <code>ovn-controller</code> to connect the integration bridge
and
>            another bridge to implement a <code>localnet</code> logical
port.
> -          Its value is the name of the physical network that the port
> -          implements.  See <code>external_ids:ovn-bridge-mappings</code>,
> -          above, for more information.
> +          Its value is the name of the logical port with type=localnet
that
> +          the port implements.
> +          See <code>external_ids:ovn-bridge-mappings</code>, above,
> +          for more information.
>          </p>
>
>          <p>
> diff --git a/ovn/controller/ovn-controller.c
b/ovn/controller/ovn-controller.c
> index b4ef675..98c6dba 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -285,11 +285,6 @@ main(int argc, char *argv[])
>          const struct ovsrec_bridge *br_int = get_br_int(&ctx);
>          const char *chassis_id = get_chassis_id(ctx.ovs_idl);
>
> -        /* Map bridges to local nets from ovn-bridge-mappings */
> -        if (br_int) {
> -            patch_run(&ctx, br_int);
> -        }
> -
>          if (chassis_id) {
>              chassis_run(&ctx, chassis_id);
>              encaps_run(&ctx, br_int, chassis_id);
> @@ -298,6 +293,8 @@ main(int argc, char *argv[])
>          }
>
>          if (br_int) {
> +            patch_run(&ctx, br_int, &local_datapaths);
> +
>              enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int);
>
>              pinctrl_run(&ctx, br_int);
> @@ -306,8 +303,7 @@ main(int argc, char *argv[])
>              lflow_run(&ctx, &flow_table, &ct_zones);
>              if (chassis_id) {
>                  physical_run(&ctx, mff_ovn_geneve,
> -                             br_int, chassis_id, &ct_zones, &flow_table,
> -                             &local_datapaths);
> +                             br_int, chassis_id, &ct_zones, &flow_table);
>              }
>              ofctrl_put(&flow_table);
>              hmap_destroy(&flow_table);
> diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
> index 07a3540..1f981b7 100644
> --- a/ovn/controller/patch.c
> +++ b/ovn/controller/patch.c
> @@ -18,6 +18,7 @@
>  #include "patch.h"
>
>  #include "hash.h"
> +#include "lib/hmap.h"
>  #include "lib/vswitch-idl.h"
>  #include "openvswitch/vlog.h"
>  #include "ovn-controller.h"
> @@ -95,27 +96,6 @@ create_patch_port(struct controller_ctx *ctx,
>      free(ports);
>  }
>
> -/* Creates a pair of patch ports that connect bridges 'b1' and 'b2',
using a
> - * port named 'name1' and 'name2' in each respective bridge.
> - * external-ids:'key' in each port is initialized to 'value'.
> - *
> - * If one or both of the ports already exists, leaves it there and
removes it
> - * from 'existing_ports'. */
> -static void
> -create_patch_ports(struct controller_ctx *ctx,
> -                   const char *key, const char *value,
> -                   const struct ovsrec_bridge *b1,
> -                   const struct ovsrec_bridge *b2,
> -                   struct shash *existing_ports)
> -{
> -    char *name1 = patch_port_name(b1->name, b2->name);
> -    char *name2 = patch_port_name(b2->name, b1->name);
> -    create_patch_port(ctx, key, value, b1, name1, b2, name2,
existing_ports);
> -    create_patch_port(ctx, key, value, b2, name2, b1, name1,
existing_ports);
> -    free(name2);
> -    free(name1);
> -}
> -
>  static void
>  remove_port(struct controller_ctx *ctx,
>              const struct ovsrec_port *port)
> @@ -153,7 +133,8 @@ remove_port(struct controller_ctx *ctx,
>  static void
>  add_bridge_mappings(struct controller_ctx *ctx,
>                      const struct ovsrec_bridge *br_int,
> -                    struct shash *existing_ports)
> +                    struct shash *existing_ports,
> +                    struct hmap *local_datapaths)
>  {
>      /* Get ovn-bridge-mappings. */
>      const char *mappings_cfg = "";
> @@ -166,7 +147,8 @@ add_bridge_mappings(struct controller_ctx *ctx,
>          }
>      }
>
> -    /* Create patch ports. */
> +    /* Parse bridge mappings. */
> +    struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings);
>      char *cur, *next, *start;
>      next = start = xstrdup(mappings_cfg);
>      while ((cur = strsep(&next, ",")) && *cur) {
> @@ -187,10 +169,53 @@ add_bridge_mappings(struct controller_ctx *ctx,
>              continue;
>          }
>
> -        create_patch_ports(ctx, "ovn-localnet-port", network,
> -                           br_int, ovs_bridge, existing_ports);
> +        shash_add(&bridge_mappings, network, ovs_bridge);
>      }
>      free(start);
> +
> +    const struct sbrec_port_binding *binding;
> +    SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
> +        if (strcmp(binding->type, "localnet")) {
> +            /* Not a binding for a localnet port. */
> +            continue;
> +        }
> +
> +        struct hmap_node *ld;
> +        ld = hmap_first_with_hash(local_datapaths,
> +                                  binding->datapath->tunnel_key);
> +        if (!ld) {
> +            /* This localnet port is on a datapath with no
> +             * logical ports bound to this chassis, so there's no need
> +             * to create patch ports for it. */
> +            continue;
> +        }
> +
> +        const char *network = smap_get(&binding->options,
"network_name");
> +        if (!network) {
> +            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
1);
> +            VLOG_ERR_RL(&rl, "localnet port '%s' has no network name.",
> +                         binding->logical_port);
> +            continue;
> +        }
> +        struct ovsrec_bridge *br_ln = shash_find_data(&bridge_mappings,
network);
> +        if (!br_ln) {
> +            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
1);
> +            VLOG_ERR_RL(&rl, "bridge not found for localnet port '%s' "
> +                    "with network name '%s'", binding->logical_port,
network);
> +            continue;
> +        }
> +
> +        char *name1 = patch_port_name(br_int->name,
binding->logical_port);
> +        char *name2 = patch_port_name(binding->logical_port,
br_int->name);
> +        create_patch_port(ctx, "ovn-localnet-port",
binding->logical_port,
> +                          br_int, name1, br_ln, name2, existing_ports);
> +        create_patch_port(ctx, "ovn-localnet-port",
binding->logical_port,
> +                          br_ln, name2, br_int, name1, existing_ports);
> +        free(name1);
> +        free(name2);
> +    }
> +
> +    shash_destroy(&bridge_mappings);
>  }
>
>  /* Add one OVS patch port for each OVN logical patch port.
> @@ -241,7 +266,8 @@ add_logical_patch_ports(struct controller_ctx *ctx,
>  }
>
>  void
> -patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
> +patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
> +          struct hmap *local_datapaths)
>  {
>      if (!ctx->ovs_idl_txn) {
>          return;
> @@ -260,7 +286,7 @@ patch_run(struct controller_ctx *ctx, const struct
ovsrec_bridge *br_int)
>      /* Create in the database any patch ports that should exist.  Remove
from
>       * 'existing_ports' any patch ports that do exist in the database and
>       * should be there. */
> -    add_bridge_mappings(ctx, br_int, &existing_ports);
> +    add_bridge_mappings(ctx, br_int, &existing_ports, local_datapaths);
>      add_logical_patch_ports(ctx, br_int, &existing_ports);
>
>      /* Now 'existing_ports' only still contains patch ports that exist
in the
> diff --git a/ovn/controller/patch.h b/ovn/controller/patch.h
> index f7db2fc..38ee7a8 100644
> --- a/ovn/controller/patch.h
> +++ b/ovn/controller/patch.h
> @@ -23,8 +23,10 @@
>   * physical bridges, as directed by other-config:ovn-bridge-mappings. */
>
>  struct controller_ctx;
> +struct hmap;
>  struct ovsrec_bridge;
>
> -void patch_run(struct controller_ctx *, const struct ovsrec_bridge
*br_int);
> +void patch_run(struct controller_ctx *, const struct ovsrec_bridge
*br_int,
> +               struct hmap *local_datapaths);
>
>  #endif /* ovn/patch.h */
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index b2772f0..8b12769 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -138,12 +138,10 @@ put_stack(enum mf_field_id field, struct
ofpact_stack *stack)
>  void
>  physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
>               const struct ovsrec_bridge *br_int, const char
*this_chassis_id,
> -             const struct simap *ct_zones, struct hmap *flow_table,
> -             struct hmap *local_datapaths)
> +             const struct simap *ct_zones, struct hmap *flow_table)
>  {
>      struct simap localvif_to_ofport =
SIMAP_INITIALIZER(&localvif_to_ofport);
>      struct hmap tunnels = HMAP_INITIALIZER(&tunnels);
> -    struct simap localnet_to_ofport =
SIMAP_INITIALIZER(&localnet_to_ofport);
>
>      for (int i = 0; i < br_int->n_ports; i++) {
>          const struct ovsrec_port *port_rec = br_int->ports[i];
> @@ -178,7 +176,8 @@ physical_run(struct controller_ctx *ctx, enum
mf_field_id mff_ovn_geneve,
>               * local logical port. */
>              bool is_patch = !strcmp(iface_rec->type, "patch");
>              if (is_patch && localnet) {
> -                simap_put(&localnet_to_ofport, localnet, ofport);
> +                /* localnet patch ports can be handled just like VIFs. */
> +                simap_put(&localvif_to_ofport, localnet, ofport);
>                  break;
>              } else if (is_patch && logpatch) {
>                  /* Logical patch ports can be handled just like VIFs. */
> @@ -219,24 +218,6 @@ physical_run(struct controller_ctx *ctx, enum
mf_field_id mff_ovn_geneve,
>      struct ofpbuf ofpacts;
>      ofpbuf_init(&ofpacts, 0);
>
> -    struct binding_elem {
> -        struct ovs_list list_elem;
> -        const struct sbrec_port_binding *binding;
> -    };
> -    /* The bindings for a given VLAN on a localnet port. */
> -    struct localnet_vlan {
> -        struct hmap_node node;
> -        int tag;
> -        struct ovs_list bindings;
> -    };
> -    /* A hash of localnet_vlans, hashed on VLAN ID, for a localnet port
*/
> -    struct localnet_bindings {
> -        ofp_port_t ofport;
> -        struct hmap vlans;
> -    };
> -    /* Maps from network name to "struct localnet_bindings". */
> -    struct shash localnet_inputs = SHASH_INITIALIZER(&localnet_inputs);
> -
>      /* Set up flows in table 0 for physical-to-logical translation and
in table
>       * 64 for logical-to-physical translation. */
>      const struct sbrec_port_binding *binding;
> @@ -247,36 +228,23 @@ physical_run(struct controller_ctx *ctx, enum
mf_field_id mff_ovn_geneve,
>           *     - If the port is a VIF on the chassis we're managing, the
>           *       OpenFlow port for the VIF.  'tun' will be NULL.
>           *
> -         *       In this or the next case, for a container nested inside
a VM
> -         *       and accessible via a VLAN, 'tag' is the VLAN ID;
otherwise
> -         *       'tag' is 0.
> +         *       The same logic handles logical patch ports, as well as
> +         *       localnet patch ports.
>           *
> -         *       The same logic handles logical patch ports.
> +         *       For a container nested inside a VM and accessible via a
VLAN,
> +         *       'tag' is the VLAN ID; otherwise 'tag' is 0.
> +         *
> +         *       For a localnet patch port, if a VLAN ID was configured,
'tag'
> +         *       is set to that VLAN ID; otherwise 'tag' is 0.
>           *
>           *     - If the port is on a remote chassis, the OpenFlow port
for a
>           *       tunnel to the VIF's remote chassis.  'tun' identifies
that
>           *       tunnel.
> -         *
> -         *     - If the port is a "localnet" port for a network that is
> -         *       attached to the chassis we're managing, the OpenFlow
port for
> -         *       the localnet port (a patch port).
> -         *
> -         *       The "localnet" port may be configured with a VLAN ID.
If so,
> -         *       'tag' will be set to that VLAN ID; otherwise 'tag' is 0.
>           */
>
>          int tag = 0;
>          ofp_port_t ofport;
> -        if (!strcmp(binding->type, "localnet")) {
> -            const char *network = smap_get(&binding->options,
"network_name");
> -            if (!network) {
> -                continue;
> -            }
> -            ofport = u16_to_ofp(simap_get(&localnet_to_ofport, network));
> -            if (ofport && binding->tag) {
> -                tag = *binding->tag;
> -            }
> -        } else if (binding->parent_port && *binding->parent_port) {
> +        if (binding->parent_port && *binding->parent_port) {
>              if (!binding->tag) {
>                  continue;
>              }
> @@ -288,6 +256,9 @@ physical_run(struct controller_ctx *ctx, enum
mf_field_id mff_ovn_geneve,
>          } else {
>              ofport = u16_to_ofp(simap_get(&localvif_to_ofport,
>                                            binding->logical_port));
> +            if (!strcmp(binding->type, "localnet") && ofport &&
binding->tag) {
> +                tag = *binding->tag;
> +            }
>          }
>
>          const struct chassis_tunnel *tun = NULL;
> @@ -324,64 +295,44 @@ physical_run(struct controller_ctx *ctx, enum
mf_field_id mff_ovn_geneve,
>               * input port, MFF_LOG_DATAPATH to the logical datapath, and
>               * resubmit into the logical ingress pipeline starting at
table
>               * 16. */
> -            if (!strcmp(binding->type, "localnet")) {
> -                /* The same OpenFlow port may correspond to localnet
ports
> -                 * attached to more than one logical datapath, so keep
track of
> -                 * all associated bindings and add a flow at the end. */
> -
> -                const char *network
> -                    = smap_get(&binding->options, "network_name");
> -                struct localnet_bindings *ln_bindings;
> -                struct hmap_node *node;
> -                struct localnet_vlan *ln_vlan;
> -
> -                ln_bindings = shash_find_data(&localnet_inputs, network);
> -                if (!ln_bindings) {
> -                    ln_bindings = xmalloc(sizeof *ln_bindings);
> -                    ln_bindings->ofport = ofport;
> -                    hmap_init(&ln_bindings->vlans);
> -                    shash_add(&localnet_inputs, network, ln_bindings);
> -                }
> -                node = hmap_first_with_hash(&ln_bindings->vlans, tag);
> -                if (node) {
> -                    ASSIGN_CONTAINER(ln_vlan, node, node);
> -                } else {
> -                    ln_vlan = xmalloc(sizeof *ln_vlan);
> -                    ln_vlan->tag = tag;
> -                    list_init(&ln_vlan->bindings);
> -                    hmap_insert(&ln_bindings->vlans, &ln_vlan->node,
tag);
> -                }
> +            ofpbuf_clear(&ofpacts);
> +            match_init_catchall(&match);
> +            match_set_in_port(&match, ofport);
>
> -                struct binding_elem *b = xmalloc(sizeof *b);
> -                b->binding = binding;
> -                list_insert(&ln_vlan->bindings, &b->list_elem);
> -            } else {
> -                ofpbuf_clear(&ofpacts);
> -                match_init_catchall(&match);
> -                match_set_in_port(&match, ofport);
> -                if (tag) {
> -                    match_set_dl_vlan(&match, htons(tag));
> -                }
> +            /* Match a VLAN tag and strip it, including stripping
priority tags
> +             * (e.g. VLAN ID 0).  In the latter case we'll add a second
flow
> +             * for frames that lack any 802.1Q header later. */
> +            if (tag || !strcmp(binding->type, "localnet")) {
> +                match_set_dl_vlan(&match, htons(tag));
> +                ofpact_put_STRIP_VLAN(&ofpacts);
> +            }
>
> -                if (zone_id) {
> -                    put_load(zone_id, MFF_LOG_CT_ZONE, 0, 32, &ofpacts);
> -                }
> +            /* Remember the size with just strip vlan added so far,
> +             * as we're going to remove this with ofpbuf_pull() later. */
> +            uint32_t ofpacts_orig_size = ofpacts.size;
>
> -                /* Set MFF_LOG_DATAPATH and MFF_LOG_INPORT. */
> -                put_load(binding->datapath->tunnel_key,
MFF_LOG_DATAPATH, 0, 64,
> -                         &ofpacts);
> -                put_load(binding->tunnel_key, MFF_LOG_INPORT, 0, 32,
> -                         &ofpacts);
> +            if (zone_id) {
> +                put_load(zone_id, MFF_LOG_CT_ZONE, 0, 32, &ofpacts);
> +            }
>
> -                /* Strip vlans. */
> -                if (tag) {
> -                    ofpact_put_STRIP_VLAN(&ofpacts);
> -                }
> +            /* Set MFF_LOG_DATAPATH and MFF_LOG_INPORT. */
> +            put_load(binding->datapath->tunnel_key, MFF_LOG_DATAPATH, 0,
64,
> +                     &ofpacts);
> +            put_load(binding->tunnel_key, MFF_LOG_INPORT, 0, 32,
> +                     &ofpacts);
>
> -                /* Resubmit to first logical ingress pipeline table. */
> -                put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts);
> -                ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG,
> -                                tag ? 150 : 100, &match, &ofpacts);
> +            /* Resubmit to first logical ingress pipeline table. */
> +            put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts);
> +            ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG,
> +                            tag ? 150 : 100, &match, &ofpacts);
> +
> +            if (!tag && !strcmp(binding->type, "localnet")) {
> +                /* Add a second flow for frames that lack any 802.1Q
> +                 * header.  For these, drop the OFPACT_STRIP_VLAN
> +                 * action. */
> +                ofpbuf_pull(&ofpacts, ofpacts_orig_size);
> +                match_set_dl_tci_masked(&match, 0, htons(VLAN_CFI));
> +                ofctrl_add_flow(flow_table, 0, 100, &match, &ofpacts);
>              }
>
>              /* Table 33, priority 100.
> @@ -536,16 +487,6 @@ physical_run(struct controller_ctx *ctx, enum
mf_field_id mff_ovn_geneve,
>                  put_resubmit(OFTABLE_DROP_LOOPBACK, &ofpacts);
>              } else if (port->chassis) {
>                  sset_add(&remote_chassis, port->chassis->name);
> -            } else if (!strcmp(port->type, "localnet")) {
> -                const char *network = smap_get(&port->options,
"network_name");
> -                if (!network) {
> -                    continue;
> -                }
> -                if (!simap_contains(&localnet_to_ofport, network)) {
> -                    continue;
> -                }
> -                put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32,
&ofpacts);
> -                put_resubmit(OFTABLE_DROP_LOOPBACK, &ofpacts);
>              }
>          }
>
> @@ -713,80 +654,4 @@ physical_run(struct controller_ctx *ctx, enum
mf_field_id mff_ovn_geneve,
>          free(tun);
>      }
>      hmap_destroy(&tunnels);
> -
> -    /* Table 0, Priority 100.
> -     * ======================
> -     *
> -     * We have now determined the full set of port bindings associated
with
> -     * each "localnet" network.  Only create flows for datapaths that
have
> -     * another local binding.  Otherwise, we know it would just be
dropped.
> -     */
> -    struct shash_node *ln_bindings_node, *ln_bindings_node_next;
> -    SHASH_FOR_EACH_SAFE (ln_bindings_node, ln_bindings_node_next,
> -                         &localnet_inputs) {
> -        struct localnet_bindings *ln_bindings = ln_bindings_node->data;
> -        struct localnet_vlan *ln_vlan, *ln_vlan_next;
> -        HMAP_FOR_EACH_SAFE (ln_vlan, ln_vlan_next, node,
&ln_bindings->vlans) {
> -            struct match match;
> -            match_init_catchall(&match);
> -            match_set_in_port(&match, ln_bindings->ofport);
> -            if (ln_vlan->tag) {
> -                match_set_dl_vlan(&match, htons(ln_vlan->tag));
> -            } else {
> -                /* Match priority-tagged frames, e.g. VLAN ID 0.
> -                 *
> -                 * We'll add a second flow for frames that lack any
802.1Q
> -                 * header later. */
> -                match_set_dl_tci_masked(&match, htons(VLAN_CFI),
> -                                        htons(VLAN_VID_MASK | VLAN_CFI));
> -            }
> -
> -            struct ofpbuf ofpacts;
> -            ofpbuf_init(&ofpacts, 0);
> -
> -            ofpact_put_STRIP_VLAN(&ofpacts);
> -            uint32_t ofpacts_orig_size = ofpacts.size;
> -
> -            struct binding_elem *b;
> -            LIST_FOR_EACH_POP (b, list_elem, &ln_vlan->bindings) {
> -                struct hmap_node *ld;
> -                ld = hmap_first_with_hash(local_datapaths,
> -
 b->binding->datapath->tunnel_key);
> -                if (ld) {
> -                    /* Set MFF_LOG_DATAPATH and MFF_LOG_INPORT. */
> -                    put_load(b->binding->datapath->tunnel_key,
MFF_LOG_DATAPATH,
> -                             0, 64, &ofpacts);
> -                    put_load(b->binding->tunnel_key, MFF_LOG_INPORT, 0,
32,
> -                             &ofpacts);
> -                    put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts);
> -                }
> -
> -                free(b);
> -            }
> -
> -            if (ofpacts.size > ofpacts_orig_size) {
> -                ofctrl_add_flow(flow_table, 0, 100, &match, &ofpacts);
> -
> -                if (!ln_vlan->tag) {
> -                    /* Add a second flow for frames that lack any 802.1Q
> -                     * header.  For these, drop the OFPACT_STRIP_VLAN
> -                     * action. */
> -                    ofpbuf_pull(&ofpacts, ofpacts_orig_size);
> -                    match_set_dl_tci_masked(&match, 0, htons(VLAN_CFI));
> -                    ofctrl_add_flow(flow_table, 0, 100, &match,
&ofpacts);
> -                }
> -            }
> -
> -            ofpbuf_uninit(&ofpacts);
> -
> -            hmap_remove(&ln_bindings->vlans, &ln_vlan->node);
> -            free(ln_vlan);
> -        }
> -        shash_delete(&localnet_inputs, ln_bindings_node);
> -        hmap_destroy(&ln_bindings->vlans);
> -        free(ln_bindings);
> -    }
> -    shash_destroy(&localnet_inputs);
> -
> -    simap_destroy(&localnet_to_ofport);
>  }
> diff --git a/ovn/controller/physical.h b/ovn/controller/physical.h
> index 826b99b..2906937 100644
> --- a/ovn/controller/physical.h
> +++ b/ovn/controller/physical.h
> @@ -43,7 +43,6 @@ struct simap;
>  void physical_register_ovs_idl(struct ovsdb_idl *);
>  void physical_run(struct controller_ctx *, enum mf_field_id
mff_ovn_geneve,
>                    const struct ovsrec_bridge *br_int, const char
*chassis_id,
> -                  const struct simap *ct_zones, struct hmap *flow_table,
> -                  struct hmap *local_datapaths);
> +                  const struct simap *ct_zones, struct hmap *flow_table);
>
>  #endif /* ovn/physical.h */
> diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml
> index c437b0d..d539db8 100644
> --- a/ovn/ovn-architecture.7.xml
> +++ b/ovn/ovn-architecture.7.xml
> @@ -702,13 +702,6 @@
>        </p>
>
>        <p>
> -        It's possible that a single ingress physical port maps to
multiple
> -        logical ports with a type of <code>localnet</code>. The logical
datapath
> -        and logical input port fields will be reset and the packet will
be
> -        resubmitted to table 16 multiple times.
> -      </p>
> -
> -      <p>
>          Packets that originate from a container nested within a VM are
treated
>          in a slightly different way.  The originating container can be
>          distinguished based on the VIF-specific VLAN ID, so the
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index bc93471..4218fbe 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -49,21 +49,30 @@ patch
>  # Initially there should be no patch ports.
>  check_patches
>
> -# Configure two ovn-bridge mappings to create two patch ports.
> +# Configure two ovn-bridge mappings, but no patch ports should be
created yet
>  AT_CHECK([ovs-vsctl set Open_vSwitch .
external-ids:ovn-bridge-mappings=physnet1:br-eth0,physnet2:br-eth1])
> -check_patches \
> -    'br-eth0 patch-br-eth0-to-br-int patch-br-int-to-br-eth0' \
> -    'br-int  patch-br-int-to-br-eth0 patch-br-eth0-to-br-int' \
> -    'br-eth1 patch-br-eth1-to-br-int patch-br-int-to-br-eth1' \
> -    'br-int  patch-br-int-to-br-eth1 patch-br-eth1-to-br-int'
> +check_patches
> +
> +# Create a localnet port, but we should still have no patch ports, as
they
> +# won't be created until there's a localnet port on a logical switch with
> +# another logical port bound to this chassis.
> +ovn-sbctl \
> +    -- --id=@dp101 create Datapath_Binding tunnel_key=101 \
> +    -- create Port_Binding datapath=@dp101 logical_port=localnet1
tunnel_key=1 \
> +        type=localnet options:network_name=physnet1
> +check_patches
>
> -# Change the mapping and the patch ports should change.
> -AT_CHECK([ovs-vsctl set Open_vSwitch .
external-ids:ovn-bridge-mappings=physnet1:br-eth2,physnet2:br-eth1])
> +# Create a localnet port on a logical switch with a port bound to this
chassis.
> +# Now we should get some patch ports created.
> +ovn-sbctl \
> +    -- --id=@dp102 create Datapath_Binding tunnel_key=102 \
> +    -- create Port_Binding datapath=@dp102 logical_port=localnet2
tunnel_key=1 \
> +        type=localnet options:network_name=physnet1 \
> +    -- create Port_Binding datapath=@dp102 logical_port=localvif2
tunnel_key=2
> +ovs-vsctl add-port br-int localvif2 -- set Interface localvif2
external_ids:iface-id=localvif2
>  check_patches \
> -    'br-eth2 patch-br-eth2-to-br-int patch-br-int-to-br-eth2' \
> -    'br-int  patch-br-int-to-br-eth2 patch-br-eth2-to-br-int' \
> -    'br-eth1 patch-br-eth1-to-br-int patch-br-int-to-br-eth1' \
> -    'br-int  patch-br-int-to-br-eth1 patch-br-eth1-to-br-int'
> +    'br-int  patch-br-int-to-localnet2 patch-localnet2-to-br-int' \
> +    'br-eth0 patch-localnet2-to-br-int patch-br-int-to-localnet2'
>
>  # Add logical patch ports.
>  AT_CHECK([ovn-sbctl \
> @@ -77,10 +86,8 @@ AT_CHECK([ovn-sbctl \
>  <3>
>  ])
>  check_patches \
> -    'br-eth2 patch-br-eth2-to-br-int patch-br-int-to-br-eth2' \
> -    'br-int  patch-br-int-to-br-eth2 patch-br-eth2-to-br-int' \
> -    'br-eth1 patch-br-eth1-to-br-int patch-br-int-to-br-eth1' \
> -    'br-int  patch-br-int-to-br-eth1 patch-br-eth1-to-br-int' \
> +    'br-int  patch-br-int-to-localnet2 patch-localnet2-to-br-int' \
> +    'br-eth0 patch-localnet2-to-br-int patch-br-int-to-localnet2' \
>      'br-int  patch-foo-to-bar        patch-bar-to-foo' \
>      'br-int  patch-bar-to-foo        patch-foo-to-bar'
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index b990116..f4117b6 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -740,7 +740,7 @@ AT_CLEANUP
>  # 2 hypervisors, 4 logical ports per HV
>  # 2 locally attached networks (one flat, one vlan tagged over same
device)
>  # 2 ports per HV on each network
> -AT_SETUP([ovn -- 2 HVs, 2 lports/HV, localnet ports])
> +AT_SETUP([ovn -- 2 HVs, 4 lports/HV, localnet ports])
>  AT_KEYWORDS([ovn-localnet])
>  AT_SKIP_IF([test $HAVE_PYTHON = no])
>  ovn_start
> @@ -830,9 +830,8 @@ test_packet 21 f00000000011 f00000000021 2111 11
>
>  # lp11 and lp12 are on the same network (phys, untagged)
>  # and on the same hypervisor
> -# TODO this is broken.
> -#test_packet 11 f00000000012 f00000000011 1112 12
> -#test_packet 12 f00000000011 f00000000012 1211 11
> +test_packet 11 f00000000012 f00000000011 1112 12
> +test_packet 12 f00000000011 f00000000012 1211 11
>
>  # lp13 and lp23 are on the same network (phys, VLAN 101)
>  # and on different hypervisors
> @@ -841,9 +840,8 @@ test_packet 23 f00000000013 f00000000023 2313 13
>
>  # lp13 and lp14 are on the same network (phys, VLAN 101)
>  # and on the same hypervisor
> -# TODO this is broken.
> -#test_packet 13 f00000000014 f00000000013 1314 14
> -#test_packet 14 f00000000013 f00000000014 1413 13
> +test_packet 13 f00000000014 f00000000013 1314 14
> +test_packet 14 f00000000013 f00000000014 1413 13
>
>  # Ports that should not be able to communicate
>  test_packet 11 f00000000013 f00000000011 1113
> --
> 2.5.0
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev


Looks good to me, and tested.

--
Best regards,
Han



More information about the dev mailing list