On 2/6/25 11:46 AM, Felix Huettner via dev wrote:
> On Thu, Feb 06, 2025 at 10:45:15AM +0100, [email protected] wrote:
>> He Felix, I have one comment regarding the publishing of SNATs that was
>> discussed in my patch series as well.
>>
>> On Tue, 2025-02-04 at 14:59 +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]>
>>> ---
>>> 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)
>>> {
>>> 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;
>>> + }
>>> 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);
>>
>> If I understand correctly, the only purpose of the call to
>> get_op_addresses is to get NAT addresses with "add_route=true" option.
>> I'm still unsure whether this fits here. With the follow-up series
>> dedicated to advertising NAT/LB addresses, wouldn't it be better if NAT
>> addresses were exclusively handled in there? i.e. only when a dynamic-
>> routing-redistribute=nat option is set?
>
> Hi Martin,
>
> i still see it in the same way as commented on the other patch.
>
> Lets assume we have LR1 with LRP1. LRP1 has 192.168.0.1/24 configured.
> Also we have LR2 with LRP2. LRP1 and LRP2 are connected (directly or via LS)
> LRP2 has 192.168.0.10/24 configured. Also LR2 has a nat rule with an external
> IP of 192.168.0.20 configured.
>
> If we enable dynamic-routing on LR1 and redistribute connected routes we
> would get an advertisement of 192.168.0.0/24. This means both the LRP
> addresses and the NAT addresses are in the scope of the advertised
> route. Traffic to other IPs of that subnet would just be dropped by LR1
> if it would receive it (as there is no destination).
>
> The idea is that the "connected-as-host" setting does not change the
> reachable addresses in any way. So we still should be able to reach both
> LRPs and the NAT addresses.
> The only change this setting does is how the routes are advertised. But
> it should not change the behaviour for any routeable address.
>
I tend to agree with Felix on this one. I think it makes sense that if
a NAT/LB IP is part of the router port subnet then redistributing
"connected" (either aggregated or as host routes) should include that IP
too.
>
> In contrast to that i would understand the
> dynamic-routing-redistribute=nat setting to advertise NAT rules that are
> configured on LR1. Because these would currently not be announced in any
> way.
>
> I hope that clarifies my perspective.
>
> Thanks a lot,
> Felix
>
>>
>> Thanks,
>> Martin.
>>
>>> + 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);
>>> +/* 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
>>
>> _______________________________________________
>> dev mailing list
>> [email protected]
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
