On Fri, Jul 25, 2025 at 2:35 PM Lucas Vargas Dias via dev <
ovs-dev@openvswitch.org> wrote:

> Handle the changes from LSP with dynamic addreses when its Logical Switch
> has mac, subnet or ipv6 prefix configured.
> Before, all logical switches were iterated even when they don't have
> changes.
>
> Test with 30000 LS and 70000 LSPs creating a LSP with dynamic address:
> ovn-nbctl --print-wait-time --wait=sb lsp-add -- lsp-set-addresses
> dynamic
>
> Without the I-P:
> Time spent on processing nb_cfg 272726:
>     ovn-northd delay before processing:    10ms
>     ovn-northd completion:            21422ms
>
> With the I-P:
> Time spent on processing nb_cfg 272733:
>     ovn-northd delay before processing:    20ms
>     ovn-northd completion:            101ms
>
> Signed-off-by: Lucas Vargas Dias <lucas.vd...@luizalabs.com>
> ---
>  northd/en-northd.c |  3 ++-
>  northd/ipam.c      |  3 ++-
>  northd/northd.c    | 56 ++++++++++++++++++++++++++++++++++++++++++++--
>  northd/northd.h    |  6 +++++
>  tests/ovn.at       | 16 ++++++-------
>  5 files changed, 72 insertions(+), 12 deletions(-)
>
> diff --git a/northd/en-northd.c b/northd/en-northd.c
> index 8402ab943..a4e90e6d0 100644
> --- a/northd/en-northd.c
> +++ b/northd/en-northd.c
> @@ -156,7 +156,8 @@ northd_nb_logical_switch_handler(struct engine_node
> *node,
>          return EN_UNHANDLED;
>      }
>
> -    if (northd_has_tracked_data(&nd->trk_data)) {
> +    bool ipam_update = northd_handle_ipam_changes(nd);
> +    if (northd_has_tracked_data(&nd->trk_data) || ipam_update) {
>          return EN_HANDLED_UPDATED;
>      }
>
> diff --git a/northd/ipam.c b/northd/ipam.c
> index ad3e03839..7ed2a2070 100644
> --- a/northd/ipam.c
> +++ b/northd/ipam.c
> @@ -71,13 +71,14 @@ init_ipam_info(struct ipam_info *info, const struct
> smap *config, const char *id
>  void
>  init_ipam_info_for_datapath(struct ovn_datapath *od)
>  {
> -    if (!od->nbs) {
> +    if (!od->nbs || od->ipam_info_initialized) {
>          return;
>      }
>
>      char uuid_s[UUID_LEN + 1];
>      sprintf(uuid_s, UUID_FMT, UUID_ARGS(&od->key));
>      init_ipam_info(&od->ipam_info, &od->nbs->other_config, uuid_s);
> +    od->ipam_info_initialized = true;
>  }
>
>  void
> diff --git a/northd/northd.c b/northd/northd.c
> index ef2271fdf..c90973c46 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -504,6 +504,7 @@ ovn_datapath_create(struct hmap *datapaths, const
> struct uuid *key,
>      od->l3dgw_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *);
>      od->localnet_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *);
>      od->lb_with_stateless_mode = false;
> +    od->ipam_info_initialized = false;
>      return od;
>  }
>
> @@ -918,6 +919,10 @@ join_datapaths(const struct
> nbrec_logical_switch_table *nbrec_ls_table,
>              ovs_list_remove(&od->list);
>              ovs_list_push_back(both, &od->list);
>              ovn_datapath_update_external_ids(od);
> +            if (od->ipam_info_initialized) {
> +                destroy_ipam_info(&od->ipam_info);
> +                od->ipam_info_initialized = false;
> +            }
>          } else {
>              od = ovn_datapath_create(datapaths, &nbs->header_.uuid,
>                                       nbs, NULL, NULL);
> @@ -4167,6 +4172,7 @@ destroy_northd_data_tracked_changes(struct
> northd_data *nd)
>      hmapx_clear(&trk_changes->trk_nat_lrs);
>      hmapx_clear(&trk_changes->ls_with_changed_lbs);
>      hmapx_clear(&trk_changes->ls_with_changed_acls);
> +    hmapx_clear(&trk_changes->ls_with_changed_ipam);
>      trk_changes->type = NORTHD_TRACKED_NONE;
>  }
>
> @@ -4183,6 +4189,7 @@ init_northd_tracked_data(struct northd_data *nd)
>      hmapx_init(&trk_data->trk_nat_lrs);
>      hmapx_init(&trk_data->ls_with_changed_lbs);
>      hmapx_init(&trk_data->ls_with_changed_acls);
> +    hmapx_init(&trk_data->ls_with_changed_ipam);
>  }
>
>  static void
> @@ -4198,6 +4205,7 @@ destroy_northd_tracked_data(struct northd_data *nd)
>      hmapx_destroy(&trk_data->trk_nat_lrs);
>      hmapx_destroy(&trk_data->ls_with_changed_lbs);
>      hmapx_destroy(&trk_data->ls_with_changed_acls);
> +    hmapx_destroy(&trk_data->ls_with_changed_ipam);
>  }
>
>  /* Check if a changed LSP can be handled incrementally within the I-P
> engine
> @@ -4224,8 +4232,9 @@ lsp_can_be_inc_processed(const struct
> nbrec_logical_switch_port *nbsp)
>      }
>
>      for (size_t j = 0; j < nbsp->n_addresses; j++) {
> -        /* Dynamic address handling is not supported for now. */
> -        if (is_dynamic_lsp_address(nbsp->addresses[j])) {
> +        /* Dynamic address was assigned in the last iteration. */
> +        if (is_dynamic_lsp_address(nbsp->addresses[j]) &&
> +            nbsp->dynamic_addresses) {
>              return false;
>          }
>          /* "unknown" address handling is not supported for now.  XXX:
> Need to
> @@ -4693,6 +4702,13 @@ northd_handle_ls_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
>          if (is_ls_acls_changed(changed_ls)) {
>              hmapx_add(&trk_data->ls_with_changed_acls, od);
>          }
> +        init_ipam_info_for_datapath(od);
> +        bool ls_has_ipam = od->ipam_info.allocated_ipv4s ||
> +                           od->ipam_info.ipv6_prefix_set ||
> +                           od->ipam_info.mac_only;
> +        if (ls_has_ipam) {
> +            hmapx_add(&trk_data->ls_with_changed_ipam, od);
> +        }
>      }
>
>      if (!hmapx_is_empty(&trk_data->trk_lsps.created)
> @@ -4774,6 +4790,42 @@ fail:
>      return false;
>  }
>
> +bool northd_handle_ipam_changes(struct northd_data *nd)
> +{
> +    struct northd_tracked_data *nd_changes = &nd->trk_data;
> +    if (hmapx_is_empty(&nd_changes->ls_with_changed_ipam)) {
> +        return false;
> +    }
> +
> +    struct vector updates =
> +        VECTOR_EMPTY_INITIALIZER(struct dynamic_address_update);
> +
> +    struct hmapx_node *hmapx_node;
> +    HMAPX_FOR_EACH (hmapx_node, &nd_changes->ls_with_changed_ipam) {
> +        struct ovn_datapath *od = hmapx_node->data;
> +        if (od->ipam_info_initialized) {
> +            destroy_ipam_info(&od->ipam_info);
> +            od->ipam_info_initialized = false;
> +        }
> +        init_ipam_info_for_datapath(od);
> +        update_ipam_ls(od, &updates, false);
> +    }
> +
> +    bool lsps_changed = false;
> +    struct dynamic_address_update *update;
> +    VECTOR_FOR_EACH_PTR (&updates, update) {
> +        if (hmapx_find(&nd_changes->trk_lsps.updated, update->op) ||
> +            hmapx_find(&nd_changes->trk_lsps.created, update->op)) {
> +            update_dynamic_addresses(update);
> +            lsps_changed = true;
> +        }
> +        destroy_lport_addresses(&update->current_addresses);
> +    }
> +    vector_destroy(&updates);
> +
> +    return lsps_changed;
> +}
> +
>  /* Returns true if the logical router has changes which can be
>   * incrementally handled.
>   * Presently supports i-p for the below changes:
> diff --git a/northd/northd.h b/northd/northd.h
> index 084634328..47ac4386a 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -161,6 +161,10 @@ struct northd_tracked_data {
>      /* Tracked logical switches whose ACLs have changed.
>       * hmapx node is 'struct ovn_datapath *'. */
>      struct hmapx ls_with_changed_acls;
> +
> +    /* Tracked logical switches with IPAM whose LSPs have changed.
> +     * hmapx node is 'struct ovn_datapath *'. */
> +    struct hmapx ls_with_changed_ipam;
>  };
>
>  struct northd_data {
> @@ -386,6 +390,7 @@ struct ovn_datapath {
>
>      /* IPAM data. */
>      struct ipam_info ipam_info;
> +    bool ipam_info_initialized;
>
>      /* Multicast data. */
>      struct mcast_info mcast_info;
> @@ -838,6 +843,7 @@ bool northd_handle_lr_changes(const struct
> northd_input *,
>                                struct northd_data *);
>  bool northd_handle_pgs_acl_changes(const struct northd_input *ni,
>                                     struct northd_data *nd);
> +bool northd_handle_ipam_changes(struct northd_data *nd);
>  void destroy_northd_data_tracked_changes(struct northd_data *);
>  void northd_destroy(struct northd_data *data);
>  void northd_init(struct northd_data *data);
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 108de260e..264dfebbc 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -9218,7 +9218,7 @@ done
>  # and addresses set by the user.
>  check ovn-nbctl lsp-set-addresses p0 "0a:00:00:a8:01:17 192.168.1.2
> 192.168.1.12 192.168.1.14"
>  check ovn-nbctl --wait=sb lsp-add sw0 p20 -- lsp-set-addresses p20 dynamic
> -check_dynamic_addresses p20 "0a:00:00:a8:01:18 192.168.1.13"
> +check_dynamic_addresses p20 "0a:00:00:a8:01:03 192.168.1.13"
>
>  # Test for logical router port address management.
>  check_uuid ovn-nbctl create Logical_Router name=R1
> @@ -9227,12 +9227,12 @@ network="192.168.1.1/24"
> mac=\"0a:00:00:a8:01:19\" \
>  -- add Logical_Router R1 ports @lrp -- lsp-add sw0 rp-sw0 \
>  -- set Logical_Switch_Port rp-sw0 type=router options:router-port=sw0
>  check ovn-nbctl --wait=sb lsp-add sw0 p21 -- lsp-set-addresses p21 dynamic
> -check_dynamic_addresses p21 "0a:00:00:a8:01:1a 192.168.1.15"
> +check_dynamic_addresses p21 "0a:00:00:a8:01:18 192.168.1.15"
>
>  # Test for address reuse after logical port is deleted.
>  check ovn-nbctl lsp-del p0
>  check ovn-nbctl --wait=sb lsp-add sw0 p23 -- lsp-set-addresses p23 dynamic
> -check_dynamic_addresses p23 "0a:00:00:a8:01:03 192.168.1.2"
> +check_dynamic_addresses p23 "0a:00:00:a8:01:1a 192.168.1.2"
>
>  # Test for multiple addresses to one logical port.
>  check ovn-nbctl lsp-add sw0 p25 -- lsp-set-addresses p25 \
> @@ -9292,19 +9292,19 @@ check_dynamic_addresses p33 "0a:00:00:a8:01:1f
> 192.168.1.22"
>  check ovn-nbctl --wait=sb lsp-add sw0 p34 -- lsp-set-addresses p34 \
>  "dynamic"
>  # 192.168.1.51 should be assigned as 192.168.1.23-192.168.1.50 is
> excluded.
> -check_dynamic_addresses p34 "0a:00:00:a8:01:34 192.168.1.51"
> +check_dynamic_addresses p34 "0a:00:00:a8:01:20 192.168.1.51"
>
>  # Now clear the exclude_ips list. 192.168.1.19 should be assigned.
>  check ovn-nbctl --wait=sb set Logical-switch sw0
> other_config:exclude_ips="invalid"
>  check ovn-nbctl --wait=sb lsp-add sw0 p35 -- lsp-set-addresses p35
> "dynamic"
> -check_dynamic_addresses p35 "0a:00:00:a8:01:20 192.168.1.19"
> +check_dynamic_addresses p35 "0a:00:00:a8:01:21 192.168.1.19"
>
>  # Set invalid data in exclude_ips list. It should be ignored.
>  check ovn-nbctl --wait=sb set Logical-switch sw0
> other_config:exclude_ips="182.168.1.30"
>  check ovn-nbctl --wait=sb lsp-add sw0 p36 -- lsp-set-addresses p36 \
>  "dynamic"
>  # 192.168.1.21 should be assigned as that's the next free one.
> -check_dynamic_addresses p36 "0a:00:00:a8:01:21 192.168.1.21"
> +check_dynamic_addresses p36 "0a:00:00:a8:01:22 192.168.1.21"
>
>  # Clear the dynamic addresses assignment request.
>  check ovn-nbctl --wait=sb clear logical_switch_port p36 addresses
> @@ -9316,7 +9316,7 @@ check ovn-nbctl --wait=sb lsp-add sw0 p37 --
> lsp-set-addresses p37 "dynamic"
>
>  # With prefix aef0 and mac 0a:00:00:00:00:26, the dynamic IPv6 should be
>  # - aef0::800:ff:fe00:26 (EUI64)
> -check_dynamic_addresses p37 "0a:00:00:a8:01:21 192.168.1.21
> aef0::800:ff:fea8:121"
> +check_dynamic_addresses p37 "0a:00:00:a8:01:22 192.168.1.21
> aef0::800:ff:fea8:122"
>
>  check ovn-nbctl --wait=sb ls-add sw4
>  check ovn-nbctl --wait=sb set Logical-switch sw4
> other_config:ipv6_prefix="bef0::" \
> @@ -9346,7 +9346,7 @@ check_dynamic_addresses p41 ''
>
>  # Set a subnet. Now p41 should have an ipv4 address, too
>  check ovn-nbctl --wait=sb add Logical-Switch sw5 other_config subnet=
> 192.168.1.0/24
> -check_dynamic_addresses <http://192.168.1.0/24-check_dynamic_addresses>
> p41 "0a:00:00:a8:01:22 192.168.1.2"
> +check_dynamic_addresses p41 "0a:00:00:a8:01:23 192.168.1.2"
>
>  # Clear the other_config. The IPv4 address should be gone
>  check ovn-nbctl --wait=sb clear Logical-Switch sw5 other_config
> --
> 2.34.1
>
>
> --
>
>
>
>
> _'Esta mensagem é direcionada apenas para os endereços constantes no
> cabeçalho inicial. Se você não está listado nos endereços constantes no
> cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa
> mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas
> estão
> imediatamente anuladas e proibidas'._
>
>
> * **'Apesar do Magazine Luiza tomar
> todas as precauções razoáveis para assegurar que nenhum vírus esteja
> presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por
> quaisquer perdas ou danos causados por esse e-mail ou por seus anexos'.*
>
>
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Thank you Lucas,

I went ahead and merged this into main.

Regards,
Ales
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to