Thanks for the patch, Alexandra. I have a few comments. On Mon, Aug 31, 2026 at 9:25 AM Alexandra Rukomoinikova via dev < [email protected]> wrote:
> Add the "options:dynamic-routing-advertise-prefixes" key to the > Logical_Router_Port table. If it is set, only the listed prefixes are > synced to the SB Advertised_Route table for that port and every route > selected by "dynamic-routing-redistribute" (connected, static, nat, lb, > hub-spoke) is skipped on it. This lets the CMS announce an aggregated > prefix instead of many individual NAT or LB addresses. > > Signed-off-by: Alexandra Rukomoinikova <[email protected]> > --- > NEWS | 4 ++ > northd/en-advertised-route-sync.c | 69 ++++++++++++++++++- > ovn-nb.xml | 30 +++++++++ > tests/ovn-northd.at | 108 ++++++++++++++++++++++++++++++ > 4 files changed, 210 insertions(+), 1 deletion(-) > > diff --git a/NEWS b/NEWS > index 40a1b9867..78af38f9f 100644 > --- a/NEWS > +++ b/NEWS > @@ -120,6 +120,10 @@ OVN v26.09.0 - xxx xx xxxx > filter the routes learned through the port by route tag. This > supersedes > "ic-route-filter-tag", which is now deprecated and is ignored when > "ic-route-learn-tag-rules" is set. > + - Logical_Router_Port: Added a new > + "options:dynamic-routing-advertise-prefixes" key that contains an > + explicit list of prefixes to be advertised via this port. If it is > + set, only these prefixes will be announced. > > OVN v26.03.0 - xxx xx xxxx > -------------------------- > diff --git a/northd/en-advertised-route-sync.c > b/northd/en-advertised-route-sync.c > index eb8a24a39..c623d9ba0 100644 > --- a/northd/en-advertised-route-sync.c > +++ b/northd/en-advertised-route-sync.c > @@ -29,6 +29,8 @@ > > VLOG_DEFINE_THIS_MODULE(en_advertised_route_sync); > > +#define DYNAMIC_ROUTING_ADVERTISE_PREFIXES > "dynamic-routing-advertise-prefixes" > + > struct ar_entry { > struct hmap_node hmap_node; > > @@ -50,6 +52,8 @@ struct ar_entry { > * unmonitored listener must remain reachable regardless. */ > bool has_ungated_lb; > struct sset health_checks; > + > + bool advertise_prefix; > Could you add an inline comment? Other members of this struct have them. > }; > > /* Add a new entries to the to-be-advertised routes. > @@ -871,6 +875,61 @@ build_connected_as_host_routes(const struct > ovn_datapath *od, > } > } > > +static const char * > +lrp_advertise_prefixes(const struct ovn_port *op) > could you add a comment describing what this function does and returns > +{ > + if (!op || !op->nbrp) { > + return NULL; > + } > + > + return smap_get(&op->nbrp->options, > DYNAMIC_ROUTING_ADVERTISE_PREFIXES); > +} > + > +static void > +build_advertise_prefix_routes(const struct ovn_datapath *od, > + struct hmap *routes) > same as lrp_advertise_prefixes > +{ > + const struct ovn_port *op; > + HMAP_FOR_EACH (op, dp_node, &od->ports) { > + const char *prefixes = lrp_advertise_prefixes(op); > + if (!prefixes || !op->sb) { > + continue; > + } > + > + char *save_ptr = NULL; > + char *tokstr = xstrdup(prefixes); > + for (char *token = strtok_r(tokstr, ",", &save_ptr); > + token != NULL; > + token = strtok_r(NULL, ",", &save_ptr)) { > + struct in6_addr prefix; > + unsigned int plen; > + > + if (!ip46_parse_cidr(token, &prefix, &plen)) { > + static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, "bad prefix '%s' in option %s of %s", > + token, DYNAMIC_ROUTING_ADVERTISE_PREFIXES, > + op->nbrp->name); > + continue; > + } > + > + char *ip_prefix = normalize_v46_prefix(&prefix, plen); > + struct ar_entry *dup = ar_entry_find(routes, od->sdp->sb_dp, > + op->sb, ip_prefix, NULL); > + if (dup && dup->advertise_prefix) { > + /* The same prefix is listed twice in the option. */ > + free(ip_prefix); > + continue; > + } > + > + struct ar_entry *route_e = > + ar_entry_add_nocopy(routes, od, op, ip_prefix, NULL, > + ROUTE_SOURCE_STATIC); > This is misleading. ROUTE_SOURCE_STATIC is derived from the northbound static route table which is not happening here. The source is only inspected by should_advertise_route, which the advertise_prefix flag buypasses and in ar_entry_sync_external_ids which only cares about ROUTE_SOURCE_LB so it is not functionally wrong. Adding another enum for ROUTE_SOURCE_EXPLICITLY_ADVERTISED or something would make the code cleaner and no one would wonder why ROUTE_SOURCE_STATIC is used. > + route_e->advertise_prefix = true; > + } > + free(tokstr); > + } > +} > + > void * > en_dynamic_routes_init(struct engine_node *node OVS_UNUSED, > struct engine_arg *arg OVS_UNUSED) > @@ -989,6 +1048,8 @@ en_dynamic_routes_run(struct engine_node *node, void > *data) > build_connected_as_host_routes(od, &northd_data->ls_ports, > dynamic_routes_data); > > + build_advertise_prefix_routes(od, &dynamic_routes_data->routes); > + > const struct lr_stateful_record *lr_stateful_rec = > lr_stateful_table_find_by_uuid(&lr_stateful_data->table, > od->key); > if (!lr_stateful_rec) { > @@ -1105,6 +1166,11 @@ should_advertise_route(const struct ovn_datapath > *advertising_od, > return false; > } > > + if (lrp_advertise_prefixes(advertising_op)) { > + /* This port advertises only the explicitly configured prefixes. > */ > + return false; > + } > + > enum dynamic_routing_redistribute_mode drr = > advertising_op->dynamic_routing_redistribute; > > @@ -1178,7 +1244,8 @@ advertised_route_table_sync( > /* Then add the set of dynamic routes that need sync-ing. */ > struct ar_entry *route_e; > HMAP_FOR_EACH (route_e, hmap_node, dynamic_routes) { > - if (!should_advertise_route(route_e->od, route_e->op, > + if (!route_e->advertise_prefix && > + !should_advertise_route(route_e->od, route_e->op, > route_e->source)) { > continue; > } > diff --git a/ovn-nb.xml b/ovn-nb.xml > index c741a3b32..c771b7e05 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -4850,6 +4850,36 @@ or > > </column> > > + <column name="options" key="dynamic-routing-advertise-prefixes" > + type='{"type": "string"}'> > + <p> > + Only relevant if <ref column="options" key="dynamic-routing" > + table="Logical_Router"/> on the respective Logical_Router is set > + to <code>true</code>. > + </p> > + > + <p> > + This is a list of IPv4 and/or IPv6 prefixes in CIDR notation, > + separated by <code>,</code>, e.g. > + <code>10.0.0.0/8,2001:db8::/32</code>. > + </p> > + > + <p> > + If this option is set, northd creates entries in the <ref > + table="Advertised_Route" db="OVN_Southbound"/> table only for > the > + prefixes listed here. Every route that would otherwise be > + advertised via this port because of <ref column="options" > + key="dynamic-routing-redistribute" > + table="Logical_Router_Port"/> (or its Logical_Router > counterpart), > + i.e. <code>connected</code>, <code>connected-as-host</code>, > + <code>static</code>, <code>nat</code>, <code>lb</code> and > + <code>hub-spoke</code> routes, is not advertised via this port. > + Setting the option to an empty string disables all > advertisements > + on this port. > + </p> > + > + </column> > + > <column name="options" key="dynamic-routing-advertise"> > If the CMS sets <ref column="options" > key="dynamic-routing-redistribute" > table="Logical_Router_Port"/> to <code>connected-as-host</code>, > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 6d191c1a0..45322ffcb 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -16525,6 +16525,114 @@ OVN_CLEANUP_NORTHD > AT_CLEANUP > ]) > > +OVN_FOR_EACH_NORTHD_NO_HV([ > +AT_SETUP([dynamic-routing - advertise only configured prefixes]) > +AT_KEYWORDS([dynamic-routing]) > +ovn_start > + > +# Start with a GW router that advertises connected, static, nat and lb > routes > +# via both of its LRPs. > +check ovn-nbctl lr-add lr0 > +check ovn-nbctl set Logical_Router lr0 \ > + options:dynamic-routing=true \ > + options:chassis=hv1 \ > + options:dynamic-routing-redistribute="connected,static,nat,lb" > +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 lr-route-add lr0 192.168.0.0/24 10.0.0.10 > +check ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.16.1.10 10.0.0.20 > +check ovn-nbctl lb-add lb0 172.16.2.10:80 10.0.0.30:80 > +check ovn-nbctl lr-lb-add lr0 lb0 > +check ovn-nbctl --wait=sb sync > + > +datapath=$(fetch_column Datapath_Binding _uuid external_ids:name=lr0) > +sw0=$(fetch_column Port_Binding _uuid logical_port=lr0-sw0) > +sw1=$(fetch_column Port_Binding _uuid logical_port=lr0-sw1) > + > +check_row_count Advertised_Route 7 > +check_row_count Advertised_Route 4 logical_port=$sw0 > +check_row_count Advertised_Route 1 logical_port=$sw0 ip_prefix= > 10.0.0.0/24 > +check_row_count Advertised_Route 1 logical_port=$sw0 ip_prefix= > 192.168.0.0/24 > +check_row_count Advertised_Route 1 logical_port=$sw0 ip_prefix=172.16.1.10 > +check_row_count Advertised_Route 1 logical_port=$sw0 ip_prefix=172.16.2.10 > +check_row_count Advertised_Route 3 logical_port=$sw1 > + > +# Setting dynamic-routing-advertise-prefixes on lr0-sw0 replaces every > route > +# advertised via this port with the configured ones. Both IPv4 and IPv6 > +# prefixes are supported. The other LRP is not affected. > +check ovn-nbctl --wait=sb set Logical_Router_Port lr0-sw0 \ > + options:dynamic-routing-advertise-prefixes=" > 172.16.0.0/16,2001:db8:1::/64" > +check_row_count Advertised_Route 5 > +check_row_count Advertised_Route 2 logical_port=$sw0 > +check_row_count Advertised_Route 1 \ > + ip_prefix="172.16.0.0/16" \ > + datapath=$datapath \ > + logical_port=$sw0 \ > + tracked_port=[[]] > +check_row_count Advertised_Route 1 \ > + ip_prefix='"2001:db8:1::/64"' \ > + datapath=$datapath \ > + logical_port=$sw0 \ > + tracked_port=[[]] > +check_row_count Advertised_Route 3 logical_port=$sw1 > +check_row_count Advertised_Route 1 logical_port=$sw1 ip_prefix= > 10.0.1.0/24 > +check_row_count Advertised_Route 1 logical_port=$sw1 ip_prefix=172.16.1.10 > +check_row_count Advertised_Route 1 logical_port=$sw1 ip_prefix=172.16.2.10 > + > +# The prefixes are normalized and duplicates are advertised only once. > +check ovn-nbctl --wait=sb set Logical_Router_Port lr0-sw0 \ > + options:dynamic-routing-advertise-prefixes=" > 10.10.10.5/24,10.10.10.0/24" > +check_row_count Advertised_Route 1 logical_port=$sw0 > +check_row_count Advertised_Route 1 logical_port=$sw0 ip_prefix= > 10.10.10.0/24 > + > +# Invalid prefixes are ignored, the valid ones are still advertised. > +check ovn-nbctl --wait=sb set Logical_Router_Port lr0-sw0 \ > + options:dynamic-routing-advertise-prefixes="not-a-prefix,10.20.0.0/16 > " > +check_row_count Advertised_Route 1 logical_port=$sw0 > +check_row_count Advertised_Route 1 logical_port=$sw0 ip_prefix= > 10.20.0.0/16 > + > +# An empty list blocks all advertisements on this port. > +check ovn-nbctl --wait=sb set Logical_Router_Port lr0-sw0 \ > + options:dynamic-routing-advertise-prefixes='""' > +check_row_count Advertised_Route 0 logical_port=$sw0 > +check_row_count Advertised_Route 3 logical_port=$sw1 > + > +# The option also overrides the redistribute modes set on the LRP itself. > +check ovn-nbctl --wait=sb set Logical_Router_Port lr0-sw0 \ > + options:dynamic-routing-redistribute="connected,static,nat,lb" \ > + options:dynamic-routing-advertise-prefixes="10.30.0.0/16" > +check_row_count Advertised_Route 1 logical_port=$sw0 > +check_row_count Advertised_Route 1 logical_port=$sw0 ip_prefix= > 10.30.0.0/16 > + > +# Removing the option brings the redistributed routes back. > +check ovn-nbctl --wait=sb remove Logical_Router_Port lr0-sw0 \ > + options dynamic-routing-advertise-prefixes > +check_row_count Advertised_Route 4 logical_port=$sw0 > +check_row_count Advertised_Route 1 logical_port=$sw0 ip_prefix= > 10.0.0.0/24 > +check_row_count Advertised_Route 1 logical_port=$sw0 ip_prefix= > 192.168.0.0/24 > +check_row_count Advertised_Route 1 logical_port=$sw0 ip_prefix=172.16.1.10 > +check_row_count Advertised_Route 1 logical_port=$sw0 ip_prefix=172.16.2.10 > + > +# The configured prefixes are advertised only while dynamic routing is > +# enabled on the Logical_Router. > +check ovn-nbctl --wait=sb set Logical_Router_Port lr0-sw0 \ > + options:dynamic-routing-advertise-prefixes="10.30.0.0/16" > +check ovn-nbctl --wait=sb remove Logical_Router lr0 options > dynamic-routing > +check_row_count Advertised_Route 0 > +check ovn-nbctl --wait=sb set Logical_Router lr0 > options:dynamic-routing=true > +check_row_count Advertised_Route 4 > +check_row_count Advertised_Route 1 logical_port=$sw0 ip_prefix= > 10.30.0.0/16 > +check_row_count Advertised_Route 3 logical_port=$sw1 > + > Could you add a dynamic-routing-advertised-prefixes entry that does not correspond to an existing OVN object? The entry in the Advertised_Route table is created if there is a matching OVN object or not and the test should exercise that. > +# Removing the LRP removes its advertised prefixes. > +check ovn-nbctl --wait=sb lrp-del lr0-sw0 > +check_row_count Advertised_Route 3 > +check_row_count Advertised_Route 0 logical_port=$sw0 > + > +OVN_CLEANUP_NORTHD > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD_NO_HV([ > AT_SETUP([dynamic-routing - learning routes from sb]) > AT_KEYWORDS([dynamic-routing]) > -- > 2.48.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thanks, Jacob _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
