Message ID | 3eac90770394f381eb5191c264802755dd825a2f.1732630355.git.felix.huettner@stackit.cloud |
---|---|
State | Changes Requested |
Headers | show |
Series | OVN Fabric integration | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
Hi Felix, Thanks for the patch. I would ask if it makes sense to have logical flows for NAT addresses that are in different subnets of the network of the LR IP. In this case, when using the active-active feature, the tear one LR would have those logical flows to know how to route the traffic back to the tear two LR (project routers). This use case happens when the user separates NAT IPs in a subnet and LRPs addresses in another one. Nowadays this scenario works when the external network (LS) has the localnet port and can rely in the ARP as the ovn-controller sends the GARP or using NDR(Neutron dynamic routing) to advertise the /32 or /64 having the next-hop as the LRP uplink address. Regards, Tiago Pires On Tue, Nov 26, 2024 at 11:40 AM Felix Huettner via dev <ovs-dev@openvswitch.org> wrote: > > sometimes we want to use individual host routes instead of the connected > routes of LRPs. > This allows the network fabric to know which adresses are actually in > use and e.g. drop traffic to adresses that are not used anyway. > > Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud> > --- > NEWS | 2 + > northd/en-lflow.c | 1 + > northd/en-routes-sync.c | 270 ++++++++++++++++++++++++++++++++++----- > northd/en-routes-sync.h | 22 +++- > northd/inc-proc-northd.c | 2 + > northd/northd.c | 32 ++--- > northd/northd.h | 25 +++- > ovn-nb.xml | 27 ++++ > ovn-sb.ovsschema | 3 +- > ovn-sb.xml | 13 ++ > tests/ovn-northd.at | 67 ++++++++++ > 11 files changed, 406 insertions(+), 58 deletions(-) > > diff --git a/NEWS b/NEWS > index efb42b32b..a780940fc 100644 > --- a/NEWS > +++ b/NEWS > @@ -14,6 +14,8 @@ Post v24.09.0 > Routes entered into the "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-lflow.c b/northd/en-lflow.c > index 8995f0300..aabba943e 100644 > --- a/northd/en-lflow.c > +++ b/northd/en-lflow.c > @@ -26,6 +26,7 @@ > #include "en-northd.h" > #include "en-meters.h" > #include "en-sampling-app.h" > +#include "en-routes-sync.h" > #include "lflow-mgr.h" > > #include "lib/inc-proc-eng.h" > diff --git a/northd/en-routes-sync.c b/northd/en-routes-sync.c > index 7ca87c76a..d5a534f86 100644 > --- a/northd/en-routes-sync.c > +++ b/northd/en-routes-sync.c > @@ -13,6 +13,7 @@ > */ > > #include <config.h> > +#include <stdbool.h> > > #include "openvswitch/vlog.h" > #include "smap.h" > @@ -20,6 +21,7 @@ > #include "northd.h" > > #include "en-routes-sync.h" > +#include "en-lr-stateful.h" > #include "lib/stopwatch-names.h" > #include "openvswitch/hmap.h" > #include "ovn-util.h" > @@ -29,15 +31,19 @@ VLOG_DEFINE_THIS_MODULE(en_routes_sync); > static void > routes_table_sync(struct ovsdb_idl_txn *ovnsb_txn, > const struct sbrec_route_table *sbrec_route_table, > + const struct lr_stateful_table *lr_stateful_table, > const struct hmap *parsed_routes, > const struct hmap *lr_ports, > const struct ovn_datapaths *lr_datapaths, > - struct hmap *parsed_routes_out); > + struct hmap *parsed_routes_out, > + struct routes_sync_tracked_data *trk_data); > > static void > routes_sync_init(struct routes_sync_data *data) > { > hmap_init(&data->parsed_routes); > + uuidset_init(&data->trk_data.nb_lr_stateful); > + uuidset_init(&data->trk_data.nb_ls); > } > > static void > @@ -48,12 +54,15 @@ routes_sync_destroy(struct routes_sync_data *data) > parsed_route_free(r); > } > hmap_destroy(&data->parsed_routes); > + uuidset_destroy(&data->trk_data.nb_lr_stateful); > + uuidset_destroy(&data->trk_data.nb_ls); > } > > bool > routes_sync_northd_change_handler(struct engine_node *node, > - void *data OVS_UNUSED) > + void *data_) > { > + struct routes_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; > @@ -69,7 +78,58 @@ routes_sync_northd_change_handler(struct engine_node *node, > * this happens and so does this node. > * Note: When we add I-P to the created/deleted logical routers or > * logical router ports, we need to revisit this handler. > + * 3. Indirectly northd_data->ls_ports if we announce host routes > + * This is what we check below > */ > + > + 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->trk_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->trk_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->trk_data.nb_ls, > + &op->od->nbs->header_.uuid)) { > + return false; > + } > + } > + > + return true; > +} > + > +bool > +routes_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 routes_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->trk_data.nb_lr_stateful, > + &lr_stateful_rec->nbr_uuid)) { > + return false; > + } > + } > + > return true; > } > > @@ -101,14 +161,18 @@ en_routes_sync_run(struct engine_node *node, void *data) > const struct sbrec_route_table *sbrec_route_table = > EN_OVSDB_GET(engine_get_input("SB_route", node)); > struct northd_data *northd_data = engine_get_input_data("northd", node); > + struct ed_type_lr_stateful *lr_stateful_data = > + engine_get_input_data("lr_stateful", node); > > stopwatch_start(ROUTES_SYNC_RUN_STOPWATCH_NAME, time_msec()); > > routes_table_sync(eng_ctx->ovnsb_idl_txn, sbrec_route_table, > + &lr_stateful_data->table, > &routes_data->parsed_routes, > &northd_data->lr_ports, > &northd_data->lr_datapaths, > - &routes_sync_data->parsed_routes); > + &routes_sync_data->parsed_routes, > + &routes_sync_data->trk_data); > > stopwatch_stop(ROUTES_SYNC_RUN_STOPWATCH_NAME, time_msec()); > engine_set_node_state(node, EN_UPDATED); > @@ -122,6 +186,7 @@ struct route_entry { > > char *logical_port; > char *ip_prefix; > + char *tracked_port; > char *type; > bool stale; > }; > @@ -129,7 +194,10 @@ struct route_entry { > static struct route_entry * > route_alloc_entry(struct hmap *routes, > const struct sbrec_datapath_binding *sb_db, > - char *logical_port, char *ip_prefix, char *route_type) > + const char *logical_port, > + const char *ip_prefix, > + const char *route_type, > + const char *tracked_port) > { > struct route_entry *route_e = xzalloc(sizeof *route_e); > > @@ -137,6 +205,9 @@ route_alloc_entry(struct hmap *routes, > route_e->logical_port = xstrdup(logical_port); > route_e->ip_prefix = xstrdup(ip_prefix); > route_e->type = xstrdup(route_type); > + if (tracked_port) { > + route_e->tracked_port = xstrdup(tracked_port); > + } > route_e->stale = false; > uint32_t hash = uuid_hash(&sb_db->header_.uuid); > hash = hash_string(logical_port, hash); > @@ -149,27 +220,72 @@ route_alloc_entry(struct hmap *routes, > static struct route_entry * > route_lookup_or_add(struct hmap *route_map, > const struct sbrec_datapath_binding *sb_db, > - char *logical_port, const struct in6_addr *prefix, > - unsigned int plen, char *route_type) > + const char *logical_port, const char *ip_prefix, > + const char *route_type, const char *tracked_port) > { > struct route_entry *route_e; > uint32_t hash; > > - char *ip_prefix = normalize_v46_prefix(prefix, plen); > - > hash = uuid_hash(&sb_db->header_.uuid); > hash = hash_string(logical_port, hash); > hash = hash_string(ip_prefix, hash); > HMAP_FOR_EACH_WITH_HASH (route_e, hmap_node, hash, route_map) { > - if (!strcmp(route_e->type, route_type)) { > - free(ip_prefix); > - return route_e; > + if (!uuid_equals(&sb_db->header_.uuid, > + &route_e->sb_db->header_.uuid)) { > + continue; > } > + > + if (strcmp(logical_port, route_e->logical_port)) { > + continue; > + } > + > + if (strcmp(ip_prefix, route_e->ip_prefix)) { > + continue; > + } > + > + if (strcmp(route_type, route_e->type)) { > + continue; > + } > + > + if (!streq(tracked_port, route_e->tracked_port)) { > + continue; > + } > + > + return route_e; > + } > + > + route_e = route_alloc_entry(route_map, sb_db, > + logical_port, ip_prefix, route_type, > + tracked_port); > + return route_e; > +} > + > +static struct route_entry * > +route_sync_to_sb(struct ovsdb_idl_txn *ovnsb_txn, struct hmap *route_map, > + const struct sbrec_datapath_binding *sb_db, > + const char *logical_port, const char *ip_prefix, > + const char *route_type, const char *tracked_port) > +{ > + struct route_entry *route_e = route_lookup_or_add(route_map, > + sb_db, > + logical_port, > + ip_prefix, > + route_type, > + tracked_port); > + route_e->stale = false; > + > + if (!route_e->sb_route) { > + const struct sbrec_route *sr = sbrec_route_insert(ovnsb_txn); > + sbrec_route_set_datapath(sr, route_e->sb_db); > + sbrec_route_set_logical_port(sr, route_e->logical_port); > + sbrec_route_set_ip_prefix(sr, route_e->ip_prefix); > + sbrec_route_set_type(sr, route_e->type); > + if (route_e->tracked_port) { > + sbrec_route_set_tracked_port(sr, route_e->tracked_port); > + } > + route_e->sb_route = sr; > } > > - route_e = route_alloc_entry(route_map, sb_db, > - logical_port, ip_prefix, route_type); > - free(ip_prefix); > return route_e; > } > > @@ -179,6 +295,7 @@ route_erase_entry(struct route_entry *route_e) > free(route_e->logical_port); > free(route_e->ip_prefix); > free(route_e->type); > + free(route_e->tracked_port); > free(route_e); > } > > @@ -270,13 +387,95 @@ parse_route_from_sbrec_route(struct hmap *parsed_routes_out, > ); > } > > +static void > +publish_lport_addresses(struct ovsdb_idl_txn *ovnsb_txn, > + struct hmap *route_map, > + const struct sbrec_datapath_binding *sb_db, > + char *logical_port, > + struct lport_addresses *addresses, > + struct ovn_port *tracking_port) > +{ > + for (int 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); > + route_sync_to_sb(ovnsb_txn, route_map, > + sb_db, > + logical_port, > + addr_s, > + "advertise", > + tracking_port->sb->logical_port); > + free(addr_s); > + } > + for (int 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); > + route_sync_to_sb(ovnsb_txn, route_map, > + sb_db, > + logical_port, > + addr_s, > + "advertise", > + tracking_port->sb->logical_port); > + free(addr_s); > + } > +} > + > + > +static void > +publish_host_routes(struct ovsdb_idl_txn *ovnsb_txn, > + struct hmap *route_map, > + const struct lr_stateful_table *lr_stateful_table, > + const struct parsed_route *route, > + struct routes_sync_tracked_data *trk_data) > +{ > + struct ovn_port *port; > + struct ovn_datapath *lsp_od = route->out_port->peer->od; > + uuidset_insert(&trk_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(ovnsb_txn, route_map, route->od->sb, > + route->out_port->key, > + 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); > + uuidset_insert(&trk_data->nb_lr_stateful, > + &lr_stateful_rec->nbr_uuid); > + struct ovn_port_routable_addresses addrs = get_op_addresses( > + port->peer, lr_stateful_rec, false); > + for (int i = 0; i < addrs.n_addrs; i++) { > + publish_lport_addresses(ovnsb_txn, route_map, route->od->sb, > + route->out_port->key, > + &addrs.laddrs[i], > + port->peer); > + } > + destroy_routable_addresses(&addrs); > + } else { > + /* This is just a plain LSP */ > + for (int i = 0; i < port->n_lsp_addrs; i++) { > + publish_lport_addresses(ovnsb_txn, route_map, route->od->sb, > + route->out_port->key, > + &port->lsp_addrs[i], > + port); > + } > + } > + } > +} > + > static void > routes_table_sync(struct ovsdb_idl_txn *ovnsb_txn, > const struct sbrec_route_table *sbrec_route_table, > + const struct lr_stateful_table *lr_stateful_table, > const struct hmap *parsed_routes, > const struct hmap *lr_ports, > const struct ovn_datapaths *lr_datapaths, > - struct hmap *parsed_routes_out) > + struct hmap *parsed_routes_out, > + struct routes_sync_tracked_data *trk_data) > { > if (!ovnsb_txn) { > return; > @@ -303,7 +502,8 @@ routes_table_sync(struct ovsdb_idl_txn *ovnsb_txn, > sb_route->datapath, > sb_route->logical_port, > sb_route->ip_prefix, > - sb_route->type); > + sb_route->type, > + sb_route->tracked_port); > route_e->stale = true; > route_e->sb_route = sb_route; > } > @@ -321,32 +521,34 @@ routes_table_sync(struct ovsdb_idl_txn *ovnsb_txn, > false)) { > continue; > } > - if (route->source == ROUTE_SOURCE_CONNECTED && > - !get_nbrp_or_nbr_option(route->out_port, > + if (route->source == ROUTE_SOURCE_CONNECTED) { > + if (!get_nbrp_or_nbr_option(route->out_port, > "dynamic-routing-connected")) { > - continue; > + continue; > + } > + if (smap_get_bool(&route->out_port->nbrp->options, > + "dynamic-routing-connected-as-host-routes", > + false)) { > + publish_host_routes(ovnsb_txn, &sync_routes, > + lr_stateful_table, route, trk_data); > + continue; > + } > } > if (route->source == ROUTE_SOURCE_STATIC && > !get_nbrp_or_nbr_option(route->out_port, > "dynamic-routing-static")) { > continue; > } > - route_e = route_lookup_or_add(&sync_routes, > - route->od->sb, > - route->out_port->key, > - &route->prefix, > - route->plen, > - "advertise"); > - route_e->stale = false; > > - if (!route_e->sb_route) { > - const struct sbrec_route *sr = sbrec_route_insert(ovnsb_txn); > - sbrec_route_set_datapath(sr, route_e->sb_db); > - sbrec_route_set_logical_port(sr, route_e->logical_port); > - sbrec_route_set_ip_prefix(sr, route_e->ip_prefix); > - sbrec_route_set_type(sr, route_e->type); > - route_e->sb_route = sr; > - } > + char *ip_prefix = normalize_v46_prefix(&route->prefix, > + route->plen); > + route_sync_to_sb(ovnsb_txn, &sync_routes, > + route->od->sb, > + route->out_port->key, > + ip_prefix, > + "advertise", > + NULL); > + free(ip_prefix); > } > > HMAP_FOR_EACH_POP (route_e, hmap_node, &sync_routes) { > diff --git a/northd/en-routes-sync.h b/northd/en-routes-sync.h > index 391f17452..10f33ce91 100644 > --- a/northd/en-routes-sync.h > +++ b/northd/en-routes-sync.h > @@ -15,9 +15,29 @@ > #define EN_ROUTES_SYNC_H 1 > > #include "lib/inc-proc-eng.h" > +#include "lib/uuidset.h" > +#include "openvswitch/hmap.h" > + > +struct routes_sync_tracked_data { > + /* Contains the uuids of all NB Logical Routers where we used a > + * lr_stateful_record during computation. */ > + struct uuidset nb_lr_stateful; > + /* Contains the uuids of all NB Logical Switches where we rely on port > + * port changes for host routes. */ > + struct uuidset nb_ls; > +}; > + > +struct routes_sync_data { > + struct hmap parsed_routes; > + > + /* Node's tracked data. */ > + struct routes_sync_tracked_data trk_data; > +}; > > bool routes_sync_northd_change_handler(struct engine_node *node, > - void *data OVS_UNUSED); > + void *data); > +bool routes_sync_lr_stateful_change_handler(struct engine_node *node, > + void *data); > void *en_routes_sync_init(struct engine_node *, struct engine_arg *); > void en_routes_sync_cleanup(void *data); > void en_routes_sync_run(struct engine_node *, void *data); > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > index 741295709..84072d6ce 100644 > --- a/northd/inc-proc-northd.c > +++ b/northd/inc-proc-northd.c > @@ -271,6 +271,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > engine_add_input(&en_routes_sync, &en_sb_route, NULL); > engine_add_input(&en_routes_sync, &en_northd, > routes_sync_northd_change_handler); > + engine_add_input(&en_routes_sync, &en_lr_stateful, > + routes_sync_lr_stateful_change_handler); > > engine_add_input(&en_sync_meters, &en_nb_acl, NULL); > engine_add_input(&en_sync_meters, &en_nb_meter, NULL); > diff --git a/northd/northd.c b/northd/northd.c > index 4439a74da..a2e59dcfc 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -1094,19 +1094,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. > @@ -1141,7 +1128,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++) { > @@ -1154,12 +1141,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) { > @@ -1192,6 +1181,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 991ba5f9f..dc51630ed 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 */ > @@ -186,10 +187,6 @@ struct routes_data { > struct hmap bfd_active_connections; > }; > > -struct routes_sync_data { > - struct hmap parsed_routes; > -}; > - > struct route_policies_data { > struct hmap route_policies; > struct hmap bfd_active_connections; > @@ -936,4 +933,24 @@ ovn_port_find_bound(const struct hmap *ports, const char *name) > return ovn_port_find__(ports, name, true); > } > > +/* 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 75fe40c01..d67a3d07a 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -3705,6 +3705,33 @@ or > key="dynamic-routing-static" table="Logical_Router_Port"/> 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. > + > + In this case 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. > + > + 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/ovn-sb.ovsschema b/ovn-sb.ovsschema > index 74540782e..01df9cc6b 100644 > --- a/ovn-sb.ovsschema > +++ b/ovn-sb.ovsschema > @@ -1,7 +1,7 @@ > { > "name": "OVN_Southbound", > "version": "20.38.0", > - "cksum": "1944407838 32212", > + "cksum": "1452226583 32264", > "tables": { > "SB_Global": { > "columns": { > @@ -626,6 +626,7 @@ > "logical_port": {"type": "string"}, > "ip_prefix": {"type": "string"}, > "nexthop": {"type": "string"}, > + "tracked_port": {"type": "string"}, > "type": {"type": {"key": {"type": "string", > "enum": ["set", ["advertise", > "receive"]]}, > diff --git a/ovn-sb.xml b/ovn-sb.xml > index 493b7e839..bf587153a 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -5224,6 +5224,19 @@ tcp.flags = RST; > </p> > </column> > > + <column name="tracked_port"> > + <p> > + Only relevant for type <code>advertise</code> and if > + <code>options:dynamic-routing-connected-as-host-routes</code> is > + set on the <code>OVN_Northbound.Logical_Router_Port</code>. > + > + In this case this lists the name of the <code>Port_Binding</code> > + that is holding this ip address. ovn-controller can use this > + information to determine the distance and therefor the route priority > + of a published route > + </p> > + </column> > + > <column name="type"> > <p> > If the route is to be exported from OVN to the outside network or if > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 213536707..b40eb24e6 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -14042,5 +14042,72 @@ 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 > +# 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-connected=true \ > + option:dynamic-routing-static=true > +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 > +check ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 10.0.1.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 > +check_row_count Route 2 type=advertise tracked_port='""' > +datapath=$(ovn-sbctl --bare --columns _uuid list datapath_binding 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 Route 2 type=advertise > +AT_CHECK([ovn-sbctl --columns ip_prefix,tracked_port --bare find Route datapath=$datapath logical_port=lr0-sw0], [0], [dnl > +10.0.0.1/32 > +lr0-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" > +check_row_count Route 3 type=advertise > +check_row_count Route 2 type=advertise tracked_port!='""' > +AT_CHECK([ovn-sbctl --columns tracked_port --bare find Route datapath=$datapath logical_port=lr0-sw0 ip_prefix=10.0.0.2/32], [0], [dnl > +sw0-vif0 > +]) > + > +# 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 > +check ovn-nbctl --wait=sb set Logical_Switch_Port sw0-lr1 type=router options:router-port=lr1-sw0 > +check_row_count Route 4 type=advertise > +check_row_count Route 3 type=advertise tracked_port!='""' > +AT_CHECK([ovn-sbctl --columns tracked_port --bare find Route datapath=$datapath logical_port=lr0-sw0 ip_prefix=10.0.0.10/32], [0], [dnl > +lr1-sw0 > +]) > + > +# 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 Route 5 type=advertise > +check_row_count Route 4 type=advertise tracked_port!='""' > +AT_CHECK([ovn-sbctl --columns tracked_port --bare find Route datapath=$datapath logical_port=lr0-sw0 ip_prefix=10.0.0.100/32], [0], [dnl > +lr1-sw0 > +]) > + > +# 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 Route 6 type=advertise > +check_row_count Route 4 type=advertise tracked_port!='""' > +AT_CHECK([ovn-sbctl --columns ip_prefix --bare find Route datapath=$datapath logical_port=lr0-sw0 ip_prefix=172.16.0.0/24], [0], [dnl > +172.16.0.0/24 > +]) > + > AT_CLEANUP > ]) > -- > 2.47.0 > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Tue, Nov 26, 2024 at 04:28:53PM -0300, Tiago Pires wrote: > Hi Felix, > > Thanks for the patch. > I would ask if it makes sense to have logical flows for NAT addresses > that are in different subnets of the network of the LR IP. In this > case, when using the active-active feature, the tear one LR would have > those logical flows to know how to route the traffic back to the tear > two LR (project routers). > This use case happens when the user separates NAT IPs in a subnet and > LRPs addresses in another one. > Nowadays this scenario works when the external network (LS) has the > localnet port and can rely in the ARP as the ovn-controller sends the > GARP or using NDR(Neutron dynamic routing) to advertise the /32 or /64 > having the next-hop as the LRP uplink address. Hi Tiago, i think in this case the tier one LR (so the one connected to the fabric) would need to have static /32 routes pointing to the tier two LRs (the project routers) for each of the NAT IPs. This series would then announce these /32 routes to the fabric. However this patch would not play part in this and we would not actually populate "tracked_port", thereby potentially causing the fabric to send traffic to a non-optimal chassis. I guess this could be quite easily added afterwards that we also populate tracked_port for static routes with the nexthop LSP/LRP. Thanks a lot Felix > > Regards, > > Tiago Pires > > On Tue, Nov 26, 2024 at 11:40 AM Felix Huettner via dev > <ovs-dev@openvswitch.org> wrote: > > > > sometimes we want to use individual host routes instead of the connected > > routes of LRPs. > > This allows the network fabric to know which adresses are actually in > > use and e.g. drop traffic to adresses that are not used anyway. > > > > Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud> > > --- > > NEWS | 2 + > > northd/en-lflow.c | 1 + > > northd/en-routes-sync.c | 270 ++++++++++++++++++++++++++++++++++----- > > northd/en-routes-sync.h | 22 +++- > > northd/inc-proc-northd.c | 2 + > > northd/northd.c | 32 ++--- > > northd/northd.h | 25 +++- > > ovn-nb.xml | 27 ++++ > > ovn-sb.ovsschema | 3 +- > > ovn-sb.xml | 13 ++ > > tests/ovn-northd.at | 67 ++++++++++ > > 11 files changed, 406 insertions(+), 58 deletions(-) > > > > diff --git a/NEWS b/NEWS > > index efb42b32b..a780940fc 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -14,6 +14,8 @@ Post v24.09.0 > > Routes entered into the "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-lflow.c b/northd/en-lflow.c > > index 8995f0300..aabba943e 100644 > > --- a/northd/en-lflow.c > > +++ b/northd/en-lflow.c > > @@ -26,6 +26,7 @@ > > #include "en-northd.h" > > #include "en-meters.h" > > #include "en-sampling-app.h" > > +#include "en-routes-sync.h" > > #include "lflow-mgr.h" > > > > #include "lib/inc-proc-eng.h" > > diff --git a/northd/en-routes-sync.c b/northd/en-routes-sync.c > > index 7ca87c76a..d5a534f86 100644 > > --- a/northd/en-routes-sync.c > > +++ b/northd/en-routes-sync.c > > @@ -13,6 +13,7 @@ > > */ > > > > #include <config.h> > > +#include <stdbool.h> > > > > #include "openvswitch/vlog.h" > > #include "smap.h" > > @@ -20,6 +21,7 @@ > > #include "northd.h" > > > > #include "en-routes-sync.h" > > +#include "en-lr-stateful.h" > > #include "lib/stopwatch-names.h" > > #include "openvswitch/hmap.h" > > #include "ovn-util.h" > > @@ -29,15 +31,19 @@ VLOG_DEFINE_THIS_MODULE(en_routes_sync); > > static void > > routes_table_sync(struct ovsdb_idl_txn *ovnsb_txn, > > const struct sbrec_route_table *sbrec_route_table, > > + const struct lr_stateful_table *lr_stateful_table, > > const struct hmap *parsed_routes, > > const struct hmap *lr_ports, > > const struct ovn_datapaths *lr_datapaths, > > - struct hmap *parsed_routes_out); > > + struct hmap *parsed_routes_out, > > + struct routes_sync_tracked_data *trk_data); > > > > static void > > routes_sync_init(struct routes_sync_data *data) > > { > > hmap_init(&data->parsed_routes); > > + uuidset_init(&data->trk_data.nb_lr_stateful); > > + uuidset_init(&data->trk_data.nb_ls); > > } > > > > static void > > @@ -48,12 +54,15 @@ routes_sync_destroy(struct routes_sync_data *data) > > parsed_route_free(r); > > } > > hmap_destroy(&data->parsed_routes); > > + uuidset_destroy(&data->trk_data.nb_lr_stateful); > > + uuidset_destroy(&data->trk_data.nb_ls); > > } > > > > bool > > routes_sync_northd_change_handler(struct engine_node *node, > > - void *data OVS_UNUSED) > > + void *data_) > > { > > + struct routes_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; > > @@ -69,7 +78,58 @@ routes_sync_northd_change_handler(struct engine_node *node, > > * this happens and so does this node. > > * Note: When we add I-P to the created/deleted logical routers or > > * logical router ports, we need to revisit this handler. > > + * 3. Indirectly northd_data->ls_ports if we announce host routes > > + * This is what we check below > > */ > > + > > + 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->trk_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->trk_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->trk_data.nb_ls, > > + &op->od->nbs->header_.uuid)) { > > + return false; > > + } > > + } > > + > > + return true; > > +} > > + > > +bool > > +routes_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 routes_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->trk_data.nb_lr_stateful, > > + &lr_stateful_rec->nbr_uuid)) { > > + return false; > > + } > > + } > > + > > return true; > > } > > > > @@ -101,14 +161,18 @@ en_routes_sync_run(struct engine_node *node, void *data) > > const struct sbrec_route_table *sbrec_route_table = > > EN_OVSDB_GET(engine_get_input("SB_route", node)); > > struct northd_data *northd_data = engine_get_input_data("northd", node); > > + struct ed_type_lr_stateful *lr_stateful_data = > > + engine_get_input_data("lr_stateful", node); > > > > stopwatch_start(ROUTES_SYNC_RUN_STOPWATCH_NAME, time_msec()); > > > > routes_table_sync(eng_ctx->ovnsb_idl_txn, sbrec_route_table, > > + &lr_stateful_data->table, > > &routes_data->parsed_routes, > > &northd_data->lr_ports, > > &northd_data->lr_datapaths, > > - &routes_sync_data->parsed_routes); > > + &routes_sync_data->parsed_routes, > > + &routes_sync_data->trk_data); > > > > stopwatch_stop(ROUTES_SYNC_RUN_STOPWATCH_NAME, time_msec()); > > engine_set_node_state(node, EN_UPDATED); > > @@ -122,6 +186,7 @@ struct route_entry { > > > > char *logical_port; > > char *ip_prefix; > > + char *tracked_port; > > char *type; > > bool stale; > > }; > > @@ -129,7 +194,10 @@ struct route_entry { > > static struct route_entry * > > route_alloc_entry(struct hmap *routes, > > const struct sbrec_datapath_binding *sb_db, > > - char *logical_port, char *ip_prefix, char *route_type) > > + const char *logical_port, > > + const char *ip_prefix, > > + const char *route_type, > > + const char *tracked_port) > > { > > struct route_entry *route_e = xzalloc(sizeof *route_e); > > > > @@ -137,6 +205,9 @@ route_alloc_entry(struct hmap *routes, > > route_e->logical_port = xstrdup(logical_port); > > route_e->ip_prefix = xstrdup(ip_prefix); > > route_e->type = xstrdup(route_type); > > + if (tracked_port) { > > + route_e->tracked_port = xstrdup(tracked_port); > > + } > > route_e->stale = false; > > uint32_t hash = uuid_hash(&sb_db->header_.uuid); > > hash = hash_string(logical_port, hash); > > @@ -149,27 +220,72 @@ route_alloc_entry(struct hmap *routes, > > static struct route_entry * > > route_lookup_or_add(struct hmap *route_map, > > const struct sbrec_datapath_binding *sb_db, > > - char *logical_port, const struct in6_addr *prefix, > > - unsigned int plen, char *route_type) > > + const char *logical_port, const char *ip_prefix, > > + const char *route_type, const char *tracked_port) > > { > > struct route_entry *route_e; > > uint32_t hash; > > > > - char *ip_prefix = normalize_v46_prefix(prefix, plen); > > - > > hash = uuid_hash(&sb_db->header_.uuid); > > hash = hash_string(logical_port, hash); > > hash = hash_string(ip_prefix, hash); > > HMAP_FOR_EACH_WITH_HASH (route_e, hmap_node, hash, route_map) { > > - if (!strcmp(route_e->type, route_type)) { > > - free(ip_prefix); > > - return route_e; > > + if (!uuid_equals(&sb_db->header_.uuid, > > + &route_e->sb_db->header_.uuid)) { > > + continue; > > } > > + > > + if (strcmp(logical_port, route_e->logical_port)) { > > + continue; > > + } > > + > > + if (strcmp(ip_prefix, route_e->ip_prefix)) { > > + continue; > > + } > > + > > + if (strcmp(route_type, route_e->type)) { > > + continue; > > + } > > + > > + if (!streq(tracked_port, route_e->tracked_port)) { > > + continue; > > + } > > + > > + return route_e; > > + } > > + > > + route_e = route_alloc_entry(route_map, sb_db, > > + logical_port, ip_prefix, route_type, > > + tracked_port); > > + return route_e; > > +} > > + > > +static struct route_entry * > > +route_sync_to_sb(struct ovsdb_idl_txn *ovnsb_txn, struct hmap *route_map, > > + const struct sbrec_datapath_binding *sb_db, > > + const char *logical_port, const char *ip_prefix, > > + const char *route_type, const char *tracked_port) > > +{ > > + struct route_entry *route_e = route_lookup_or_add(route_map, > > + sb_db, > > + logical_port, > > + ip_prefix, > > + route_type, > > + tracked_port); > > + route_e->stale = false; > > + > > + if (!route_e->sb_route) { > > + const struct sbrec_route *sr = sbrec_route_insert(ovnsb_txn); > > + sbrec_route_set_datapath(sr, route_e->sb_db); > > + sbrec_route_set_logical_port(sr, route_e->logical_port); > > + sbrec_route_set_ip_prefix(sr, route_e->ip_prefix); > > + sbrec_route_set_type(sr, route_e->type); > > + if (route_e->tracked_port) { > > + sbrec_route_set_tracked_port(sr, route_e->tracked_port); > > + } > > + route_e->sb_route = sr; > > } > > > > - route_e = route_alloc_entry(route_map, sb_db, > > - logical_port, ip_prefix, route_type); > > - free(ip_prefix); > > return route_e; > > } > > > > @@ -179,6 +295,7 @@ route_erase_entry(struct route_entry *route_e) > > free(route_e->logical_port); > > free(route_e->ip_prefix); > > free(route_e->type); > > + free(route_e->tracked_port); > > free(route_e); > > } > > > > @@ -270,13 +387,95 @@ parse_route_from_sbrec_route(struct hmap *parsed_routes_out, > > ); > > } > > > > +static void > > +publish_lport_addresses(struct ovsdb_idl_txn *ovnsb_txn, > > + struct hmap *route_map, > > + const struct sbrec_datapath_binding *sb_db, > > + char *logical_port, > > + struct lport_addresses *addresses, > > + struct ovn_port *tracking_port) > > +{ > > + for (int 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); > > + route_sync_to_sb(ovnsb_txn, route_map, > > + sb_db, > > + logical_port, > > + addr_s, > > + "advertise", > > + tracking_port->sb->logical_port); > > + free(addr_s); > > + } > > + for (int 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); > > + route_sync_to_sb(ovnsb_txn, route_map, > > + sb_db, > > + logical_port, > > + addr_s, > > + "advertise", > > + tracking_port->sb->logical_port); > > + free(addr_s); > > + } > > +} > > + > > + > > +static void > > +publish_host_routes(struct ovsdb_idl_txn *ovnsb_txn, > > + struct hmap *route_map, > > + const struct lr_stateful_table *lr_stateful_table, > > + const struct parsed_route *route, > > + struct routes_sync_tracked_data *trk_data) > > +{ > > + struct ovn_port *port; > > + struct ovn_datapath *lsp_od = route->out_port->peer->od; > > + uuidset_insert(&trk_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(ovnsb_txn, route_map, route->od->sb, > > + route->out_port->key, > > + 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); > > + uuidset_insert(&trk_data->nb_lr_stateful, > > + &lr_stateful_rec->nbr_uuid); > > + struct ovn_port_routable_addresses addrs = get_op_addresses( > > + port->peer, lr_stateful_rec, false); > > + for (int i = 0; i < addrs.n_addrs; i++) { > > + publish_lport_addresses(ovnsb_txn, route_map, route->od->sb, > > + route->out_port->key, > > + &addrs.laddrs[i], > > + port->peer); > > + } > > + destroy_routable_addresses(&addrs); > > + } else { > > + /* This is just a plain LSP */ > > + for (int i = 0; i < port->n_lsp_addrs; i++) { > > + publish_lport_addresses(ovnsb_txn, route_map, route->od->sb, > > + route->out_port->key, > > + &port->lsp_addrs[i], > > + port); > > + } > > + } > > + } > > +} > > + > > static void > > routes_table_sync(struct ovsdb_idl_txn *ovnsb_txn, > > const struct sbrec_route_table *sbrec_route_table, > > + const struct lr_stateful_table *lr_stateful_table, > > const struct hmap *parsed_routes, > > const struct hmap *lr_ports, > > const struct ovn_datapaths *lr_datapaths, > > - struct hmap *parsed_routes_out) > > + struct hmap *parsed_routes_out, > > + struct routes_sync_tracked_data *trk_data) > > { > > if (!ovnsb_txn) { > > return; > > @@ -303,7 +502,8 @@ routes_table_sync(struct ovsdb_idl_txn *ovnsb_txn, > > sb_route->datapath, > > sb_route->logical_port, > > sb_route->ip_prefix, > > - sb_route->type); > > + sb_route->type, > > + sb_route->tracked_port); > > route_e->stale = true; > > route_e->sb_route = sb_route; > > } > > @@ -321,32 +521,34 @@ routes_table_sync(struct ovsdb_idl_txn *ovnsb_txn, > > false)) { > > continue; > > } > > - if (route->source == ROUTE_SOURCE_CONNECTED && > > - !get_nbrp_or_nbr_option(route->out_port, > > + if (route->source == ROUTE_SOURCE_CONNECTED) { > > + if (!get_nbrp_or_nbr_option(route->out_port, > > "dynamic-routing-connected")) { > > - continue; > > + continue; > > + } > > + if (smap_get_bool(&route->out_port->nbrp->options, > > + "dynamic-routing-connected-as-host-routes", > > + false)) { > > + publish_host_routes(ovnsb_txn, &sync_routes, > > + lr_stateful_table, route, trk_data); > > + continue; > > + } > > } > > if (route->source == ROUTE_SOURCE_STATIC && > > !get_nbrp_or_nbr_option(route->out_port, > > "dynamic-routing-static")) { > > continue; > > } > > - route_e = route_lookup_or_add(&sync_routes, > > - route->od->sb, > > - route->out_port->key, > > - &route->prefix, > > - route->plen, > > - "advertise"); > > - route_e->stale = false; > > > > - if (!route_e->sb_route) { > > - const struct sbrec_route *sr = sbrec_route_insert(ovnsb_txn); > > - sbrec_route_set_datapath(sr, route_e->sb_db); > > - sbrec_route_set_logical_port(sr, route_e->logical_port); > > - sbrec_route_set_ip_prefix(sr, route_e->ip_prefix); > > - sbrec_route_set_type(sr, route_e->type); > > - route_e->sb_route = sr; > > - } > > + char *ip_prefix = normalize_v46_prefix(&route->prefix, > > + route->plen); > > + route_sync_to_sb(ovnsb_txn, &sync_routes, > > + route->od->sb, > > + route->out_port->key, > > + ip_prefix, > > + "advertise", > > + NULL); > > + free(ip_prefix); > > } > > > > HMAP_FOR_EACH_POP (route_e, hmap_node, &sync_routes) { > > diff --git a/northd/en-routes-sync.h b/northd/en-routes-sync.h > > index 391f17452..10f33ce91 100644 > > --- a/northd/en-routes-sync.h > > +++ b/northd/en-routes-sync.h > > @@ -15,9 +15,29 @@ > > #define EN_ROUTES_SYNC_H 1 > > > > #include "lib/inc-proc-eng.h" > > +#include "lib/uuidset.h" > > +#include "openvswitch/hmap.h" > > + > > +struct routes_sync_tracked_data { > > + /* Contains the uuids of all NB Logical Routers where we used a > > + * lr_stateful_record during computation. */ > > + struct uuidset nb_lr_stateful; > > + /* Contains the uuids of all NB Logical Switches where we rely on port > > + * port changes for host routes. */ > > + struct uuidset nb_ls; > > +}; > > + > > +struct routes_sync_data { > > + struct hmap parsed_routes; > > + > > + /* Node's tracked data. */ > > + struct routes_sync_tracked_data trk_data; > > +}; > > > > bool routes_sync_northd_change_handler(struct engine_node *node, > > - void *data OVS_UNUSED); > > + void *data); > > +bool routes_sync_lr_stateful_change_handler(struct engine_node *node, > > + void *data); > > void *en_routes_sync_init(struct engine_node *, struct engine_arg *); > > void en_routes_sync_cleanup(void *data); > > void en_routes_sync_run(struct engine_node *, void *data); > > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > > index 741295709..84072d6ce 100644 > > --- a/northd/inc-proc-northd.c > > +++ b/northd/inc-proc-northd.c > > @@ -271,6 +271,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > > engine_add_input(&en_routes_sync, &en_sb_route, NULL); > > engine_add_input(&en_routes_sync, &en_northd, > > routes_sync_northd_change_handler); > > + engine_add_input(&en_routes_sync, &en_lr_stateful, > > + routes_sync_lr_stateful_change_handler); > > > > engine_add_input(&en_sync_meters, &en_nb_acl, NULL); > > engine_add_input(&en_sync_meters, &en_nb_meter, NULL); > > diff --git a/northd/northd.c b/northd/northd.c > > index 4439a74da..a2e59dcfc 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -1094,19 +1094,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. > > @@ -1141,7 +1128,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++) { > > @@ -1154,12 +1141,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) { > > @@ -1192,6 +1181,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 991ba5f9f..dc51630ed 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 */ > > @@ -186,10 +187,6 @@ struct routes_data { > > struct hmap bfd_active_connections; > > }; > > > > -struct routes_sync_data { > > - struct hmap parsed_routes; > > -}; > > - > > struct route_policies_data { > > struct hmap route_policies; > > struct hmap bfd_active_connections; > > @@ -936,4 +933,24 @@ ovn_port_find_bound(const struct hmap *ports, const char *name) > > return ovn_port_find__(ports, name, true); > > } > > > > +/* 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 75fe40c01..d67a3d07a 100644 > > --- a/ovn-nb.xml > > +++ b/ovn-nb.xml > > @@ -3705,6 +3705,33 @@ or > > key="dynamic-routing-static" table="Logical_Router_Port"/> 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. > > + > > + In this case 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. > > + > > + 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/ovn-sb.ovsschema b/ovn-sb.ovsschema > > index 74540782e..01df9cc6b 100644 > > --- a/ovn-sb.ovsschema > > +++ b/ovn-sb.ovsschema > > @@ -1,7 +1,7 @@ > > { > > "name": "OVN_Southbound", > > "version": "20.38.0", > > - "cksum": "1944407838 32212", > > + "cksum": "1452226583 32264", > > "tables": { > > "SB_Global": { > > "columns": { > > @@ -626,6 +626,7 @@ > > "logical_port": {"type": "string"}, > > "ip_prefix": {"type": "string"}, > > "nexthop": {"type": "string"}, > > + "tracked_port": {"type": "string"}, > > "type": {"type": {"key": {"type": "string", > > "enum": ["set", ["advertise", > > "receive"]]}, > > diff --git a/ovn-sb.xml b/ovn-sb.xml > > index 493b7e839..bf587153a 100644 > > --- a/ovn-sb.xml > > +++ b/ovn-sb.xml > > @@ -5224,6 +5224,19 @@ tcp.flags = RST; > > </p> > > </column> > > > > + <column name="tracked_port"> > > + <p> > > + Only relevant for type <code>advertise</code> and if > > + <code>options:dynamic-routing-connected-as-host-routes</code> is > > + set on the <code>OVN_Northbound.Logical_Router_Port</code>. > > + > > + In this case this lists the name of the <code>Port_Binding</code> > > + that is holding this ip address. ovn-controller can use this > > + information to determine the distance and therefor the route priority > > + of a published route > > + </p> > > + </column> > > + > > <column name="type"> > > <p> > > If the route is to be exported from OVN to the outside network or if > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > index 213536707..b40eb24e6 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -14042,5 +14042,72 @@ 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 > > +# 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-connected=true \ > > + option:dynamic-routing-static=true > > +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 > > +check ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 10.0.1.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 > > +check_row_count Route 2 type=advertise tracked_port='""' > > +datapath=$(ovn-sbctl --bare --columns _uuid list datapath_binding 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 Route 2 type=advertise > > +AT_CHECK([ovn-sbctl --columns ip_prefix,tracked_port --bare find Route datapath=$datapath logical_port=lr0-sw0], [0], [dnl > > +10.0.0.1/32 > > +lr0-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" > > +check_row_count Route 3 type=advertise > > +check_row_count Route 2 type=advertise tracked_port!='""' > > +AT_CHECK([ovn-sbctl --columns tracked_port --bare find Route datapath=$datapath logical_port=lr0-sw0 ip_prefix=10.0.0.2/32], [0], [dnl > > +sw0-vif0 > > +]) > > + > > +# 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 > > +check ovn-nbctl --wait=sb set Logical_Switch_Port sw0-lr1 type=router options:router-port=lr1-sw0 > > +check_row_count Route 4 type=advertise > > +check_row_count Route 3 type=advertise tracked_port!='""' > > +AT_CHECK([ovn-sbctl --columns tracked_port --bare find Route datapath=$datapath logical_port=lr0-sw0 ip_prefix=10.0.0.10/32], [0], [dnl > > +lr1-sw0 > > +]) > > + > > +# 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 Route 5 type=advertise > > +check_row_count Route 4 type=advertise tracked_port!='""' > > +AT_CHECK([ovn-sbctl --columns tracked_port --bare find Route datapath=$datapath logical_port=lr0-sw0 ip_prefix=10.0.0.100/32], [0], [dnl > > +lr1-sw0 > > +]) > > + > > +# 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 Route 6 type=advertise > > +check_row_count Route 4 type=advertise tracked_port!='""' > > +AT_CHECK([ovn-sbctl --columns ip_prefix --bare find Route datapath=$datapath logical_port=lr0-sw0 ip_prefix=172.16.0.0/24], [0], [dnl > > +172.16.0.0/24 > > +]) > > + > > AT_CLEANUP > > ]) > > -- > > 2.47.0 > > > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > -- > > > > > _‘Esta mensagem é direcionada apenas para os endereços constantes no > cabeçalho inicial. Se você não está listado nos endereços constantes no > cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa > mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão > imediatamente anuladas e proibidas’._ > > > * **‘Apesar do Magazine Luiza tomar > todas as precauções razoáveis para assegurar que nenhum vírus esteja > presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por > quaisquer perdas ou danos causados por esse e-mail ou por seus anexos’.* > > >
diff --git a/NEWS b/NEWS index efb42b32b..a780940fc 100644 --- a/NEWS +++ b/NEWS @@ -14,6 +14,8 @@ Post v24.09.0 Routes entered into the "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-lflow.c b/northd/en-lflow.c index 8995f0300..aabba943e 100644 --- a/northd/en-lflow.c +++ b/northd/en-lflow.c @@ -26,6 +26,7 @@ #include "en-northd.h" #include "en-meters.h" #include "en-sampling-app.h" +#include "en-routes-sync.h" #include "lflow-mgr.h" #include "lib/inc-proc-eng.h" diff --git a/northd/en-routes-sync.c b/northd/en-routes-sync.c index 7ca87c76a..d5a534f86 100644 --- a/northd/en-routes-sync.c +++ b/northd/en-routes-sync.c @@ -13,6 +13,7 @@ */ #include <config.h> +#include <stdbool.h> #include "openvswitch/vlog.h" #include "smap.h" @@ -20,6 +21,7 @@ #include "northd.h" #include "en-routes-sync.h" +#include "en-lr-stateful.h" #include "lib/stopwatch-names.h" #include "openvswitch/hmap.h" #include "ovn-util.h" @@ -29,15 +31,19 @@ VLOG_DEFINE_THIS_MODULE(en_routes_sync); static void routes_table_sync(struct ovsdb_idl_txn *ovnsb_txn, const struct sbrec_route_table *sbrec_route_table, + const struct lr_stateful_table *lr_stateful_table, const struct hmap *parsed_routes, const struct hmap *lr_ports, const struct ovn_datapaths *lr_datapaths, - struct hmap *parsed_routes_out); + struct hmap *parsed_routes_out, + struct routes_sync_tracked_data *trk_data); static void routes_sync_init(struct routes_sync_data *data) { hmap_init(&data->parsed_routes); + uuidset_init(&data->trk_data.nb_lr_stateful); + uuidset_init(&data->trk_data.nb_ls); } static void @@ -48,12 +54,15 @@ routes_sync_destroy(struct routes_sync_data *data) parsed_route_free(r); } hmap_destroy(&data->parsed_routes); + uuidset_destroy(&data->trk_data.nb_lr_stateful); + uuidset_destroy(&data->trk_data.nb_ls); } bool routes_sync_northd_change_handler(struct engine_node *node, - void *data OVS_UNUSED) + void *data_) { + struct routes_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; @@ -69,7 +78,58 @@ routes_sync_northd_change_handler(struct engine_node *node, * this happens and so does this node. * Note: When we add I-P to the created/deleted logical routers or * logical router ports, we need to revisit this handler. + * 3. Indirectly northd_data->ls_ports if we announce host routes + * This is what we check below */ + + 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->trk_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->trk_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->trk_data.nb_ls, + &op->od->nbs->header_.uuid)) { + return false; + } + } + + return true; +} + +bool +routes_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 routes_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->trk_data.nb_lr_stateful, + &lr_stateful_rec->nbr_uuid)) { + return false; + } + } + return true; } @@ -101,14 +161,18 @@ en_routes_sync_run(struct engine_node *node, void *data) const struct sbrec_route_table *sbrec_route_table = EN_OVSDB_GET(engine_get_input("SB_route", node)); struct northd_data *northd_data = engine_get_input_data("northd", node); + struct ed_type_lr_stateful *lr_stateful_data = + engine_get_input_data("lr_stateful", node); stopwatch_start(ROUTES_SYNC_RUN_STOPWATCH_NAME, time_msec()); routes_table_sync(eng_ctx->ovnsb_idl_txn, sbrec_route_table, + &lr_stateful_data->table, &routes_data->parsed_routes, &northd_data->lr_ports, &northd_data->lr_datapaths, - &routes_sync_data->parsed_routes); + &routes_sync_data->parsed_routes, + &routes_sync_data->trk_data); stopwatch_stop(ROUTES_SYNC_RUN_STOPWATCH_NAME, time_msec()); engine_set_node_state(node, EN_UPDATED); @@ -122,6 +186,7 @@ struct route_entry { char *logical_port; char *ip_prefix; + char *tracked_port; char *type; bool stale; }; @@ -129,7 +194,10 @@ struct route_entry { static struct route_entry * route_alloc_entry(struct hmap *routes, const struct sbrec_datapath_binding *sb_db, - char *logical_port, char *ip_prefix, char *route_type) + const char *logical_port, + const char *ip_prefix, + const char *route_type, + const char *tracked_port) { struct route_entry *route_e = xzalloc(sizeof *route_e); @@ -137,6 +205,9 @@ route_alloc_entry(struct hmap *routes, route_e->logical_port = xstrdup(logical_port); route_e->ip_prefix = xstrdup(ip_prefix); route_e->type = xstrdup(route_type); + if (tracked_port) { + route_e->tracked_port = xstrdup(tracked_port); + } route_e->stale = false; uint32_t hash = uuid_hash(&sb_db->header_.uuid); hash = hash_string(logical_port, hash); @@ -149,27 +220,72 @@ route_alloc_entry(struct hmap *routes, static struct route_entry * route_lookup_or_add(struct hmap *route_map, const struct sbrec_datapath_binding *sb_db, - char *logical_port, const struct in6_addr *prefix, - unsigned int plen, char *route_type) + const char *logical_port, const char *ip_prefix, + const char *route_type, const char *tracked_port) { struct route_entry *route_e; uint32_t hash; - char *ip_prefix = normalize_v46_prefix(prefix, plen); - hash = uuid_hash(&sb_db->header_.uuid); hash = hash_string(logical_port, hash); hash = hash_string(ip_prefix, hash); HMAP_FOR_EACH_WITH_HASH (route_e, hmap_node, hash, route_map) { - if (!strcmp(route_e->type, route_type)) { - free(ip_prefix); - return route_e; + if (!uuid_equals(&sb_db->header_.uuid, + &route_e->sb_db->header_.uuid)) { + continue; } + + if (strcmp(logical_port, route_e->logical_port)) { + continue; + } + + if (strcmp(ip_prefix, route_e->ip_prefix)) { + continue; + } + + if (strcmp(route_type, route_e->type)) { + continue; + } + + if (!streq(tracked_port, route_e->tracked_port)) { + continue; + } + + return route_e; + } + + route_e = route_alloc_entry(route_map, sb_db, + logical_port, ip_prefix, route_type, + tracked_port); + return route_e; +} + +static struct route_entry * +route_sync_to_sb(struct ovsdb_idl_txn *ovnsb_txn, struct hmap *route_map, + const struct sbrec_datapath_binding *sb_db, + const char *logical_port, const char *ip_prefix, + const char *route_type, const char *tracked_port) +{ + struct route_entry *route_e = route_lookup_or_add(route_map, + sb_db, + logical_port, + ip_prefix, + route_type, + tracked_port); + route_e->stale = false; + + if (!route_e->sb_route) { + const struct sbrec_route *sr = sbrec_route_insert(ovnsb_txn); + sbrec_route_set_datapath(sr, route_e->sb_db); + sbrec_route_set_logical_port(sr, route_e->logical_port); + sbrec_route_set_ip_prefix(sr, route_e->ip_prefix); + sbrec_route_set_type(sr, route_e->type); + if (route_e->tracked_port) { + sbrec_route_set_tracked_port(sr, route_e->tracked_port); + } + route_e->sb_route = sr; } - route_e = route_alloc_entry(route_map, sb_db, - logical_port, ip_prefix, route_type); - free(ip_prefix); return route_e; } @@ -179,6 +295,7 @@ route_erase_entry(struct route_entry *route_e) free(route_e->logical_port); free(route_e->ip_prefix); free(route_e->type); + free(route_e->tracked_port); free(route_e); } @@ -270,13 +387,95 @@ parse_route_from_sbrec_route(struct hmap *parsed_routes_out, ); } +static void +publish_lport_addresses(struct ovsdb_idl_txn *ovnsb_txn, + struct hmap *route_map, + const struct sbrec_datapath_binding *sb_db, + char *logical_port, + struct lport_addresses *addresses, + struct ovn_port *tracking_port) +{ + for (int 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); + route_sync_to_sb(ovnsb_txn, route_map, + sb_db, + logical_port, + addr_s, + "advertise", + tracking_port->sb->logical_port); + free(addr_s); + } + for (int 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); + route_sync_to_sb(ovnsb_txn, route_map, + sb_db, + logical_port, + addr_s, + "advertise", + tracking_port->sb->logical_port); + free(addr_s); + } +} + + +static void +publish_host_routes(struct ovsdb_idl_txn *ovnsb_txn, + struct hmap *route_map, + const struct lr_stateful_table *lr_stateful_table, + const struct parsed_route *route, + struct routes_sync_tracked_data *trk_data) +{ + struct ovn_port *port; + struct ovn_datapath *lsp_od = route->out_port->peer->od; + uuidset_insert(&trk_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(ovnsb_txn, route_map, route->od->sb, + route->out_port->key, + 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); + uuidset_insert(&trk_data->nb_lr_stateful, + &lr_stateful_rec->nbr_uuid); + struct ovn_port_routable_addresses addrs = get_op_addresses( + port->peer, lr_stateful_rec, false); + for (int i = 0; i < addrs.n_addrs; i++) { + publish_lport_addresses(ovnsb_txn, route_map, route->od->sb, + route->out_port->key, + &addrs.laddrs[i], + port->peer); + } + destroy_routable_addresses(&addrs); + } else { + /* This is just a plain LSP */ + for (int i = 0; i < port->n_lsp_addrs; i++) { + publish_lport_addresses(ovnsb_txn, route_map, route->od->sb, + route->out_port->key, + &port->lsp_addrs[i], + port); + } + } + } +} + static void routes_table_sync(struct ovsdb_idl_txn *ovnsb_txn, const struct sbrec_route_table *sbrec_route_table, + const struct lr_stateful_table *lr_stateful_table, const struct hmap *parsed_routes, const struct hmap *lr_ports, const struct ovn_datapaths *lr_datapaths, - struct hmap *parsed_routes_out) + struct hmap *parsed_routes_out, + struct routes_sync_tracked_data *trk_data) { if (!ovnsb_txn) { return; @@ -303,7 +502,8 @@ routes_table_sync(struct ovsdb_idl_txn *ovnsb_txn, sb_route->datapath, sb_route->logical_port, sb_route->ip_prefix, - sb_route->type); + sb_route->type, + sb_route->tracked_port); route_e->stale = true; route_e->sb_route = sb_route; } @@ -321,32 +521,34 @@ routes_table_sync(struct ovsdb_idl_txn *ovnsb_txn, false)) { continue; } - if (route->source == ROUTE_SOURCE_CONNECTED && - !get_nbrp_or_nbr_option(route->out_port, + if (route->source == ROUTE_SOURCE_CONNECTED) { + if (!get_nbrp_or_nbr_option(route->out_port, "dynamic-routing-connected")) { - continue; + continue; + } + if (smap_get_bool(&route->out_port->nbrp->options, + "dynamic-routing-connected-as-host-routes", + false)) { + publish_host_routes(ovnsb_txn, &sync_routes, + lr_stateful_table, route, trk_data); + continue; + } } if (route->source == ROUTE_SOURCE_STATIC && !get_nbrp_or_nbr_option(route->out_port, "dynamic-routing-static")) { continue; } - route_e = route_lookup_or_add(&sync_routes, - route->od->sb, - route->out_port->key, - &route->prefix, - route->plen, - "advertise"); - route_e->stale = false; - if (!route_e->sb_route) { - const struct sbrec_route *sr = sbrec_route_insert(ovnsb_txn); - sbrec_route_set_datapath(sr, route_e->sb_db); - sbrec_route_set_logical_port(sr, route_e->logical_port); - sbrec_route_set_ip_prefix(sr, route_e->ip_prefix); - sbrec_route_set_type(sr, route_e->type); - route_e->sb_route = sr; - } + char *ip_prefix = normalize_v46_prefix(&route->prefix, + route->plen); + route_sync_to_sb(ovnsb_txn, &sync_routes, + route->od->sb, + route->out_port->key, + ip_prefix, + "advertise", + NULL); + free(ip_prefix); } HMAP_FOR_EACH_POP (route_e, hmap_node, &sync_routes) { diff --git a/northd/en-routes-sync.h b/northd/en-routes-sync.h index 391f17452..10f33ce91 100644 --- a/northd/en-routes-sync.h +++ b/northd/en-routes-sync.h @@ -15,9 +15,29 @@ #define EN_ROUTES_SYNC_H 1 #include "lib/inc-proc-eng.h" +#include "lib/uuidset.h" +#include "openvswitch/hmap.h" + +struct routes_sync_tracked_data { + /* Contains the uuids of all NB Logical Routers where we used a + * lr_stateful_record during computation. */ + struct uuidset nb_lr_stateful; + /* Contains the uuids of all NB Logical Switches where we rely on port + * port changes for host routes. */ + struct uuidset nb_ls; +}; + +struct routes_sync_data { + struct hmap parsed_routes; + + /* Node's tracked data. */ + struct routes_sync_tracked_data trk_data; +}; bool routes_sync_northd_change_handler(struct engine_node *node, - void *data OVS_UNUSED); + void *data); +bool routes_sync_lr_stateful_change_handler(struct engine_node *node, + void *data); void *en_routes_sync_init(struct engine_node *, struct engine_arg *); void en_routes_sync_cleanup(void *data); void en_routes_sync_run(struct engine_node *, void *data); diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index 741295709..84072d6ce 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -271,6 +271,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_routes_sync, &en_sb_route, NULL); engine_add_input(&en_routes_sync, &en_northd, routes_sync_northd_change_handler); + engine_add_input(&en_routes_sync, &en_lr_stateful, + routes_sync_lr_stateful_change_handler); engine_add_input(&en_sync_meters, &en_nb_acl, NULL); engine_add_input(&en_sync_meters, &en_nb_meter, NULL); diff --git a/northd/northd.c b/northd/northd.c index 4439a74da..a2e59dcfc 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -1094,19 +1094,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. @@ -1141,7 +1128,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++) { @@ -1154,12 +1141,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) { @@ -1192,6 +1181,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 991ba5f9f..dc51630ed 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 */ @@ -186,10 +187,6 @@ struct routes_data { struct hmap bfd_active_connections; }; -struct routes_sync_data { - struct hmap parsed_routes; -}; - struct route_policies_data { struct hmap route_policies; struct hmap bfd_active_connections; @@ -936,4 +933,24 @@ ovn_port_find_bound(const struct hmap *ports, const char *name) return ovn_port_find__(ports, name, true); } +/* 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 75fe40c01..d67a3d07a 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -3705,6 +3705,33 @@ or key="dynamic-routing-static" table="Logical_Router_Port"/> 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. + + In this case 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. + + 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/ovn-sb.ovsschema b/ovn-sb.ovsschema index 74540782e..01df9cc6b 100644 --- a/ovn-sb.ovsschema +++ b/ovn-sb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Southbound", "version": "20.38.0", - "cksum": "1944407838 32212", + "cksum": "1452226583 32264", "tables": { "SB_Global": { "columns": { @@ -626,6 +626,7 @@ "logical_port": {"type": "string"}, "ip_prefix": {"type": "string"}, "nexthop": {"type": "string"}, + "tracked_port": {"type": "string"}, "type": {"type": {"key": {"type": "string", "enum": ["set", ["advertise", "receive"]]}, diff --git a/ovn-sb.xml b/ovn-sb.xml index 493b7e839..bf587153a 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -5224,6 +5224,19 @@ tcp.flags = RST; </p> </column> + <column name="tracked_port"> + <p> + Only relevant for type <code>advertise</code> and if + <code>options:dynamic-routing-connected-as-host-routes</code> is + set on the <code>OVN_Northbound.Logical_Router_Port</code>. + + In this case this lists the name of the <code>Port_Binding</code> + that is holding this ip address. ovn-controller can use this + information to determine the distance and therefor the route priority + of a published route + </p> + </column> + <column name="type"> <p> If the route is to be exported from OVN to the outside network or if diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 213536707..b40eb24e6 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -14042,5 +14042,72 @@ 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 +# 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-connected=true \ + option:dynamic-routing-static=true +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 +check ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 10.0.1.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 +check_row_count Route 2 type=advertise tracked_port='""' +datapath=$(ovn-sbctl --bare --columns _uuid list datapath_binding 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 Route 2 type=advertise +AT_CHECK([ovn-sbctl --columns ip_prefix,tracked_port --bare find Route datapath=$datapath logical_port=lr0-sw0], [0], [dnl +10.0.0.1/32 +lr0-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" +check_row_count Route 3 type=advertise +check_row_count Route 2 type=advertise tracked_port!='""' +AT_CHECK([ovn-sbctl --columns tracked_port --bare find Route datapath=$datapath logical_port=lr0-sw0 ip_prefix=10.0.0.2/32], [0], [dnl +sw0-vif0 +]) + +# 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 +check ovn-nbctl --wait=sb set Logical_Switch_Port sw0-lr1 type=router options:router-port=lr1-sw0 +check_row_count Route 4 type=advertise +check_row_count Route 3 type=advertise tracked_port!='""' +AT_CHECK([ovn-sbctl --columns tracked_port --bare find Route datapath=$datapath logical_port=lr0-sw0 ip_prefix=10.0.0.10/32], [0], [dnl +lr1-sw0 +]) + +# 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 Route 5 type=advertise +check_row_count Route 4 type=advertise tracked_port!='""' +AT_CHECK([ovn-sbctl --columns tracked_port --bare find Route datapath=$datapath logical_port=lr0-sw0 ip_prefix=10.0.0.100/32], [0], [dnl +lr1-sw0 +]) + +# 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 Route 6 type=advertise +check_row_count Route 4 type=advertise tracked_port!='""' +AT_CHECK([ovn-sbctl --columns ip_prefix --bare find Route datapath=$datapath logical_port=lr0-sw0 ip_prefix=172.16.0.0/24], [0], [dnl +172.16.0.0/24 +]) + AT_CLEANUP ])
sometimes we want to use individual host routes instead of the connected routes of LRPs. This allows the network fabric to know which adresses are actually in use and e.g. drop traffic to adresses that are not used anyway. Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud> --- NEWS | 2 + northd/en-lflow.c | 1 + northd/en-routes-sync.c | 270 ++++++++++++++++++++++++++++++++++----- northd/en-routes-sync.h | 22 +++- northd/inc-proc-northd.c | 2 + northd/northd.c | 32 ++--- northd/northd.h | 25 +++- ovn-nb.xml | 27 ++++ ovn-sb.ovsschema | 3 +- ovn-sb.xml | 13 ++ tests/ovn-northd.at | 67 ++++++++++ 11 files changed, 406 insertions(+), 58 deletions(-)