diff mbox series

[ovs-dev,2/3] encaps: Create separate tunnels for multiple local encap IPs.

Message ID 20240117054745.4027120-3-hzhou@ovn.org
State Accepted
Headers show
Series Support VIF-based local encap IPs selection. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_ovn-kubernetes success github build: passed
ovsrobot/github-robot-_Build_and_Test fail github build: failed

Commit Message

Han Zhou Jan. 17, 2024, 5:47 a.m. UTC 
In commit dd527a283cd8, it created separate tunnels for different remote
encap IPs of the same remote chassis, but when the current chassis is
the one that has multiple encap IPs configured, it only uses the first
encap IP. This patch creates separate tunnels taking into consideration
the multiple encap IPs in current chassis and sets corresponding
local_ip for each tunnel interface in such cases.

Co-authored-by: Lei Huang <leih@ovn.org>
Signed-off-by: Lei Huang <leih@ovn.org>
Signed-off-by: Han Zhou <hzhou@ovn.org>
---
 controller/bfd.c        |   4 +-
 controller/encaps.c     | 158 ++++++++++++++++++++++------------------
 controller/encaps.h     |   9 ++-
 controller/lflow.c      |   2 +-
 controller/local_data.c |  14 ++--
 controller/local_data.h |   5 +-
 controller/physical.c   |  28 +++----
 controller/pinctrl.c    |   2 +-
 tests/ovn-ipsec.at      |  49 -------------
 tests/ovn.at            |  88 +++++++++++++++++-----
 10 files changed, 192 insertions(+), 167 deletions(-)

Comments

Han Zhou Jan. 17, 2024, 5:54 a.m. UTC | #1 
On Tue, Jan 16, 2024 at 9:48 PM Han Zhou <hzhou@ovn.org> wrote:
>
> In commit dd527a283cd8, it created separate tunnels for different remote
> encap IPs of the same remote chassis, but when the current chassis is
> the one that has multiple encap IPs configured, it only uses the first
> encap IP. This patch creates separate tunnels taking into consideration
> the multiple encap IPs in current chassis and sets corresponding
> local_ip for each tunnel interface in such cases.
>
> Co-authored-by: Lei Huang <leih@ovn.org>
> Signed-off-by: Lei Huang <leih@ovn.org>

Oops, I made a silly mistake in Lei's email address, which should be
leih@nvidia.com instead. Sorry for that.

Han

> Signed-off-by: Han Zhou <hzhou@ovn.org>
> ---
>  controller/bfd.c        |   4 +-
>  controller/encaps.c     | 158 ++++++++++++++++++++++------------------
>  controller/encaps.h     |   9 ++-
>  controller/lflow.c      |   2 +-
>  controller/local_data.c |  14 ++--
>  controller/local_data.h |   5 +-
>  controller/physical.c   |  28 +++----
>  controller/pinctrl.c    |   2 +-
>  tests/ovn-ipsec.at      |  49 -------------
>  tests/ovn.at            |  88 +++++++++++++++++-----
>  10 files changed, 192 insertions(+), 167 deletions(-)
>
> diff --git a/controller/bfd.c b/controller/bfd.c
> index cf011e382c6c..f24bfd063888 100644
> --- a/controller/bfd.c
> +++ b/controller/bfd.c
> @@ -75,7 +75,7 @@ bfd_calculate_active_tunnels(const struct ovsrec_bridge
*br_int,
>                              char *chassis_name = NULL;
>
>                              if (encaps_tunnel_id_parse(id, &chassis_name,
> -                                                       NULL)) {
> +                                                       NULL, NULL)) {
>                                  if (!sset_contains(active_tunnels,
>                                                     chassis_name)) {
>                                      sset_add(active_tunnels,
chassis_name);
> @@ -204,7 +204,7 @@ bfd_run(const struct ovsrec_interface_table
*interface_table,
>
>              sset_add(&tunnels, port_name);
>
> -            if (encaps_tunnel_id_parse(tunnel_id, &chassis_name, NULL)) {
> +            if (encaps_tunnel_id_parse(tunnel_id, &chassis_name, NULL,
NULL)) {
>                  if (sset_contains(&bfd_chassis, chassis_name)) {
>                      sset_add(&bfd_ifaces, port_name);
>                  }
> diff --git a/controller/encaps.c b/controller/encaps.c
> index 1f6e667a606c..28237f6191c8 100644
> --- a/controller/encaps.c
> +++ b/controller/encaps.c
> @@ -32,10 +32,9 @@ VLOG_DEFINE_THIS_MODULE(encaps);
>  /*
>   * Given there could be multiple tunnels with different IPs to the same
>   * chassis we annotate the external_ids:ovn-chassis-id in tunnel port
with
> - * <chassis_name>OVN_MVTEP_CHASSISID_DELIM<IP>. The external_id key
> + * <chassis_name>@<remote IP>%<local IP>. The external_id key
>   * "ovn-chassis-id" is kept for backward compatibility.
>   */
> -#define        OVN_MVTEP_CHASSISID_DELIM '@'
>  #define OVN_TUNNEL_ID "ovn-chassis-id"
>
>  static char *current_br_int_name = NULL;
> @@ -95,72 +94,93 @@ tunnel_create_name(struct tunnel_ctx *tc, const char
*chassis_id)
>  }
>
>  /*
> - * Returns a tunnel-id of the form 'chassis_id'-delimiter-'encap_ip'.
> + * Returns a tunnel-id of the form chassis_id@remote_encap_ip
%local_encap_ip.
>   */
>  char *
> -encaps_tunnel_id_create(const char *chassis_id, const char *encap_ip)
> +encaps_tunnel_id_create(const char *chassis_id, const char
*remote_encap_ip,
> +                        const char *local_encap_ip)
>  {
> -    return xasprintf("%s%c%s", chassis_id, OVN_MVTEP_CHASSISID_DELIM,
> -                     encap_ip);
> +    return xasprintf("%s%c%s%c%s", chassis_id, '@', remote_encap_ip,
> +                     '%', local_encap_ip);
>  }
>
>  /*
> - * Parses a 'tunnel_id' of the form <chassis_name><delimiter><IP>.
> + * Parses a 'tunnel_id' of the form <chassis_name>@<remote IP>%<local
IP>.
>   * If the 'chassis_id' argument is not NULL the function will allocate
memory
>   * and store the chassis_name part of the tunnel-id at '*chassis_id'.
> - * If the 'encap_ip' argument is not NULL the function will allocate
memory
> - * and store the encapsulation IP part of the tunnel-id at '*encap_ip'.
> + * Same for remote_encap_ip and local_encap_ip.
>   */
>  bool
>  encaps_tunnel_id_parse(const char *tunnel_id, char **chassis_id,
> -                       char **encap_ip)
> +                       char **remote_encap_ip, char **local_encap_ip)
>  {
> -    /* Find the delimiter.  Fail if there is no delimiter or if
<chassis_name>
> -     * or <IP> is the empty string.*/
> -    const char *d = strchr(tunnel_id, OVN_MVTEP_CHASSISID_DELIM);
> +    /* Find the @.  Fail if there is no @ or if any part is empty. */
> +    const char *d = strchr(tunnel_id, '@');
>      if (d == tunnel_id || !d || !d[1]) {
>          return false;
>      }
>
> +    /* Find the %.  Fail if there is no % or if any part is empty. */
> +    const char *d2 = strchr(d + 1, '%');
> +    if (d2 == d + 1 || !d2 || !d2[1]) {
> +        return false;
> +    }
> +
>      if (chassis_id) {
>          *chassis_id = xmemdup0(tunnel_id, d - tunnel_id);
>      }
> -    if (encap_ip) {
> -        *encap_ip = xstrdup(d + 1);
> +
> +    if (remote_encap_ip) {
> +        *remote_encap_ip = xmemdup0(d + 1, d2 - (d + 1));
> +    }
> +
> +    if (local_encap_ip) {
> +        *local_encap_ip = xstrdup(d2 + 1);
>      }
>      return true;
>  }
>
>  /*
> - * Returns true if 'tunnel_id' contains 'chassis_id' and, if specified,
the
> - * given 'encap_ip'. Returns false otherwise.
> + * Returns true if 'tunnel_id' in the format
> + *      <chassis_id>@<remote_encap_ip>%<local_encap_ip>
> + * contains 'chassis_id' and, if specified, the given 'remote_encap_ip'
and
> + * 'local_encap_ip'. Returns false otherwise.
>   */
>  bool
>  encaps_tunnel_id_match(const char *tunnel_id, const char *chassis_id,
> -                       const char *encap_ip)
> +                       const char *remote_encap_ip, const char
*local_encap_ip)
>  {
> -    while (*tunnel_id == *chassis_id) {
> -        if (!*tunnel_id) {
> -            /* 'tunnel_id' and 'chassis_id' are equal strings.  This is a
> -             * mismatch because 'tunnel_id' is missing the delimiter and
IP. */
> -            return false;
> -        }
> -        tunnel_id++;
> -        chassis_id++;
> +    char *tokstr = xstrdup(tunnel_id);
> +    char *saveptr = NULL;
> +    bool ret = false;
> +
> +    char *token_chassis = strtok_r(tokstr, "@", &saveptr);
> +    if (!token_chassis || strcmp(token_chassis, chassis_id)) {
> +        goto out;
> +    }
> +
> +    char *token_remote_ip = strtok_r(NULL, "%", &saveptr);
> +    if (remote_encap_ip &&
> +        (!token_remote_ip || strcmp(token_remote_ip, remote_encap_ip))) {
> +        goto out;
>      }
>
> -    /* We found the first byte that disagrees between 'tunnel_id' and
> -     * 'chassis_id'.  If we consumed all of 'chassis_id' and arrived at
the
> -     * delimiter in 'tunnel_id' (and if 'encap_ip' is correct, if it was
> -     * supplied), it's a match. */
> -    return (*tunnel_id == OVN_MVTEP_CHASSISID_DELIM
> -            && *chassis_id == '\0'
> -            && (!encap_ip || !strcmp(tunnel_id + 1, encap_ip)));
> +    char *token_local_ip = strtok_r(NULL, "", &saveptr);
> +    if (local_encap_ip &&
> +        (!token_local_ip || strcmp(token_local_ip, local_encap_ip))) {
> +        goto out;
> +    }
> +
> +    ret = true;
> +out:
> +    free(tokstr);
> +    return ret;
>  }
>
>  static void
>  tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg,
>             const char *new_chassis_id, const struct sbrec_encap *encap,
> +           bool must_set_local_ip, const char *local_ip,
>             const struct ovsrec_open_vswitch_table *ovs_table)
>  {
>      struct smap options = SMAP_INITIALIZER(&options);
> @@ -173,10 +193,11 @@ tunnel_add(struct tunnel_ctx *tc, const struct
sbrec_sb_global *sbg,
>      /*
>       * Since a chassis may have multiple encap-ip, we can't just add the
>       * chassis name as the OVN_TUNNEL_ID for the port; we use the
> -     * combination of the chassis_name and the encap-ip to identify
> -     * a specific tunnel to the chassis.
> +     * combination of the chassis_name and the remote and local
encap-ips to
> +     * identify a specific tunnel to the remote chassis.
>       */
> -    tunnel_entry_id = encaps_tunnel_id_create(new_chassis_id, encap->ip);
> +    tunnel_entry_id = encaps_tunnel_id_create(new_chassis_id, encap->ip,
> +                                              local_ip);
>      if (csum && (!strcmp(csum, "true") || !strcmp(csum, "false"))) {
>          smap_add(&options, "csum", csum);
>      }
> @@ -187,7 +208,7 @@ tunnel_add(struct tunnel_ctx *tc, const struct
sbrec_sb_global *sbg,
>      const struct ovsrec_open_vswitch *cfg =
>          ovsrec_open_vswitch_table_first(ovs_table);
>
> -    bool set_local_ip = false;
> +    bool set_local_ip = must_set_local_ip;
>      if (cfg) {
>          /* If the tos option is configured, get it */
>          const char *encap_tos =
> @@ -208,11 +229,13 @@ tunnel_add(struct tunnel_ctx *tc, const struct
sbrec_sb_global *sbg,
>              smap_add(&options, "df_default", encap_df);
>          }
>
> -        /* If ovn-set-local-ip option is configured, get it */
> -        set_local_ip =
> -            get_chassis_external_id_value_bool(
> -                &cfg->external_ids, tc->this_chassis->name,
> -                "ovn-set-local-ip", false);
> +        if (!set_local_ip) {
> +            /* If ovn-set-local-ip option is configured, get it */
> +            set_local_ip =
> +                get_chassis_external_id_value_bool(
> +                    &cfg->external_ids, tc->this_chassis->name,
> +                    "ovn-set-local-ip", false);
> +        }
>      }
>
>      /* Add auth info if ipsec is enabled. */
> @@ -237,30 +260,7 @@ tunnel_add(struct tunnel_ctx *tc, const struct
sbrec_sb_global *sbg,
>      }
>
>      if (set_local_ip) {
> -        const struct sbrec_chassis *this_chassis = tc->this_chassis;
> -        const char *local_ip = NULL;
> -
> -        /* Determine 'ovn-encap-ip' of the local chassis as this will be
the
> -         * tunnel port's 'local_ip'. We do not support the case in which
> -         * 'ovn-encap-ip' holds multiple comma-delimited IP addresses.
> -         */
> -        for (int i = 0; i < this_chassis->n_encaps; i++) {
> -            if (local_ip && strcmp(local_ip,
this_chassis->encaps[i]->ip)) {
> -                static struct vlog_rate_limit rl =
VLOG_RATE_LIMIT_INIT(5, 1);
> -                VLOG_ERR_RL(&rl, "ovn-encap-ip has been configured as a
list. "
> -                            "This is unsupported for IPsec and explicit "
> -                            "local_ip configuration.");
> -                /* No need to loop further as we know this condition has
been
> -                 * hit */
> -                break;
> -            } else {
> -                local_ip = this_chassis->encaps[i]->ip;
> -            }
> -        }
> -
> -        if (local_ip) {
> -            smap_add(&options, "local_ip", local_ip);
> -        }
> +        smap_add(&options, "local_ip", local_ip);
>      }
>
>      /* If there's an existing tunnel record that does not need any
change,
> @@ -362,9 +362,29 @@ chassis_tunnel_add(const struct sbrec_chassis
*chassis_rec,
>          if (tun_type != pref_type) {
>              continue;
>          }
> -        tunnel_add(tc, sbg, chassis_rec->name, chassis_rec->encaps[i],
> -                   ovs_table);
> -        tuncnt++;
> +
> +        /* Check if need to pass the local ip. We always set local ip if
there
> +         * are multiple local IPs for the selected encap type. */
> +        int count = 0;
> +        bool set_local_ip = false;
> +        for (int j = 0; j < this_chassis->n_encaps; j++) {
> +            if (pref_type ==
get_tunnel_type(this_chassis->encaps[j]->type) &&
> +                count++ > 0) {
> +                set_local_ip = true;
> +                break;
> +            }
> +        }
> +
> +        for (int j = 0; j < this_chassis->n_encaps; j++) {
> +            if (pref_type !=
get_tunnel_type(this_chassis->encaps[j]->type)) {
> +                continue;
> +            }
> +            VLOG_DBG("tunnel_add: '%s', local ip: %s", chassis_rec->name,
> +                     this_chassis->encaps[j]->ip);
> +            tunnel_add(tc, sbg, chassis_rec->name,
chassis_rec->encaps[i],
> +                       set_local_ip, this_chassis->encaps[j]->ip,
ovs_table);
> +            tuncnt++;
> +        }
>      }
>      return tuncnt;
>  }
> diff --git a/controller/encaps.h b/controller/encaps.h
> index 3e58b3c82805..6f9891ee5907 100644
> --- a/controller/encaps.h
> +++ b/controller/encaps.h
> @@ -41,11 +41,14 @@ void encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
>  bool encaps_cleanup(struct ovsdb_idl_txn *ovs_idl_txn,
>                      const struct ovsrec_bridge *br_int);
>
> -char *encaps_tunnel_id_create(const char *chassis_id, const char
*encap_ip);
> +char *encaps_tunnel_id_create(const char *chassis_id,
> +                              const char *remote_encap_ip,
> +                              const char *local_encap_ip);
>  bool  encaps_tunnel_id_parse(const char *tunnel_id, char **chassis_id,
> -                             char **encap_ip);
> +                             char **remote_encap_ip, char
**local_encap_ip);
>  bool  encaps_tunnel_id_match(const char *tunnel_id, const char
*chassis_id,
> -                             const char *encap_ip);
> +                             const char *remote_encap_ip,
> +                             const char *local_encap_ip);
>
>  void encaps_destroy(void);
>
> diff --git a/controller/lflow.c b/controller/lflow.c
> index b0cf4253c482..c0cf0aa10646 100644
> --- a/controller/lflow.c
> +++ b/controller/lflow.c
> @@ -164,7 +164,7 @@ tunnel_ofport_cb(const void *aux_, const char
*port_name, ofp_port_t *ofport)
>      }
>
>      if (!get_chassis_tunnel_ofport(aux->chassis_tunnels,
pb->chassis->name,
> -                                   NULL, ofport)) {
> +                                   ofport)) {
>          return false;
>      }
>
> diff --git a/controller/local_data.c b/controller/local_data.c
> index 3a7d3afebe0b..a9092783958f 100644
> --- a/controller/local_data.c
> +++ b/controller/local_data.c
> @@ -388,7 +388,7 @@ local_nonvif_data_run(const struct ovsrec_bridge
*br_int,
>                                           "ovn-chassis-id");
>          if (tunnel_id && encaps_tunnel_id_match(tunnel_id,
>                                                  chassis_rec->name,
> -                                                NULL)) {
> +                                                NULL, NULL)) {
>              continue;
>          }
>
> @@ -439,7 +439,7 @@ local_nonvif_data_run(const struct ovsrec_bridge
*br_int,
>                  char *hash_id = NULL;
>                  char *ip = NULL;
>
> -                if (!encaps_tunnel_id_parse(tunnel_id, &hash_id, &ip)) {
> +                if (!encaps_tunnel_id_parse(tunnel_id, &hash_id, &ip,
NULL)) {
>                      continue;
>                  }
>                  struct chassis_tunnel *tun = xmalloc(sizeof *tun);
> @@ -477,11 +477,10 @@ local_nonvif_data_handle_ovs_iface_changes(
>
>  bool
>  get_chassis_tunnel_ofport(const struct hmap *chassis_tunnels,
> -                          const char *chassis_name, char *encap_ip,
> -                          ofp_port_t *ofport)
> +                          const char *chassis_name, ofp_port_t *ofport)
>  {
>      struct chassis_tunnel *tun = NULL;
> -    tun = chassis_tunnel_find(chassis_tunnels, chassis_name, encap_ip);
> +    tun = chassis_tunnel_find(chassis_tunnels, chassis_name, NULL, NULL);
>      if (!tun) {
>          return false;
>      }
> @@ -515,7 +514,7 @@ chassis_tunnels_destroy(struct hmap *chassis_tunnels)
>   */
>  struct chassis_tunnel *
>  chassis_tunnel_find(const struct hmap *chassis_tunnels, const char
*chassis_id,
> -                    char *encap_ip)
> +                    char *remote_encap_ip, char *local_encap_ip)
>  {
>      /*
>       * If the specific encap_ip is given, look for the chassisid_ip
entry,
> @@ -524,7 +523,8 @@ chassis_tunnel_find(const struct hmap
*chassis_tunnels, const char *chassis_id,
>      struct chassis_tunnel *tun = NULL;
>      HMAP_FOR_EACH_WITH_HASH (tun, hmap_node, hash_string(chassis_id, 0),
>                               chassis_tunnels) {
> -        if (encaps_tunnel_id_match(tun->chassis_id, chassis_id,
encap_ip)) {
> +        if (encaps_tunnel_id_match(tun->chassis_id, chassis_id,
> +                                   remote_encap_ip, local_encap_ip)) {
>              return tun;
>          }
>      }
> diff --git a/controller/local_data.h b/controller/local_data.h
> index f6d8f725f805..bab95bcc3824 100644
> --- a/controller/local_data.h
> +++ b/controller/local_data.h
> @@ -149,10 +149,11 @@ bool local_nonvif_data_handle_ovs_iface_changes(
>
>  struct chassis_tunnel *chassis_tunnel_find(const struct hmap
*chassis_tunnels,
>                                             const char *chassis_id,
> -                                           char *encap_ip);
> +                                           char *remote_encap_ip,
> +                                           char *local_encap_ip);
>
>  bool get_chassis_tunnel_ofport(const struct hmap *chassis_tunnels,
> -                               const char *chassis_name, char *encap_ip,
> +                               const char *chassis_name,
>                                 ofp_port_t *ofport);
>
>  void chassis_tunnels_destroy(struct hmap *chassis_tunnels);
> diff --git a/controller/physical.c b/controller/physical.c
> index eda0854410b6..e93bfbd2cffb 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -137,16 +137,18 @@ put_resubmit(uint8_t table_id, struct ofpbuf
*ofpacts)
>   * port.
>   */
>  static struct chassis_tunnel *
> -get_port_binding_tun(const struct sbrec_encap *encap,
> +get_port_binding_tun(const struct sbrec_encap *remote_encap,
> +                     const struct sbrec_encap *local_encap,
>                       const struct sbrec_chassis *chassis,
>                       const struct hmap *chassis_tunnels)
>  {
>      struct chassis_tunnel *tun = NULL;
> -    if (encap) {
> -        tun = chassis_tunnel_find(chassis_tunnels, chassis->name,
encap->ip);
> -    }
> +    tun = chassis_tunnel_find(chassis_tunnels, chassis->name,
> +                              remote_encap ? remote_encap->ip : NULL,
> +                              local_encap ? local_encap->ip : NULL);
> +
>      if (!tun) {
> -        tun = chassis_tunnel_find(chassis_tunnels, chassis->name, NULL);
> +        tun = chassis_tunnel_find(chassis_tunnels, chassis->name, NULL,
NULL);
>      }
>      return tun;
>  }
> @@ -335,7 +337,7 @@ get_remote_tunnels(const struct sbrec_port_binding
*binding,
>      ovs_list_init(tunnels);
>
>      if (binding->chassis && binding->chassis != chassis) {
> -        tun = get_port_binding_tun(binding->encap, binding->chassis,
> +        tun = get_port_binding_tun(binding->encap, NULL,
binding->chassis,
>                                     chassis_tunnels);
>          if (!tun) {
>              static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
1);
> @@ -356,7 +358,7 @@ get_remote_tunnels(const struct sbrec_port_binding
*binding,
>          }
>          const struct sbrec_encap *additional_encap;
>          additional_encap = find_additional_encap_for_chassis(binding,
chassis);
> -        tun = get_port_binding_tun(additional_encap,
> +        tun = get_port_binding_tun(additional_encap, NULL,
>                                     binding->additional_chassis[i],
>                                     chassis_tunnels);
>          if (!tun) {
> @@ -432,10 +434,10 @@ put_remote_port_redirect_overlay_ha_remote(
>              continue;
>          }
>          if (!tun) {
> -            tun = chassis_tunnel_find(chassis_tunnels, ch->name, NULL);
> +            tun = chassis_tunnel_find(chassis_tunnels, ch->name, NULL,
NULL);
>          } else {
>              struct chassis_tunnel *chassis_tunnel =
> -                chassis_tunnel_find(chassis_tunnels, ch->name, NULL);
> +                chassis_tunnel_find(chassis_tunnels, ch->name, NULL,
NULL);
>              if (chassis_tunnel &&
>                  tun->type != chassis_tunnel->type) {
>                  static struct vlog_rate_limit rl =
> @@ -469,7 +471,7 @@ put_remote_port_redirect_overlay_ha_remote(
>          if (!ch) {
>              continue;
>          }
> -        tun = chassis_tunnel_find(chassis_tunnels, ch->name, NULL);
> +        tun = chassis_tunnel_find(chassis_tunnels, ch->name, NULL, NULL);
>          if (!tun) {
>              continue;
>          }
> @@ -1930,7 +1932,7 @@ tunnel_to_chassis(enum mf_field_id mff_ovn_geneve,
>                    uint16_t outport, struct ofpbuf *remote_ofpacts)
>  {
>      const struct chassis_tunnel *tun
> -        = chassis_tunnel_find(chassis_tunnels, chassis_name, NULL);
> +        = chassis_tunnel_find(chassis_tunnels, chassis_name, NULL, NULL);
>      if (!tun) {
>          return;
>      }
> @@ -1953,7 +1955,7 @@ fanout_to_chassis(enum mf_field_id mff_ovn_geneve,
>      const struct chassis_tunnel *prev = NULL;
>      SSET_FOR_EACH (chassis_name, remote_chassis) {
>          const struct chassis_tunnel *tun
> -            = chassis_tunnel_find(chassis_tunnels, chassis_name, NULL);
> +            = chassis_tunnel_find(chassis_tunnels, chassis_name, NULL,
NULL);
>          if (!tun) {
>              continue;
>          }
> @@ -2492,7 +2494,7 @@ physical_run(struct physical_ctx *p_ctx,
>
>              if (!binding->chassis ||
>                  !encaps_tunnel_id_match(tun->chassis_id,
> -                                        binding->chassis->name, NULL)) {
> +                                        binding->chassis->name, NULL,
NULL)) {
>                  continue;
>              }
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 4992eab0892b..4e35af0fb074 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -5780,7 +5780,7 @@ get_localnet_vifs_l3gwports(
>          const char *tunnel_id = smap_get(&port_rec->external_ids,
>                                            "ovn-chassis-id");
>          if (tunnel_id &&
> -                encaps_tunnel_id_match(tunnel_id, chassis->name, NULL)) {
> +                encaps_tunnel_id_match(tunnel_id, chassis->name, NULL,
NULL)) {
>              continue;
>          }
>          const char *localnet = smap_get(&port_rec->external_ids,
> diff --git a/tests/ovn-ipsec.at b/tests/ovn-ipsec.at
> index f8df8d60e4b3..7579124db2e1 100644
> --- a/tests/ovn-ipsec.at
> +++ b/tests/ovn-ipsec.at
> @@ -58,52 +58,3 @@ AT_CHECK([as hv1 ovs-vsctl get Interface ovn-hv2-0
options:remote_name | tr -d '
>  AT_CHECK([as hv1 ovs-vsctl get Interface ovn-hv2-0
options:ipsec_encapsulation | tr -d '\n'], [0], [yes])
>
>  AT_CLEANUP
> -
> -AT_SETUP([ipsec -- unsupported multiple ovn-encap-ip values])
> -ovn_start
> -
> -# Configure the Northbound database
> -ovn-nbctl ls-add lsw0
> -
> -ovn-nbctl lsp-add lsw0 lp1
> -ovn-nbctl lsp-set-addresses lp1 "f0:00:00:00:00:01 10.1.1.1"
> -
> -ovn-nbctl lsp-add lsw0 lp2
> -ovn-nbctl lsp-set-addresses lp2 "f0:00:00:00:00:02 10.1.1.2"
> -
> -net_add n1               # Network to connect hv1 and hv2
> -
> -# Enable IPsec
> -ovn-nbctl set nb_global . ipsec=true
> -
> -# Create hypervisor hv1 connected to n1
> -sim_add hv1
> -as hv1
> -ovs-vsctl add-br br-phys
> -ovn_attach n1 br-phys 192.168.0.1
> -ovs-vsctl add-port br-int vif1 -- set Interface vif1
external-ids:iface-id=lp1
> -ovs-vsctl \
> -    -- set Open_vSwitch . external-ids:system-id=hv1 \
> -    -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
> -    -- set Open_vSwitch . external-ids:ovn-encap-ip="192.168.0.1,
192.169.0.1" \
> -    -- set Open_vSwitch . other_config:certificate=dummy-cert.pem \
> -    -- set Open_vSwitch . other_config:private_key=dummy-privkey.pem \
> -    -- set Open_vSwitch . other_config:ca_cert=dummy-cacert.pem
> -
> -# Create hypervisor hv2 connected to n1
> -sim_add hv2
> -as hv2
> -ovs-vsctl add-br br-phys
> -ovn_attach n1 br-phys 192.168.0.2
> -ovs-vsctl add-port br-int vif2 -- set Interface vif2
external-ids:iface-id=lp2
> -ovs-vsctl \
> -    -- set Open_vSwitch . external-ids:system-id=hv2 \
> -    -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
> -    -- set Open_vSwitch . external-ids:ovn-encap-ip="192.168.0.2,
192.169.0.2" \
> -    -- set Open_vSwitch . other_config:certificate=dummy-cert.pem \
> -    -- set Open_vSwitch . other_config:private_key=dummy-privkey.pem \
> -    -- set Open_vSwitch . other_config:ca_cert=dummy-cacert.pem
> -
> -OVS_WAIT_UNTIL([grep "ovn-encap-ip has been configured as a list. This
is unsupported for IPsec." hv1/ovn-controller.log])
> -
> -AT_CLEANUP
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 2dd46fd79452..243fe0b8246c 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -30333,6 +30333,54 @@ OVN_CLEANUP([hv1])
>  AT_CLEANUP
>  ])
>
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([multiple encap ips tunnel creation])
> +ovn_start
> +net_add n1
> +
> +# 2 HVs, each with 2 encap-ips.
> +for i in 1 2; do
> +    sim_add hv$i
> +    as hv$i
> +    ovs-vsctl add-br br-phys-$j
> +    ovn_attach n1 br-phys-$j 192.168.0.${i}1
> +    ovs-vsctl set open .
external_ids:ovn-encap-ip=192.168.0.${i}1,192.168.0.${i}2
> +done
> +
> +check ovn-nbctl --wait=hv sync
> +
> +check_tunnel_port() {
> +    local hv=$1
> +    local br=$2
> +    local id=$3
> +
> +    as $hv
> +    OVS_WAIT_UNTIL([
> +        test "$(ovs-vsctl --format=table --no-headings find port
external_ids:ovn-chassis-id="$id" | wc -l)" = "1"
> +    ])
> +    local tunnel_id=$(ovs-vsctl --bare --columns _uuid find port
external_ids:ovn-chassis-id="$id")
> +    AT_CHECK([ovs-vsctl --bare --columns ports find bridge name="$br" |
grep -q "$tunnel_id"])
> +}
> +
> +# Check that both chassis have tunnels.
> +# 'tunnel_id' in the format:
> +#   <chassis_id>@<remote_encap_ip>%<local_encap_ip>
> +check_tunnel_port hv1 br-int hv2@192.168.0.21%192.168.0.11
> +check_tunnel_port hv1 br-int hv2@192.168.0.22%192.168.0.11
> +check_tunnel_port hv1 br-int hv2@192.168.0.21%192.168.0.12
> +check_tunnel_port hv1 br-int hv2@192.168.0.22%192.168.0.12
> +
> +check_tunnel_port hv2 br-int hv1@192.168.0.11%192.168.0.21
> +check_tunnel_port hv2 br-int hv1@192.168.0.12%192.168.0.21
> +check_tunnel_port hv2 br-int hv1@192.168.0.11%192.168.0.22
> +check_tunnel_port hv2 br-int hv1@192.168.0.12%192.168.0.22
> +
> +OVN_CLEANUP([hv1],[hv2])
> +AT_CLEANUP
> +])
> +
> +
>  OVN_FOR_EACH_NORTHD([
>  AT_SETUP([Load Balancer LS hairpin OF flows])
>  ovn_start
> @@ -36174,14 +36222,14 @@ check_tunnel_port() {
>  }
>
>  # Check that both chassis have tunnel
> -check_tunnel_port hv1 br-int hv2@192.168.0.2
> -check_tunnel_port hv2 br-int hv1@192.168.0.1
> +check_tunnel_port hv1 br-int hv2@192.168.0.2%192.168.0.1
> +check_tunnel_port hv2 br-int hv1@192.168.0.1%192.168.0.2
>
>  # Stop ovn-controller on hv1
>  check as hv1 ovn-appctl -t ovn-controller exit --restart
>
>  # The tunnel should remain intact
> -check_tunnel_port hv1 br-int hv2@192.168.0.2
> +check_tunnel_port hv1 br-int hv2@192.168.0.2%192.168.0.1
>
>  # Change the bridge to br-int1 on hv1
>  as hv1
> @@ -36191,8 +36239,8 @@ start_daemon ovn-controller --verbose="encaps:dbg"
>  check ovn-nbctl --wait=hv sync
>
>  # Check that the tunnel was created on br-int1 instead
> -check_tunnel_port hv1 br-int1 hv2@192.168.0.2
> -check grep -q "Clearing old tunnel port \"ovn-hv2-0\" (hv2@192.168.0.2)
from bridge \"br-int\"" hv1/ovn-controller.log
> +check_tunnel_port hv1 br-int1 hv2@192.168.0.2%192.168.0.1
> +check grep -q "Clearing old tunnel port \"ovn-hv2-0\" (hv2@192.168.0.2%192.168.0.1)
from bridge \"br-int\"" hv1/ovn-controller.log
>
>  # Change the bridge to br-int1 on hv2
>  as hv2
> @@ -36203,21 +36251,21 @@ check ovn-nbctl --wait=hv sync
>
>
>  # Check that the tunnel was created on br-int1 instead
> -check_tunnel_port hv2 br-int1 hv1@192.168.0.1
> -check grep -q "Clearing old tunnel port \"ovn-hv1-0\" (hv1@192.168.0.1)
from bridge \"br-int\"" hv2/ovn-controller.log
> +check_tunnel_port hv2 br-int1 hv1@192.168.0.1%192.168.0.2
> +check grep -q "Clearing old tunnel port \"ovn-hv1-0\" (hv1@192.168.0.1%192.168.0.2)
from bridge \"br-int\"" hv2/ovn-controller.log
>
>  # Stop ovn-controller on hv1
>  check as hv1 ovn-appctl -t ovn-controller exit --restart
>
>  # The tunnel should remain intact
> -check_tunnel_port hv1 br-int1 hv2@192.168.0.2
> -prev_id=$(ovs-vsctl --bare --columns _uuid find port
external_ids:ovn-chassis-id="hv2@192.168.0.2")
> +check_tunnel_port hv1 br-int1 hv2@192.168.0.2%192.168.0.1
> +prev_id=$(ovs-vsctl --bare --columns _uuid find port
external_ids:ovn-chassis-id="hv2@192.168.0.2%192.168.0.1")
>
>  # Start the controller again
>  start_daemon ovn-controller --verbose="encaps:dbg"
>  check ovn-nbctl --wait=hv sync
> -check_tunnel_port hv1 br-int1 hv2@192.168.0.2
> -current_id=$(ovs-vsctl --bare --columns _uuid find port
external_ids:ovn-chassis-id="hv2@192.168.0.2")
> +check_tunnel_port hv1 br-int1 hv2@192.168.0.2%192.168.0.1
> +current_id=$(ovs-vsctl --bare --columns _uuid find port
external_ids:ovn-chassis-id="hv2@192.168.0.2%192.168.0.1")
>
>  # The tunnel should be the same after restart
>  check test "$current_id" = "$prev_id"
> @@ -36276,10 +36324,10 @@ check_tunnel_port() {
>      AT_CHECK([ovs-vsctl --bare --columns ports find bridge name="$br" |
grep -q "$tunnel_id"])
>  }
>
> -check_tunnel_port hv1 br-int hv2@192.168.0.2
> -check_tunnel_port hv1 br-int-2 hv1@192.168.0.1
> -prev_id1=$(ovs-vsctl --bare --columns _uuid find port
external_ids:ovn-chassis-id="hv1@192.168.0.1")
> -prev_id2=$(ovs-vsctl --bare --columns _uuid find port
external_ids:ovn-chassis-id="hv2@192.168.0.2")
> +check_tunnel_port hv1 br-int hv2@192.168.0.2%192.168.0.1
> +check_tunnel_port hv1 br-int-2 hv1@192.168.0.1%192.168.0.2
> +prev_id1=$(ovs-vsctl --bare --columns _uuid find port
external_ids:ovn-chassis-id="hv1@192.168.0.1%192.168.0.2")
> +prev_id2=$(ovs-vsctl --bare --columns _uuid find port
external_ids:ovn-chassis-id="hv2@192.168.0.2%192.168.0.1")
>
>  # The hv2 is running we can remove the override file
>  rm -f ${OVN_SYSCONFDIR}/system-id-override
> @@ -36300,13 +36348,13 @@ start_daemon ovn-controller
--verbose="encaps:dbg" \
>
>  check ovn-nbctl --wait=hv sync
>
> -check_tunnel_port hv1 br-int hv2@192.168.0.2
> -check_tunnel_port hv1 br-int-2 hv1@192.168.0.1
> -current_id1=$(ovs-vsctl --bare --columns _uuid find port
external_ids:ovn-chassis-id="hv1@192.168.0.1")
> -current_id2=$(ovs-vsctl --bare --columns _uuid find port
external_ids:ovn-chassis-id="hv2@192.168.0.2")
> +check_tunnel_port hv1 br-int hv2@192.168.0.2%192.168.0.1
> +check_tunnel_port hv1 br-int-2 hv1@192.168.0.1%192.168.0.2
> +current_id1=$(ovs-vsctl --bare --columns _uuid find port
external_ids:ovn-chassis-id="hv1@192.168.0.1%192.168.0.2")
> +current_id2=$(ovs-vsctl --bare --columns _uuid find port
external_ids:ovn-chassis-id="hv2@192.168.0.2%192.168.0.1")
>
>  # Check that restart of hv1 ovn-controller did not interfere with hv2
> -AT_CHECK([grep -q "Clearing old tunnel port \"ovn0-hv1-0\" (
hv1@192.168.0.1) from bridge \"br-int-2\"" hv1/ovn-controller.log], [1])
> +AT_CHECK([grep -q "Clearing old tunnel port \"ovn0-hv1-0\" (
hv1@192.168.0.1%192.168.0.2) from bridge \"br-int-2\""
hv1/ovn-controller.log], [1])
>  check test "$current_id1" = "$prev_id1"
>  check test "$current_id2" = "$prev_id2"
>
> --
> 2.38.1
>
Ales Musil Jan. 23, 2024, 1:21 p.m. UTC | #2 
On Wed, Jan 17, 2024 at 6:48 AM Han Zhou <hzhou@ovn.org> wrote:

> In commit dd527a283cd8, it created separate tunnels for different remote
> encap IPs of the same remote chassis, but when the current chassis is
> the one that has multiple encap IPs configured, it only uses the first
> encap IP. This patch creates separate tunnels taking into consideration
> the multiple encap IPs in current chassis and sets corresponding
> local_ip for each tunnel interface in such cases.
>
> Co-authored-by: Lei Huang <leih@ovn.org>
> Signed-off-by: Lei Huang <leih@ovn.org>
> Signed-off-by: Han Zhou <hzhou@ovn.org>
> ---
>  controller/bfd.c        |   4 +-
>  controller/encaps.c     | 158 ++++++++++++++++++++++------------------
>  controller/encaps.h     |   9 ++-
>  controller/lflow.c      |   2 +-
>  controller/local_data.c |  14 ++--
>  controller/local_data.h |   5 +-
>  controller/physical.c   |  28 +++----
>  controller/pinctrl.c    |   2 +-
>  tests/ovn-ipsec.at      |  49 -------------
>  tests/ovn.at            |  88 +++++++++++++++++-----
>  10 files changed, 192 insertions(+), 167 deletions(-)
>
> diff --git a/controller/bfd.c b/controller/bfd.c
> index cf011e382c6c..f24bfd063888 100644
> --- a/controller/bfd.c
> +++ b/controller/bfd.c
> @@ -75,7 +75,7 @@ bfd_calculate_active_tunnels(const struct ovsrec_bridge
> *br_int,
>                              char *chassis_name = NULL;
>
>                              if (encaps_tunnel_id_parse(id, &chassis_name,
> -                                                       NULL)) {
> +                                                       NULL, NULL)) {
>                                  if (!sset_contains(active_tunnels,
>                                                     chassis_name)) {
>                                      sset_add(active_tunnels,
> chassis_name);
> @@ -204,7 +204,7 @@ bfd_run(const struct ovsrec_interface_table
> *interface_table,
>
>              sset_add(&tunnels, port_name);
>
> -            if (encaps_tunnel_id_parse(tunnel_id, &chassis_name, NULL)) {
> +            if (encaps_tunnel_id_parse(tunnel_id, &chassis_name, NULL,
> NULL)) {
>                  if (sset_contains(&bfd_chassis, chassis_name)) {
>                      sset_add(&bfd_ifaces, port_name);
>                  }
> diff --git a/controller/encaps.c b/controller/encaps.c
> index 1f6e667a606c..28237f6191c8 100644
> --- a/controller/encaps.c
> +++ b/controller/encaps.c
> @@ -32,10 +32,9 @@ VLOG_DEFINE_THIS_MODULE(encaps);
>  /*
>   * Given there could be multiple tunnels with different IPs to the same
>   * chassis we annotate the external_ids:ovn-chassis-id in tunnel port with
> - * <chassis_name>OVN_MVTEP_CHASSISID_DELIM<IP>. The external_id key
> + * <chassis_name>@<remote IP>%<local IP>. The external_id key
>   * "ovn-chassis-id" is kept for backward compatibility.
>   */
> -#define        OVN_MVTEP_CHASSISID_DELIM '@'
>  #define OVN_TUNNEL_ID "ovn-chassis-id"
>
>  static char *current_br_int_name = NULL;
> @@ -95,72 +94,93 @@ tunnel_create_name(struct tunnel_ctx *tc, const char
> *chassis_id)
>  }
>
>  /*
> - * Returns a tunnel-id of the form 'chassis_id'-delimiter-'encap_ip'.
> + * Returns a tunnel-id of the form chassis_id@remote_encap_ip
> %local_encap_ip.
>   */
>  char *
> -encaps_tunnel_id_create(const char *chassis_id, const char *encap_ip)
> +encaps_tunnel_id_create(const char *chassis_id, const char
> *remote_encap_ip,
> +                        const char *local_encap_ip)
>  {
> -    return xasprintf("%s%c%s", chassis_id, OVN_MVTEP_CHASSISID_DELIM,
> -                     encap_ip);
> +    return xasprintf("%s%c%s%c%s", chassis_id, '@', remote_encap_ip,
> +                     '%', local_encap_ip);
>  }
>
>  /*
> - * Parses a 'tunnel_id' of the form <chassis_name><delimiter><IP>.
> + * Parses a 'tunnel_id' of the form <chassis_name>@<remote IP>%<local IP>.
>   * If the 'chassis_id' argument is not NULL the function will allocate
> memory
>   * and store the chassis_name part of the tunnel-id at '*chassis_id'.
> - * If the 'encap_ip' argument is not NULL the function will allocate
> memory
> - * and store the encapsulation IP part of the tunnel-id at '*encap_ip'.
> + * Same for remote_encap_ip and local_encap_ip.
>   */
>  bool
>  encaps_tunnel_id_parse(const char *tunnel_id, char **chassis_id,
> -                       char **encap_ip)
> +                       char **remote_encap_ip, char **local_encap_ip)
>  {
> -    /* Find the delimiter.  Fail if there is no delimiter or if
> <chassis_name>
> -     * or <IP> is the empty string.*/
> -    const char *d = strchr(tunnel_id, OVN_MVTEP_CHASSISID_DELIM);
> +    /* Find the @.  Fail if there is no @ or if any part is empty. */
> +    const char *d = strchr(tunnel_id, '@');
>      if (d == tunnel_id || !d || !d[1]) {
>          return false;
>      }
>
> +    /* Find the %.  Fail if there is no % or if any part is empty. */
> +    const char *d2 = strchr(d + 1, '%');
> +    if (d2 == d + 1 || !d2 || !d2[1]) {
> +        return false;
> +    }
> +
>      if (chassis_id) {
>          *chassis_id = xmemdup0(tunnel_id, d - tunnel_id);
>      }
> -    if (encap_ip) {
> -        *encap_ip = xstrdup(d + 1);
> +
> +    if (remote_encap_ip) {
> +        *remote_encap_ip = xmemdup0(d + 1, d2 - (d + 1));
> +    }
> +
> +    if (local_encap_ip) {
> +        *local_encap_ip = xstrdup(d2 + 1);
>      }
>      return true;
>  }
>
>  /*
> - * Returns true if 'tunnel_id' contains 'chassis_id' and, if specified,
> the
> - * given 'encap_ip'. Returns false otherwise.
> + * Returns true if 'tunnel_id' in the format
> + *      <chassis_id>@<remote_encap_ip>%<local_encap_ip>
> + * contains 'chassis_id' and, if specified, the given 'remote_encap_ip'
> and
> + * 'local_encap_ip'. Returns false otherwise.
>   */
>  bool
>  encaps_tunnel_id_match(const char *tunnel_id, const char *chassis_id,
> -                       const char *encap_ip)
> +                       const char *remote_encap_ip, const char
> *local_encap_ip)
>  {
> -    while (*tunnel_id == *chassis_id) {
> -        if (!*tunnel_id) {
> -            /* 'tunnel_id' and 'chassis_id' are equal strings.  This is a
> -             * mismatch because 'tunnel_id' is missing the delimiter and
> IP. */
> -            return false;
> -        }
> -        tunnel_id++;
> -        chassis_id++;
> +    char *tokstr = xstrdup(tunnel_id);
> +    char *saveptr = NULL;
> +    bool ret = false;
> +
> +    char *token_chassis = strtok_r(tokstr, "@", &saveptr);
> +    if (!token_chassis || strcmp(token_chassis, chassis_id)) {
> +        goto out;
> +    }
> +
> +    char *token_remote_ip = strtok_r(NULL, "%", &saveptr);
> +    if (remote_encap_ip &&
> +        (!token_remote_ip || strcmp(token_remote_ip, remote_encap_ip))) {
> +        goto out;
>      }
>
> -    /* We found the first byte that disagrees between 'tunnel_id' and
> -     * 'chassis_id'.  If we consumed all of 'chassis_id' and arrived at
> the
> -     * delimiter in 'tunnel_id' (and if 'encap_ip' is correct, if it was
> -     * supplied), it's a match. */
> -    return (*tunnel_id == OVN_MVTEP_CHASSISID_DELIM
> -            && *chassis_id == '\0'
> -            && (!encap_ip || !strcmp(tunnel_id + 1, encap_ip)));
> +    char *token_local_ip = strtok_r(NULL, "", &saveptr);
> +    if (local_encap_ip &&
> +        (!token_local_ip || strcmp(token_local_ip, local_encap_ip))) {
> +        goto out;
> +    }
> +
> +    ret = true;
> +out:
> +    free(tokstr);
> +    return ret;
>  }
>
>  static void
>  tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg,
>             const char *new_chassis_id, const struct sbrec_encap *encap,
> +           bool must_set_local_ip, const char *local_ip,
>             const struct ovsrec_open_vswitch_table *ovs_table)
>  {
>      struct smap options = SMAP_INITIALIZER(&options);
> @@ -173,10 +193,11 @@ tunnel_add(struct tunnel_ctx *tc, const struct
> sbrec_sb_global *sbg,
>      /*
>       * Since a chassis may have multiple encap-ip, we can't just add the
>       * chassis name as the OVN_TUNNEL_ID for the port; we use the
> -     * combination of the chassis_name and the encap-ip to identify
> -     * a specific tunnel to the chassis.
> +     * combination of the chassis_name and the remote and local encap-ips
> to
> +     * identify a specific tunnel to the remote chassis.
>       */
> -    tunnel_entry_id = encaps_tunnel_id_create(new_chassis_id, encap->ip);
> +    tunnel_entry_id = encaps_tunnel_id_create(new_chassis_id, encap->ip,
> +                                              local_ip);
>      if (csum && (!strcmp(csum, "true") || !strcmp(csum, "false"))) {
>          smap_add(&options, "csum", csum);
>      }
> @@ -187,7 +208,7 @@ tunnel_add(struct tunnel_ctx *tc, const struct
> sbrec_sb_global *sbg,
>      const struct ovsrec_open_vswitch *cfg =
>          ovsrec_open_vswitch_table_first(ovs_table);
>
> -    bool set_local_ip = false;
> +    bool set_local_ip = must_set_local_ip;
>      if (cfg) {
>          /* If the tos option is configured, get it */
>          const char *encap_tos =
> @@ -208,11 +229,13 @@ tunnel_add(struct tunnel_ctx *tc, const struct
> sbrec_sb_global *sbg,
>              smap_add(&options, "df_default", encap_df);
>          }
>
> -        /* If ovn-set-local-ip option is configured, get it */
> -        set_local_ip =
> -            get_chassis_external_id_value_bool(
> -                &cfg->external_ids, tc->this_chassis->name,
> -                "ovn-set-local-ip", false);
> +        if (!set_local_ip) {
> +            /* If ovn-set-local-ip option is configured, get it */
> +            set_local_ip =
> +                get_chassis_external_id_value_bool(
> +                    &cfg->external_ids, tc->this_chassis->name,
> +                    "ovn-set-local-ip", false);
> +        }
>      }
>
>      /* Add auth info if ipsec is enabled. */
> @@ -237,30 +260,7 @@ tunnel_add(struct tunnel_ctx *tc, const struct
> sbrec_sb_global *sbg,
>      }
>
>      if (set_local_ip) {
> -        const struct sbrec_chassis *this_chassis = tc->this_chassis;
> -        const char *local_ip = NULL;
> -
> -        /* Determine 'ovn-encap-ip' of the local chassis as this will be
> the
> -         * tunnel port's 'local_ip'. We do not support the case in which
> -         * 'ovn-encap-ip' holds multiple comma-delimited IP addresses.
> -         */
> -        for (int i = 0; i < this_chassis->n_encaps; i++) {
> -            if (local_ip && strcmp(local_ip,
> this_chassis->encaps[i]->ip)) {
> -                static struct vlog_rate_limit rl =
> VLOG_RATE_LIMIT_INIT(5, 1);
> -                VLOG_ERR_RL(&rl, "ovn-encap-ip has been configured as a
> list. "
> -                            "This is unsupported for IPsec and explicit "
> -                            "local_ip configuration.");
> -                /* No need to loop further as we know this condition has
> been
> -                 * hit */
> -                break;
> -            } else {
> -                local_ip = this_chassis->encaps[i]->ip;
> -            }
> -        }
> -
> -        if (local_ip) {
> -            smap_add(&options, "local_ip", local_ip);
> -        }
> +        smap_add(&options, "local_ip", local_ip);
>      }
>
>      /* If there's an existing tunnel record that does not need any change,
> @@ -362,9 +362,29 @@ chassis_tunnel_add(const struct sbrec_chassis
> *chassis_rec,
>          if (tun_type != pref_type) {
>              continue;
>          }
> -        tunnel_add(tc, sbg, chassis_rec->name, chassis_rec->encaps[i],
> -                   ovs_table);
> -        tuncnt++;
> +
> +        /* Check if need to pass the local ip. We always set local ip if
> there
> +         * are multiple local IPs for the selected encap type. */
> +        int count = 0;
> +        bool set_local_ip = false;
> +        for (int j = 0; j < this_chassis->n_encaps; j++) {
> +            if (pref_type ==
> get_tunnel_type(this_chassis->encaps[j]->type) &&
> +                count++ > 0) {
> +                set_local_ip = true;
> +                break;
> +            }
> +        }
> +
> +        for (int j = 0; j < this_chassis->n_encaps; j++) {
> +            if (pref_type !=
> get_tunnel_type(this_chassis->encaps[j]->type)) {
> +                continue;
> +            }
> +            VLOG_DBG("tunnel_add: '%s', local ip: %s", chassis_rec->name,
> +                     this_chassis->encaps[j]->ip);
> +            tunnel_add(tc, sbg, chassis_rec->name, chassis_rec->encaps[i],
> +                       set_local_ip, this_chassis->encaps[j]->ip,
> ovs_table);
> +            tuncnt++;
> +        }
>      }
>      return tuncnt;
>  }
> diff --git a/controller/encaps.h b/controller/encaps.h
> index 3e58b3c82805..6f9891ee5907 100644
> --- a/controller/encaps.h
> +++ b/controller/encaps.h
> @@ -41,11 +41,14 @@ void encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
>  bool encaps_cleanup(struct ovsdb_idl_txn *ovs_idl_txn,
>                      const struct ovsrec_bridge *br_int);
>
> -char *encaps_tunnel_id_create(const char *chassis_id, const char
> *encap_ip);
> +char *encaps_tunnel_id_create(const char *chassis_id,
> +                              const char *remote_encap_ip,
> +                              const char *local_encap_ip);
>  bool  encaps_tunnel_id_parse(const char *tunnel_id, char **chassis_id,
> -                             char **encap_ip);
> +                             char **remote_encap_ip, char
> **local_encap_ip);
>  bool  encaps_tunnel_id_match(const char *tunnel_id, const char
> *chassis_id,
> -                             const char *encap_ip);
> +                             const char *remote_encap_ip,
> +                             const char *local_encap_ip);
>
>  void encaps_destroy(void);
>
> diff --git a/controller/lflow.c b/controller/lflow.c
> index b0cf4253c482..c0cf0aa10646 100644
> --- a/controller/lflow.c
> +++ b/controller/lflow.c
> @@ -164,7 +164,7 @@ tunnel_ofport_cb(const void *aux_, const char
> *port_name, ofp_port_t *ofport)
>      }
>
>      if (!get_chassis_tunnel_ofport(aux->chassis_tunnels,
> pb->chassis->name,
> -                                   NULL, ofport)) {
> +                                   ofport)) {
>          return false;
>      }
>
> diff --git a/controller/local_data.c b/controller/local_data.c
> index 3a7d3afebe0b..a9092783958f 100644
> --- a/controller/local_data.c
> +++ b/controller/local_data.c
> @@ -388,7 +388,7 @@ local_nonvif_data_run(const struct ovsrec_bridge
> *br_int,
>                                           "ovn-chassis-id");
>          if (tunnel_id && encaps_tunnel_id_match(tunnel_id,
>                                                  chassis_rec->name,
> -                                                NULL)) {
> +                                                NULL, NULL)) {
>              continue;
>          }
>
> @@ -439,7 +439,7 @@ local_nonvif_data_run(const struct ovsrec_bridge
> *br_int,
>                  char *hash_id = NULL;
>                  char *ip = NULL;
>
> -                if (!encaps_tunnel_id_parse(tunnel_id, &hash_id, &ip)) {
> +                if (!encaps_tunnel_id_parse(tunnel_id, &hash_id, &ip,
> NULL)) {
>                      continue;
>                  }
>                  struct chassis_tunnel *tun = xmalloc(sizeof *tun);
> @@ -477,11 +477,10 @@ local_nonvif_data_handle_ovs_iface_changes(
>
>  bool
>  get_chassis_tunnel_ofport(const struct hmap *chassis_tunnels,
> -                          const char *chassis_name, char *encap_ip,
> -                          ofp_port_t *ofport)
> +                          const char *chassis_name, ofp_port_t *ofport)
>  {
>      struct chassis_tunnel *tun = NULL;
> -    tun = chassis_tunnel_find(chassis_tunnels, chassis_name, encap_ip);
> +    tun = chassis_tunnel_find(chassis_tunnels, chassis_name, NULL, NULL);
>      if (!tun) {
>          return false;
>      }
> @@ -515,7 +514,7 @@ chassis_tunnels_destroy(struct hmap *chassis_tunnels)
>   */
>  struct chassis_tunnel *
>  chassis_tunnel_find(const struct hmap *chassis_tunnels, const char
> *chassis_id,
> -                    char *encap_ip)
> +                    char *remote_encap_ip, char *local_encap_ip)
>  {
>      /*
>       * If the specific encap_ip is given, look for the chassisid_ip entry,
> @@ -524,7 +523,8 @@ chassis_tunnel_find(const struct hmap
> *chassis_tunnels, const char *chassis_id,
>      struct chassis_tunnel *tun = NULL;
>      HMAP_FOR_EACH_WITH_HASH (tun, hmap_node, hash_string(chassis_id, 0),
>                               chassis_tunnels) {
> -        if (encaps_tunnel_id_match(tun->chassis_id, chassis_id,
> encap_ip)) {
> +        if (encaps_tunnel_id_match(tun->chassis_id, chassis_id,
> +                                   remote_encap_ip, local_encap_ip)) {
>              return tun;
>          }
>      }
> diff --git a/controller/local_data.h b/controller/local_data.h
> index f6d8f725f805..bab95bcc3824 100644
> --- a/controller/local_data.h
> +++ b/controller/local_data.h
> @@ -149,10 +149,11 @@ bool local_nonvif_data_handle_ovs_iface_changes(
>
>  struct chassis_tunnel *chassis_tunnel_find(const struct hmap
> *chassis_tunnels,
>                                             const char *chassis_id,
> -                                           char *encap_ip);
> +                                           char *remote_encap_ip,
> +                                           char *local_encap_ip);
>
>  bool get_chassis_tunnel_ofport(const struct hmap *chassis_tunnels,
> -                               const char *chassis_name, char *encap_ip,
> +                               const char *chassis_name,
>                                 ofp_port_t *ofport);
>
>  void chassis_tunnels_destroy(struct hmap *chassis_tunnels);
> diff --git a/controller/physical.c b/controller/physical.c
> index eda0854410b6..e93bfbd2cffb 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -137,16 +137,18 @@ put_resubmit(uint8_t table_id, struct ofpbuf
> *ofpacts)
>   * port.
>   */
>  static struct chassis_tunnel *
> -get_port_binding_tun(const struct sbrec_encap *encap,
> +get_port_binding_tun(const struct sbrec_encap *remote_encap,
> +                     const struct sbrec_encap *local_encap,
>                       const struct sbrec_chassis *chassis,
>                       const struct hmap *chassis_tunnels)
>  {
>      struct chassis_tunnel *tun = NULL;
> -    if (encap) {
> -        tun = chassis_tunnel_find(chassis_tunnels, chassis->name,
> encap->ip);
> -    }
> +    tun = chassis_tunnel_find(chassis_tunnels, chassis->name,
> +                              remote_encap ? remote_encap->ip : NULL,
> +                              local_encap ? local_encap->ip : NULL);
> +
>      if (!tun) {
> -        tun = chassis_tunnel_find(chassis_tunnels, chassis->name, NULL);
> +        tun = chassis_tunnel_find(chassis_tunnels, chassis->name, NULL,
> NULL);
>      }
>      return tun;
>  }
> @@ -335,7 +337,7 @@ get_remote_tunnels(const struct sbrec_port_binding
> *binding,
>      ovs_list_init(tunnels);
>
>      if (binding->chassis && binding->chassis != chassis) {
> -        tun = get_port_binding_tun(binding->encap, binding->chassis,
> +        tun = get_port_binding_tun(binding->encap, NULL, binding->chassis,
>                                     chassis_tunnels);
>          if (!tun) {
>              static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> @@ -356,7 +358,7 @@ get_remote_tunnels(const struct sbrec_port_binding
> *binding,
>          }
>          const struct sbrec_encap *additional_encap;
>          additional_encap = find_additional_encap_for_chassis(binding,
> chassis);
> -        tun = get_port_binding_tun(additional_encap,
> +        tun = get_port_binding_tun(additional_encap, NULL,
>                                     binding->additional_chassis[i],
>                                     chassis_tunnels);
>          if (!tun) {
> @@ -432,10 +434,10 @@ put_remote_port_redirect_overlay_ha_remote(
>              continue;
>          }
>          if (!tun) {
> -            tun = chassis_tunnel_find(chassis_tunnels, ch->name, NULL);
> +            tun = chassis_tunnel_find(chassis_tunnels, ch->name, NULL,
> NULL);
>          } else {
>              struct chassis_tunnel *chassis_tunnel =
> -                chassis_tunnel_find(chassis_tunnels, ch->name, NULL);
> +                chassis_tunnel_find(chassis_tunnels, ch->name, NULL,
> NULL);
>              if (chassis_tunnel &&
>                  tun->type != chassis_tunnel->type) {
>                  static struct vlog_rate_limit rl =
> @@ -469,7 +471,7 @@ put_remote_port_redirect_overlay_ha_remote(
>          if (!ch) {
>              continue;
>          }
> -        tun = chassis_tunnel_find(chassis_tunnels, ch->name, NULL);
> +        tun = chassis_tunnel_find(chassis_tunnels, ch->name, NULL, NULL);
>          if (!tun) {
>              continue;
>          }
> @@ -1930,7 +1932,7 @@ tunnel_to_chassis(enum mf_field_id mff_ovn_geneve,
>                    uint16_t outport, struct ofpbuf *remote_ofpacts)
>  {
>      const struct chassis_tunnel *tun
> -        = chassis_tunnel_find(chassis_tunnels, chassis_name, NULL);
> +        = chassis_tunnel_find(chassis_tunnels, chassis_name, NULL, NULL);
>      if (!tun) {
>          return;
>      }
> @@ -1953,7 +1955,7 @@ fanout_to_chassis(enum mf_field_id mff_ovn_geneve,
>      const struct chassis_tunnel *prev = NULL;
>      SSET_FOR_EACH (chassis_name, remote_chassis) {
>          const struct chassis_tunnel *tun
> -            = chassis_tunnel_find(chassis_tunnels, chassis_name, NULL);
> +            = chassis_tunnel_find(chassis_tunnels, chassis_name, NULL,
> NULL);
>          if (!tun) {
>              continue;
>          }
> @@ -2492,7 +2494,7 @@ physical_run(struct physical_ctx *p_ctx,
>
>              if (!binding->chassis ||
>                  !encaps_tunnel_id_match(tun->chassis_id,
> -                                        binding->chassis->name, NULL)) {
> +                                        binding->chassis->name, NULL,
> NULL)) {
>                  continue;
>              }
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 4992eab0892b..4e35af0fb074 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -5780,7 +5780,7 @@ get_localnet_vifs_l3gwports(
>          const char *tunnel_id = smap_get(&port_rec->external_ids,
>                                            "ovn-chassis-id");
>          if (tunnel_id &&
> -                encaps_tunnel_id_match(tunnel_id, chassis->name, NULL)) {
> +                encaps_tunnel_id_match(tunnel_id, chassis->name, NULL,
> NULL)) {
>              continue;
>          }
>          const char *localnet = smap_get(&port_rec->external_ids,
> diff --git a/tests/ovn-ipsec.at b/tests/ovn-ipsec.at
> index f8df8d60e4b3..7579124db2e1 100644
> --- a/tests/ovn-ipsec.at
> +++ b/tests/ovn-ipsec.at
> @@ -58,52 +58,3 @@ AT_CHECK([as hv1 ovs-vsctl get Interface ovn-hv2-0
> options:remote_name | tr -d '
>  AT_CHECK([as hv1 ovs-vsctl get Interface ovn-hv2-0
> options:ipsec_encapsulation | tr -d '\n'], [0], [yes])
>
>  AT_CLEANUP
> -
> -AT_SETUP([ipsec -- unsupported multiple ovn-encap-ip values])
> -ovn_start
> -
> -# Configure the Northbound database
> -ovn-nbctl ls-add lsw0
> -
> -ovn-nbctl lsp-add lsw0 lp1
> -ovn-nbctl lsp-set-addresses lp1 "f0:00:00:00:00:01 10.1.1.1"
> -
> -ovn-nbctl lsp-add lsw0 lp2
> -ovn-nbctl lsp-set-addresses lp2 "f0:00:00:00:00:02 10.1.1.2"
> -
> -net_add n1               # Network to connect hv1 and hv2
> -
> -# Enable IPsec
> -ovn-nbctl set nb_global . ipsec=true
> -
> -# Create hypervisor hv1 connected to n1
> -sim_add hv1
> -as hv1
> -ovs-vsctl add-br br-phys
> -ovn_attach n1 br-phys 192.168.0.1
> -ovs-vsctl add-port br-int vif1 -- set Interface vif1
> external-ids:iface-id=lp1
> -ovs-vsctl \
> -    -- set Open_vSwitch . external-ids:system-id=hv1 \
> -    -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
> -    -- set Open_vSwitch . external-ids:ovn-encap-ip="192.168.0.1,
> 192.169.0.1" \
> -    -- set Open_vSwitch . other_config:certificate=dummy-cert.pem \
> -    -- set Open_vSwitch . other_config:private_key=dummy-privkey.pem \
> -    -- set Open_vSwitch . other_config:ca_cert=dummy-cacert.pem
> -
> -# Create hypervisor hv2 connected to n1
> -sim_add hv2
> -as hv2
> -ovs-vsctl add-br br-phys
> -ovn_attach n1 br-phys 192.168.0.2
> -ovs-vsctl add-port br-int vif2 -- set Interface vif2
> external-ids:iface-id=lp2
> -ovs-vsctl \
> -    -- set Open_vSwitch . external-ids:system-id=hv2 \
> -    -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
> -    -- set Open_vSwitch . external-ids:ovn-encap-ip="192.168.0.2,
> 192.169.0.2" \
> -    -- set Open_vSwitch . other_config:certificate=dummy-cert.pem \
> -    -- set Open_vSwitch . other_config:private_key=dummy-privkey.pem \
> -    -- set Open_vSwitch . other_config:ca_cert=dummy-cacert.pem
> -
> -OVS_WAIT_UNTIL([grep "ovn-encap-ip has been configured as a list. This is
> unsupported for IPsec." hv1/ovn-controller.log])
> -
> -AT_CLEANUP
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 2dd46fd79452..243fe0b8246c 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -30333,6 +30333,54 @@ OVN_CLEANUP([hv1])
>  AT_CLEANUP
>  ])
>
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([multiple encap ips tunnel creation])
> +ovn_start
> +net_add n1
> +
> +# 2 HVs, each with 2 encap-ips.
> +for i in 1 2; do
> +    sim_add hv$i
> +    as hv$i
> +    ovs-vsctl add-br br-phys-$j
> +    ovn_attach n1 br-phys-$j 192.168.0.${i}1
> +    ovs-vsctl set open .
> external_ids:ovn-encap-ip=192.168.0.${i}1,192.168.0.${i}2
> +done
> +
> +check ovn-nbctl --wait=hv sync
> +
> +check_tunnel_port() {
> +    local hv=$1
> +    local br=$2
> +    local id=$3
> +
> +    as $hv
> +    OVS_WAIT_UNTIL([
> +        test "$(ovs-vsctl --format=table --no-headings find port
> external_ids:ovn-chassis-id="$id" | wc -l)" = "1"
> +    ])
> +    local tunnel_id=$(ovs-vsctl --bare --columns _uuid find port
> external_ids:ovn-chassis-id="$id")
> +    AT_CHECK([ovs-vsctl --bare --columns ports find bridge name="$br" |
> grep -q "$tunnel_id"])
> +}
> +
> +# Check that both chassis have tunnels.
> +# 'tunnel_id' in the format:
> +#   <chassis_id>@<remote_encap_ip>%<local_encap_ip>
> +check_tunnel_port hv1 br-int hv2@192.168.0.21%192.168.0.11
> +check_tunnel_port hv1 br-int hv2@192.168.0.22%192.168.0.11
> +check_tunnel_port hv1 br-int hv2@192.168.0.21%192.168.0.12
> +check_tunnel_port hv1 br-int hv2@192.168.0.22%192.168.0.12
> +
> +check_tunnel_port hv2 br-int hv1@192.168.0.11%192.168.0.21
> +check_tunnel_port hv2 br-int hv1@192.168.0.12%192.168.0.21
> +check_tunnel_port hv2 br-int hv1@192.168.0.11%192.168.0.22
> +check_tunnel_port hv2 br-int hv1@192.168.0.12%192.168.0.22
> +
> +OVN_CLEANUP([hv1],[hv2])
> +AT_CLEANUP
> +])
> +
> +
>  OVN_FOR_EACH_NORTHD([
>  AT_SETUP([Load Balancer LS hairpin OF flows])
>  ovn_start
> @@ -36174,14 +36222,14 @@ check_tunnel_port() {
>  }
>
>  # Check that both chassis have tunnel
> -check_tunnel_port hv1 br-int hv2@192.168.0.2
> -check_tunnel_port hv2 br-int hv1@192.168.0.1
> +check_tunnel_port hv1 br-int hv2@192.168.0.2%192.168.0.1
> +check_tunnel_port hv2 br-int hv1@192.168.0.1%192.168.0.2
>
>  # Stop ovn-controller on hv1
>  check as hv1 ovn-appctl -t ovn-controller exit --restart
>
>  # The tunnel should remain intact
> -check_tunnel_port hv1 br-int hv2@192.168.0.2
> +check_tunnel_port hv1 br-int hv2@192.168.0.2%192.168.0.1
>
>  # Change the bridge to br-int1 on hv1
>  as hv1
> @@ -36191,8 +36239,8 @@ start_daemon ovn-controller --verbose="encaps:dbg"
>  check ovn-nbctl --wait=hv sync
>
>  # Check that the tunnel was created on br-int1 instead
> -check_tunnel_port hv1 br-int1 hv2@192.168.0.2
> -check grep -q "Clearing old tunnel port \"ovn-hv2-0\" (hv2@192.168.0.2)
> from bridge \"br-int\"" hv1/ovn-controller.log
> +check_tunnel_port hv1 br-int1 hv2@192.168.0.2%192.168.0.1
> +check grep -q "Clearing old tunnel port \"ovn-hv2-0\" (hv2@192.168.0.2%192.168.0.1)
> from bridge \"br-int\"" hv1/ovn-controller.log
>
>  # Change the bridge to br-int1 on hv2
>  as hv2
> @@ -36203,21 +36251,21 @@ check ovn-nbctl --wait=hv sync
>
>
>  # Check that the tunnel was created on br-int1 instead
> -check_tunnel_port hv2 br-int1 hv1@192.168.0.1
> -check grep -q "Clearing old tunnel port \"ovn-hv1-0\" (hv1@192.168.0.1)
> from bridge \"br-int\"" hv2/ovn-controller.log
> +check_tunnel_port hv2 br-int1 hv1@192.168.0.1%192.168.0.2
> +check grep -q "Clearing old tunnel port \"ovn-hv1-0\" (hv1@192.168.0.1%192.168.0.2)
> from bridge \"br-int\"" hv2/ovn-controller.log
>
>  # Stop ovn-controller on hv1
>  check as hv1 ovn-appctl -t ovn-controller exit --restart
>
>  # The tunnel should remain intact
> -check_tunnel_port hv1 br-int1 hv2@192.168.0.2
> -prev_id=$(ovs-vsctl --bare --columns _uuid find port
> external_ids:ovn-chassis-id="hv2@192.168.0.2")
> +check_tunnel_port hv1 br-int1 hv2@192.168.0.2%192.168.0.1
> +prev_id=$(ovs-vsctl --bare --columns _uuid find port
> external_ids:ovn-chassis-id="hv2@192.168.0.2%192.168.0.1")
>
>  # Start the controller again
>  start_daemon ovn-controller --verbose="encaps:dbg"
>  check ovn-nbctl --wait=hv sync
> -check_tunnel_port hv1 br-int1 hv2@192.168.0.2
> -current_id=$(ovs-vsctl --bare --columns _uuid find port
> external_ids:ovn-chassis-id="hv2@192.168.0.2")
> +check_tunnel_port hv1 br-int1 hv2@192.168.0.2%192.168.0.1
> +current_id=$(ovs-vsctl --bare --columns _uuid find port
> external_ids:ovn-chassis-id="hv2@192.168.0.2%192.168.0.1")
>
>  # The tunnel should be the same after restart
>  check test "$current_id" = "$prev_id"
> @@ -36276,10 +36324,10 @@ check_tunnel_port() {
>      AT_CHECK([ovs-vsctl --bare --columns ports find bridge name="$br" |
> grep -q "$tunnel_id"])
>  }
>
> -check_tunnel_port hv1 br-int hv2@192.168.0.2
> -check_tunnel_port hv1 br-int-2 hv1@192.168.0.1
> -prev_id1=$(ovs-vsctl --bare --columns _uuid find port
> external_ids:ovn-chassis-id="hv1@192.168.0.1")
> -prev_id2=$(ovs-vsctl --bare --columns _uuid find port
> external_ids:ovn-chassis-id="hv2@192.168.0.2")
> +check_tunnel_port hv1 br-int hv2@192.168.0.2%192.168.0.1
> +check_tunnel_port hv1 br-int-2 hv1@192.168.0.1%192.168.0.2
> +prev_id1=$(ovs-vsctl --bare --columns _uuid find port
> external_ids:ovn-chassis-id="hv1@192.168.0.1%192.168.0.2")
> +prev_id2=$(ovs-vsctl --bare --columns _uuid find port
> external_ids:ovn-chassis-id="hv2@192.168.0.2%192.168.0.1")
>
>  # The hv2 is running we can remove the override file
>  rm -f ${OVN_SYSCONFDIR}/system-id-override
> @@ -36300,13 +36348,13 @@ start_daemon ovn-controller
> --verbose="encaps:dbg" \
>
>  check ovn-nbctl --wait=hv sync
>
> -check_tunnel_port hv1 br-int hv2@192.168.0.2
> -check_tunnel_port hv1 br-int-2 hv1@192.168.0.1
> -current_id1=$(ovs-vsctl --bare --columns _uuid find port
> external_ids:ovn-chassis-id="hv1@192.168.0.1")
> -current_id2=$(ovs-vsctl --bare --columns _uuid find port
> external_ids:ovn-chassis-id="hv2@192.168.0.2")
> +check_tunnel_port hv1 br-int hv2@192.168.0.2%192.168.0.1
> +check_tunnel_port hv1 br-int-2 hv1@192.168.0.1%192.168.0.2
> +current_id1=$(ovs-vsctl --bare --columns _uuid find port
> external_ids:ovn-chassis-id="hv1@192.168.0.1%192.168.0.2")
> +current_id2=$(ovs-vsctl --bare --columns _uuid find port
> external_ids:ovn-chassis-id="hv2@192.168.0.2%192.168.0.1")
>
>  # Check that restart of hv1 ovn-controller did not interfere with hv2
> -AT_CHECK([grep -q "Clearing old tunnel port \"ovn0-hv1-0\" (
> hv1@192.168.0.1) from bridge \"br-int-2\"" hv1/ovn-controller.log], [1])
> +AT_CHECK([grep -q "Clearing old tunnel port \"ovn0-hv1-0\" (
> hv1@192.168.0.1%192.168.0.2) from bridge \"br-int-2\""
> hv1/ovn-controller.log], [1])
>  check test "$current_id1" = "$prev_id1"
>  check test "$current_id2" = "$prev_id2"
>
> --
> 2.38.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Looks good to me, thanks.

Acked-by: Ales Musil <amusil@redhat.com>
diff mbox series

Patch

diff --git a/controller/bfd.c b/controller/bfd.c
index cf011e382c6c..f24bfd063888 100644
--- a/controller/bfd.c
+++ b/controller/bfd.c
@@ -75,7 +75,7 @@  bfd_calculate_active_tunnels(const struct ovsrec_bridge *br_int,
                             char *chassis_name = NULL;
 
                             if (encaps_tunnel_id_parse(id, &chassis_name,
-                                                       NULL)) {
+                                                       NULL, NULL)) {
                                 if (!sset_contains(active_tunnels,
                                                    chassis_name)) {
                                     sset_add(active_tunnels, chassis_name);
@@ -204,7 +204,7 @@  bfd_run(const struct ovsrec_interface_table *interface_table,
 
             sset_add(&tunnels, port_name);
 
-            if (encaps_tunnel_id_parse(tunnel_id, &chassis_name, NULL)) {
+            if (encaps_tunnel_id_parse(tunnel_id, &chassis_name, NULL, NULL)) {
                 if (sset_contains(&bfd_chassis, chassis_name)) {
                     sset_add(&bfd_ifaces, port_name);
                 }
diff --git a/controller/encaps.c b/controller/encaps.c
index 1f6e667a606c..28237f6191c8 100644
--- a/controller/encaps.c
+++ b/controller/encaps.c
@@ -32,10 +32,9 @@  VLOG_DEFINE_THIS_MODULE(encaps);
 /*
  * Given there could be multiple tunnels with different IPs to the same
  * chassis we annotate the external_ids:ovn-chassis-id in tunnel port with
- * <chassis_name>OVN_MVTEP_CHASSISID_DELIM<IP>. The external_id key
+ * <chassis_name>@<remote IP>%<local IP>. The external_id key
  * "ovn-chassis-id" is kept for backward compatibility.
  */
-#define	OVN_MVTEP_CHASSISID_DELIM '@'
 #define OVN_TUNNEL_ID "ovn-chassis-id"
 
 static char *current_br_int_name = NULL;
@@ -95,72 +94,93 @@  tunnel_create_name(struct tunnel_ctx *tc, const char *chassis_id)
 }
 
 /*
- * Returns a tunnel-id of the form 'chassis_id'-delimiter-'encap_ip'.
+ * Returns a tunnel-id of the form chassis_id@remote_encap_ip%local_encap_ip.
  */
 char *
-encaps_tunnel_id_create(const char *chassis_id, const char *encap_ip)
+encaps_tunnel_id_create(const char *chassis_id, const char *remote_encap_ip,
+                        const char *local_encap_ip)
 {
-    return xasprintf("%s%c%s", chassis_id, OVN_MVTEP_CHASSISID_DELIM,
-                     encap_ip);
+    return xasprintf("%s%c%s%c%s", chassis_id, '@', remote_encap_ip,
+                     '%', local_encap_ip);
 }
 
 /*
- * Parses a 'tunnel_id' of the form <chassis_name><delimiter><IP>.
+ * Parses a 'tunnel_id' of the form <chassis_name>@<remote IP>%<local IP>.
  * If the 'chassis_id' argument is not NULL the function will allocate memory
  * and store the chassis_name part of the tunnel-id at '*chassis_id'.
- * If the 'encap_ip' argument is not NULL the function will allocate memory
- * and store the encapsulation IP part of the tunnel-id at '*encap_ip'.
+ * Same for remote_encap_ip and local_encap_ip.
  */
 bool
 encaps_tunnel_id_parse(const char *tunnel_id, char **chassis_id,
-                       char **encap_ip)
+                       char **remote_encap_ip, char **local_encap_ip)
 {
-    /* Find the delimiter.  Fail if there is no delimiter or if <chassis_name>
-     * or <IP> is the empty string.*/
-    const char *d = strchr(tunnel_id, OVN_MVTEP_CHASSISID_DELIM);
+    /* Find the @.  Fail if there is no @ or if any part is empty. */
+    const char *d = strchr(tunnel_id, '@');
     if (d == tunnel_id || !d || !d[1]) {
         return false;
     }
 
+    /* Find the %.  Fail if there is no % or if any part is empty. */
+    const char *d2 = strchr(d + 1, '%');
+    if (d2 == d + 1 || !d2 || !d2[1]) {
+        return false;
+    }
+
     if (chassis_id) {
         *chassis_id = xmemdup0(tunnel_id, d - tunnel_id);
     }
-    if (encap_ip) {
-        *encap_ip = xstrdup(d + 1);
+
+    if (remote_encap_ip) {
+        *remote_encap_ip = xmemdup0(d + 1, d2 - (d + 1));
+    }
+
+    if (local_encap_ip) {
+        *local_encap_ip = xstrdup(d2 + 1);
     }
     return true;
 }
 
 /*
- * Returns true if 'tunnel_id' contains 'chassis_id' and, if specified, the
- * given 'encap_ip'. Returns false otherwise.
+ * Returns true if 'tunnel_id' in the format
+ *      <chassis_id>@<remote_encap_ip>%<local_encap_ip>
+ * contains 'chassis_id' and, if specified, the given 'remote_encap_ip' and
+ * 'local_encap_ip'. Returns false otherwise.
  */
 bool
 encaps_tunnel_id_match(const char *tunnel_id, const char *chassis_id,
-                       const char *encap_ip)
+                       const char *remote_encap_ip, const char *local_encap_ip)
 {
-    while (*tunnel_id == *chassis_id) {
-        if (!*tunnel_id) {
-            /* 'tunnel_id' and 'chassis_id' are equal strings.  This is a
-             * mismatch because 'tunnel_id' is missing the delimiter and IP. */
-            return false;
-        }
-        tunnel_id++;
-        chassis_id++;
+    char *tokstr = xstrdup(tunnel_id);
+    char *saveptr = NULL;
+    bool ret = false;
+
+    char *token_chassis = strtok_r(tokstr, "@", &saveptr);
+    if (!token_chassis || strcmp(token_chassis, chassis_id)) {
+        goto out;
+    }
+
+    char *token_remote_ip = strtok_r(NULL, "%", &saveptr);
+    if (remote_encap_ip &&
+        (!token_remote_ip || strcmp(token_remote_ip, remote_encap_ip))) {
+        goto out;
     }
 
-    /* We found the first byte that disagrees between 'tunnel_id' and
-     * 'chassis_id'.  If we consumed all of 'chassis_id' and arrived at the
-     * delimiter in 'tunnel_id' (and if 'encap_ip' is correct, if it was
-     * supplied), it's a match. */
-    return (*tunnel_id == OVN_MVTEP_CHASSISID_DELIM
-            && *chassis_id == '\0'
-            && (!encap_ip || !strcmp(tunnel_id + 1, encap_ip)));
+    char *token_local_ip = strtok_r(NULL, "", &saveptr);
+    if (local_encap_ip &&
+        (!token_local_ip || strcmp(token_local_ip, local_encap_ip))) {
+        goto out;
+    }
+
+    ret = true;
+out:
+    free(tokstr);
+    return ret;
 }
 
 static void
 tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg,
            const char *new_chassis_id, const struct sbrec_encap *encap,
+           bool must_set_local_ip, const char *local_ip,
            const struct ovsrec_open_vswitch_table *ovs_table)
 {
     struct smap options = SMAP_INITIALIZER(&options);
@@ -173,10 +193,11 @@  tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg,
     /*
      * Since a chassis may have multiple encap-ip, we can't just add the
      * chassis name as the OVN_TUNNEL_ID for the port; we use the
-     * combination of the chassis_name and the encap-ip to identify
-     * a specific tunnel to the chassis.
+     * combination of the chassis_name and the remote and local encap-ips to
+     * identify a specific tunnel to the remote chassis.
      */
-    tunnel_entry_id = encaps_tunnel_id_create(new_chassis_id, encap->ip);
+    tunnel_entry_id = encaps_tunnel_id_create(new_chassis_id, encap->ip,
+                                              local_ip);
     if (csum && (!strcmp(csum, "true") || !strcmp(csum, "false"))) {
         smap_add(&options, "csum", csum);
     }
@@ -187,7 +208,7 @@  tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg,
     const struct ovsrec_open_vswitch *cfg =
         ovsrec_open_vswitch_table_first(ovs_table);
 
-    bool set_local_ip = false;
+    bool set_local_ip = must_set_local_ip;
     if (cfg) {
         /* If the tos option is configured, get it */
         const char *encap_tos =
@@ -208,11 +229,13 @@  tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg,
             smap_add(&options, "df_default", encap_df);
         }
 
-        /* If ovn-set-local-ip option is configured, get it */
-        set_local_ip =
-            get_chassis_external_id_value_bool(
-                &cfg->external_ids, tc->this_chassis->name,
-                "ovn-set-local-ip", false);
+        if (!set_local_ip) {
+            /* If ovn-set-local-ip option is configured, get it */
+            set_local_ip =
+                get_chassis_external_id_value_bool(
+                    &cfg->external_ids, tc->this_chassis->name,
+                    "ovn-set-local-ip", false);
+        }
     }
 
     /* Add auth info if ipsec is enabled. */
@@ -237,30 +260,7 @@  tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg,
     }
 
     if (set_local_ip) {
-        const struct sbrec_chassis *this_chassis = tc->this_chassis;
-        const char *local_ip = NULL;
-
-        /* Determine 'ovn-encap-ip' of the local chassis as this will be the
-         * tunnel port's 'local_ip'. We do not support the case in which
-         * 'ovn-encap-ip' holds multiple comma-delimited IP addresses.
-         */
-        for (int i = 0; i < this_chassis->n_encaps; i++) {
-            if (local_ip && strcmp(local_ip, this_chassis->encaps[i]->ip)) {
-                static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
-                VLOG_ERR_RL(&rl, "ovn-encap-ip has been configured as a list. "
-                            "This is unsupported for IPsec and explicit "
-                            "local_ip configuration.");
-                /* No need to loop further as we know this condition has been
-                 * hit */
-                break;
-            } else {
-                local_ip = this_chassis->encaps[i]->ip;
-            }
-        }
-
-        if (local_ip) {
-            smap_add(&options, "local_ip", local_ip);
-        }
+        smap_add(&options, "local_ip", local_ip);
     }
 
     /* If there's an existing tunnel record that does not need any change,
@@ -362,9 +362,29 @@  chassis_tunnel_add(const struct sbrec_chassis *chassis_rec,
         if (tun_type != pref_type) {
             continue;
         }
-        tunnel_add(tc, sbg, chassis_rec->name, chassis_rec->encaps[i],
-                   ovs_table);
-        tuncnt++;
+
+        /* Check if need to pass the local ip. We always set local ip if there
+         * are multiple local IPs for the selected encap type. */
+        int count = 0;
+        bool set_local_ip = false;
+        for (int j = 0; j < this_chassis->n_encaps; j++) {
+            if (pref_type == get_tunnel_type(this_chassis->encaps[j]->type) &&
+                count++ > 0) {
+                set_local_ip = true;
+                break;
+            }
+        }
+
+        for (int j = 0; j < this_chassis->n_encaps; j++) {
+            if (pref_type != get_tunnel_type(this_chassis->encaps[j]->type)) {
+                continue;
+            }
+            VLOG_DBG("tunnel_add: '%s', local ip: %s", chassis_rec->name,
+                     this_chassis->encaps[j]->ip);
+            tunnel_add(tc, sbg, chassis_rec->name, chassis_rec->encaps[i],
+                       set_local_ip, this_chassis->encaps[j]->ip, ovs_table);
+            tuncnt++;
+        }
     }
     return tuncnt;
 }
diff --git a/controller/encaps.h b/controller/encaps.h
index 3e58b3c82805..6f9891ee5907 100644
--- a/controller/encaps.h
+++ b/controller/encaps.h
@@ -41,11 +41,14 @@  void encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
 bool encaps_cleanup(struct ovsdb_idl_txn *ovs_idl_txn,
                     const struct ovsrec_bridge *br_int);
 
-char *encaps_tunnel_id_create(const char *chassis_id, const char *encap_ip);
+char *encaps_tunnel_id_create(const char *chassis_id,
+                              const char *remote_encap_ip,
+                              const char *local_encap_ip);
 bool  encaps_tunnel_id_parse(const char *tunnel_id, char **chassis_id,
-                             char **encap_ip);
+                             char **remote_encap_ip, char **local_encap_ip);
 bool  encaps_tunnel_id_match(const char *tunnel_id, const char *chassis_id,
-                             const char *encap_ip);
+                             const char *remote_encap_ip,
+                             const char *local_encap_ip);
 
 void encaps_destroy(void);
 
diff --git a/controller/lflow.c b/controller/lflow.c
index b0cf4253c482..c0cf0aa10646 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -164,7 +164,7 @@  tunnel_ofport_cb(const void *aux_, const char *port_name, ofp_port_t *ofport)
     }
 
     if (!get_chassis_tunnel_ofport(aux->chassis_tunnels, pb->chassis->name,
-                                   NULL, ofport)) {
+                                   ofport)) {
         return false;
     }
 
diff --git a/controller/local_data.c b/controller/local_data.c
index 3a7d3afebe0b..a9092783958f 100644
--- a/controller/local_data.c
+++ b/controller/local_data.c
@@ -388,7 +388,7 @@  local_nonvif_data_run(const struct ovsrec_bridge *br_int,
                                          "ovn-chassis-id");
         if (tunnel_id && encaps_tunnel_id_match(tunnel_id,
                                                 chassis_rec->name,
-                                                NULL)) {
+                                                NULL, NULL)) {
             continue;
         }
 
@@ -439,7 +439,7 @@  local_nonvif_data_run(const struct ovsrec_bridge *br_int,
                 char *hash_id = NULL;
                 char *ip = NULL;
 
-                if (!encaps_tunnel_id_parse(tunnel_id, &hash_id, &ip)) {
+                if (!encaps_tunnel_id_parse(tunnel_id, &hash_id, &ip, NULL)) {
                     continue;
                 }
                 struct chassis_tunnel *tun = xmalloc(sizeof *tun);
@@ -477,11 +477,10 @@  local_nonvif_data_handle_ovs_iface_changes(
 
 bool
 get_chassis_tunnel_ofport(const struct hmap *chassis_tunnels,
-                          const char *chassis_name, char *encap_ip,
-                          ofp_port_t *ofport)
+                          const char *chassis_name, ofp_port_t *ofport)
 {
     struct chassis_tunnel *tun = NULL;
-    tun = chassis_tunnel_find(chassis_tunnels, chassis_name, encap_ip);
+    tun = chassis_tunnel_find(chassis_tunnels, chassis_name, NULL, NULL);
     if (!tun) {
         return false;
     }
@@ -515,7 +514,7 @@  chassis_tunnels_destroy(struct hmap *chassis_tunnels)
  */
 struct chassis_tunnel *
 chassis_tunnel_find(const struct hmap *chassis_tunnels, const char *chassis_id,
-                    char *encap_ip)
+                    char *remote_encap_ip, char *local_encap_ip)
 {
     /*
      * If the specific encap_ip is given, look for the chassisid_ip entry,
@@ -524,7 +523,8 @@  chassis_tunnel_find(const struct hmap *chassis_tunnels, const char *chassis_id,
     struct chassis_tunnel *tun = NULL;
     HMAP_FOR_EACH_WITH_HASH (tun, hmap_node, hash_string(chassis_id, 0),
                              chassis_tunnels) {
-        if (encaps_tunnel_id_match(tun->chassis_id, chassis_id, encap_ip)) {
+        if (encaps_tunnel_id_match(tun->chassis_id, chassis_id,
+                                   remote_encap_ip, local_encap_ip)) {
             return tun;
         }
     }
diff --git a/controller/local_data.h b/controller/local_data.h
index f6d8f725f805..bab95bcc3824 100644
--- a/controller/local_data.h
+++ b/controller/local_data.h
@@ -149,10 +149,11 @@  bool local_nonvif_data_handle_ovs_iface_changes(
 
 struct chassis_tunnel *chassis_tunnel_find(const struct hmap *chassis_tunnels,
                                            const char *chassis_id,
-                                           char *encap_ip);
+                                           char *remote_encap_ip,
+                                           char *local_encap_ip);
 
 bool get_chassis_tunnel_ofport(const struct hmap *chassis_tunnels,
-                               const char *chassis_name, char *encap_ip,
+                               const char *chassis_name,
                                ofp_port_t *ofport);
 
 void chassis_tunnels_destroy(struct hmap *chassis_tunnels);
diff --git a/controller/physical.c b/controller/physical.c
index eda0854410b6..e93bfbd2cffb 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -137,16 +137,18 @@  put_resubmit(uint8_t table_id, struct ofpbuf *ofpacts)
  * port.
  */
 static struct chassis_tunnel *
-get_port_binding_tun(const struct sbrec_encap *encap,
+get_port_binding_tun(const struct sbrec_encap *remote_encap,
+                     const struct sbrec_encap *local_encap,
                      const struct sbrec_chassis *chassis,
                      const struct hmap *chassis_tunnels)
 {
     struct chassis_tunnel *tun = NULL;
-    if (encap) {
-        tun = chassis_tunnel_find(chassis_tunnels, chassis->name, encap->ip);
-    }
+    tun = chassis_tunnel_find(chassis_tunnels, chassis->name,
+                              remote_encap ? remote_encap->ip : NULL,
+                              local_encap ? local_encap->ip : NULL);
+
     if (!tun) {
-        tun = chassis_tunnel_find(chassis_tunnels, chassis->name, NULL);
+        tun = chassis_tunnel_find(chassis_tunnels, chassis->name, NULL, NULL);
     }
     return tun;
 }
@@ -335,7 +337,7 @@  get_remote_tunnels(const struct sbrec_port_binding *binding,
     ovs_list_init(tunnels);
 
     if (binding->chassis && binding->chassis != chassis) {
-        tun = get_port_binding_tun(binding->encap, binding->chassis,
+        tun = get_port_binding_tun(binding->encap, NULL, binding->chassis,
                                    chassis_tunnels);
         if (!tun) {
             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
@@ -356,7 +358,7 @@  get_remote_tunnels(const struct sbrec_port_binding *binding,
         }
         const struct sbrec_encap *additional_encap;
         additional_encap = find_additional_encap_for_chassis(binding, chassis);
-        tun = get_port_binding_tun(additional_encap,
+        tun = get_port_binding_tun(additional_encap, NULL,
                                    binding->additional_chassis[i],
                                    chassis_tunnels);
         if (!tun) {
@@ -432,10 +434,10 @@  put_remote_port_redirect_overlay_ha_remote(
             continue;
         }
         if (!tun) {
-            tun = chassis_tunnel_find(chassis_tunnels, ch->name, NULL);
+            tun = chassis_tunnel_find(chassis_tunnels, ch->name, NULL, NULL);
         } else {
             struct chassis_tunnel *chassis_tunnel =
-                chassis_tunnel_find(chassis_tunnels, ch->name, NULL);
+                chassis_tunnel_find(chassis_tunnels, ch->name, NULL, NULL);
             if (chassis_tunnel &&
                 tun->type != chassis_tunnel->type) {
                 static struct vlog_rate_limit rl =
@@ -469,7 +471,7 @@  put_remote_port_redirect_overlay_ha_remote(
         if (!ch) {
             continue;
         }
-        tun = chassis_tunnel_find(chassis_tunnels, ch->name, NULL);
+        tun = chassis_tunnel_find(chassis_tunnels, ch->name, NULL, NULL);
         if (!tun) {
             continue;
         }
@@ -1930,7 +1932,7 @@  tunnel_to_chassis(enum mf_field_id mff_ovn_geneve,
                   uint16_t outport, struct ofpbuf *remote_ofpacts)
 {
     const struct chassis_tunnel *tun
-        = chassis_tunnel_find(chassis_tunnels, chassis_name, NULL);
+        = chassis_tunnel_find(chassis_tunnels, chassis_name, NULL, NULL);
     if (!tun) {
         return;
     }
@@ -1953,7 +1955,7 @@  fanout_to_chassis(enum mf_field_id mff_ovn_geneve,
     const struct chassis_tunnel *prev = NULL;
     SSET_FOR_EACH (chassis_name, remote_chassis) {
         const struct chassis_tunnel *tun
-            = chassis_tunnel_find(chassis_tunnels, chassis_name, NULL);
+            = chassis_tunnel_find(chassis_tunnels, chassis_name, NULL, NULL);
         if (!tun) {
             continue;
         }
@@ -2492,7 +2494,7 @@  physical_run(struct physical_ctx *p_ctx,
 
             if (!binding->chassis ||
                 !encaps_tunnel_id_match(tun->chassis_id,
-                                        binding->chassis->name, NULL)) {
+                                        binding->chassis->name, NULL, NULL)) {
                 continue;
             }
 
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 4992eab0892b..4e35af0fb074 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -5780,7 +5780,7 @@  get_localnet_vifs_l3gwports(
         const char *tunnel_id = smap_get(&port_rec->external_ids,
                                           "ovn-chassis-id");
         if (tunnel_id &&
-                encaps_tunnel_id_match(tunnel_id, chassis->name, NULL)) {
+                encaps_tunnel_id_match(tunnel_id, chassis->name, NULL, NULL)) {
             continue;
         }
         const char *localnet = smap_get(&port_rec->external_ids,
diff --git a/tests/ovn-ipsec.at b/tests/ovn-ipsec.at
index f8df8d60e4b3..7579124db2e1 100644
--- a/tests/ovn-ipsec.at
+++ b/tests/ovn-ipsec.at
@@ -58,52 +58,3 @@  AT_CHECK([as hv1 ovs-vsctl get Interface ovn-hv2-0 options:remote_name | tr -d '
 AT_CHECK([as hv1 ovs-vsctl get Interface ovn-hv2-0 options:ipsec_encapsulation | tr -d '\n'], [0], [yes])
 
 AT_CLEANUP
-
-AT_SETUP([ipsec -- unsupported multiple ovn-encap-ip values])
-ovn_start
-
-# Configure the Northbound database
-ovn-nbctl ls-add lsw0
-
-ovn-nbctl lsp-add lsw0 lp1
-ovn-nbctl lsp-set-addresses lp1 "f0:00:00:00:00:01 10.1.1.1"
-
-ovn-nbctl lsp-add lsw0 lp2
-ovn-nbctl lsp-set-addresses lp2 "f0:00:00:00:00:02 10.1.1.2"
-
-net_add n1               # Network to connect hv1 and hv2
-
-# Enable IPsec
-ovn-nbctl set nb_global . ipsec=true
-
-# Create hypervisor hv1 connected to n1
-sim_add hv1
-as hv1
-ovs-vsctl add-br br-phys
-ovn_attach n1 br-phys 192.168.0.1
-ovs-vsctl add-port br-int vif1 -- set Interface vif1 external-ids:iface-id=lp1
-ovs-vsctl \
-    -- set Open_vSwitch . external-ids:system-id=hv1 \
-    -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
-    -- set Open_vSwitch . external-ids:ovn-encap-ip="192.168.0.1, 192.169.0.1" \
-    -- set Open_vSwitch . other_config:certificate=dummy-cert.pem \
-    -- set Open_vSwitch . other_config:private_key=dummy-privkey.pem \
-    -- set Open_vSwitch . other_config:ca_cert=dummy-cacert.pem
-
-# Create hypervisor hv2 connected to n1
-sim_add hv2
-as hv2
-ovs-vsctl add-br br-phys
-ovn_attach n1 br-phys 192.168.0.2
-ovs-vsctl add-port br-int vif2 -- set Interface vif2 external-ids:iface-id=lp2
-ovs-vsctl \
-    -- set Open_vSwitch . external-ids:system-id=hv2 \
-    -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
-    -- set Open_vSwitch . external-ids:ovn-encap-ip="192.168.0.2, 192.169.0.2" \
-    -- set Open_vSwitch . other_config:certificate=dummy-cert.pem \
-    -- set Open_vSwitch . other_config:private_key=dummy-privkey.pem \
-    -- set Open_vSwitch . other_config:ca_cert=dummy-cacert.pem
-
-OVS_WAIT_UNTIL([grep "ovn-encap-ip has been configured as a list. This is unsupported for IPsec." hv1/ovn-controller.log])
-
-AT_CLEANUP
diff --git a/tests/ovn.at b/tests/ovn.at
index 2dd46fd79452..243fe0b8246c 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -30333,6 +30333,54 @@  OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
 
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([multiple encap ips tunnel creation])
+ovn_start
+net_add n1
+
+# 2 HVs, each with 2 encap-ips.
+for i in 1 2; do
+    sim_add hv$i
+    as hv$i
+    ovs-vsctl add-br br-phys-$j
+    ovn_attach n1 br-phys-$j 192.168.0.${i}1
+    ovs-vsctl set open . external_ids:ovn-encap-ip=192.168.0.${i}1,192.168.0.${i}2
+done
+
+check ovn-nbctl --wait=hv sync
+
+check_tunnel_port() {
+    local hv=$1
+    local br=$2
+    local id=$3
+
+    as $hv
+    OVS_WAIT_UNTIL([
+        test "$(ovs-vsctl --format=table --no-headings find port external_ids:ovn-chassis-id="$id" | wc -l)" = "1"
+    ])
+    local tunnel_id=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="$id")
+    AT_CHECK([ovs-vsctl --bare --columns ports find bridge name="$br" | grep -q "$tunnel_id"])
+}
+
+# Check that both chassis have tunnels.
+# 'tunnel_id' in the format:
+#   <chassis_id>@<remote_encap_ip>%<local_encap_ip>
+check_tunnel_port hv1 br-int hv2@192.168.0.21%192.168.0.11
+check_tunnel_port hv1 br-int hv2@192.168.0.22%192.168.0.11
+check_tunnel_port hv1 br-int hv2@192.168.0.21%192.168.0.12
+check_tunnel_port hv1 br-int hv2@192.168.0.22%192.168.0.12
+
+check_tunnel_port hv2 br-int hv1@192.168.0.11%192.168.0.21
+check_tunnel_port hv2 br-int hv1@192.168.0.12%192.168.0.21
+check_tunnel_port hv2 br-int hv1@192.168.0.11%192.168.0.22
+check_tunnel_port hv2 br-int hv1@192.168.0.12%192.168.0.22
+
+OVN_CLEANUP([hv1],[hv2])
+AT_CLEANUP
+])
+
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([Load Balancer LS hairpin OF flows])
 ovn_start
@@ -36174,14 +36222,14 @@  check_tunnel_port() {
 }
 
 # Check that both chassis have tunnel
-check_tunnel_port hv1 br-int hv2@192.168.0.2
-check_tunnel_port hv2 br-int hv1@192.168.0.1
+check_tunnel_port hv1 br-int hv2@192.168.0.2%192.168.0.1
+check_tunnel_port hv2 br-int hv1@192.168.0.1%192.168.0.2
 
 # Stop ovn-controller on hv1
 check as hv1 ovn-appctl -t ovn-controller exit --restart
 
 # The tunnel should remain intact
-check_tunnel_port hv1 br-int hv2@192.168.0.2
+check_tunnel_port hv1 br-int hv2@192.168.0.2%192.168.0.1
 
 # Change the bridge to br-int1 on hv1
 as hv1
@@ -36191,8 +36239,8 @@  start_daemon ovn-controller --verbose="encaps:dbg"
 check ovn-nbctl --wait=hv sync
 
 # Check that the tunnel was created on br-int1 instead
-check_tunnel_port hv1 br-int1 hv2@192.168.0.2
-check grep -q "Clearing old tunnel port \"ovn-hv2-0\" (hv2@192.168.0.2) from bridge \"br-int\"" hv1/ovn-controller.log
+check_tunnel_port hv1 br-int1 hv2@192.168.0.2%192.168.0.1
+check grep -q "Clearing old tunnel port \"ovn-hv2-0\" (hv2@192.168.0.2%192.168.0.1) from bridge \"br-int\"" hv1/ovn-controller.log
 
 # Change the bridge to br-int1 on hv2
 as hv2
@@ -36203,21 +36251,21 @@  check ovn-nbctl --wait=hv sync
 
 
 # Check that the tunnel was created on br-int1 instead
-check_tunnel_port hv2 br-int1 hv1@192.168.0.1
-check grep -q "Clearing old tunnel port \"ovn-hv1-0\" (hv1@192.168.0.1) from bridge \"br-int\"" hv2/ovn-controller.log
+check_tunnel_port hv2 br-int1 hv1@192.168.0.1%192.168.0.2
+check grep -q "Clearing old tunnel port \"ovn-hv1-0\" (hv1@192.168.0.1%192.168.0.2) from bridge \"br-int\"" hv2/ovn-controller.log
 
 # Stop ovn-controller on hv1
 check as hv1 ovn-appctl -t ovn-controller exit --restart
 
 # The tunnel should remain intact
-check_tunnel_port hv1 br-int1 hv2@192.168.0.2
-prev_id=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="hv2@192.168.0.2")
+check_tunnel_port hv1 br-int1 hv2@192.168.0.2%192.168.0.1
+prev_id=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="hv2@192.168.0.2%192.168.0.1")
 
 # Start the controller again
 start_daemon ovn-controller --verbose="encaps:dbg"
 check ovn-nbctl --wait=hv sync
-check_tunnel_port hv1 br-int1 hv2@192.168.0.2
-current_id=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="hv2@192.168.0.2")
+check_tunnel_port hv1 br-int1 hv2@192.168.0.2%192.168.0.1
+current_id=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="hv2@192.168.0.2%192.168.0.1")
 
 # The tunnel should be the same after restart
 check test "$current_id" = "$prev_id"
@@ -36276,10 +36324,10 @@  check_tunnel_port() {
     AT_CHECK([ovs-vsctl --bare --columns ports find bridge name="$br" | grep -q "$tunnel_id"])
 }
 
-check_tunnel_port hv1 br-int hv2@192.168.0.2
-check_tunnel_port hv1 br-int-2 hv1@192.168.0.1
-prev_id1=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="hv1@192.168.0.1")
-prev_id2=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="hv2@192.168.0.2")
+check_tunnel_port hv1 br-int hv2@192.168.0.2%192.168.0.1
+check_tunnel_port hv1 br-int-2 hv1@192.168.0.1%192.168.0.2
+prev_id1=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="hv1@192.168.0.1%192.168.0.2")
+prev_id2=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="hv2@192.168.0.2%192.168.0.1")
 
 # The hv2 is running we can remove the override file
 rm -f ${OVN_SYSCONFDIR}/system-id-override
@@ -36300,13 +36348,13 @@  start_daemon ovn-controller --verbose="encaps:dbg" \
 
 check ovn-nbctl --wait=hv sync
 
-check_tunnel_port hv1 br-int hv2@192.168.0.2
-check_tunnel_port hv1 br-int-2 hv1@192.168.0.1
-current_id1=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="hv1@192.168.0.1")
-current_id2=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="hv2@192.168.0.2")
+check_tunnel_port hv1 br-int hv2@192.168.0.2%192.168.0.1
+check_tunnel_port hv1 br-int-2 hv1@192.168.0.1%192.168.0.2
+current_id1=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="hv1@192.168.0.1%192.168.0.2")
+current_id2=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="hv2@192.168.0.2%192.168.0.1")
 
 # Check that restart of hv1 ovn-controller did not interfere with hv2
-AT_CHECK([grep -q "Clearing old tunnel port \"ovn0-hv1-0\" (hv1@192.168.0.1) from bridge \"br-int-2\"" hv1/ovn-controller.log], [1])
+AT_CHECK([grep -q "Clearing old tunnel port \"ovn0-hv1-0\" (hv1@192.168.0.1%192.168.0.2) from bridge \"br-int-2\"" hv1/ovn-controller.log], [1])
 check test "$current_id1" = "$prev_id1"
 check test "$current_id2" = "$prev_id2"