On 2/7/25 9:00 AM, [email protected] wrote:
> Hi Felix,
> I noticed small nits in the documentation, I'll list them below.
> 
> On Thu, 2025-02-06 at 15:19 +0100, 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]>
>> ---
>> v6->v7:
>>   * switch the setting to a dynamic-routing-redistribute option
>>   * support lrp<->lrp peer connection for host routes
>>   * addressed review comments
>> 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 | 249
>> ++++++++++++++++++++++++++++--
>>  northd/en-advertised-route-sync.h |  11 ++
>>  northd/inc-proc-northd.c          |   4 +
>>  northd/northd.c                   |  37 ++---
>>  northd/northd.h                   |  25 ++-
>>  ovn-nb.xml                        | 123 +++++++++++----
>>  tests/ovn-northd.at               | 127 ++++++++++++++-
>>  8 files changed, 514 insertions(+), 65 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index 73a06843c..60d1e793a 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -53,6 +53,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 value "connected-as-host" to the
>> +       "dynamic-routing-redistribute" LR and LRP option. If set 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 0fa91f5aa..9e8636806 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,22 @@ 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)
>>  {
>>      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;
>> +    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 +192,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 +216,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 +233,118 @@ 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,
>> +                        const 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 via this LRP. */
>> +static void
>> +publish_host_routes_lrp(struct hmap *sync_routes,
>> +                        const struct lr_stateful_table
>> *lr_stateful_table,
>> +                        const struct parsed_route *route,
>> +                        struct advertised_route_sync_data *data,
>> +                        const struct ovn_port *lrp)
>> +{
>> +    /* This is a LSP connected to an LRP */
>> +    const struct lport_addresses *addresses = &lrp->lrp_networks;
>> +    publish_lport_addresses(sync_routes, route->od->sb,
>> +                            route->out_port,
>> +                            addresses, lrp);
>> +
>> +    const struct lr_stateful_record *lr_stateful_rec;
>> +    lr_stateful_rec = lr_stateful_table_find_by_index(
>> +        lr_stateful_table, lrp->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(
>> +        lrp, 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],
>> +                                lrp);
>> +    }
>> +    destroy_routable_addresses(&addrs);
>> +}
>> +
>> +/* 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)
>> +{
>> +    if (!route->out_port->peer) {
>> +        return;
>> +    }
>> +
>> +    struct ovn_datapath *peer_od = route->out_port->peer->od;
>> +    if (!peer_od->nbs && !peer_od->nbr) {
>> +        return;
>> +    }
>> +
>> +    if (peer_od->nbr) {
>> +        /* This is a LRP directly connected to another LRP. */
>> +        publish_host_routes_lrp(sync_routes, lr_stateful_table,
>> route,
>> +                                data, route->out_port->peer);
>> +        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, &peer_od->nbs->header_.uuid);
>> +
>> +    struct ovn_port *port;
>> +    HMAP_FOR_EACH (port, dp_node, &peer_od->ports) {
>> +        if (port->peer) {
>> +            /* This is a LSP connected to an LRP */
>> +            publish_host_routes_lrp(sync_routes, lr_stateful_table,
>> route,
>> +                                    data, port->peer);
>> +        } 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;
>> @@ -151,9 +362,21 @@ advertised_route_table_sync(
>>  
>>          enum dynamic_routing_redistribute_mode drr =
>>              route->out_port->dynamic_routing_redistribute;
>> -        if (route->source == ROUTE_SOURCE_CONNECTED &&
>> -              (drr & DRRM_CONNECTED) == 0) {
>> -            continue;
>> +        if (route->source == ROUTE_SOURCE_CONNECTED) {
>> +            if ((drr & DRRM_CONNECTED) == 0) {
>> +                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 (drr & DRRM_CONNECTED_AS_HOST &&
>> +                !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 && (drr &
>> DRRM_STATIC) == 0) {
>>              continue;
>> @@ -161,14 +384,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);
>> @@ -183,6 +407,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 51d8a6b56..3cd1ba9a6 100644
>> --- a/northd/northd.c
>> +++ b/northd/northd.c
>> @@ -828,6 +828,11 @@ parse_dynamic_routing_redistribute(
>>              out |= DRRM_CONNECTED;
>>              continue;
>>          }
>> +        if (!strcmp(token, "connected-as-host")) {
>> +            /* Setting connected-as-host implies connected. */
>> +            out |= DRRM_CONNECTED | DRRM_CONNECTED_AS_HOST;
>> +            continue;
>> +        }
>>          if (!strcmp(token, "static")) {
>>              out |= DRRM_STATIC;
>>              continue;
>> @@ -1119,19 +1124,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.
>> @@ -1177,7 +1169,7 @@ is_transit_router_port(struct ovn_port *op)
>>             smap_get_bool(&op->sb->chassis->other_config, "is-
>> remote", false);
>>  }
>>  
>> -static void
>> +void
>>  destroy_routable_addresses(struct ovn_port_routable_addresses *ra)
>>  {
>>      for (size_t i = 0; i < ra->n_addrs; i++) {
>> @@ -1190,12 +1182,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(const 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) {
>> @@ -1228,6 +1222,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,
>> diff --git a/northd/northd.h b/northd/northd.h
>> index 1fa0d6365..a4bff1987 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 */
>> @@ -302,12 +303,14 @@ struct mcast_port_info {
>>  
>>  enum dynamic_routing_redistribute_mode_bits {
>>      DRRM_CONNECTED_BIT = 0,
>> -    DRRM_STATIC_BIT    = 1,
>> +    DRRM_CONNECTED_AS_HOST_BIT = 1,
>> +    DRRM_STATIC_BIT    = 2,
>>  };
>>  
>>  enum dynamic_routing_redistribute_mode {
>>      DRRM_NONE      = 0,
>>      DRRM_CONNECTED = (1 << DRRM_CONNECTED_BIT),
>> +    DRRM_CONNECTED_AS_HOST = (1 << DRRM_CONNECTED_AS_HOST_BIT),
>>      DRRM_STATIC    = (1 << DRRM_STATIC_BIT),
>>  };
>>  
>> @@ -954,4 +957,24 @@ void build_igmp_lflows(struct hmap *igmp_groups,
>>                         const struct hmap *ls_datapaths,
>>                         struct lflow_table *lflows,
>>                         struct lflow_ref *lflow_ref);
>> +
>> +/* 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(
>> +    const 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 895d1b3a1..cc94937bc 100644
>> --- a/ovn-nb.xml
>> +++ b/ovn-nb.xml
>> @@ -2978,31 +2978,63 @@ or
>>          route types that should be advertised. See:
>>          <ul>
>>          <li><ref column="options" key="dynamic-routing-redistribute"
>> -                 table="Logical_Router"/></li>
>> +                 table="Logical_Router"/> on Logical_Router</li>
>>          <li><ref column="options" key="dynamic-routing-redistribute"
>> -                 table="Logical_Router_Port"/></li>
>> +                 table="Logical_Router_Port"/> on
>> Logical_Router_Port</li>
>>          </ul>
>>        </column>
>>  
>>        <column name="options" key="dynamic-routing-redistribute"
>>                type='{"type": "string"}'>
>> -        Only relevant if <ref column="options" key="dynamic-routing"
>> -        table="Logical_Router"/> is set to <code>true</code>.
>> +        <p>
>> +          Only relevant if <ref column="options" key="dynamic-
>> routing"
>> +          table="Logical_Router"/> is set to <code>true</code>.
>> +        </p>
>>  
>> -        This is a list of elements separated by <code>;</code>.
>> +        <p>
>> +          This is a list of elements separated by <code>;</code>.
>> +        </p>
>>  
>> -        If <code>connected</code> is in the list then northd will
>> synchronize
>> -        all "connected" routes to the southbound <ref table="Route"
>> -        db="OVN_SB"/> table. "Connected" here means routes
>> implicitly created
>> -        by networks associated with the LRPs.
>> +        <p>
>> +          If <code>connected</code> is in the list then northd will
>> synchronize
>> +          all "connected" routes to the southbound <ref
>> table="Route"
>> +          db="OVN_SB"/> table. "Connected" here means routes
>> implicitly created
>> +          by networks associated with the LRPs.
>> +        </p>
>>  
>> -        If <code>static</code> is in the list then northd will
>> synchronize all
>> -        <ref table="Logical_Router_Static_Route"/> to the southbound
>> -        <ref table="Route" db="OVN_SB"/> table.
>> +        <p>
>> +          If <code>connected-as-host</code> is in the list then
>> northd will
>> +          enumerate all actively used individual IPs of a
>> "connected" route and
>> +          announce these IPs as host routes instead of announcing
>> the subnet.
>> +          Setting this implies the setting <code>connected</code>.
>>  
>> -        This value can be overwritten on a per LRP basis using
>> -        <ref column="options" key="dynamic-routing-redistribute"
>> -             table="Logical_Router_Port"/>.
>> +          This setting 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>
>> +        </p>
>> +
>> +        <p>
>> +          If <code>static</code> is in the list then northd will
>> synchronize
>> +          all <ref table="Logical_Router_Static_Route"/> to the
>> southbound
>> +          <ref table="Route" db="OVN_SB"/> table.
>> +        </p>
>> +
>> +        <p>
>> +          This value can be overwritten on a per LRP basis using
>> +          <ref column="options" key="dynamic-routing-redistribute"
>> +               table="Logical_Router_Port"/> on the
>> Logical_Router_Port.
>> +        </p>
>>        </column>
>>      </group>
>>  
>> @@ -3774,24 +3806,57 @@ or
>>  
>>        <column name="options" key="dynamic-routing-redistribute"
>>                type='{"type": "string"}'>
>> -        Only relevant if <ref column="options" key="dynamic-routing"
>> -        table="Logical_Router"/> on the respective Logical_Router is
>> set
>> -        to <code>true</code>.
>> +        <p>
>> +          Only relevant if <ref column="options" key="dynamic-
>> routing"
>> +          table="Logical_Router"/> on the respective Logical_Router
>> is set
>> +          to <code>true</code>.
>> +        </p>
>>  
>> -        This is a list of elements separated by <code>;</code>.
>> +        <p>
>> +          This is a list of elements separated by <code>;</code>.
>> +        </p>
>>  
>> -        If <code>connected</code> is in the list then northd will
>> synchronize
>> -        all "connected" routes to the southbound <ref table="Route"
>> -        db="OVN_SB"/> table. "Connected" here means routes
>> implicitly created
>> -        by networks associated with the LRPs.
>> +        <p>
>> +          If <code>connected</code> is in the list then northd will
>> synchronize
>> +          all "connected" routes to the southbound <ref
>> table="Route"
>> +          db="OVN_SB"/> table. "Connected" here means routes
>> implicitly created
> 
> Two nits that apply here and in the rest of the ovn-nb.xml file.
> 
> nit1: you use db="OVN_SB", but the rest of the file seems to
> historically use db="OVN_Southbound".
> 
> nit2: You reference table=Route. Did you mean table="Advertised_Route"?
> 
> Thanks,
> Martin.
> 

With Martin's comments addressed feel free to add my ack too:
Acked-by: Dumitru Ceara <[email protected]>

Thanks,
Dumitru

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to