On Wed, Feb 05, 2025 at 02:31:07PM +0100, Dumitru Ceara wrote:
> On 2/4/25 2:59 PM, Felix Huettner via dev wrote:
> > Sometimes we want to use individual host routes instead of the connected
> > routes of LRPs.
> > This allows the network fabric to know which addresses are actually in
> > use and e.g. drop traffic to addresses that are not used anyway.
> > 
> > Signed-off-by: Felix Huettner <[email protected]>
> > ---
> 
> Hi Felix,
> 
> I have 3 minor comments below.  If those are the only changes in v7,
> feel free to add my ack:

Hi Dumitru,

thanks for the review.
I will fix the changes below in v7.

There will be other changes on this patch based on comments by Lorenzo.

Thanks a lot,
Felix

> 
> Acked-by: Dumitru Ceara <[email protected]>
> 
> > v5->v6:
> >   * addressed review comments
> > v4->v5: skipped
> > v3->v4:
> >   * fix a memory leak
> > v2->v3:
> >   * A lot of minor review comments.
> > 
> >  NEWS                              |   3 +
> >  northd/en-advertised-route-sync.c | 227 ++++++++++++++++++++++++++++--
> >  northd/en-advertised-route-sync.h |  11 ++
> >  northd/inc-proc-northd.c          |   4 +
> >  northd/northd.c                   |  35 +++--
> >  northd/northd.h                   |  22 +++
> >  ovn-nb.xml                        |  27 ++++
> >  tests/ovn-northd.at               | 113 ++++++++++++++-
> >  8 files changed, 407 insertions(+), 35 deletions(-)
> > 
> > diff --git a/NEWS b/NEWS
> > index b0ca1992e..3547f659f 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -50,6 +50,9 @@ Post v24.09.0
> >         fabric. Routes entered into the "Learned_Route" table in the 
> > southbound
> >         database will be learned by the respective LR. They are included in 
> > the
> >         route table with a lower priority than static routes.
> > +     * Add the option "dynamic-routing-connected-as-host-routes" to LRPs. 
> > If
> > +       set to true then connected routes are announced as individual host
> > +       routes.
> >  
> >  OVN v24.09.0 - 13 Sep 2024
> >  --------------------------
> > diff --git a/northd/en-advertised-route-sync.c 
> > b/northd/en-advertised-route-sync.c
> > index d4360763f..a0087c71a 100644
> > --- a/northd/en-advertised-route-sync.c
> > +++ b/northd/en-advertised-route-sync.c
> > @@ -20,6 +20,7 @@
> >  #include "northd.h"
> >  
> >  #include "en-advertised-route-sync.h"
> > +#include "en-lr-stateful.h"
> >  #include "lib/stopwatch-names.h"
> >  #include "openvswitch/hmap.h"
> >  #include "ovn-util.h"
> > @@ -28,34 +29,131 @@ static void
> >  advertised_route_table_sync(
> >      struct ovsdb_idl_txn *ovnsb_txn,
> >      const struct sbrec_advertised_route_table 
> > *sbrec_advertised_route_table,
> > -    const struct hmap *parsed_routes);
> > +    const struct lr_stateful_table *lr_stateful_table,
> > +    const struct hmap *parsed_routes,
> > +    struct advertised_route_sync_data *data);
> > +
> > +bool
> > +advertised_route_sync_lr_stateful_change_handler(struct engine_node *node,
> > +                                                 void *data_)
> > +{
> > +    /* We only actually use lr_stateful data if we expose individual host
> > +     * routes. In this case we for now just recompute.
> > +     * */
> > +    struct ed_type_lr_stateful *lr_stateful_data =
> > +        engine_get_input_data("lr_stateful", node);
> > +    struct advertised_route_sync_data *data = data_;
> > +
> > +    struct hmapx_node *hmapx_node;
> > +    const struct lr_stateful_record *lr_stateful_rec;
> > +    HMAPX_FOR_EACH (hmapx_node, &lr_stateful_data->trk_data.crupdated) {
> > +        lr_stateful_rec = hmapx_node->data;
> > +        if (uuidset_contains(&data->nb_lr,
> > +                             &lr_stateful_rec->nbr_uuid)) {
> > +            return false;
> > +        }
> > +    }
> > +
> > +    return true;
> > +}
> > +
> > +bool
> > +advertised_route_sync_northd_change_handler(struct engine_node *node,
> > +                                            void *data_)
> > +{
> > +    struct advertised_route_sync_data *data = data_;
> > +    struct northd_data *northd_data = engine_get_input_data("northd", 
> > node);
> > +    if (!northd_has_tracked_data(&northd_data->trk_data)) {
> > +        return false;
> > +    }
> > +
> > +    /* We indirectly use northd_data->ls_ports if we announce host routes.
> > +     * For now we just recompute on any change to lsps that are relevant 
> > to us.
> > +     */
> > +    struct hmapx_node *hmapx_node;
> > +    const struct ovn_port *op;
> > +    HMAPX_FOR_EACH (hmapx_node, &northd_data->trk_data.trk_lsps.created) {
> > +        op = hmapx_node->data;
> > +        if (uuidset_contains(&data->nb_ls,
> > +                             &op->od->nbs->header_.uuid)) {
> > +            return false;
> > +        }
> > +    }
> > +    HMAPX_FOR_EACH (hmapx_node, &northd_data->trk_data.trk_lsps.updated) {
> > +        op = hmapx_node->data;
> > +        if (uuidset_contains(&data->nb_ls,
> > +                             &op->od->nbs->header_.uuid)) {
> > +            return false;
> > +        }
> > +    }
> > +    HMAPX_FOR_EACH (hmapx_node, &northd_data->trk_data.trk_lsps.deleted) {
> > +        op = hmapx_node->data;
> > +        if (uuidset_contains(&data->nb_ls,
> > +                             &op->od->nbs->header_.uuid)) {
> > +            return false;
> > +        }
> > +    }
> > +
> > +    return true;
> > +}
> > +
> > +static void
> > +routes_sync_init(struct advertised_route_sync_data *data)
> > +{
> > +    uuidset_init(&data->nb_lr);
> > +    uuidset_init(&data->nb_ls);
> > +}
> > +
> > +static void
> > +routes_sync_clear(struct advertised_route_sync_data *data)
> > +{
> > +    uuidset_clear(&data->nb_lr);
> > +    uuidset_clear(&data->nb_ls);
> > +}
> > +
> > +static void
> > +routes_sync_destroy(struct advertised_route_sync_data *data)
> > +{
> > +    uuidset_destroy(&data->nb_lr);
> > +    uuidset_destroy(&data->nb_ls);
> > +}
> >  
> >  void
> >  *en_advertised_route_sync_init(struct engine_node *node OVS_UNUSED,
> >                       struct engine_arg *arg OVS_UNUSED)
> >  {
> > -    return NULL;
> > +    struct advertised_route_sync_data *data = xzalloc(sizeof *data);
> > +    routes_sync_init(data);
> > +    return data;
> >  }
> >  
> >  void
> >  en_advertised_route_sync_cleanup(void *data OVS_UNUSED)
> >  {
> > +    routes_sync_destroy(data);
> >  }
> >  
> >  void
> >  en_advertised_route_sync_run(struct engine_node *node, void *data 
> > OVS_UNUSED)
> >  {
> > +    routes_sync_clear(data);
> > +
> > +    struct advertised_route_sync_data *routes_sync_data = data;
> >      struct routes_data *routes_data
> >          = engine_get_input_data("routes", node);
> >      const struct engine_context *eng_ctx = engine_get_context();
> >      const struct sbrec_advertised_route_table 
> > *sbrec_advertised_route_table =
> >          EN_OVSDB_GET(engine_get_input("SB_advertised_route", node));
> > +    struct ed_type_lr_stateful *lr_stateful_data =
> > +        engine_get_input_data("lr_stateful", node);
> >  
> >      stopwatch_start(ADVERTISED_ROUTE_SYNC_RUN_STOPWATCH_NAME, time_msec());
> >  
> >      advertised_route_table_sync(eng_ctx->ovnsb_idl_txn,
> >                                  sbrec_advertised_route_table,
> > -                                &routes_data->parsed_routes);
> > +                                &lr_stateful_data->table,
> > +                                &routes_data->parsed_routes,
> > +                                routes_sync_data);
> >  
> >      stopwatch_stop(ADVERTISED_ROUTE_SYNC_RUN_STOPWATCH_NAME, time_msec());
> >      engine_set_node_state(node, EN_UPDATED);
> > @@ -68,19 +166,24 @@ struct ar_entry {
> >  
> >      const struct sbrec_port_binding *logical_port;
> >      char *ip_prefix;
> > +    const struct sbrec_port_binding *tracked_port;
> >  };
> >  
> >  /* Add a new entries to the to-be-advertised routes.
> >   * Takes ownership of ip_prefix. */
> >  static struct ar_entry *
> >  ar_add_entry(struct hmap *routes, const struct sbrec_datapath_binding 
> > *sb_db,
> > -             const struct sbrec_port_binding *logical_port, char 
> > *ip_prefix)
> > +               const struct sbrec_port_binding *logical_port, char 
> > *ip_prefix,
> > +               const struct sbrec_port_binding *tracked_port)
> 
> Indentation is off here, these two lines should start 2 columns to the left.
> 
> >  {
> >      struct ar_entry *route_e = xzalloc(sizeof *route_e);
> >  
> >      route_e->sb_db = sb_db;
> >      route_e->logical_port = logical_port;
> >      route_e->ip_prefix = ip_prefix;
> > +    if (tracked_port) {
> > +        route_e->tracked_port = tracked_port;
> > +    }
> 
> Nit: the if is not needed, this can be:
> 
> route_e->tracked_port = tracked_port;
> 
> >      uint32_t hash = uuid_hash(&sb_db->header_.uuid);
> >      hash = hash_string(logical_port->logical_port, hash);
> >      hash = hash_string(ip_prefix, hash);
> > @@ -91,7 +194,8 @@ ar_add_entry(struct hmap *routes, const struct 
> > sbrec_datapath_binding *sb_db,
> >  
> >  static struct ar_entry *
> >  ar_find(struct hmap *route_map, const struct sbrec_datapath_binding *sb_db,
> > -        const struct sbrec_port_binding *logical_port, const char 
> > *ip_prefix)
> > +        const struct sbrec_port_binding *logical_port, const char 
> > *ip_prefix,
> > +        const struct sbrec_port_binding *tracked_port)
> >  {
> >      struct ar_entry *route_e;
> >      uint32_t hash;
> > @@ -114,6 +218,10 @@ ar_find(struct hmap *route_map, const struct 
> > sbrec_datapath_binding *sb_db,
> >              continue;
> >          }
> >  
> > +        if (tracked_port != route_e->tracked_port) {
> > +            continue;
> > +        }
> > +
> >          return route_e;
> >      }
> >  
> > @@ -127,13 +235,94 @@ ar_entry_free(struct ar_entry *route_e)
> >      free(route_e);
> >  }
> >  
> > +static void
> > +publish_lport_addresses(struct hmap *sync_routes,
> > +                        const struct sbrec_datapath_binding *sb_db,
> > +                        const struct ovn_port *logical_port,
> > +                        struct lport_addresses *addresses,
> > +                        const struct ovn_port *tracking_port)
> > +{
> > +    for (size_t i = 0; i < addresses->n_ipv4_addrs; i++) {
> > +        const struct ipv4_netaddr *addr = &addresses->ipv4_addrs[i];
> > +        char *addr_s = xasprintf("%s/32", addr->addr_s);
> > +        ar_add_entry(sync_routes, sb_db, logical_port->sb,
> > +                       addr_s, tracking_port->sb);
> > +    }
> > +    for (size_t i = 0; i < addresses->n_ipv6_addrs; i++) {
> > +        if (in6_is_lla(&addresses->ipv6_addrs[i].network)) {
> > +            continue;
> > +        }
> > +        const struct ipv6_netaddr *addr = &addresses->ipv6_addrs[i];
> > +        char *addr_s = xasprintf("%s/128", addr->addr_s);
> > +        ar_add_entry(sync_routes, sb_db, logical_port->sb,
> > +                       addr_s, tracking_port->sb);
> > +    }
> > +}
> > +
> > +/* Collect all IP addresses connected to the out_port of a route.
> > + * This traverses all LSPs on the LS connected to the out_port. */
> > +static void
> > +publish_host_routes(struct hmap *sync_routes,
> > +                    const struct lr_stateful_table *lr_stateful_table,
> > +                    const struct parsed_route *route,
> > +                    struct advertised_route_sync_data *data)
> > +{
> > +    struct ovn_port *port;
> > +    struct ovn_datapath *lsp_od = route->out_port->peer->od;
> > +
> > +    if (!lsp_od->nbs) {
> > +        return;
> > +    }
> > +
> > +    /* We need to track the LS we are publishing routes from, so that we 
> > can
> > +     * recompute when any port on there changes. */
> > +    uuidset_insert(&data->nb_ls, &lsp_od->nbs->header_.uuid);
> > +    HMAP_FOR_EACH (port, dp_node, &lsp_od->ports) {
> > +        if (port->peer) {
> > +            /* This is a LSP connected to an LRP */
> > +            struct lport_addresses *addresses = &port->peer->lrp_networks;
> > +            publish_lport_addresses(sync_routes, route->od->sb,
> > +                                    route->out_port,
> > +                                    addresses, port->peer);
> > +
> > +            const struct lr_stateful_record *lr_stateful_rec;
> > +            lr_stateful_rec = lr_stateful_table_find_by_index(
> > +                lr_stateful_table, port->peer->od->index);
> > +            /* We also need to track this LR as we need to recompute when
> > +             * any of its IPs change. */
> > +            uuidset_insert(&data->nb_lr,
> > +                           &lr_stateful_rec->nbr_uuid);
> > +            struct ovn_port_routable_addresses addrs = get_op_addresses(
> > +                port->peer, lr_stateful_rec, false);
> > +            for (size_t i = 0; i < addrs.n_addrs; i++) {
> > +                publish_lport_addresses(sync_routes, route->od->sb,
> > +                                        route->out_port,
> > +                                        &addrs.laddrs[i],
> > +                                        port->peer);
> > +            }
> > +            destroy_routable_addresses(&addrs);
> > +        } else {
> > +            /* This is just a plain LSP */
> > +            for (size_t i = 0; i < port->n_lsp_addrs; i++) {
> > +                publish_lport_addresses(sync_routes, route->od->sb,
> > +                                        route->out_port,
> > +                                        &port->lsp_addrs[i],
> > +                                        port);
> > +            }
> > +        }
> > +    }
> > +}
> > +
> >  static void
> >  advertised_route_table_sync(
> >      struct ovsdb_idl_txn *ovnsb_txn,
> >      const struct sbrec_advertised_route_table 
> > *sbrec_advertised_route_table,
> > -    const struct hmap *parsed_routes)
> > +    const struct lr_stateful_table *lr_stateful_table,
> > +    const struct hmap *parsed_routes,
> > +    struct advertised_route_sync_data *data)
> >  {
> >      struct hmap sync_routes = HMAP_INITIALIZER(&sync_routes);
> > +    struct uuidset host_route_lrps = UUIDSET_INITIALIZER(&host_route_lrps);
> >      const struct parsed_route *route;
> >  
> >      struct ar_entry *route_e;
> > @@ -148,9 +337,21 @@ advertised_route_table_sync(
> >          if (!route->od->dynamic_routing) {
> >              continue;
> >          }
> > -        if (route->source == ROUTE_SOURCE_CONNECTED &&
> > -                !route->out_port->dynamic_routing_connected) {
> > -            continue;
> > +        if (route->source == ROUTE_SOURCE_CONNECTED) {
> > +            if (!route->out_port->dynamic_routing_connected) {
> > +                continue;
> > +            }
> > +            /* If we advertise host routes, we only need to do so once per
> > +             * LRP. */
> > +            const struct uuid *lrp_uuid =
> > +                &route->out_port->nbrp->header_.uuid;
> > +            if (route->out_port->dynamic_routing_connected_as_host_routes 
> > &&
> > +                !uuidset_contains(&host_route_lrps, lrp_uuid)) {
> > +                uuidset_insert(&host_route_lrps, lrp_uuid);
> > +                publish_host_routes(&sync_routes, lr_stateful_table,
> > +                                    route, data);
> > +                continue;
> > +            }
> >          }
> >          if (route->source == ROUTE_SOURCE_STATIC &&
> >                  !route->out_port->dynamic_routing_static) {
> > @@ -160,14 +361,15 @@ advertised_route_table_sync(
> >          char *ip_prefix = normalize_v46_prefix(&route->prefix,
> >                                                 route->plen);
> >          route_e = ar_add_entry(&sync_routes, route->od->sb,
> > -                               route->out_port->sb, ip_prefix);
> > +                               route->out_port->sb, ip_prefix, NULL);
> >      }
> > +    uuidset_destroy(&host_route_lrps);
> >  
> >      SBREC_ADVERTISED_ROUTE_TABLE_FOR_EACH_SAFE (sb_route,
> >                                                  
> > sbrec_advertised_route_table) {
> >          route_e = ar_find(&sync_routes, sb_route->datapath,
> > -                          sb_route->logical_port,
> > -                          sb_route->ip_prefix);
> > +                          sb_route->logical_port, sb_route->ip_prefix,
> > +                          sb_route->tracked_port);
> >          if (route_e) {
> >            hmap_remove(&sync_routes, &route_e->hmap_node);
> >            ar_entry_free(route_e);
> > @@ -182,6 +384,7 @@ advertised_route_table_sync(
> >          sbrec_advertised_route_set_datapath(sr, route_e->sb_db);
> >          sbrec_advertised_route_set_logical_port(sr, route_e->logical_port);
> >          sbrec_advertised_route_set_ip_prefix(sr, route_e->ip_prefix);
> > +        sbrec_advertised_route_set_tracked_port(sr, route_e->tracked_port);
> >          ar_entry_free(route_e);
> >      }
> >  
> > diff --git a/northd/en-advertised-route-sync.h 
> > b/northd/en-advertised-route-sync.h
> > index 30e7cae1f..1f24fd329 100644
> > --- a/northd/en-advertised-route-sync.h
> > +++ b/northd/en-advertised-route-sync.h
> > @@ -17,10 +17,21 @@
> >  #define EN_ADVERTISED_ROUTE_SYNC_H 1
> >  
> >  #include "lib/inc-proc-eng.h"
> > +#include "lib/uuidset.h"
> >  
> >  struct advertised_route_sync_data {
> > +  /* Contains the uuids of all NB Logical Routers where we used a
> > +   * lr_stateful_record during computation. */
> > +  struct uuidset nb_lr;
> > +  /* Contains the uuids of all NB Logical Switches where we rely on port
> > +   * changes for host routes. */
> > +  struct uuidset nb_ls;
> >  };
> >  
> > +bool advertised_route_sync_lr_stateful_change_handler(struct engine_node *,
> > +                                                      void *data);
> > +bool advertised_route_sync_northd_change_handler(struct engine_node *,
> > +                                                 void *data);
> >  void *en_advertised_route_sync_init(struct engine_node *, struct 
> > engine_arg *);
> >  void en_advertised_route_sync_cleanup(void *data);
> >  void en_advertised_route_sync_run(struct engine_node *, void *data);
> > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> > index fa7ca6f2d..86b7b999e 100644
> > --- a/northd/inc-proc-northd.c
> > +++ b/northd/inc-proc-northd.c
> > @@ -284,6 +284,10 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> >      engine_add_input(&en_advertised_route_sync, &en_routes, NULL);
> >      engine_add_input(&en_advertised_route_sync, &en_sb_advertised_route,
> >                       NULL);
> > +    engine_add_input(&en_advertised_route_sync, &en_lr_stateful,
> > +                     advertised_route_sync_lr_stateful_change_handler);
> > +    engine_add_input(&en_advertised_route_sync, &en_northd,
> > +                     advertised_route_sync_northd_change_handler);
> >  
> >      engine_add_input(&en_learned_route_sync, &en_routes, NULL);
> >      engine_add_input(&en_learned_route_sync, &en_sb_learned_route, NULL);
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 5cbf6c800..3f14cc75c 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -1122,19 +1122,6 @@ build_datapaths(struct ovsdb_idl_txn *ovnsb_txn,
> >      ods_build_array_index(lr_datapaths);
> >  }
> >  
> > -/* Structure representing logical router port
> > - * routable addresses. This includes DNAT and Load Balancer
> > - * addresses. This structure will only be filled in if the
> > - * router port is a gateway router port. Otherwise, all pointers
> > - * will be NULL and n_addrs will be 0.
> > - */
> > -struct ovn_port_routable_addresses {
> > -    /* The parsed routable addresses */
> > -    struct lport_addresses *laddrs;
> > -    /* Number of items in the laddrs array */
> > -    size_t n_addrs;
> > -};
> > -
> >  static bool lsp_can_be_inc_processed(const struct 
> > nbrec_logical_switch_port *);
> >  
> >  /* This function returns true if 'op' is a gateway router port.
> > @@ -1169,7 +1156,7 @@ is_cr_port(const struct ovn_port *op)
> >      return op->primary_port;
> >  }
> >  
> > -static void
> > +void
> >  destroy_routable_addresses(struct ovn_port_routable_addresses *ra)
> >  {
> >      for (size_t i = 0; i < ra->n_addrs; i++) {
> > @@ -1182,12 +1169,14 @@ static char **get_nat_addresses(const struct 
> > ovn_port *op, size_t *n,
> >                                  bool routable_only, bool include_lb_ips,
> >                                  const struct lr_stateful_record *);
> >  
> > -static struct ovn_port_routable_addresses
> > -get_op_routable_addresses(struct ovn_port *op,
> > -                          const struct lr_stateful_record *lr_stateful_rec)
> > +struct ovn_port_routable_addresses
> > +get_op_addresses(struct ovn_port *op,
> > +                 const struct lr_stateful_record *lr_stateful_rec,
> > +                 bool routable_only)
> >  {
> >      size_t n;
> > -    char **nats = get_nat_addresses(op, &n, true, true, lr_stateful_rec);
> > +    char **nats = get_nat_addresses(op, &n, routable_only, true,
> > +                                    lr_stateful_rec);
> >  
> >      if (!nats) {
> >          return (struct ovn_port_routable_addresses) {
> > @@ -1220,6 +1209,13 @@ get_op_routable_addresses(struct ovn_port *op,
> >      };
> >  }
> >  
> > +static struct ovn_port_routable_addresses
> > +get_op_routable_addresses(struct ovn_port *op,
> > +                          const struct lr_stateful_record *lr_stateful_rec)
> > +{
> > +    return get_op_addresses(op, lr_stateful_rec, true);
> > +}
> > +
> >  
> >  static void
> >  ovn_port_set_nb(struct ovn_port *op,
> > @@ -2267,6 +2263,9 @@ join_logical_ports_lrp(struct hmap *ports,
> >  
> >      op->prefix_delegation = smap_get_bool(&op->nbrp->options,
> >                                            "prefix_delegation", false);
> > +    op->dynamic_routing_connected_as_host_routes = smap_get_bool(
> > +        &op->nbrp->options,
> > +        "dynamic-routing-connected-as-host-routes", false);
> >      parse_dynamic_routing_redistribute(&op->nbrp->options,
> >                                         &op->dynamic_routing_connected,
> >                                         od->dynamic_routing_connected,
> > diff --git a/northd/northd.h b/northd/northd.h
> > index 75a3df617..15bd01c05 100644
> > --- a/northd/northd.h
> > +++ b/northd/northd.h
> > @@ -25,6 +25,7 @@
> >  #include "openvswitch/hmap.h"
> >  #include "simap.h"
> >  #include "ovs-thread.h"
> > +#include "en-lr-stateful.h"
> >  
> >  struct northd_input {
> >      /* Northbound table references */
> > @@ -625,6 +626,8 @@ struct ovn_port {
> >       * If the option is unset it will be initialized based on the nbr
> >       * option. */
> >      bool dynamic_routing_connected;
> > +    /* nbrp option "dynamic-routing-connected-as-host-routes" is "true". */
> > +    bool dynamic_routing_connected_as_host_routes;
> >      /* nbrp option "dynamic-routing-redistribute" contains "static".
> >       * If the option is unset it will be initialized based on the nbr
> >       * option. */
> > @@ -935,4 +938,23 @@ void build_igmp_lflows(struct hmap *igmp_groups,
> >                         const struct hmap *ls_datapaths,
> >                         struct lflow_table *lflows,
> >                         struct lflow_ref *lflow_ref);
> 
> We could use an empty line here.
> 
> > +/* Structure representing logical router port routable addresses. This
> > + * includes DNAT and Load Balancer addresses. This structure will only
> > + * be filled in if the router port is a gateway router port. Otherwise,
> > + * all pointers will be NULL and n_addrs will be 0.
> > + */
> > +struct ovn_port_routable_addresses {
> > +    /* The parsed routable addresses */
> > +    struct lport_addresses *laddrs;
> > +    /* Number of items in the laddrs array */
> > +    size_t n_addrs;
> > +};
> > +
> > +struct ovn_port_routable_addresses get_op_addresses(
> > +    struct ovn_port *op,
> > +    const struct lr_stateful_record *lr_stateful_rec,
> > +    bool routable_only);
> > +
> > +void destroy_routable_addresses(struct ovn_port_routable_addresses *ra);
> > +
> >  #endif /* NORTHD_H */
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index bf785e4d3..d0bdb5058 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -3752,6 +3752,33 @@ or
> >          key="dynamic-routing-redistribute" table="Logical_Router"/> will be
> >          used.
> >        </column>
> > +      <column name="options" key="dynamic-routing-connected-as-host-routes"
> > +              type='{"type": "boolean"}'>
> > +        Only relevant if <ref column="options" key="dynamic-routing"
> > +        table="Logical_Router"/> on the respective Logical_Router is set
> > +        to <code>true</code> and also
> > +        <ref column="options" key="dynamic-routing-connected"/> is enabled 
> > on
> > +        the LR or LRP.
> > +
> > +        If set to true the prefix connected to the LRP is not advertised 
> > as a
> > +        whole. Rather each individual IP address that is actually in use 
> > inside
> > +        this prefix is announced as a host route. Default is false.
> > +
> > +        This can be used to:
> > +        <ul>
> > +          <li>
> > +            allow the fabric outside of OVN to drop traffic towards IP
> > +            addresses that are not actually used. This traffic would 
> > otherwise
> > +            hit this LR and then be dropped.
> > +          </li>
> > +
> > +          <li>
> > +            If this LR has multiple LRPs connected to the fabric on 
> > different
> > +            chassis: allows the fabric outside of OVN to steer packets to 
> > the
> > +            chassis which already hosts this backing ip address.
> > +          </li>
> > +        </ul>
> > +      </column>
> >      </group>
> >  
> >      <group title="Attachment">
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 52a458e35..18d367fa5 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -14845,6 +14845,66 @@ AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | 
> > ovn_strip_lflows], [0], [dnl
> >  AT_CLEANUP
> >  ])
> >  
> > +OVN_FOR_EACH_NORTHD_NO_HV([
> > +AT_SETUP([dynamic-routing - host routes])
> > +AT_KEYWORDS([dynamic-routing])
> > +ovn_start
> > +
> > +# We start with announcing routes on a lr with 2 lrps.
> > +# LRP lr0-sw0 is connected to ls sw0.
> > +check ovn-nbctl lr-add lr0
> > +check ovn-nbctl set Logical_Router lr0 option:dynamic-routing=true \
> > +                                 
> > option:dynamic-routing-redistribute="connected;static"
> > +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
> > +sw0=$(fetch_column port_binding _uuid logical_port=lr0-sw0)
> > +check ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 10.0.1.1/24
> > +sw1=$(fetch_column port_binding _uuid logical_port=lr0-sw1)
> > +check ovn-nbctl ls-add sw0
> > +check ovn-nbctl lsp-add sw0 sw0-lr0
> > +check ovn-nbctl --wait=sb set Logical_Switch_Port sw0-lr0 type=router 
> > options:router-port=lr0-sw0
> > +check_row_count Advertised_Route 2 tracked_port='[[]]'
> > +datapath=$(fetch_column datapath_binding _uuid external_ids:name=lr0)
> > +
> > +# Configuring the LRP lr0-sw0 to send host routes.
> > +# As sw0 is quite empty we will only see the addresses of lr0-sw0.
> > +check ovn-nbctl --wait=sb set Logical_Router_Port lr0-sw0 
> > options:dynamic-routing-connected-as-host-routes=true
> > +check_row_count Advertised_Route 2
> > +check_column 10.0.0.1/32 Advertised_Route ip_prefix datapath=$datapath 
> > logical_port=$sw0
> > +check_column $sw0 Advertised_Route tracked_port datapath=$datapath 
> > logical_port=$sw0
> > +
> > +# Adding a VIF to the LS sw0 will advertise it as well.
> > +check ovn-nbctl lsp-add sw0 sw0-vif0
> > +check ovn-nbctl --wait=sb lsp-set-addresses sw0-vif0 "00:aa:bb:cc:dd:ee 
> > 10.0.0.2"
> > +vif0=$(fetch_column port_binding _uuid logical_port=sw0-vif0)
> > +check_row_count Advertised_Route 3
> > +check_row_count Advertised_Route 2 tracked_port!='[[]]'
> > +check_column $vif0 Advertised_Route tracked_port datapath=$datapath 
> > logical_port=$sw0 ip_prefix=10.0.0.2/32
> > +
> > +# Adding a LR lr1 to the LS sw0 will advertise the LRP of the new router.
> > +check ovn-nbctl lr-add lr1
> > +check ovn-nbctl lrp-add lr1 lr1-sw0 00:00:00:01:ff:01 10.0.0.10/24
> > +check ovn-nbctl lsp-add sw0 sw0-lr1
> > +lr1=$(fetch_column port_binding _uuid logical_port=lr1-sw0)
> > +check ovn-nbctl --wait=sb set Logical_Switch_Port sw0-lr1 type=router 
> > options:router-port=lr1-sw0
> > +check_row_count Advertised_Route 4
> > +check_row_count Advertised_Route 3 tracked_port!='[[]]'
> > +check_column $lr1 Advertised_Route tracked_port datapath=$datapath 
> > logical_port=$sw0 ip_prefix=10.0.0.10/32
> > +
> > +# Adding a NAT rule to lr1 will advertise it as well.
> > +check ovn-nbctl --wait=sb lr-nat-add lr1 dnat_and_snat 10.0.0.100 
> > 192.168.0.1
> > +check_row_count Advertised_Route 5
> > +check_row_count Advertised_Route 4 tracked_port!='[[]]'
> > +check_column $lr1 Advertised_Route tracked_port datapath=$datapath 
> > logical_port=$sw0 ip_prefix=10.0.0.100/32
> > +
> > +# Adding a static route to lr1 will be advertised just normally.
> > +check ovn-nbctl --wait=sb lr-route-add lr0 172.16.0.0/24 10.0.0.200
> > +check_row_count Advertised_Route 6
> > +check_row_count Advertised_Route 4 tracked_port!='[[]]'
> > +check_row_count Advertised_Route 1 datapath=$datapath logical_port=$sw0 
> > ip_prefix=172.16.0.0/24
> > +
> > +AT_CLEANUP
> > +])
> > +
> >  OVN_FOR_EACH_NORTHD_NO_HV([
> >  AT_SETUP([dynamic-routing incremental processing])
> >  AT_KEYWORDS([dynamic-routing])
> > @@ -14868,13 +14928,16 @@ check_engine_stats lflow recompute nocompute
> >  CHECK_NO_CHANGE_AFTER_RECOMPUTE
> >  
> >  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > -check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
> > +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
> > +check ovn-nbctl ls-add sw0
> > +check ovn-nbctl lsp-add sw0 sw0-lr0
> > +check ovn-nbctl --wait=sb set Logical_Switch_Port sw0-lr0 type=router 
> > options:router-port=lr0-sw0
> >  sw0=$(fetch_column port_binding _uuid logical_port=lr0-sw0)
> >  check_engine_stats northd recompute compute
> > -check_engine_stats routes recompute nocompute
> > -check_engine_stats advertised_route_sync recompute nocompute
> > -check_engine_stats learned_route_sync recompute nocompute
> > -check_engine_stats lflow recompute nocompute
> > +check_engine_stats routes recompute compute
> > +check_engine_stats advertised_route_sync recompute compute
> > +check_engine_stats learned_route_sync recompute compute
> > +check_engine_stats lflow recompute compute
> >  CHECK_NO_CHANGE_AFTER_RECOMPUTE
> >  
> >  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > @@ -14980,6 +15043,46 @@ check_engine_stats learned_route_sync recompute 
> > nocompute
> >  check_engine_stats lflow recompute nocompute
> >  CHECK_NO_CHANGE_AFTER_RECOMPUTE
> >  
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +check ovn-nbctl --wait=sb set Logical_Router_Port lr0-sw0 
> > options:dynamic-routing-connected-as-host-routes=true
> > +check_engine_stats northd recompute nocompute
> > +check_engine_stats routes recompute nocompute
> > +check_engine_stats advertised_route_sync recompute nocompute
> > +check_engine_stats learned_route_sync recompute nocompute
> > +check_engine_stats lflow recompute nocompute
> > +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > +
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +check ovn-nbctl lsp-add sw0 sw0-vif0
> > +check ovn-nbctl --wait=sb lsp-set-addresses sw0-vif0 "00:aa:bb:cc:dd:ee 
> > 10.0.0.2"
> > +check_engine_stats northd norecompute compute
> > +check_engine_stats routes norecompute compute
> > +check_engine_stats advertised_route_sync recompute nocompute
> > +check_engine_stats learned_route_sync norecompute compute
> > +check_engine_stats lflow norecompute compute
> > +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > +
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +check ovn-nbctl lr-add lr1
> > +check ovn-nbctl lrp-add lr1 lr1-sw0 00:00:00:01:ff:01 10.0.0.10/24
> > +check ovn-nbctl lsp-add sw0 sw0-lr1
> > +check ovn-nbctl --wait=sb set Logical_Switch_Port sw0-lr1 type=router 
> > options:router-port=lr1-sw0
> > +check_engine_stats northd recompute compute
> > +check_engine_stats routes recompute compute
> > +check_engine_stats advertised_route_sync recompute nocompute
> > +check_engine_stats learned_route_sync recompute compute
> > +check_engine_stats lflow recompute compute
> > +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > +
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +check ovn-nbctl --wait=sb lr-nat-add lr1 dnat_and_snat 10.0.0.100 
> > 192.168.0.1
> > +check_engine_stats northd norecompute compute
> > +check_engine_stats routes norecompute compute
> > +check_engine_stats advertised_route_sync recompute nocompute
> > +check_engine_stats learned_route_sync norecompute compute
> > +check_engine_stats lflow norecompute compute
> > +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > +
> >  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> >  check ovn-nbctl --wait=sb lrp-del lr0-sw0
> >  check_engine_stats northd recompute compute
> 
> Thanks,
> Dumitru
> 
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to