[ovs-dev,1/1] northd: Add requested-encap-ip for remote ports.

From: Lei Huang <leih@nvidia.com>

Use requested-encap-ip (with requested-chassis) to set Port_Binding.encap,
clear on removal, and prefer geneve when both types exist. Add a northd
test and document the ovn-k8s interconnect use case.

CC: Han Zhou <hzhou@ovn.org>
Signed-off-by: Lei Huang <leihuang.dev8@gmail.com>
---
 NEWS                |   3 ++
 northd/northd.c     | 116 +++++++++++++++++++++++++++++++++++++++++---
 ovn-nb.xml          |  18 +++++++
 tests/ovn-northd.at |  63 ++++++++++++++++++++++++
 4 files changed, 193 insertions(+), 7 deletions(-)

Bleep bloop.  Greetings Lei Huang, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Author Lei Huang <leih@nvidia.com> needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Lei Huang <leihuang.dev8@gmail.com>
Lines checked: 398, Warnings: 1, Errors: 1


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot

Hello Lei, thanks for the patch. I have some notes inline

On Tue, Feb 10, 2026 at 8:55 PM Lei Huang <leihuang.dev8@gmail.com> wrote:
>
> From: Lei Huang <leih@nvidia.com>
>
> Use requested-encap-ip (with requested-chassis) to set Port_Binding.encap,
> clear on removal, and prefer geneve when both types exist. Add a northd
> test and document the ovn-k8s interconnect use case.
>
> CC: Han Zhou <hzhou@ovn.org>
> Signed-off-by: Lei Huang <leihuang.dev8@gmail.com>

0-day Robot is complaining because the email in the Signed-off-by line
does not match the one in the From line.

> ---
>  NEWS                |   3 ++
>  northd/northd.c     | 116 +++++++++++++++++++++++++++++++++++++++++---
>  ovn-nb.xml          |  18 +++++++
>  tests/ovn-northd.at |  63 ++++++++++++++++++++++++
>  4 files changed, 193 insertions(+), 7 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 2a2b5e12d..040577519 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,5 +1,8 @@
>  Post v25.09.0
>  -------------
> +   - Added LSP/LRP option "requested-encap-ip" to let CMS request a specific
> +     SB Port_Binding encap IP (e.g., for remote transit ports in ovn-k8s
> +     interconnect mode).
>     - Added DNS query statistics tracking in ovn-controller using OVS coverage
>       counters. Statistics can be queried using "ovn-appctl -t ovn-controller
>       coverage/read-counter <counter_name>" or "coverage/show". Tracked metrics
> diff --git a/northd/northd.c b/northd/northd.c
> index b4bb4ba6d..3e8e9a994 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -2531,6 +2531,82 @@ ovn_port_update_sbrec_chassis(
>      free(requested_chassis_sb);
>  }
>
> +static void
> +encap_ip_map_init(struct shash *encap_by_ip,
> +                  const struct sbrec_chassis_table *sbrec_chassis_table)
> +{

I'm not 100% certain this encap_by_ip mapping is necessary.

The OVSDB IDL provides a way of creating indexes of tables. In this
case, you could create an ovsdb_idl_index that specifically allows for
you to look up encaps by chassis name and ip.  Then you can use this
index to find any matching encaps. In case this has more than one
match, you can prefer the one with the higher priority tunnel type.

However, if you have done some benchmarking and found that this
outperforms the IDL index method, then we can go with this instead.

> +    shash_init(encap_by_ip);
> +    if (!sbrec_chassis_table) {
> +        return;
> +    }
> +
> +    const struct sbrec_chassis *chassis;
> +    SBREC_CHASSIS_TABLE_FOR_EACH (chassis, sbrec_chassis_table) {
> +        for (size_t i = 0; i < chassis->n_encaps; i++) {
> +            const struct sbrec_encap *encap = chassis->encaps[i];
> +            if (!encap || !encap->ip || !encap->type || !encap->chassis_name) {

Since "type" and "ip" are indexes on the Encap table, it is not
possible for encap->ip or encap->type to be NULL.

> +                continue;
> +            }
> +
> +            enum chassis_tunnel_type tun_type = get_tunnel_type(encap->type);
> +            if (tun_type == TUNNEL_TYPE_INVALID) {
> +                continue;
> +            }
> +
> +            char *key = xasprintf("%s@%s", encap->chassis_name, encap->ip);

I recommend creating a function that creates this string. This way,
both encap_ip_map_init() and encap_ip_map_lookup() can use the same
function to create the string.

> +            struct shash_node *node = shash_find(encap_by_ip, key);
> +            if (!node) {
> +                shash_add_nocopy(encap_by_ip, key, (void *) encap);
> +            } else {
> +                free(key);
> +                const struct sbrec_encap *existing = node->data;
> +                /* Pick the highest-preference tunnel type (geneve > vxlan)
> +                 * when multiple encap types share the same chassis+IP. */
> +                if (get_tunnel_type(existing->type) < tun_type) {
> +                    node->data = (void *) encap;
> +                }
> +            }
> +        }
> +    }
> +}
> +
> +static const struct sbrec_encap *
> +encap_ip_map_lookup(const struct shash *encap_by_ip, const char *chassis_name,
> +                    const char *ip)
> +{
> +    if (!encap_by_ip || !chassis_name || !chassis_name[0] || !ip || !ip[0]) {

I don't know how it's possible for encap_by_ip to be NULL here. The
southbound schema for the Chassis table ensures that the chassis_name
is non-NULL and not zero-length. The only caller of
encap_ip_map_lookup() also does its own validity checks for ip before
calling.

Therefore, I think this NULL return is impossible to hit, and you can remove it.

Alternatively, you could remove the validation checks from the caller
of encap_ip_map_lookup() and just check the validity of ip here.

> +        return NULL;
> +    }
> +    char *key = xasprintf("%s@%s", chassis_name, ip);
> +    const struct sbrec_encap *encap = shash_find_data(encap_by_ip, key);
> +    free(key);
> +    return encap;
> +}
> +
> +static void
> +ovn_port_update_requested_encap(const struct shash *encap_by_ip,
> +                                const struct ovn_port *op)
> +{
> +    if (is_cr_port(op)) {
> +        return;
> +    }
> +
> +    /* requested-chassis is resolved into SB first; reuse that binding. */
> +    const struct smap *options = op->nbsp ? &op->nbsp->options
> +                                          : &op->nbrp->options;
> +    const char *requested_ip = smap_get(options, "requested-encap-ip");
> +    const struct sbrec_encap *encap = NULL;
> +    if (requested_ip && requested_ip[0] && op->sb->requested_chassis) {
> +        encap = encap_ip_map_lookup(encap_by_ip,
> +                                    op->sb->requested_chassis->name,
> +                                    requested_ip);
> +    }
> +
> +    if (op->sb->encap != encap) {
> +        sbrec_port_binding_set_encap(op->sb, encap);
> +    }
> +}
> +
>  static void
>  check_and_do_sb_mirror_deletion(const struct ovn_port *op)
>  {
> @@ -2601,6 +2677,7 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn,
>                        struct ovsdb_idl_index *sbrec_chassis_by_hostname,
>                        struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name,
>                        const struct sbrec_mirror_table *sbrec_mirror_table,
> +                      const struct shash *encap_by_ip,
>                        const struct ovn_port *op,
>                        unsigned long *queue_id_bitmap,
>                        struct sset *active_ha_chassis_grps)
> @@ -2937,6 +3014,10 @@ common:
>          sbrec_port_binding_set_tunnel_key(op->sb, op->tunnel_key);
>      }
>
> +    if (encap_by_ip) {

Under what circumstances would encap_by_ip be NULL here?

> +        ovn_port_update_requested_encap(encap_by_ip, op);
> +    }
> +
>      /* ovn-controller will update 'Port_Binding.up' only if it was explicitly
>       * set to 'false'.
>       */
> @@ -4096,6 +4177,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
>      const struct sbrec_mirror_table *sbrec_mirror_table,
>      const struct sbrec_mac_binding_table *sbrec_mac_binding_table,
>      const struct sbrec_ha_chassis_group_table *sbrec_ha_chassis_group_table,
> +    const struct sbrec_chassis_table *sbrec_chassis_table,
>      struct ovsdb_idl_index *sbrec_chassis_by_name,
>      struct ovsdb_idl_index *sbrec_chassis_by_hostname,
>      struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name,
> @@ -4113,6 +4195,9 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
>      struct sset active_ha_chassis_grps =
>          SSET_INITIALIZER(&active_ha_chassis_grps);
>
> +    struct shash encap_by_ip;
> +    encap_ip_map_init(&encap_by_ip, sbrec_chassis_table);
> +
>      /* Borrow ls_ports for joining NB and SB for both LSPs and LRPs.
>       * We will split them later. */
>      struct hmap *ports = ls_ports;
> @@ -4172,6 +4257,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
>                                sbrec_chassis_by_hostname,
>                                sbrec_ha_chassis_grp_by_name,
>                                sbrec_mirror_table,
> +                              &encap_by_ip,
>                                op, queue_id_bitmap,
>                                &active_ha_chassis_grps);
>          op->od->is_transit_router |= is_transit_router_port(op);
> @@ -4185,6 +4271,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
>                                sbrec_chassis_by_hostname,
>                                sbrec_ha_chassis_grp_by_name,
>                                sbrec_mirror_table,
> +                              &encap_by_ip,
>                                op, queue_id_bitmap,
>                                &active_ha_chassis_grps);
>          sbrec_port_binding_set_logical_port(op->sb, op->key);
> @@ -4215,6 +4302,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
>          cleanup_mac_bindings(sbrec_mac_binding_table, lr_datapaths, lr_ports);
>      }
>
> +    shash_destroy(&encap_by_ip);
>      tag_alloc_destroy(&tag_alloc_table);
>      bitmap_free(queue_id_bitmap);
>      cleanup_sb_ha_chassis_groups(sbrec_ha_chassis_group_table,
> @@ -4401,7 +4489,8 @@ ls_port_init(struct ovn_port *op, struct ovsdb_idl_txn *ovnsb_txn,
>               const struct sbrec_port_binding *sb,
>               const struct sbrec_mirror_table *sbrec_mirror_table,
>               struct ovsdb_idl_index *sbrec_chassis_by_name,
> -             struct ovsdb_idl_index *sbrec_chassis_by_hostname)
> +             struct ovsdb_idl_index *sbrec_chassis_by_hostname,
> +             const struct shash *encap_by_ip)
>  {
>      op->od = od;
>      parse_lsp_addrs(op);
> @@ -4431,6 +4520,7 @@ ls_port_init(struct ovn_port *op, struct ovsdb_idl_txn *ovnsb_txn,
>      }
>      ovn_port_update_sbrec(ovnsb_txn, sbrec_chassis_by_name,
>                            sbrec_chassis_by_hostname, NULL, sbrec_mirror_table,
> +                          encap_by_ip,
>                            op, NULL, NULL);
>      return true;
>  }
> @@ -4441,13 +4531,15 @@ ls_port_create(struct ovsdb_idl_txn *ovnsb_txn, struct hmap *ls_ports,
>                 struct ovn_datapath *od,
>                 const struct sbrec_mirror_table *sbrec_mirror_table,
>                 struct ovsdb_idl_index *sbrec_chassis_by_name,
> -               struct ovsdb_idl_index *sbrec_chassis_by_hostname)
> +               struct ovsdb_idl_index *sbrec_chassis_by_hostname,
> +               const struct shash *encap_by_ip)
>  {
>      struct ovn_port *op = ovn_port_create(ls_ports, key, nbsp, NULL,
>                                            NULL);
>      hmap_insert(&od->ports, &op->dp_node, hmap_node_hash(&op->key_node));
>      if (!ls_port_init(op, ovnsb_txn, od, NULL, sbrec_mirror_table,
> -                      sbrec_chassis_by_name, sbrec_chassis_by_hostname)) {
> +                      sbrec_chassis_by_name, sbrec_chassis_by_hostname,
> +                      encap_by_ip)) {
>          ovn_port_destroy(ls_ports, op);
>          return NULL;
>      }
> @@ -4462,14 +4554,16 @@ ls_port_reinit(struct ovn_port *op, struct ovsdb_idl_txn *ovnsb_txn,
>                  const struct sbrec_port_binding *sb,
>                  const struct sbrec_mirror_table *sbrec_mirror_table,
>                  struct ovsdb_idl_index *sbrec_chassis_by_name,
> -                struct ovsdb_idl_index *sbrec_chassis_by_hostname)
> +                struct ovsdb_idl_index *sbrec_chassis_by_hostname,
> +                const struct shash *encap_by_ip)
>  {
>      ovn_port_cleanup(op);
>      op->sb = sb;
>      ovn_port_set_nb(op, nbsp, NULL);
>      op->primary_port = op->cr_port = NULL;
>      return ls_port_init(op, ovnsb_txn, od, sb, sbrec_mirror_table,
> -                        sbrec_chassis_by_name, sbrec_chassis_by_hostname);
> +                        sbrec_chassis_by_name, sbrec_chassis_by_hostname,
> +                        encap_by_ip);
>  }
>
>  /* Returns true if the logical switch has changes which can be
> @@ -4632,6 +4726,9 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
>          return true;
>      }
>
> +    struct shash encap_by_ip;
> +    encap_ip_map_init(&encap_by_ip, ni->sbrec_chassis_table);
> +
>      bool ls_had_only_router_ports = (!vector_is_empty(&od->router_ports)
>              && (vector_len(&od->router_ports) == hmap_count(&od->ports)));
>
> @@ -4658,7 +4755,8 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
>                                      new_nbsp->name, new_nbsp, od,
>                                      ni->sbrec_mirror_table,
>                                      ni->sbrec_chassis_by_name,
> -                                    ni->sbrec_chassis_by_hostname);
> +                                    ni->sbrec_chassis_by_hostname,
> +                                    &encap_by_ip);
>                  if (!op) {
>                      goto fail;
>                  }
> @@ -4697,7 +4795,8 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
>                                      new_nbsp,
>                                      od, sb, ni->sbrec_mirror_table,
>                                      ni->sbrec_chassis_by_name,
> -                                    ni->sbrec_chassis_by_hostname)) {
> +                                    ni->sbrec_chassis_by_hostname,
> +                                    &encap_by_ip)) {
>                      if (sb) {
>                          sbrec_port_binding_delete(sb);
>                      }
> @@ -4798,9 +4897,11 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
>      }
>      sset_destroy(&created_or_deleted_ports);
>
> +    shash_destroy(&encap_by_ip);
>      return true;
>
>  fail:
> +    shash_destroy(&encap_by_ip);
>      destroy_tracked_ovn_ports(trk_lsps);
>      return false;
>  }
> @@ -20622,6 +20723,7 @@ ovnnb_db_run(struct northd_input *input_data,
>                  input_data->sbrec_mirror_table,
>                  input_data->sbrec_mac_binding_table,
>                  input_data->sbrec_ha_chassis_group_table,
> +                input_data->sbrec_chassis_table,
>                  input_data->sbrec_chassis_by_name,
>                  input_data->sbrec_chassis_by_hostname,
>                  input_data->sbrec_ha_chassis_grp_by_name,
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 1acbf202b..33bf8993a 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -1550,6 +1550,15 @@
>            </p>
>          </column>
>
> +        <column name="options" key="requested-encap-ip">
> +          Requests the encapsulation IP address for the port binding. If set,
> +          <code>ovn-northd</code> uses this IP to select the
> +          <ref table="Encap" db="OVN_Southbound"/> entry written to
> +          <ref table="Port_Binding" column="encap" db="OVN_Southbound"/>.
> +          This is intended for ports without a local OVS interface, e.g. remote
> +          transit switch ports in ovn-kubernetes interconnect mode.
> +        </column>
> +
>          <column name="options" key="activation-strategy">
>            If used with multiple chassis set in
>            <ref column="requested-chassis"/>, specifies an activation strategy
> @@ -4393,6 +4402,15 @@ or
>          </p>
>        </column>
>
> +      <column name="options" key="requested-encap-ip">
> +        Requests the encapsulation IP address for the port binding. If set,
> +        <code>ovn-northd</code> uses this IP to select the
> +        <ref table="Encap" db="OVN_Southbound"/> entry written to
> +        <ref table="Port_Binding" column="encap" db="OVN_Southbound"/>.
> +        This is intended for ports without a local OVS interface, e.g. remote
> +        transit router ports in ovn-kubernetes interconnect mode.
> +      </column>
> +
>        <column name="options" key="dynamic-routing-redistribute"
>                type='{"type": "string"}'>
>          <p>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 512e42036..9245ff639 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -2896,6 +2896,69 @@ OVN_CLEANUP_NORTHD
>  AT_CLEANUP
>  ])
>
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([check options:requested-encap-ip fills port binding encap col])
> +AT_KEYWORDS([requested encap ip])
> +ovn_start
> +
> +check_uuid ovn-sbctl \
> +    -- --id=@e11 create encap chassis_name=ch1 ip="192.168.1.1" type="geneve" \
> +    -- --id=@e12 create encap chassis_name=ch1 ip="192.168.1.2" type="geneve" \
> +    -- --id=@c1 create chassis name=ch1 encaps=@e11,@e12
> +check_uuid ovn-sbctl \
> +    -- --id=@e21 create encap chassis_name=ch2 ip="192.168.2.1" type="geneve" \
> +    -- --id=@e22 create encap chassis_name=ch2 ip="192.168.2.2" type="geneve" \
> +    -- --id=@c2 create chassis name=ch2 encaps=@e21,@e22
> +
> +wait_row_count Chassis 2
> +wait_row_count Encap 4
> +en11_uuid=`ovn-sbctl --bare --columns _uuid find Encap ip="192.168.1.1"`

Nit: for this and all other lines where you look up the UUID of the
encap, you can instead use the fetch_column() function provided by the
testsuite:

en11_uuid=$(fetch_column Encap _uuid ip="192.168.1.1")

> +en12_uuid=`ovn-sbctl --bare --columns _uuid find Encap ip="192.168.1.2"`
> +en21_uuid=`ovn-sbctl --bare --columns _uuid find Encap ip="192.168.2.1"`
> +en22_uuid=`ovn-sbctl --bare --columns _uuid find Encap ip="192.168.2.2"`
> +ovn-sbctl show
> +
> +echo "__file__:__line__: encap uuid: $en11_uuid, ip: 192.168.1.1"
> +echo "__file__:__line__: encap uuid: $en12_uuid, ip: 192.168.1.2"
> +echo "__file__:__line__: encap uuid: $en21_uuid, ip: 192.168.2.1"
> +echo "__file__:__line__: encap uuid: $en22_uuid, ip: 192.168.2.2"
> +
> +check ovn-nbctl --wait=sb ls-add ls1
> +check ovn-nbctl --wait=sb lsp-add ls1 lsp1
> +check ovn-nbctl --wait=sb lsp-add ls1 lsp2
> +ovn-nbctl show
> +
> +echo "options:requested-chassis is required to set options:requested-encap-ip"
> +check ovn-nbctl --wait=sb set logical-switch-port lsp1 \
> +    options:requested-encap-ip=192.168.1.1
> +check ovn-nbctl --wait=sb sync
> +wait_row_count Port_Binding 1 logical_port=lsp1 'encap=[[]]'
> +
> +# Now set both options
> +check ovn-nbctl --wait=sb set logical-switch-port lsp1 \
> +    options:requested-chassis=ch1 \
> +    options:requested-encap-ip=192.168.1.1
> +check ovn-nbctl --wait=sb set logical-switch-port lsp2 \
> +    options:requested-chassis=ch2 \
> +    options:requested-encap-ip=192.168.2.2
> +
> +wait_row_count Port_Binding 1 logical_port=lsp1 encap="$en11_uuid"
> +wait_row_count Port_Binding 1 logical_port=lsp2 encap="$en22_uuid"
> +
> +# remove options:requested-encap-ip from lsp1
> +check ovn-nbctl --wait=sb remove logical_switch_port lsp1 \
> +    options requested-encap-ip=192.168.1.1
> +wait_row_count Port_Binding 1 logical_port=lsp1 'encap=[[]]'
> +
> +# remove options:requested-chassis from lsp2
> +check ovn-nbctl --wait=sb remove logical_switch_port lsp2 \
> +    options requested-chassis=ch2
> +wait_row_count Port_Binding 1 logical_port=lsp2 'encap=[[]]'
> +

One thing this test does not validate is that geneve encaps take
priority over vxlan encaps.

> +OVN_CLEANUP_NORTHD
> +AT_CLEANUP
> +])
> +
>  OVN_FOR_EACH_NORTHD_NO_HV([
>  AT_SETUP([port requested-tnl-key])
>  AT_KEYWORDS([requested tnl tunnel key keys])
> --
> 2.39.5 (Apple Git-154)
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

Hi Mark,

Thanks for reviewing this patch. I’ve pushed v2 with the updates based on
your comments. Please take a look.

Lei


On Wed, Feb 18, 2026 at 12:59 PM Mark Michelson <mmichels@redhat.com> wrote:

> Hello Lei, thanks for the patch. I have some notes inline
>
> On Tue, Feb 10, 2026 at 8:55 PM Lei Huang <leihuang.dev8@gmail.com> wrote:
> >
> > From: Lei Huang <leih@nvidia.com>
> >
> > Use requested-encap-ip (with requested-chassis) to set
> Port_Binding.encap,
> > clear on removal, and prefer geneve when both types exist. Add a northd
> > test and document the ovn-k8s interconnect use case.
> >
> > CC: Han Zhou <hzhou@ovn.org>
> > Signed-off-by: Lei Huang <leihuang.dev8@gmail.com>
>
> 0-day Robot is complaining because the email in the Signed-off-by line
> does not match the one in the From line.
>
> > ---
> >  NEWS                |   3 ++
> >  northd/northd.c     | 116 +++++++++++++++++++++++++++++++++++++++++---
> >  ovn-nb.xml          |  18 +++++++
> >  tests/ovn-northd.at |  63 ++++++++++++++++++++++++
> >  4 files changed, 193 insertions(+), 7 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index 2a2b5e12d..040577519 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -1,5 +1,8 @@
> >  Post v25.09.0
> >  -------------
> > +   - Added LSP/LRP option "requested-encap-ip" to let CMS request a
> specific
> > +     SB Port_Binding encap IP (e.g., for remote transit ports in ovn-k8s
> > +     interconnect mode).
> >     - Added DNS query statistics tracking in ovn-controller using OVS
> coverage
> >       counters. Statistics can be queried using "ovn-appctl -t
> ovn-controller
> >       coverage/read-counter <counter_name>" or "coverage/show". Tracked
> metrics
> > diff --git a/northd/northd.c b/northd/northd.c
> > index b4bb4ba6d..3e8e9a994 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -2531,6 +2531,82 @@ ovn_port_update_sbrec_chassis(
> >      free(requested_chassis_sb);
> >  }
> >
> > +static void
> > +encap_ip_map_init(struct shash *encap_by_ip,
> > +                  const struct sbrec_chassis_table *sbrec_chassis_table)
> > +{
>
> I'm not 100% certain this encap_by_ip mapping is necessary.
>
> The OVSDB IDL provides a way of creating indexes of tables. In this
> case, you could create an ovsdb_idl_index that specifically allows for
> you to look up encaps by chassis name and ip.  Then you can use this
> index to find any matching encaps. In case this has more than one
> match, you can prefer the one with the higher priority tunnel type.
>
> However, if you have done some benchmarking and found that this
> outperforms the IDL index method, then we can go with this instead.
>
> > +    shash_init(encap_by_ip);
> > +    if (!sbrec_chassis_table) {
> > +        return;
> > +    }
> > +
> > +    const struct sbrec_chassis *chassis;
> > +    SBREC_CHASSIS_TABLE_FOR_EACH (chassis, sbrec_chassis_table) {
> > +        for (size_t i = 0; i < chassis->n_encaps; i++) {
> > +            const struct sbrec_encap *encap = chassis->encaps[i];
> > +            if (!encap || !encap->ip || !encap->type ||
> !encap->chassis_name) {
>
> Since "type" and "ip" are indexes on the Encap table, it is not
> possible for encap->ip or encap->type to be NULL.
>
> > +                continue;
> > +            }
> > +
> > +            enum chassis_tunnel_type tun_type =
> get_tunnel_type(encap->type);
> > +            if (tun_type == TUNNEL_TYPE_INVALID) {
> > +                continue;
> > +            }
> > +
> > +            char *key = xasprintf("%s@%s", encap->chassis_name,
> encap->ip);
>
> I recommend creating a function that creates this string. This way,
> both encap_ip_map_init() and encap_ip_map_lookup() can use the same
> function to create the string.
>
> > +            struct shash_node *node = shash_find(encap_by_ip, key);
> > +            if (!node) {
> > +                shash_add_nocopy(encap_by_ip, key, (void *) encap);
> > +            } else {
> > +                free(key);
> > +                const struct sbrec_encap *existing = node->data;
> > +                /* Pick the highest-preference tunnel type (geneve >
> vxlan)
> > +                 * when multiple encap types share the same chassis+IP.
> */
> > +                if (get_tunnel_type(existing->type) < tun_type) {
> > +                    node->data = (void *) encap;
> > +                }
> > +            }
> > +        }
> > +    }
> > +}
> > +
> > +static const struct sbrec_encap *
> > +encap_ip_map_lookup(const struct shash *encap_by_ip, const char
> *chassis_name,
> > +                    const char *ip)
> > +{
> > +    if (!encap_by_ip || !chassis_name || !chassis_name[0] || !ip ||
> !ip[0]) {
>
> I don't know how it's possible for encap_by_ip to be NULL here. The
> southbound schema for the Chassis table ensures that the chassis_name
> is non-NULL and not zero-length. The only caller of
> encap_ip_map_lookup() also does its own validity checks for ip before
> calling.
>
> Therefore, I think this NULL return is impossible to hit, and you can
> remove it.
>
> Alternatively, you could remove the validation checks from the caller
> of encap_ip_map_lookup() and just check the validity of ip here.
>
> > +        return NULL;
> > +    }
> > +    char *key = xasprintf("%s@%s", chassis_name, ip);
> > +    const struct sbrec_encap *encap = shash_find_data(encap_by_ip, key);
> > +    free(key);
> > +    return encap;
> > +}
> > +
> > +static void
> > +ovn_port_update_requested_encap(const struct shash *encap_by_ip,
> > +                                const struct ovn_port *op)
> > +{
> > +    if (is_cr_port(op)) {
> > +        return;
> > +    }
> > +
> > +    /* requested-chassis is resolved into SB first; reuse that binding.
> */
> > +    const struct smap *options = op->nbsp ? &op->nbsp->options
> > +                                          : &op->nbrp->options;
> > +    const char *requested_ip = smap_get(options, "requested-encap-ip");
> > +    const struct sbrec_encap *encap = NULL;
> > +    if (requested_ip && requested_ip[0] && op->sb->requested_chassis) {
> > +        encap = encap_ip_map_lookup(encap_by_ip,
> > +                                    op->sb->requested_chassis->name,
> > +                                    requested_ip);
> > +    }
> > +
> > +    if (op->sb->encap != encap) {
> > +        sbrec_port_binding_set_encap(op->sb, encap);
> > +    }
> > +}
> > +
> >  static void
> >  check_and_do_sb_mirror_deletion(const struct ovn_port *op)
> >  {
> > @@ -2601,6 +2677,7 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn
> *ovnsb_txn,
> >                        struct ovsdb_idl_index *sbrec_chassis_by_hostname,
> >                        struct ovsdb_idl_index
> *sbrec_ha_chassis_grp_by_name,
> >                        const struct sbrec_mirror_table
> *sbrec_mirror_table,
> > +                      const struct shash *encap_by_ip,
> >                        const struct ovn_port *op,
> >                        unsigned long *queue_id_bitmap,
> >                        struct sset *active_ha_chassis_grps)
> > @@ -2937,6 +3014,10 @@ common:
> >          sbrec_port_binding_set_tunnel_key(op->sb, op->tunnel_key);
> >      }
> >
> > +    if (encap_by_ip) {
>
> Under what circumstances would encap_by_ip be NULL here?
>
> > +        ovn_port_update_requested_encap(encap_by_ip, op);
> > +    }
> > +
> >      /* ovn-controller will update 'Port_Binding.up' only if it was
> explicitly
> >       * set to 'false'.
> >       */
> > @@ -4096,6 +4177,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
> >      const struct sbrec_mirror_table *sbrec_mirror_table,
> >      const struct sbrec_mac_binding_table *sbrec_mac_binding_table,
> >      const struct sbrec_ha_chassis_group_table
> *sbrec_ha_chassis_group_table,
> > +    const struct sbrec_chassis_table *sbrec_chassis_table,
> >      struct ovsdb_idl_index *sbrec_chassis_by_name,
> >      struct ovsdb_idl_index *sbrec_chassis_by_hostname,
> >      struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name,
> > @@ -4113,6 +4195,9 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
> >      struct sset active_ha_chassis_grps =
> >          SSET_INITIALIZER(&active_ha_chassis_grps);
> >
> > +    struct shash encap_by_ip;
> > +    encap_ip_map_init(&encap_by_ip, sbrec_chassis_table);
> > +
> >      /* Borrow ls_ports for joining NB and SB for both LSPs and LRPs.
> >       * We will split them later. */
> >      struct hmap *ports = ls_ports;
> > @@ -4172,6 +4257,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
> >                                sbrec_chassis_by_hostname,
> >                                sbrec_ha_chassis_grp_by_name,
> >                                sbrec_mirror_table,
> > +                              &encap_by_ip,
> >                                op, queue_id_bitmap,
> >                                &active_ha_chassis_grps);
> >          op->od->is_transit_router |= is_transit_router_port(op);
> > @@ -4185,6 +4271,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
> >                                sbrec_chassis_by_hostname,
> >                                sbrec_ha_chassis_grp_by_name,
> >                                sbrec_mirror_table,
> > +                              &encap_by_ip,
> >                                op, queue_id_bitmap,
> >                                &active_ha_chassis_grps);
> >          sbrec_port_binding_set_logical_port(op->sb, op->key);
> > @@ -4215,6 +4302,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
> >          cleanup_mac_bindings(sbrec_mac_binding_table, lr_datapaths,
> lr_ports);
> >      }
> >
> > +    shash_destroy(&encap_by_ip);
> >      tag_alloc_destroy(&tag_alloc_table);
> >      bitmap_free(queue_id_bitmap);
> >      cleanup_sb_ha_chassis_groups(sbrec_ha_chassis_group_table,
> > @@ -4401,7 +4489,8 @@ ls_port_init(struct ovn_port *op, struct
> ovsdb_idl_txn *ovnsb_txn,
> >               const struct sbrec_port_binding *sb,
> >               const struct sbrec_mirror_table *sbrec_mirror_table,
> >               struct ovsdb_idl_index *sbrec_chassis_by_name,
> > -             struct ovsdb_idl_index *sbrec_chassis_by_hostname)
> > +             struct ovsdb_idl_index *sbrec_chassis_by_hostname,
> > +             const struct shash *encap_by_ip)
> >  {
> >      op->od = od;
> >      parse_lsp_addrs(op);
> > @@ -4431,6 +4520,7 @@ ls_port_init(struct ovn_port *op, struct
> ovsdb_idl_txn *ovnsb_txn,
> >      }
> >      ovn_port_update_sbrec(ovnsb_txn, sbrec_chassis_by_name,
> >                            sbrec_chassis_by_hostname, NULL,
> sbrec_mirror_table,
> > +                          encap_by_ip,
> >                            op, NULL, NULL);
> >      return true;
> >  }
> > @@ -4441,13 +4531,15 @@ ls_port_create(struct ovsdb_idl_txn *ovnsb_txn,
> struct hmap *ls_ports,
> >                 struct ovn_datapath *od,
> >                 const struct sbrec_mirror_table *sbrec_mirror_table,
> >                 struct ovsdb_idl_index *sbrec_chassis_by_name,
> > -               struct ovsdb_idl_index *sbrec_chassis_by_hostname)
> > +               struct ovsdb_idl_index *sbrec_chassis_by_hostname,
> > +               const struct shash *encap_by_ip)
> >  {
> >      struct ovn_port *op = ovn_port_create(ls_ports, key, nbsp, NULL,
> >                                            NULL);
> >      hmap_insert(&od->ports, &op->dp_node,
> hmap_node_hash(&op->key_node));
> >      if (!ls_port_init(op, ovnsb_txn, od, NULL, sbrec_mirror_table,
> > -                      sbrec_chassis_by_name,
> sbrec_chassis_by_hostname)) {
> > +                      sbrec_chassis_by_name, sbrec_chassis_by_hostname,
> > +                      encap_by_ip)) {
> >          ovn_port_destroy(ls_ports, op);
> >          return NULL;
> >      }
> > @@ -4462,14 +4554,16 @@ ls_port_reinit(struct ovn_port *op, struct
> ovsdb_idl_txn *ovnsb_txn,
> >                  const struct sbrec_port_binding *sb,
> >                  const struct sbrec_mirror_table *sbrec_mirror_table,
> >                  struct ovsdb_idl_index *sbrec_chassis_by_name,
> > -                struct ovsdb_idl_index *sbrec_chassis_by_hostname)
> > +                struct ovsdb_idl_index *sbrec_chassis_by_hostname,
> > +                const struct shash *encap_by_ip)
> >  {
> >      ovn_port_cleanup(op);
> >      op->sb = sb;
> >      ovn_port_set_nb(op, nbsp, NULL);
> >      op->primary_port = op->cr_port = NULL;
> >      return ls_port_init(op, ovnsb_txn, od, sb, sbrec_mirror_table,
> > -                        sbrec_chassis_by_name,
> sbrec_chassis_by_hostname);
> > +                        sbrec_chassis_by_name,
> sbrec_chassis_by_hostname,
> > +                        encap_by_ip);
> >  }
> >
> >  /* Returns true if the logical switch has changes which can be
> > @@ -4632,6 +4726,9 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> >          return true;
> >      }
> >
> > +    struct shash encap_by_ip;
> > +    encap_ip_map_init(&encap_by_ip, ni->sbrec_chassis_table);
> > +
> >      bool ls_had_only_router_ports = (!vector_is_empty(&od->router_ports)
> >              && (vector_len(&od->router_ports) ==
> hmap_count(&od->ports)));
> >
> > @@ -4658,7 +4755,8 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> >                                      new_nbsp->name, new_nbsp, od,
> >                                      ni->sbrec_mirror_table,
> >                                      ni->sbrec_chassis_by_name,
> > -                                    ni->sbrec_chassis_by_hostname);
> > +                                    ni->sbrec_chassis_by_hostname,
> > +                                    &encap_by_ip);
> >                  if (!op) {
> >                      goto fail;
> >                  }
> > @@ -4697,7 +4795,8 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> >                                      new_nbsp,
> >                                      od, sb, ni->sbrec_mirror_table,
> >                                      ni->sbrec_chassis_by_name,
> > -                                    ni->sbrec_chassis_by_hostname)) {
> > +                                    ni->sbrec_chassis_by_hostname,
> > +                                    &encap_by_ip)) {
> >                      if (sb) {
> >                          sbrec_port_binding_delete(sb);
> >                      }
> > @@ -4798,9 +4897,11 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> >      }
> >      sset_destroy(&created_or_deleted_ports);
> >
> > +    shash_destroy(&encap_by_ip);
> >      return true;
> >
> >  fail:
> > +    shash_destroy(&encap_by_ip);
> >      destroy_tracked_ovn_ports(trk_lsps);
> >      return false;
> >  }
> > @@ -20622,6 +20723,7 @@ ovnnb_db_run(struct northd_input *input_data,
> >                  input_data->sbrec_mirror_table,
> >                  input_data->sbrec_mac_binding_table,
> >                  input_data->sbrec_ha_chassis_group_table,
> > +                input_data->sbrec_chassis_table,
> >                  input_data->sbrec_chassis_by_name,
> >                  input_data->sbrec_chassis_by_hostname,
> >                  input_data->sbrec_ha_chassis_grp_by_name,
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index 1acbf202b..33bf8993a 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -1550,6 +1550,15 @@
> >            </p>
> >          </column>
> >
> > +        <column name="options" key="requested-encap-ip">
> > +          Requests the encapsulation IP address for the port binding.
> If set,
> > +          <code>ovn-northd</code> uses this IP to select the
> > +          <ref table="Encap" db="OVN_Southbound"/> entry written to
> > +          <ref table="Port_Binding" column="encap"
> db="OVN_Southbound"/>.
> > +          This is intended for ports without a local OVS interface,
> e.g. remote
> > +          transit switch ports in ovn-kubernetes interconnect mode.
> > +        </column>
> > +
> >          <column name="options" key="activation-strategy">
> >            If used with multiple chassis set in
> >            <ref column="requested-chassis"/>, specifies an activation
> strategy
> > @@ -4393,6 +4402,15 @@ or
> >          </p>
> >        </column>
> >
> > +      <column name="options" key="requested-encap-ip">
> > +        Requests the encapsulation IP address for the port binding. If
> set,
> > +        <code>ovn-northd</code> uses this IP to select the
> > +        <ref table="Encap" db="OVN_Southbound"/> entry written to
> > +        <ref table="Port_Binding" column="encap" db="OVN_Southbound"/>.
> > +        This is intended for ports without a local OVS interface, e.g.
> remote
> > +        transit router ports in ovn-kubernetes interconnect mode.
> > +      </column>
> > +
> >        <column name="options" key="dynamic-routing-redistribute"
> >                type='{"type": "string"}'>
> >          <p>
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 512e42036..9245ff639 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -2896,6 +2896,69 @@ OVN_CLEANUP_NORTHD
> >  AT_CLEANUP
> >  ])
> >
> > +OVN_FOR_EACH_NORTHD_NO_HV([
> > +AT_SETUP([check options:requested-encap-ip fills port binding encap
> col])
> > +AT_KEYWORDS([requested encap ip])
> > +ovn_start
> > +
> > +check_uuid ovn-sbctl \
> > +    -- --id=@e11 create encap chassis_name=ch1 ip="192.168.1.1"
> type="geneve" \
> > +    -- --id=@e12 create encap chassis_name=ch1 ip="192.168.1.2"
> type="geneve" \
> > +    -- --id=@c1 create chassis name=ch1 encaps=@e11,@e12
> > +check_uuid ovn-sbctl \
> > +    -- --id=@e21 create encap chassis_name=ch2 ip="192.168.2.1"
> type="geneve" \
> > +    -- --id=@e22 create encap chassis_name=ch2 ip="192.168.2.2"
> type="geneve" \
> > +    -- --id=@c2 create chassis name=ch2 encaps=@e21,@e22
> > +
> > +wait_row_count Chassis 2
> > +wait_row_count Encap 4
> > +en11_uuid=`ovn-sbctl --bare --columns _uuid find Encap ip="192.168.1.1"`
>
> Nit: for this and all other lines where you look up the UUID of the
> encap, you can instead use the fetch_column() function provided by the
> testsuite:
>
> en11_uuid=$(fetch_column Encap _uuid ip="192.168.1.1")
>
> > +en12_uuid=`ovn-sbctl --bare --columns _uuid find Encap ip="192.168.1.2"`
> > +en21_uuid=`ovn-sbctl --bare --columns _uuid find Encap ip="192.168.2.1"`
> > +en22_uuid=`ovn-sbctl --bare --columns _uuid find Encap ip="192.168.2.2"`
> > +ovn-sbctl show
> > +
> > +echo "__file__:__line__: encap uuid: $en11_uuid, ip: 192.168.1.1"
> > +echo "__file__:__line__: encap uuid: $en12_uuid, ip: 192.168.1.2"
> > +echo "__file__:__line__: encap uuid: $en21_uuid, ip: 192.168.2.1"
> > +echo "__file__:__line__: encap uuid: $en22_uuid, ip: 192.168.2.2"
> > +
> > +check ovn-nbctl --wait=sb ls-add ls1
> > +check ovn-nbctl --wait=sb lsp-add ls1 lsp1
> > +check ovn-nbctl --wait=sb lsp-add ls1 lsp2
> > +ovn-nbctl show
> > +
> > +echo "options:requested-chassis is required to set
> options:requested-encap-ip"
> > +check ovn-nbctl --wait=sb set logical-switch-port lsp1 \
> > +    options:requested-encap-ip=192.168.1.1
> > +check ovn-nbctl --wait=sb sync
> > +wait_row_count Port_Binding 1 logical_port=lsp1 'encap=[[]]'
> > +
> > +# Now set both options
> > +check ovn-nbctl --wait=sb set logical-switch-port lsp1 \
> > +    options:requested-chassis=ch1 \
> > +    options:requested-encap-ip=192.168.1.1
> > +check ovn-nbctl --wait=sb set logical-switch-port lsp2 \
> > +    options:requested-chassis=ch2 \
> > +    options:requested-encap-ip=192.168.2.2
> > +
> > +wait_row_count Port_Binding 1 logical_port=lsp1 encap="$en11_uuid"
> > +wait_row_count Port_Binding 1 logical_port=lsp2 encap="$en22_uuid"
> > +
> > +# remove options:requested-encap-ip from lsp1
> > +check ovn-nbctl --wait=sb remove logical_switch_port lsp1 \
> > +    options requested-encap-ip=192.168.1.1
> > +wait_row_count Port_Binding 1 logical_port=lsp1 'encap=[[]]'
> > +
> > +# remove options:requested-chassis from lsp2
> > +check ovn-nbctl --wait=sb remove logical_switch_port lsp2 \
> > +    options requested-chassis=ch2
> > +wait_row_count Port_Binding 1 logical_port=lsp2 'encap=[[]]'
> > +
>
> One thing this test does not validate is that geneve encaps take
> priority over vxlan encaps.
>
> > +OVN_CLEANUP_NORTHD
> > +AT_CLEANUP
> > +])
> > +
> >  OVN_FOR_EACH_NORTHD_NO_HV([
> >  AT_SETUP([port requested-tnl-key])
> >  AT_KEYWORDS([requested tnl tunnel key keys])
> > --
> > 2.39.5 (Apple Git-154)
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
>