On 5/20/25 7:20 PM, Paulo Guilherme da Silva via dev wrote:
> This commit adds the ability for ovn-ic to filter routes between VPCs.
> In use cases where the user is using vpc-peering(route tag) handling
> multiple points of redistribution, when we connect more than one TS in
> the same Logical Router it is essential that we have control over which
> routes will be filtered or advertised by transit switch.
>
> Co-authored-by: Lucas Vargas Dias <[email protected]>
> Signed-off-by: Lucas Vargas Dias <[email protected]>
> Signed-off-by: Paulo Guilherme da Silva <[email protected]>
> ---
Hi Paulo, Lucas,
Thanks for v6!
> NEWS | 3 ++
> ic/ovn-ic.c | 124 ++++++++++++++++++++++++++++--------------------
> lib/ovn-util.c | 47 ++++++++++++++++++
> lib/ovn-util.h | 4 ++
> ovn-nb.xml | 44 ++++++++++++++++-
> tests/ovn-ic.at | 113 +++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 283 insertions(+), 52 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index f82f25754..2dc89c801 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -20,6 +20,9 @@ Post v25.03.0
> - Introduce exclude-router-ips-from-garp in logical_switch_port column
> so that the router port IPs are not advertised in the GARPs.
> - Added support for port mirroring in OVN overlay.
> + - Added "ic-route-filter-adv" and "ic-route-filter-learn" options to
Nit: indentation is a bit off here.
> + the Logical_Router/Logical_Router_Port tables to allow users to
> + filter advertised/learned IC routes.
>
> OVN v25.03.0 - 07 Mar 2025
> --------------------------
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index 682ecb2a1..e74cc1fe3 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -1068,69 +1068,76 @@ get_nexthop_from_lport_addresses(bool is_v4,
> return true;
> }
>
> +static bool
> +prefix_is_filtered(struct in6_addr *prefix,
> + unsigned int plen,
> + const struct nbrec_logical_router *nb_lr,
> + const struct nbrec_logical_router_port *ts_lrp,
> + bool is_advertisement)
> +{
> + struct ds filter_list = DS_EMPTY_INITIALIZER;
> + const char *filter_direction = is_advertisement ? "ic-route-filter-adv" :
> +
> "ic-route-filter-learn";
> + if (ts_lrp) {
> + const char *lrp_route_filter = smap_get(&ts_lrp->options,
> + filter_direction);
> + if (lrp_route_filter) {
> + ds_put_format(&filter_list, "%s,", lrp_route_filter);
> + }
> + }
> + const char *lr_route_filter = smap_get(&nb_lr->options,
> + filter_direction);
> + if (lr_route_filter) {
> + ds_put_format(&filter_list, "%s,", lr_route_filter);
> + }
> +
> + struct sset prefix_list = SSET_INITIALIZER(&prefix_list);
> + sset_from_delimited_string(&prefix_list, ds_cstr(&filter_list), ",");
> +
> + bool matched = true;
> + if (!sset_is_empty(&prefix_list)) {
> + matched = find_prefix_in_list(prefix, plen, &prefix_list,
> + filter_direction);
> + }
> +
> + ds_destroy(&filter_list);
> + sset_destroy(&prefix_list);
> + return matched;
> +}
> +
> static bool
> prefix_is_deny_listed(const struct smap *nb_options,
> struct in6_addr *prefix,
> unsigned int plen)
> {
> - const char *denylist = smap_get(nb_options, "ic-route-denylist");
> + const char *filter_name = "ic-route-denylist";
> + const char *denylist = smap_get(nb_options, filter_name);
> if (!denylist || !denylist[0]) {
> denylist = smap_get(nb_options, "ic-route-blacklist");
> if (!denylist || !denylist[0]) {
> return false;
> }
> }
> - struct in6_addr bl_prefix;
> - unsigned int bl_plen;
> - char *cur, *next, *start;
> - next = start = xstrdup(denylist);
> - bool matched = false;
> - while ((cur = strsep(&next, ",")) && *cur) {
> - if (!ip46_parse_cidr(cur, &bl_prefix, &bl_plen)) {
> - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> - VLOG_WARN_RL(&rl, "Bad format in nb_global options:"
> - "ic-route-denylist: %s. CIDR expected.", cur);
> - continue;
> - }
> -
> - if (IN6_IS_ADDR_V4MAPPED(&bl_prefix) !=
> IN6_IS_ADDR_V4MAPPED(prefix)) {
> - continue;
> - }
>
> - /* 192.168.0.0/16 does not belong to 192.168.0.0/17 */
> - if (plen < bl_plen) {
> - continue;
> - }
> -
> - if (IN6_IS_ADDR_V4MAPPED(prefix)) {
> - ovs_be32 bl_prefix_v4 = in6_addr_get_mapped_ipv4(&bl_prefix);
> - ovs_be32 prefix_v4 = in6_addr_get_mapped_ipv4(prefix);
> - ovs_be32 mask = be32_prefix_mask(bl_plen);
> + struct sset prefix_list = SSET_INITIALIZER(&prefix_list);
> + sset_from_delimited_string(&prefix_list, denylist, ",");
>
> - if ((prefix_v4 & mask) != (bl_prefix_v4 & mask)) {
> - continue;
> - }
> - } else {
> - struct in6_addr bl_mask = ipv6_create_mask(bl_plen);
> - struct in6_addr m_prefix = ipv6_addr_bitand(prefix, &bl_mask);
> - struct in6_addr m_bl_prefix = ipv6_addr_bitand(&bl_prefix,
> - &bl_mask);
> - if (!ipv6_addr_equals(&m_prefix, &m_bl_prefix)) {
> - continue;
> - }
> - }
> - matched = true;
> - break;
> + bool denied = false;
> + if (!sset_is_empty(&prefix_list)) {
> + denied = find_prefix_in_list(prefix, plen, &prefix_list,
> + filter_name);
> }
> - free(start);
> - return matched;
> + sset_destroy(&prefix_list);
> + return denied;
> }
>
> static bool
> route_need_advertise(const char *policy,
> struct in6_addr *prefix,
> unsigned int plen,
> - const struct smap *nb_options)
> + const struct smap *nb_options,
> + const struct nbrec_logical_router *nb_lr,
> + const struct nbrec_logical_router_port *ts_lrp)
> {
> if (!smap_get_bool(nb_options, "ic-route-adv", false)) {
> return false;
> @@ -1152,6 +1159,11 @@ route_need_advertise(const char *policy,
> if (prefix_is_deny_listed(nb_options, prefix, plen)) {
> return false;
> }
> +
> + if (!prefix_is_filtered(prefix, plen, nb_lr, ts_lrp, true)) {
> + return false;
> + }
> +
> return true;
> }
>
> @@ -1206,7 +1218,8 @@ add_static_to_routes_ad(
> const struct nbrec_logical_router *nb_lr,
> const struct lport_addresses *nexthop_addresses,
> const struct smap *nb_options,
> - const char *route_tag)
> + const char *route_tag,
> + const struct nbrec_logical_router_port *ts_lrp)
> {
> struct in6_addr prefix, nexthop;
> unsigned int plen;
> @@ -1215,7 +1228,8 @@ add_static_to_routes_ad(
> return;
> }
>
> - if (!route_need_advertise(nb_route->policy, &prefix, plen, nb_options)) {
> + if (!route_need_advertise(nb_route->policy, &prefix, plen, nb_options,
> + nb_lr, ts_lrp)) {
> return;
> }
>
> @@ -1256,7 +1270,8 @@ add_network_to_routes_ad(struct hmap *routes_ad, const
> char *network,
> const struct lport_addresses *nexthop_addresses,
> const struct smap *nb_options,
> const struct nbrec_logical_router *nb_lr,
> - const char *route_tag)
> + const char *route_tag,
> + const struct nbrec_logical_router_port *ts_lrp)
> {
> struct in6_addr prefix, nexthop;
> unsigned int plen;
> @@ -1264,7 +1279,8 @@ add_network_to_routes_ad(struct hmap *routes_ad, const
> char *network,
> return;
> }
>
> - if (!route_need_advertise(NULL, &prefix, plen, nb_options)) {
> + if (!route_need_advertise(NULL, &prefix, plen, nb_options,
> + nb_lr, ts_lrp)) {
> VLOG_DBG("Route ad: skip network %s of lrp %s.",
> network, nb_lrp->name);
> return;
> @@ -1378,6 +1394,11 @@ route_need_learn(const struct nbrec_logical_router *lr,
> return false;
> }
>
> + if (!prefix_is_filtered(prefix, plen, lr, ts_lrp, false)) {
> + return false;
> + }
> +
> +
> if (route_has_local_gw(lr, isb_route->route_table,
> isb_route->ip_prefix)) {
> VLOG_DBG("Skip learning %s (rtb:%s) route, as we've got one with "
> "local GW", isb_route->ip_prefix, isb_route->route_table);
> @@ -1756,7 +1777,8 @@ build_ts_routes_to_adv(struct ic_context *ctx,
> struct lport_addresses *ts_port_addrs,
> const struct nbrec_nb_global *nb_global,
> const char *ts_route_table,
> - const char *route_tag)
> + const char *route_tag,
> + const struct nbrec_logical_router_port *ts_lrp)
> {
> const struct nbrec_logical_router *lr = ic_lr->lr;
>
> @@ -1779,7 +1801,7 @@ build_ts_routes_to_adv(struct ic_context *ctx,
> } else if (!strcmp(ts_route_table, nb_route->route_table)) {
> /* It may be a route to be advertised */
> add_static_to_routes_ad(routes_ad, nb_route, lr, ts_port_addrs,
> - &nb_global->options, route_tag);
> + &nb_global->options, route_tag, ts_lrp);
> }
> }
>
> @@ -1791,7 +1813,7 @@ build_ts_routes_to_adv(struct ic_context *ctx,
> add_network_to_routes_ad(routes_ad, lrp->networks[j], lrp,
> ts_port_addrs,
> &nb_global->options,
> - lr, route_tag);
> + lr, route_tag, ts_lrp);
> }
> } else {
> /* The router port of the TS port is ignored. */
> @@ -1854,7 +1876,7 @@ collect_lr_routes(struct ic_context *ctx,
> route_tag = "";
> }
> build_ts_routes_to_adv(ctx, ic_lr, routes_ad, &ts_port_addrs,
> - nb_global, route_table, route_tag);
> + nb_global, route_table, route_tag, lrp);
> destroy_lport_addresses(&ts_port_addrs);
> }
> }
> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> index 45a230b03..2cfb477a5 100644
> --- a/lib/ovn-util.c
> +++ b/lib/ovn-util.c
> @@ -1392,6 +1392,53 @@ prefix_is_link_local(const struct in6_addr *prefix,
> unsigned int plen)
> ((prefix->s6_addr[1] & 0xc0) == 0x80));
> }
>
> +bool
> +find_prefix_in_list(const struct in6_addr *prefix, unsigned int plen,
> + const struct sset *prefix_list, const char *filter_name)
I should've probably mentioned this in the v5 review but this is a
function that searches in a prefix set; it feels more natural if it's
called find_prefix_in_set(). Also, 'prefix_list' should probably be
called 'prefix_set' instead everywhere (in the callers too).
> +{
> + struct in6_addr lt_prefix;
> + const char *cur_prefix;
> + unsigned int lt_plen;
> +
> + SSET_FOR_EACH (cur_prefix, prefix_list) {
> + if (!ip46_parse_cidr(cur_prefix, <_prefix, <_plen)) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> + VLOG_WARN_RL(&rl, "Bad prefix (%s) format: %s. CIDR expected.",
> + filter_name, cur_prefix);
> + continue;
> + }
> +
> + if (IN6_IS_ADDR_V4MAPPED(<_prefix) !=
> IN6_IS_ADDR_V4MAPPED(prefix)) {
> + continue;
> + }
> +
> + /* 192.168.0.0/16 does not belong to 192.168.0.0/17 */
> + if (plen < lt_plen) {
> + continue;
> + }
> +
> + if (IN6_IS_ADDR_V4MAPPED(prefix)) {
> + ovs_be32 bl_prefix_v4 = in6_addr_get_mapped_ipv4(<_prefix);
> + ovs_be32 prefix_v4 = in6_addr_get_mapped_ipv4(prefix);
> + ovs_be32 mask = be32_prefix_mask(lt_plen);
> +
> + if ((prefix_v4 & mask) != (bl_prefix_v4 & mask)) {
> + continue;
Nit: To avoid the negated logic, I'd change the condition to == and
return true here.
> + }
> + } else {
> + struct in6_addr bl_mask = ipv6_create_mask(lt_plen);
> + struct in6_addr m_prefix = ipv6_addr_bitand(prefix, &bl_mask);
> + struct in6_addr m_bl_prefix = ipv6_addr_bitand(<_prefix,
> + &bl_mask);
> + if (!ipv6_addr_equals(&m_prefix, &m_bl_prefix)) {
> + continue;
Same here.
> + }
> + }
> + return true;
Then this could be removed.
> + }
> + return false;
> +}
> +
> const struct sbrec_port_binding *
> lport_lookup_by_name(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> const char *name)
> diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> index 5d3cb95bd..872c1e5e1 100644
> --- a/lib/ovn-util.h
> +++ b/lib/ovn-util.h
> @@ -493,6 +493,10 @@ bool ovn_update_swconn_at(struct rconn *swconn, const
> char *target,
>
> bool prefix_is_link_local(const struct in6_addr *prefix, unsigned int plen);
>
> +bool find_prefix_in_list(const struct in6_addr *prefix, unsigned int plen,
> + const struct sset *prefix_list,
> + const char *filter_name);
> +
> void ovn_debug_commands_register(void);
>
> const struct sbrec_port_binding *lport_lookup_by_name(
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 4396023a4..5f8d08da0 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -3200,8 +3200,29 @@ or
> for established sessions as we will commit everything in addition
> to already existing stateful NATs and LBs.
> </column>
> - </group>
>
> + <column name="options" key="ic-route-filter-adv">
> + <p>
> + This option expects list of CIDRs delimited by "," that's present
> + in the Logical Router. A route will not be advertised if the
> + route's prefix belongs to any of the CIDRs listed.
> +
> + This allows to filter CIDR prefixes in the process of advertising
> + routes in <code>ovn-ic</code> daemon.
> + </p>
> + </column>
> +
> + <column name="options" key="ic-route-filter-learn">
> + <p>
> + This option expects list of CIDRs delimited by "," that's present
> + in the Logical Router. A route will not be learned if the
> + route's prefix belongs to any of the CIDRs listed.
> +
> + This allows to filter CIDR prefixes in the process of learning
> + routes in <code>ovn-ic</code> daemon.
> + </p>
> + </column>
> + </group>
> <group title="Common Columns">
> <column name="external_ids">
> See <em>External IDs</em> at the beginning of this document.
> @@ -4179,6 +4200,27 @@ or
> instead of the autodiscovery. Also it is then irrelevant if the port
> is bound locally.
> </column>
> + <column name="options" key="ic-route-filter-adv">
> + <p>
> + This option expects list of CIDRs delimited by "," that's present
> + in the Logical Router Port. A route will be advertised if the
> + route's prefix belongs to any of the CIDRs listed.
> +
> + This allows to filter CIDR prefixes in the process of advertising
> + routes in <code>ovn-ic</code> daemon.
> + </p>
> + </column>
> +
> + <column name="options" key="ic-route-filter-learn">
> + <p>
> + This option expects list of CIDRs delimited by "," that's present
> + in the Logical Router Port. A route will be learned if the
> + route's prefix belongs to any of the CIDRs listed.
> +
> + This allows to filter CIDR prefixes in the process of learning
> + routes in <code>ovn-ic</code> daemon.
> + </p>
> + </column>
> </group>
>
> <group title="Attachment">
> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
> index b3d25e663..159ffb594 100644
> --- a/tests/ovn-ic.at
> +++ b/tests/ovn-ic.at
> @@ -3247,3 +3247,116 @@ OVN_CLEANUP_IC([az1], [az2])
>
> AT_CLEANUP
> ])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn-ic -- prefix filter -- filtering routes])
> +ovn_init_ic_db
> +ovn-ic-nbctl ts-add ts1
> +for i in 1 2; do
> + ovn_start az$i
> + ovn_as az$i
> + # Enable route learning at AZ level.
> + check ovn-nbctl set nb_global . options:ic-route-learn=true
> + # Enable route advertising at AZ level.
> + check ovn-nbctl set nb_global . options:ic-route-adv=true
> +done
> +
> +# Create new transit switche and LRs. Test topology is next:
Typo: switche
> +# logical router (lr11) - transit switch (ts1) - logical router (lr12)
> +
> +# Create lr11, lr12 and connect them to ts1.
> +for i in 1 2; do
> + ovn_as az$i
> + lr=lr1$i
> + check ovn-nbctl lr-add $lr
> + lrp=lrp-$lr-ts1
> + lsp=lsp-ts1-$lr
> + # Create LRP and connect to TS
Nit: missing period.
> + check ovn-nbctl lrp-add $lr $lrp aa:aa:aa:aa:ab:0$i 169.254.101.$i/24
> fe80:10::$i/64
> + check ovn-nbctl lsp-add ts1 $lsp \
> + -- lsp-set-addresses $lsp router \
> + -- lsp-set-type $lsp router \
> + -- lsp-set-options $lsp router-port=$lrp
> +done
> +
> +# Create directly-connected route in lr12.
> +ovn_as az2 check ovn-nbctl lrp-add lr12 lrp-lr12-1 aa:aa:aa:aa:bb:01
> "192.168.12.1/24" "2001:db12::1/64"
> +ovn_as az2 check ovn-nbctl lrp-add lr12 lrp-lr12-2 aa:aa:aa:aa:bb:02
> "192.168.2.1/24" "2001:db22::1/64"
> +
> +# Create directly-connected route in lr11.
> +ovn_as az1 check ovn-nbctl --wait=sb lrp-add lr11 lrp-lr11 aa:aa:aa:aa:cc:01
> "192.168.11.1/24" "2001:db11::1/64"
> +check ovn-ic-nbctl --wait=sb sync
> +
> +# Test direct routes from lr12 were learned to lr11.
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep 192.168 |
> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> +192.168.12.0/24 169.254.101.2
> +192.168.2.0/24 169.254.101.2
> +])
> +
> +# Test direct routes from lr11 were learned to lr12.
> +OVS_WAIT_FOR_OUTPUT([ovn_as az2 ovn-nbctl lr-route-list lr12 | grep 192.168 |
> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> +192.168.11.0/24 169.254.101.1
> +])
> +
> +ovn_as az2 check ovn-nbctl set logical_router_port lrp-lr12-ts1
> options:ic-route-filter-adv=192.168.12.0/24
> +# Test direct routes from lr12 were learned to lr11.
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep 192.168 |
> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> +192.168.12.0/24 169.254.101.2
> +])
> +
> +# Set ic-route-filter-adv with invalid CIDR to advertise just the valid
> route.
> +ovn_as az2 check ovn-nbctl set logical_router_port lrp-lr12-ts1
> options:ic-route-filter-adv="192.168.12.0/24,invalid"
> +# Test direct routes from lr12 were learned to lr11.
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep 192.168 |
> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> +192.168.12.0/24 169.254.101.2
> +])
> +
> +ovn_as az2 check ovn-nbctl remove logical_router_port lrp-lr12-ts1 options
> ic-route-filter-adv
> +ovn_as az1 check ovn-nbctl set logical_router_port lrp-lr11-ts1
> options:ic-route-filter-learn=192.168.2.0/24
> +
> +# Test direct routes from lr12 were learned to lr11.
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep 192.168 |
> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> +192.168.2.0/24 169.254.101.2
> +])
> +
> +ovn_as az1 check ovn-nbctl remove logical_router_port lrp-lr11-ts1 options
> ic-route-filter-learn
> +
> +# Test direct routes from lr12 were learned to lr11.
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep 2001 |
> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> +2001:db12::/64 fe80:10::2
> +2001:db22::/64 fe80:10::2
> +])
> +ovn_as az2 check ovn-nbctl set logical_router_port lrp-lr12-ts1
> options:ic-route-filter-adv=2001:db22::/64
> +
> +# Test direct routes from lr12 were learned to lr11.
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep 2001 |
> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> +2001:db22::/64 fe80:10::2
> +])
> +
> +ovn_as az2 check ovn-nbctl remove logical_router_port lrp-lr12-ts1 options
> ic-route-filter-adv
> +
> +# Test direct routes from lr12 were learned to lr11.
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep 2001 |
> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> +2001:db12::/64 fe80:10::2
> +2001:db22::/64 fe80:10::2
> +])
> +
> +ovn_as az1 check ovn-nbctl set logical_router_port lrp-lr11-ts1
> options:ic-route-filter-learn=2001:db12::/64
> +
> +# Test direct routes from lr12 were learned to lr11.
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep 2001 |
> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> +2001:db12::/64 fe80:10::2
> +])
> +
> +OVN_CLEANUP_IC([az1], [az2])
> +AT_CLEANUP
> +])
> \ No newline at end of file
No newline at end of file.
To avoid a v7 I took care of the minor comments I had above and applied
the patch to main. I hope that's fine with you.
Thanks,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
