[ovs-dev] [PATCH 9/9] ovn-controller: Drop most uses of OVS patch ports.

Guru Shetty guru at ovn.org
Wed Dec 7 18:08:25 UTC 2016


On 4 December 2016 at 23:17, Ben Pfaff <blp at ovn.org> wrote:

> Until now, ovn-controller has implemented OVN logical patch ports and
> l3gateway ports in terms of OVS patch ports.  It is a hassle to create and
> destroy ports, and it is also wasteful compared to what the patch ports
> actually buy us: the ability to "save and restore" a packet around a
> recursive trip through the flow table.  The "clone" action can do that too,
> without the need to create a port.  This commit takes advantage of the
> clone action for that purpose, getting rid of most of the patch ports
> previously created by ovn-controller.
>
> Signed-off-by: Ben Pfaff <blp at ovn.org>
>

Though all the unit tests pass, all the system tests related to the gateway
fail.
make check-kernel TESTSUITEFLAGS="-k ovn"





> ---
>  ovn/controller/binding.c        |   2 +-
>  ovn/controller/ovn-controller.c |   8 ++--
>  ovn/controller/patch.c          | 101 +++++++++++-------------------
> ----------
>  ovn/controller/patch.h          |   3 +-
>  ovn/controller/physical.c       |  96 +++++++++++++++++++++++++++++-
> --------
>  ovn/controller/physical.h       |   7 +--
>  ovn/controller/pinctrl.c        |  36 ++++++++------
>  ovn/controller/pinctrl.h        |   4 +-
>  tests/ovn-controller.at         |  57 ++---------------------
>  9 files changed, 141 insertions(+), 173 deletions(-)
>
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index b53af98..2759e23 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
> @@ -391,7 +391,7 @@ consider_local_datapath(struct controller_ctx *ctx,
>                                         "l3gateway-chassis");
>          if (!strcmp(chassis, chassis_rec->name) && ctx->ovnsb_idl_txn) {
>              add_local_datapath(ldatapaths, lports, binding_rec->datapath,
> -                               false, local_datapaths);
> +                               true, local_datapaths);
>          }
>      } else if (chassis_rec && binding_rec->chassis == chassis_rec) {
>          if (ctx->ovnsb_idl_txn) {
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-
> controller.c
> index ff48a94..7d372b4 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -515,12 +515,12 @@ main(int argc, char *argv[])
>          }
>
>          if (br_int && chassis) {
> -            patch_run(&ctx, br_int, chassis_id, &local_datapaths);
> +            patch_run(&ctx, br_int, chassis, &local_datapaths);
>
>              enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int,
>
> &pending_ct_zones);
>
> -            pinctrl_run(&ctx, &lports, br_int, chassis_id,
> &local_datapaths);
> +            pinctrl_run(&ctx, &lports, br_int, chassis, &local_datapaths);
>              update_ct_zones(&all_lports, &local_datapaths, &ct_zones,
>                              ct_zone_bitmap, &pending_ct_zones);
>              if (ctx.ovs_idl_txn) {
> @@ -531,8 +531,8 @@ main(int argc, char *argv[])
>                            &group_table, &ct_zones, &flow_table);
>
>                  physical_run(&ctx, mff_ovn_geneve,
> -                             br_int, chassis_id, &ct_zones, &flow_table,
> -                             &local_datapaths);
> +                             br_int, chassis, &ct_zones, &lports,
> +                             &flow_table, &local_datapaths);
>
>                  ofctrl_put(&flow_table, &pending_ct_zones,
>                             get_nb_cfg(ctx.ovnsb_idl));
> diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
> index ce82b89..9893c79 100644
> --- a/ovn/controller/patch.c
> +++ b/ovn/controller/patch.c
> @@ -137,7 +137,7 @@ add_bridge_mappings(struct controller_ctx *ctx,
>                      const struct ovsrec_bridge *br_int,
>                      struct shash *existing_ports,
>                      struct hmap *local_datapaths,
> -                    const char *chassis_id)
> +                    const struct sbrec_chassis *chassis)
>  {
>      /* Get ovn-bridge-mappings. */
>      const char *mappings_cfg = "";
> @@ -207,7 +207,7 @@ add_bridge_mappings(struct controller_ctx *ctx,
>              patch_port_id = "ovn-localnet-port";
>          } else if (!strcmp(binding->type, "l2gateway")) {
>              if (!binding->chassis
> -                || strcmp(chassis_id, binding->chassis->name)) {
> +                || strcmp(chassis->name, binding->chassis->name)) {
>                  /* This L2 gateway port is not bound to this chassis,
>                   * so we should not create any patch ports for it. */
>                  continue;
> @@ -247,86 +247,39 @@ add_bridge_mappings(struct controller_ctx *ctx,
>      shash_destroy(&bridge_mappings);
>  }
>
> -/* Add one OVS patch port for each OVN logical patch port.
> - *
> - * It's wasteful to create an OVS patch port per OVN logical patch port,
> when
> - * really there's no benefit to them beyond a way to identify how a packet
> - * ingressed into a logical datapath.
> - *
> - * There are two obvious ways to improve the situation here, by modifying
> - * OVS:
> - *
> - *     1. Add a way to configure in OVS which fields are preserved on a
> hop
> - *        across an OVS patch port.  If MFF_LOG_DATAPATH and
> MFF_LOG_INPORT
> - *        were preserved, then only a single pair of OVS patch ports
> would be
> - *        required regardless of the number of OVN logical patch ports.
> - *
> - *     2. Add a new OpenFlow extension action modeled on "resubmit" that
> also
> - *        saves and restores the packet data and metadata (the inability
> to do
> - *        this is the only reason that "resubmit" can't be used
> already).  Or
> - *        add OpenFlow extension actions to otherwise save and restore
> packet
> - *        data and metadata.
> - */
> -static void
> -add_logical_patch_ports(struct controller_ctx *ctx,
> -                        const struct ovsrec_bridge *br_int,
> -                        const char *local_chassis_id,
> -                        struct hmap *local_datapaths,
> -                        struct shash *existing_ports)
> +void
> +patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
> +          const struct sbrec_chassis *chassis, struct hmap
> *local_datapaths)
>  {
> -    const struct sbrec_chassis *chassis_rec;
> -    chassis_rec = get_chassis(ctx->ovnsb_idl, local_chassis_id);
> -    if (!chassis_rec) {
> -        return;
> -    }
> -
> -    const struct local_datapath *ld;
> -    HMAP_FOR_EACH (ld, hmap_node, local_datapaths) {
> -        for (size_t i = 0; i < ld->ldatapath->n_lports; i++) {
> -            const struct sbrec_port_binding *pb =
> ld->ldatapath->lports[i];
> -            const char *patch_port_id = "ovn-logical-patch-port";
> -
> -            bool is_local_l3gateway = false;
> -            if (!strcmp(pb->type, "l3gateway")) {
> -                const char *chassis = smap_get(&pb->options,
> -                                               "l3gateway-chassis");
> -                if (chassis && !strcmp(local_chassis_id, chassis)) {
> -                    is_local_l3gateway = true;
> -                    patch_port_id = "ovn-l3gateway-port";
> -                    if (pb->chassis != chassis_rec && ctx->ovnsb_idl_txn)
> {
> -                        sbrec_port_binding_set_chassis(pb, chassis_rec);
> -                    }
> -                }
> -            }
> -
> -            if (!strcmp(pb->type, "patch") || is_local_l3gateway) {
> -                const char *local = pb->logical_port;
> -                const char *peer = smap_get(&pb->options, "peer");
> -                if (!peer) {
> -                    continue;
> +    /* Bind l3gateway ports to chassis.
> +     *
> +     * (It would make sense for ovn-northd to do this, but if it is ever
> +     * modified to do so, ovn-controller still needs to do the binding as
> long
> +     * as OVN supports upgrades against older ovn-northd.) */
> +    if (ctx->ovnsb_idl_txn && chassis) {
> +        const struct sbrec_port_binding *binding;
> +        SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
> +            if (!strcmp(binding->type, "l3gateway")) {
> +                const char *l3gw_chassis = smap_get(&binding->options,
> +                                                    "l3gateway-chassis");
> +                if (!strcmp(l3gw_chassis, chassis->name)
> +                    && binding->chassis != chassis) {
> +                    sbrec_port_binding_set_chassis(binding, chassis);
>                  }
> -
> -                char *src_name = patch_port_name(local, peer);
> -                char *dst_name = patch_port_name(peer, local);
> -                create_patch_port(ctx, patch_port_id, local,
> -                                  br_int, src_name, br_int, dst_name,
> -                                  existing_ports);
> -                free(dst_name);
> -                free(src_name);
>              }
>          }
>      }
> -}
>
> -void
> -patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
> -          const char *chassis_id, struct hmap *local_datapaths)
> -{
>      if (!ctx->ovs_idl_txn) {
>          return;
>      }
>
> -    /* Figure out what patch ports already exist. */
> +    /* Figure out what patch ports already exist.
> +     *
> +     * ovn-controller does not create or use ports of type
> "ovn-l3gateway-port"
> +     * or "ovn-logical-patch-port", but older version did.  We still
> recognize
> +     * them here, so that we delete them at the end of this function, to
> avoid
> +     * leaving useless ports on upgrade. */
>      struct shash existing_ports = SHASH_INITIALIZER(&existing_ports);
>      const struct ovsrec_port *port;
>      OVSREC_PORT_FOR_EACH (port, ctx->ovs_idl) {
> @@ -342,9 +295,7 @@ patch_run(struct controller_ctx *ctx, const struct
> ovsrec_bridge *br_int,
>       * 'existing_ports' any patch ports that do exist in the database and
>       * should be there. */
>      add_bridge_mappings(ctx, br_int, &existing_ports, local_datapaths,
> -                        chassis_id);
> -    add_logical_patch_ports(ctx, br_int, chassis_id, local_datapaths,
> -                            &existing_ports);
> +                        chassis);
>
>      /* Now 'existing_ports' only still contains patch ports that exist in
> the
>       * database but shouldn't.  Delete them from the database. */
> diff --git a/ovn/controller/patch.h b/ovn/controller/patch.h
> index 12ebeb9..2feb44b 100644
> --- a/ovn/controller/patch.h
> +++ b/ovn/controller/patch.h
> @@ -25,8 +25,9 @@
>  struct controller_ctx;
>  struct hmap;
>  struct ovsrec_bridge;
> +struct sbrec_chassis;
>
>  void patch_run(struct controller_ctx *, const struct ovsrec_bridge
> *br_int,
> -               const char *chassis_id, struct hmap *local_datapaths);
> +               const struct sbrec_chassis *, struct hmap
> *local_datapaths);
>
>  #endif /* ovn/patch.h */
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index 5de3530..747bb13 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -18,6 +18,7 @@
>  #include "byte-order.h"
>  #include "flow.h"
>  #include "lflow.h"
> +#include "lport.h"
>  #include "lib/poll-loop.h"
>  #include "ofctrl.h"
>  #include "openvswitch/hmap.h"
> @@ -286,8 +287,10 @@ load_logical_ingress_metadata(const struct
> sbrec_port_binding *binding,
>  static void
>  consider_port_binding(enum mf_field_id mff_ovn_geneve,
>                        const struct simap *ct_zones,
> +                      const struct lport_index *lports,
>                        struct hmap *local_datapaths,
>                        const struct sbrec_port_binding *binding,
> +                      const struct sbrec_chassis *chassis,
>                        struct ofpbuf *ofpacts_p,
>                        struct hmap *flow_table)
>  {
> @@ -297,14 +300,76 @@ consider_port_binding(enum mf_field_id
> mff_ovn_geneve,
>          return;
>      }
>
> +    struct match match;
> +    if (!strcmp(binding->type, "patch")
> +        || (!strcmp(binding->type, "l3gateway")
> +            && binding->chassis == chassis)) {
> +        const char *peer_name = smap_get(&binding->options, "peer");
> +        if (!peer_name) {
> +            return;
> +        }
> +
> +        const struct sbrec_port_binding *peer = lport_lookup_by_name(
> +            lports, peer_name);
> +        if (!peer || strcmp(peer->type, binding->type)) {
> +            return;
> +        }
> +        const char *peer_peer_name = smap_get(&peer->options, "peer");
> +        if (!peer_peer_name || strcmp(peer_peer_name,
> binding->logical_port)) {
> +            return;
> +        }
> +
> +        struct zone_ids zone_ids = { .ct = 0 };
> +        if (!strcmp(binding->type, "l3gateway")) {
> +            char *dnat = alloc_nat_zone_key(&binding->
> datapath->header_.uuid,
> +                                            "dnat");
> +            zone_ids.dnat = simap_get(ct_zones, dnat);
> +            free(dnat);
> +
> +            char *snat = alloc_nat_zone_key(&binding->
> datapath->header_.uuid,
> +                                            "snat");
> +            zone_ids.snat = simap_get(ct_zones, snat);
> +            free(snat);
> +        }
> +
> +        put_local_common_flows(dp_key, port_key, false, &zone_ids,
> +                               ofpacts_p, flow_table);
> +
> +        match_init_catchall(&match);
> +        ofpbuf_clear(ofpacts_p);
> +        match_set_metadata(&match, htonll(dp_key));
> +        match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0, port_key);
> +
> +        size_t clone_ofs = ofpacts_p->size;
> +        struct ofpact_clone *clone = ofpact_put_CLONE(ofpacts_p);
> +        put_load(0, MFF_LOG_DNAT_ZONE, 0, 32, ofpacts_p);
> +        put_load(0, MFF_LOG_SNAT_ZONE, 0, 32, ofpacts_p);
> +        put_load(0, MFF_LOG_CT_ZONE, 0, 32, ofpacts_p);
> +        load_logical_ingress_metadata(peer, &zone_ids, ofpacts_p);
> +        put_load(0, MFF_LOG_FLAGS, 0, 32, ofpacts_p);
> +        put_load(0, MFF_LOG_OUTPORT, 0, 32, ofpacts_p);
> +        for (int i = 0; i < MFF_N_LOG_REGS; i++) {
> +            put_load(0, MFF_LOG_REG0 + i, 0, 32, ofpacts_p);
> +        }
> +        put_load(0, MFF_IN_PORT, 0, 16, ofpacts_p);
> +        put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
> +        clone = ofpbuf_at_assert(ofpacts_p, clone_ofs, sizeof *clone);
> +        ofpacts_p->header = clone;
> +        ofpact_finish_CLONE(ofpacts_p, &clone);
> +
> +        ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100,
> +                        &match, ofpacts_p);
> +        return;
> +    }
> +
>      /* Find the OpenFlow port for the logical port, as 'ofport'.  This is
>       * one of:
>       *
>       *     - If the port is a VIF on the chassis we're managing, the
>       *       OpenFlow port for the VIF.  'tun' will be NULL.
>       *
> -     *       The same logic handles logical patch ports, as well as
> -     *       localnet patch ports.
> +     *       The same logic handles ports that OVN implements as Open
> vSwitch
> +     *       patch ports, that is, "localnet" and "l2gateway" ports.
>       *
>       *       For a container nested inside a VM and accessible via a VLAN,
>       *       'tag' is the VLAN ID; otherwise 'tag' is 0.
> @@ -365,7 +430,6 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
>          }
>      }
>
> -    struct match match;
>      if (!is_remote) {
>          /* Packets that arrive from a vif can belong to a VM or
>           * to a container located inside that VM. Packets that
> @@ -647,9 +711,10 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
>
>  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 ovsrec_bridge *br_int,
> +             const struct sbrec_chassis *chassis,
> +             const struct simap *ct_zones, struct lport_index *lports,
> +             struct hmap *flow_table, struct hmap *local_datapaths)
>  {
>
>      /* This bool tracks physical mapping changes. */
> @@ -667,7 +732,7 @@ physical_run(struct controller_ctx *ctx, enum
> mf_field_id mff_ovn_geneve,
>
>          const char *chassis_id = smap_get(&port_rec->external_ids,
>                                            "ovn-chassis-id");
> -        if (chassis_id && !strcmp(chassis_id, this_chassis_id)) {
> +        if (chassis_id && !strcmp(chassis_id, chassis->name)) {
>              continue;
>          }
>
> @@ -675,10 +740,6 @@ physical_run(struct controller_ctx *ctx, enum
> mf_field_id mff_ovn_geneve,
>                                          "ovn-localnet-port");
>          const char *l2gateway = smap_get(&port_rec->external_ids,
>                                          "ovn-l2gateway-port");
> -        const char *l3gateway = smap_get(&port_rec->external_ids,
> -                                        "ovn-l3gateway-port");
> -        const char *logpatch = smap_get(&port_rec->external_ids,
> -                                        "ovn-logical-patch-port");
>
>          for (int j = 0; j < port_rec->n_interfaces; j++) {
>              const struct ovsrec_interface *iface_rec =
> port_rec->interfaces[j];
> @@ -703,14 +764,6 @@ physical_run(struct controller_ctx *ctx, enum
> mf_field_id mff_ovn_geneve,
>                  /* L2 gateway patch ports can be handled just like VIFs.
> */
>                  simap_put(&new_localvif_to_ofport, l2gateway, ofport);
>                  break;
> -            } else if (is_patch && l3gateway) {
> -                /* L3 gateway patch ports can be handled just like VIFs.
> */
> -                simap_put(&new_localvif_to_ofport, l3gateway, ofport);
> -                break;
> -            } else if (is_patch && logpatch) {
> -                /* Logical patch ports can be handled just like VIFs. */
> -                simap_put(&new_localvif_to_ofport, logpatch, ofport);
> -                break;
>              } else if (chassis_id) {
>                  enum chassis_tunnel_type tunnel_type;
>                  if (!strcmp(iface_rec->type, "geneve")) {
> @@ -799,8 +852,9 @@ physical_run(struct controller_ctx *ctx, enum
> mf_field_id mff_ovn_geneve,
>       * 64 for logical-to-physical translation. */
>      const struct sbrec_port_binding *binding;
>      SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
> -        consider_port_binding(mff_ovn_geneve, ct_zones, local_datapaths,
> -                              binding, &ofpacts, flow_table);
> +        consider_port_binding(mff_ovn_geneve, ct_zones, lports,
> +                              local_datapaths, binding, chassis,
> +                              &ofpacts, flow_table);
>      }
>
>      /* Handle output to multicast groups, in tables 32 and 33. */
> diff --git a/ovn/controller/physical.h b/ovn/controller/physical.h
> index ab8a9d2..e2debed 100644
> --- a/ovn/controller/physical.h
> +++ b/ovn/controller/physical.h
> @@ -42,8 +42,9 @@ 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 ovsrec_bridge *br_int,
> +                  const struct sbrec_chassis *chassis,
> +                  const struct simap *ct_zones, struct lport_index *,
> +                  struct hmap *flow_table, struct hmap *local_datapaths);
>
>  #endif /* ovn/physical.h */
> diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> index db9e441..4accf69 100644
> --- a/ovn/controller/pinctrl.c
> +++ b/ovn/controller/pinctrl.c
> @@ -68,7 +68,7 @@ static void init_send_garps(void);
>  static void destroy_send_garps(void);
>  static void send_garp_wait(void);
>  static void send_garp_run(const struct ovsrec_bridge *,
> -                          const char *chassis_id,
> +                          const struct sbrec_chassis *,
>                            const struct lport_index *lports,
>                            struct hmap *local_datapaths);
>  static void pinctrl_handle_nd_na(const struct flow *ip_flow,
> @@ -755,7 +755,7 @@ pinctrl_recv(const struct ofp_header *oh, enum ofptype
> type)
>  void
>  pinctrl_run(struct controller_ctx *ctx, const struct lport_index *lports,
>              const struct ovsrec_bridge *br_int,
> -            const char *chassis_id,
> +            const struct sbrec_chassis *chassis,
>              struct hmap *local_datapaths)
>  {
>      char *target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(),
> br_int->name);
> @@ -791,7 +791,7 @@ pinctrl_run(struct controller_ctx *ctx, const struct
> lport_index *lports,
>      }
>
>      run_put_mac_bindings(ctx, lports);
> -    send_garp_run(br_int, chassis_id, lports, local_datapaths);
> +    send_garp_run(br_int, chassis, lports, local_datapaths);
>  }
>
>  void
> @@ -1148,7 +1148,7 @@ send_garp(struct garp_data *garp, long long int
> current_time)
>  /* Get localnet vifs, local l3gw ports and ofport for localnet patch
> ports. */
>  static void
>  get_localnet_vifs_l3gwports(const struct ovsrec_bridge *br_int,
> -                  const char *this_chassis_id,
> +                  const struct sbrec_chassis *chassis,
>                    const struct lport_index *lports,
>                    struct hmap *local_datapaths,
>                    struct sset *localnet_vifs,
> @@ -1162,13 +1162,11 @@ get_localnet_vifs_l3gwports(const struct
> ovsrec_bridge *br_int,
>          }
>          const char *chassis_id = smap_get(&port_rec->external_ids,
>                                            "ovn-chassis-id");
> -        if (chassis_id && !strcmp(chassis_id, this_chassis_id)) {
> +        if (chassis_id && !strcmp(chassis_id, chassis->name)) {
>              continue;
>          }
>          const char *localnet = smap_get(&port_rec->external_ids,
>                                          "ovn-localnet-port");
> -        const char *l3_gateway_port = smap_get(&port_rec->external_ids,
> -                                               "ovn-l3gateway-port");
>          for (int j = 0; j < port_rec->n_interfaces; j++) {
>              const struct ovsrec_interface *iface_rec =
> port_rec->interfaces[j];
>              if (!iface_rec->n_ofport) {
> @@ -1182,10 +1180,6 @@ get_localnet_vifs_l3gwports(const struct
> ovsrec_bridge *br_int,
>                  simap_put(localnet_ofports, localnet, ofport);
>                  continue;
>              }
> -            if (l3_gateway_port) {
> -                sset_add(local_l3gw_ports, l3_gateway_port);
> -                continue;
> -            }
>              const char *iface_id = smap_get(&iface_rec->external_ids,
>                                              "iface-id");
>              if (!iface_id) {
> @@ -1204,6 +1198,21 @@ get_localnet_vifs_l3gwports(const struct
> ovsrec_bridge *br_int,
>              }
>          }
>      }
> +
> +    const struct local_datapath *ld;
> +    HMAP_FOR_EACH (ld, hmap_node, local_datapaths) {
> +        if (!ld->has_local_l3gateway) {
> +            continue;
> +        }
> +
> +        for (size_t i = 0; i < ld->ldatapath->n_lports; i++) {
> +            const struct sbrec_port_binding *pb =
> ld->ldatapath->lports[i];
> +            if (!strcmp(pb->type, "l3gateway")
> +                /* && it's on this chassis */) {
> +                sset_add(local_l3gw_ports, pb->logical_port);
> +            }
> +        }
> +    }
>  }
>
>  static void
> @@ -1248,7 +1257,8 @@ send_garp_wait(void)
>  }
>
>  static void
> -send_garp_run(const struct ovsrec_bridge *br_int, const char *chassis_id,
> +send_garp_run(const struct ovsrec_bridge *br_int,
> +              const struct sbrec_chassis *chassis,
>                const struct lport_index *lports,
>                struct hmap *local_datapaths)
>  {
> @@ -1260,7 +1270,7 @@ send_garp_run(const struct ovsrec_bridge *br_int,
> const char *chassis_id,
>
>      shash_init(&nat_addresses);
>
> -    get_localnet_vifs_l3gwports(br_int, chassis_id, lports,
> local_datapaths,
> +    get_localnet_vifs_l3gwports(br_int, chassis, lports, local_datapaths,
>                        &localnet_vifs, &localnet_ofports,
> &local_l3gw_ports);
>
>      get_nat_addresses_and_keys(&nat_ip_keys, &local_l3gw_ports, lports,
> diff --git a/ovn/controller/pinctrl.h b/ovn/controller/pinctrl.h
> index 553eace..af3c4b0 100644
> --- a/ovn/controller/pinctrl.h
> +++ b/ovn/controller/pinctrl.h
> @@ -25,11 +25,11 @@ struct hmap;
>  struct lport_index;
>  struct ovsrec_bridge;
>  struct controller_ctx;
> +struct sbrec_chassis;
>
>  void pinctrl_init(void);
>  void pinctrl_run(struct controller_ctx *, const struct lport_index *,
> -                 const struct ovsrec_bridge *,
> -                 const char *chassis_id,
> +                 const struct ovsrec_bridge *, const struct sbrec_chassis
> *,
>                   struct hmap *local_datapaths);
>  void pinctrl_wait(struct controller_ctx *);
>  void pinctrl_destroy(void);
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index 8ad8f67..2b28279 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -88,8 +88,8 @@ check_patches \
>
>  # Add logical patch ports to connect new logical datapath.
>  #
> -# (Also add a vif on this hypervisor on one of the datapaths,
> -# otherwise the hypervisor will ignore both of them.)
> +# OVN no longer uses OVS patch ports to implement logical patch ports, so
> +# the set of OVS patch ports doesn't change.
>  AT_CHECK([ovn-sbctl \
>      -- --id=@dp1 create Datapath_Binding tunnel_key=1 \
>      -- --id=@dp2 create Datapath_Binding tunnel_key=2 \
> @@ -105,66 +105,17 @@ AT_CHECK([ovn-sbctl \
>  ovs-vsctl add-port br-int dp1vif -- set Interface dp1vif
> external_ids:iface-id=dp1vif
>  check_patches \
>      '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'
> +    'br-eth0 patch-localnet2-to-br-int patch-br-int-to-localnet2'
>
> -# Delete the mapping and the ovn-bridge-mapping patch ports should go
> away;
> -# the ones from the logical patch port remain.
> +# Delete the mapping and the ovn-bridge-mapping patch ports should go
> away.
>  AT_CHECK([ovs-vsctl remove Open_vSwitch . external-ids
> ovn-bridge-mappings])
>  check_bridge_mappings
> -check_patches \
> -    'br-int patch-foo-to-bar patch-bar-to-foo' \
> -    'br-int patch-bar-to-foo patch-foo-to-bar'
> -
> -# Change name of logical patch port, check that the OVS patch ports
> -# get updated.
> -AT_CHECK([ovn-sbctl \
> -    -- set Port_Binding foo logical_port=quux options:peer=baz \
> -    -- set Port_Binding bar logical_port=baz  options:peer=quux])
> -check_patches \
> -    'br-int patch-quux-to-baz patch-baz-to-quux' \
> -    'br-int patch-baz-to-quux patch-quux-to-baz'
> -
> -# Drop the vif from the patched-together datapaths and verify that the
> patch
> -# ports disappear.
> -AT_CHECK([ovs-vsctl del-port dp1vif])
> -check_patches
> -
> -# Put the vif back and the patch ports reappear.
> -AT_CHECK([ovs-vsctl add-port br-int dp1vif -- set Interface dp1vif
> external_ids:iface-id=dp1vif])
> -check_patches \
> -    'br-int patch-quux-to-baz patch-baz-to-quux' \
> -    'br-int patch-baz-to-quux patch-quux-to-baz'
> -
> -# Create an empty database, serve it and switch to it
> -# and verify that the OVS patch ports disappear
> -# then put it back and verify that they reappear
> -
> -on_exit 'kill `cat $ovs_base/ovn-sb/ovsdb-server-2.pid`'
> -
> -ovsdb-tool create $ovs_base/ovn-sb/ovn-sb1.db
> "$abs_top_srcdir"/ovn/ovn-sb.ovsschema
> -as ovn-sb ovsdb-server --detach --pidfile=$ovs_base/ovn-sb/ovsdb-server-2.pid
> --remote=punix:$ovs_base/ovn-sb/ovn-sb1.sock $ovs_base/ovn-sb/ovn-sb1.db \
> -   --unixctl=$ovs_base/ovn-sb/ovsdb-server-2.ctl
> -AT_CHECK([ovs-vsctl -- set Open_vSwitch . external-ids:ovn-remote=unix:$
> ovs_base/ovn-sb/ovn-sb1.sock])
> -check_patches
> -AT_CHECK([ovs-vsctl -- set Open_vSwitch . external-ids:ovn-remote=unix:$
> ovs_base/ovn-sb/ovn-sb.sock])
> -check_patches \
> -    'br-int patch-quux-to-baz patch-baz-to-quux' \
> -    'br-int patch-baz-to-quux patch-quux-to-baz'
> -
> -# Change the logical patch ports to VIFs and verify that the OVS patch
> -# ports disappear.
> -AT_CHECK([ovn-sbctl \
> -    -- set Port_Binding quux type='""' options='{}' \
> -    -- set Port_Binding baz type='""' options='{}'])
>  check_patches
>
>  # Gracefully terminate daemons
>  OVN_CLEANUP_SBOX([hv])
>  OVN_CLEANUP_VSWITCH([main])
>  as ovn-sb
> -OVS_APP_EXIT_AND_WAIT_BY_TARGET([$ovs_base/ovn-sb/ovsdb-server-2.ctl],
> [$ovs_base/ovn-sb/ovsdb-server-2.pid])
>  OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>
>  AT_CLEANUP
> --
> 2.10.2
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list