Hi! Thanks for the contribution.
The code change looks good to me. However, there are a couple of issues
with the documentation updates.
1. The new ic-route-deny-adv and ic-route-deny-learn options are only
documented for the Logical_Router table. They also need to be documented
for the Logical_Router_Port table.
2. The commit message says that it fixes a documentation error for the
ic-route-filter-adv option. If I've read the code correctly, the issue
with the documentation is that it says that listed CIDRs will NOT be
advertised, but the docs should say that the listed CIDRs WILL be
advertised. With this change, you've updated the ic-route-filter-learn
option to have the correct information, but the ic-route-filter-adv
option still has the "not" present.
I have a few comments on the test in line below.
On 8/7/25 5:42 AM, Indrajitt Valsaraj wrote:
This commit adds the ability for ovn-ic to deny filter routes
learnt/advertised between AZs. This commit also fixes a documentation
error for the ic-route-filter-adv option.
Signed-off-by: Indrajitt Valsaraj <indrajitt.valsa...@nutanix.com>
---
NEWS | 3 +
ic/ovn-ic.c | 53 ++++++++++++----
ovn-nb.xml | 22 +++++++
tests/ovn-ic.at | 165 ++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 229 insertions(+), 14 deletions(-)
diff --git a/NEWS b/NEWS
index 0cce1790d..44ec47b5e 100644
--- a/NEWS
+++ b/NEWS
@@ -41,6 +41,9 @@ Post v25.03.0
- Added support for running tests from the 'check-kernel' system test
target
under retis by setting OVS_TEST_WITH_RETIS=yes. See the 'Testing'
section
of the documentation for more details.
+ - Added "ic-route-deny-adv" and "ic-route-deny-learn" options to
+ the Logical_Router/Logical_Router_Port tables to allow users to
+ deny 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 caffa6fe0..71174ae01 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -1109,26 +1109,50 @@ prefix_is_filtered(struct in6_addr *prefix,
}
static bool
-prefix_is_deny_listed(const struct smap *nb_options,
- struct in6_addr *prefix,
- unsigned int plen)
-{
- 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;
+prefix_is_deny_filtered(struct in6_addr *prefix,
+ unsigned int plen,
+ const struct smap *nb_options,
+ const struct nbrec_logical_router *nb_lr,
+ const struct nbrec_logical_router_port *ts_lrp,
+ bool is_advertisement)
+{
+ struct ds deny_list = DS_EMPTY_INITIALIZER;
+ const char *deny_key = is_advertisement ? "ic-route-deny-adv" :
+ "ic-route-deny-learn";
+
+ if (ts_lrp) {
+ const char *lrp_deny_filter = smap_get(&ts_lrp->options, deny_key);
+ if (lrp_deny_filter) {
+ ds_put_format(&deny_list, "%s,", lrp_deny_filter);
+ }
+ }
+
+ if (nb_lr) {
+ const char *lr_deny_filter = smap_get(&nb_lr->options, deny_key);
+ if (lr_deny_filter) {
+ ds_put_format(&deny_list, "%s,", lr_deny_filter);
+ }
+ }
+
+ if (nb_options) {
+ const char *global_deny = smap_get(nb_options, "ic-route-denylist");
+ if (!global_deny || !global_deny[0]) {
+ global_deny = smap_get(nb_options, "ic-route-blacklist");
+ }
+ if (global_deny && global_deny[0]) {
+ ds_put_format(&deny_list, "%s,", global_deny);
}
}
struct sset prefix_set = SSET_INITIALIZER(&prefix_set);
- sset_from_delimited_string(&prefix_set, denylist, ",");
+ sset_from_delimited_string(&prefix_set, ds_cstr(&deny_list), ",");
bool denied = false;
if (!sset_is_empty(&prefix_set)) {
- denied = find_prefix_in_set(prefix, plen, &prefix_set, filter_name);
+ denied = find_prefix_in_set(prefix, plen, &prefix_set, deny_key);
}
+
+ ds_destroy(&deny_list);
sset_destroy(&prefix_set);
return denied;
}
@@ -1158,7 +1182,8 @@ route_need_advertise(const char *policy,
return false;
}
- if (prefix_is_deny_listed(nb_options, prefix, plen)) {
+ if (prefix_is_deny_filtered(prefix, plen, nb_options,
+ nb_lr, ts_lrp, true)) {
return false;
}
@@ -1511,7 +1536,7 @@ route_need_learn(const struct nbrec_logical_router *lr,
return false;
}
- if (prefix_is_deny_listed(nb_options, prefix, plen)) {
+ if (prefix_is_deny_filtered(prefix, plen, nb_options, lr, ts_lrp, false)) {
return false;
}
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 4a7581807..f4b63d129 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -3240,6 +3240,28 @@ or
</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 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>
+
+ <column name="options" key="ic-route-deny-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-deny-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
diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
index 49a409015..f1e47740e 100644
--- a/tests/ovn-ic.at
+++ b/tests/ovn-ic.at
@@ -3640,6 +3640,171 @@ OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list
lr11 | grep 2001 |
2001:db12::/64 fe80:10::2
])
+OVN_CLEANUP_IC([az1], [az2])
+AT_CLEANUP
+])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-ic -- prefix filter -- deny route adv])
+ovn_init_ic_db
+ovn-ic-nbctl ts-add ts1
+for i in 1 2; do
+ ovn_start az$i
+ ovn_as az$i
+ check ovn-nbctl set nb_global . options:ic-route-learn=true
+ check ovn-nbctl set nb_global . options:ic-route-adv=true
+done
+
+# Create routers and connect to transit switch
+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
+ 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
+
+# Add directly connected routes to lr12 (first two test prefixes reused)
+ovn_as az2 check ovn-nbctl lrp-add lr12 lrp-lr12-1 aa:aa:aa:aa:cc:01 "192.168.100.1/24"
"2001:db12::1/64"
+ovn_as az2 check ovn-nbctl lrp-add lr12 lrp-lr12-2 aa:aa:aa:aa:cc:02 "192.168.200.1/24"
"2001:db22::1/64"
+
+# Sync IC DB
+check ovn-ic-nbctl --wait=sb sync
+
+# Validate lr11 learns both routes from lr12
+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.100.0/24 169.254.101.2
+192.168.200.0/24 169.254.101.2
+])
For this baseline check, it's a good idea to ensure that both the IPv4
and IPv6 routes are present, similar to what you do after removing the
deny-adv options below.
> +> +# Set deny-adv on lrp-lr12-ts1 for 192.168.100.0/24
+ovn_as az2 check ovn-nbctl set logical_router_port lrp-lr12-ts1
options:ic-route-deny-adv=192.168.100.0/24
+
+# Only 192.168.200.0/24 should now be learned
+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.200.0/24 169.254.101.2
+])
+
+# Add IPv6 deny prefix too
+ovn_as az2 check ovn-nbctl set logical_router_port lrp-lr12-ts1
options:ic-route-deny-adv="192.168.100.0/24,2001:db12::/64"
+
+# Only 2001:db22::/64 should be learned now
+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
+])
One thing that would be good to test is how these options interact with
ic-route-filter-adv and ic-route-filter-learn. For instance, if lr12
lists the same CIDR in both ic-route-filter-adv and ic-route-deny-adv,
then what happens? From the code, the ic-route-deny-adv should take
precedence and the route should not be advertised. But it's probably a
good idea to ensure that this actually is true.
Also, what happens if lr12 has ic-route-filter-adv but lr11 has
ic-route-deny-learn for the same CIDR? That's also worth ensuring that
the route is not learned by lr11.
+# Remove deny-adv and validate all are learned again
+ovn_as az2 check ovn-nbctl remove logical_router_port lrp-lr12-ts1 options
ic-route-deny-adv
+
+OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep learned |
awk '{print $1, $2}' | sort], [0], [dnl
+192.168.100.0/24 169.254.101.2
+192.168.200.0/24 169.254.101.2
+2001:db12::/64 fe80:10::2
+2001:db22::/64 fe80:10::2
+])
+
+# --- Test ic-route-deny-learn ---
+
+# Validate lr11 learns both v4 and v6 routes from lr12
+OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep -E
'192.168|2001' | grep learned | awk '{print $1, $2}' | sort], [0], [dnl
+192.168.100.0/24 169.254.101.2
+192.168.200.0/24 169.254.101.2
+2001:db12::/64 fe80:10::2
+2001:db22::/64 fe80:10::2
+])
The test just checked this information already. There's no need to check
it again here.
+
+# Set deny-learn on lrp-lr11-ts1 for 192.168.200.0/24 and 2001:db22::/64
+ovn_as az1 check ovn-nbctl set logical_router_port lrp-lr11-ts1
options:ic-route-deny-learn="192.168.200.0/24,2001:db22::/64"
+
+# Now lr11 should only learn 192.168.100.0/24 and 2001:db12::/64
+OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep -E
'192.168|2001' | grep learned | awk '{print $1, $2}' | sort], [0], [dnl
+192.168.100.0/24 169.254.101.2
+2001:db12::/64 fe80:10::2
+])
> +> +# Remove deny-learn and confirm full route learning resumes
+ovn_as az1 check ovn-nbctl remove logical_router_port lrp-lr11-ts1 options
ic-route-deny-learn
+
+OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep -E
'192.168|2001' | grep learned | awk '{print $1, $2}' | sort], [0], [dnl
+192.168.100.0/24 169.254.101.2
+192.168.200.0/24 169.254.101.2
+2001:db12::/64 fe80:10::2
+2001:db22::/64 fe80:10::2
+])
+
+# --- Test setting deny options on logical router ---
+
+# Set deny-adv on lr12 for 192.168.100.0/24
+ovn_as az2 check ovn-nbctl set logical_router lr12
options:ic-route-deny-adv="192.168.100.0/24"
+
+# Sync again after router option is applied
+check ovn-ic-nbctl --wait=sb sync
+
+# Only 192.168.200.0/24 should now be learned
+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.200.0/24 169.254.101.2
+])
+
+# Set deny-learn on lr11 for 2001:db22::/64
+ovn_as az1 check ovn-nbctl set logical_router lr11
options:ic-route-deny-learn="2001:db22::/64"
+
+# Only 2001:db12::/64 should now be learned
+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
+])
+
+# Clean up router-level options
+ovn_as az2 check ovn-nbctl remove logical_router lr12 options ic-route-deny-adv
+ovn_as az1 check ovn-nbctl remove logical_router lr11 options
ic-route-deny-learn
+
+# Confirm all routes are back
+OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep learned |
awk '{print $1, $2}' | sort], [0], [dnl
+192.168.100.0/24 169.254.101.2
+192.168.200.0/24 169.254.101.2
+2001:db12::/64 fe80:10::2
+2001:db22::/64 fe80:10::2
+])
+
+# --- Test ic-route-denylist (global option) ---
+
+# Set global denylist to block 192.168.100.0/24 and 2001:db12::/64
+ovn_as az2 check ovn-nbctl set nb_global .
options:ic-route-denylist="192.168.100.0/24,2001:db12::/64"
+
+# Now lr11 should only learn 192.168.200.0/24 and 2001:db22::/64
+OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep -E
'192.168|2001' | grep learned | awk '{print $1, $2}' | sort], [0], [dnl
+192.168.200.0/24 169.254.101.2
+2001:db22::/64 fe80:10::2
+])
+
+# Remove global denylist
+ovn_as az2 check ovn-nbctl remove nb_global . options ic-route-denylist
+
+# Confirm all routes are learned again
+OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep -E
'192.168|2001' | grep learned | awk '{print $1, $2}' | sort], [0], [dnl
+192.168.100.0/24 169.254.101.2
+192.168.200.0/24 169.254.101.2
+2001:db12::/64 fe80:10::2
+2001:db22::/64 fe80:10::2
+])
+
+# --- Test combination of deny options ---
+
+# Set all deny filters
+ovn_as az2 check ovn-nbctl set logical_router_port lrp-lr12-ts1
options:ic-route-deny-adv="192.168.100.0/24"
+ovn_as az1 check ovn-nbctl set logical_router_port lrp-lr11-ts1
options:ic-route-deny-learn="192.168.200.0/24"
+ovn_as az2 check ovn-nbctl set nb_global .
options:ic-route-denylist="2001:db12::/64"
+
+# Validate only 2001:db22::/64 is learned
+OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep -E
'192.168|2001' | grep learned | awk '{print $1, $2}' | sort], [0], [dnl
+2001:db22::/64 fe80:10::2
+])
> +
+
OVN_CLEANUP_IC([az1], [az2])
AT_CLEANUP
])
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
