Hi Lucas, Sorry I didn't respond to you explaining your use-case for having both the block and allowlist at the same time. I guess I've already shared my thoughts on that so it would be nice if someone else could chime in and share what they think. I'm fine with it if others are okay with it. And that's fair that there's already similar behavior in ovn-ic. However as a general practice I would say we shouldn't mirror less-than-ideal behavior just because it's already in the codebase :) But as I said I would appreciate others' opinions!
I hope you can see Lucas' response to my review. It's not showing up for me on patchwork but it's this thread: [ovs-dev] [PATCH ovn v1] ic: Add allowlist filter for learned IC route tags. @Ilya @Dumitru, or anyone else, any thoughts? Also one nit below. On 8/11/26 4:36 PM, Lucas Vargas Dias wrote: > The "ic-route-filter-tag" option on a Logical_Router_Port acts as a > blocklist: routes whose "ic-route-tag" matches the configured tag are > not learned. There was no way to express the opposite - "learn only > routes carrying one of these tags" - which is useful when a router port > should import routes from a known set of VPCs and drop everything else. > > Add an "ic-route-allow-tag" option that takes a comma-separated list > of route tags. When set, only IC routes whose "ic-route-tag" matches > one of the listed tags are learned; every other route, including > untagged ones, is filtered out. The existing "ic-route-filter-tag" > blocklist still takes precedence, so a route whose tag is filtered is > skipped even if the same tag is allowlisted. > > Assisted-by: Claude Opus 4.8, Claude Code > Signed-off-by: Lucas Vargas Dias <[email protected]> > --- > NEWS | 4 ++++ > ic/ovn-ic.c | 26 +++++++++++++++++++++++++- > ovn-nb.xml | 24 ++++++++++++++++++++++++ > tests/ovn-ic.at | 35 +++++++++++++++++++++++++++++++++++ > 4 files changed, 88 insertions(+), 1 deletion(-) > > diff --git a/NEWS b/NEWS > index 384e30820..e0c1f1779 100644 > --- a/NEWS > +++ b/NEWS > @@ -68,6 +68,10 @@ Post v26.03.0 > (type 11) and Parameter Problem (type 12) - generated by an external > router are un-NATed correctly. This makes Path MTU discovery and > traceroute work through stateless NAT. > + - Added "ic-route-allow-tag" option to the Logical_Router_Port > + table, it accepts a comma-separated list of route tags and > + only IC routes whose "ic-route-tag" matches one of them are learned. > + "ic-route-filter-tag" takes precedence over it. > > OVN v26.03.0 - xxx xx xxxx > -------------------------- > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > index af6879dda..26f5791ec 100644 > --- a/ic/ovn-ic.c > +++ b/ic/ovn-ic.c > @@ -2352,7 +2352,8 @@ sync_learned_routes(struct ic_context *ctx, > nbrec_nb_global_first(ctx->ovnnb_idl); > ovs_assert(nb_global); > > - const char *lrp_name, *ts_route_table, *route_tag_filter; > + const char *lrp_name, *ts_route_table, *route_tag_filter, > + *route_tag_allow; > const struct icsbrec_port_binding *isb_pb; > const struct nbrec_logical_router_port *lrp; > VECTOR_FOR_EACH (&ic_lr->isb_pbs, isb_pb) { > @@ -2365,15 +2366,25 @@ sync_learned_routes(struct ic_context *ctx, > ts_route_table = smap_get_def(&lrp->options, "route_table", ""); > route_tag_filter = smap_get_def(&lrp->options, > "ic-route-filter-tag", ""); > + route_tag_allow = smap_get_def(&lrp->options, > + "ic-route-allow-tag", ""); > } else { > ts_route_table = ""; > route_tag_filter = ""; > + route_tag_allow = ""; > } > > /* The filter tag option accepts a comma-separated list of tags. */ > struct sset filter_tags = SSET_INITIALIZER(&filter_tags); > sset_from_delimited_string(&filter_tags, route_tag_filter, ","); > > + > + /* Allowlist of route tags to learn. When non-empty, only routes > + * whose "ic-route-tag" is in this set are learned; every other > + * route (including untagged ones) is filtered out. */ > + struct sset allow_tag_set = SSET_INITIALIZER(&allow_tag_set); > + sset_from_delimited_string(&allow_tag_set, route_tag_allow, ","); > + > isb_route_key = > icsbrec_route_index_init_row(ctx->icsbrec_route_by_ts); > icsbrec_route_index_set_transit_switch(isb_route_key, > isb_pb->transit_switch); > @@ -2405,6 +2416,18 @@ sync_learned_routes(struct ic_context *ctx, > continue; > } > > + if (!sset_is_empty(&allow_tag_set)) { > + if (!isb_route_tag || > + !sset_contains(&allow_tag_set, isb_route_tag)) { > + VLOG_DBG("Skip learning route %s -> %s as its route tag " > + "[%s] is not in the allow tag list [%s] of TS" > + "LRP ", isb_route->ip_prefix, > isb_route->nexthop, > + isb_route_tag ? isb_route_tag : "(none)", > + route_tag_allow); Nit: indentation > + continue; > + } > + } > + > if (isb_route->route_table[0] && > strcmp(isb_route->route_table, ts_route_table)) { > if (VLOG_IS_DBG_ENABLED()) { > @@ -2470,6 +2493,7 @@ sync_learned_routes(struct ic_context *ctx, > } > icsbrec_route_index_destroy_row(isb_route_key); > sset_destroy(&filter_tags); > + sset_destroy(&allow_tag_set); > } > > /* Delete extra learned routes. */ > diff --git a/ovn-nb.xml b/ovn-nb.xml > index a13c2083c..ad9fd405f 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -4584,6 +4584,30 @@ or > </p> > </column> > > + <column name="options" key="ic-route-allow-tag" > + type='{"type": "string"}'> > + <p> > + This option expects a comma-separated list of allowed route-tags, > + for example <code>vpc1,vpc2</code>. When set on the Logical Router > + Port, it acts as an allowlist for the route learning process: only > + routes whose <code>ic-route-tag</code> (present in the > + <code>external_ids</code> register of the advertised route entry in > + the <ref table="Route" db="OVN_IC_Southbound"/> table of the > + <ref db="OVN_IC_Southbound"/> database) matches one of the listed > + tags are learned by the <code>ovn-ic</code> daemon. Every other > + route, including routes that carry no <code>ic-route-tag</code> at > + all, is filtered and not learned. > + </p> > + > + <p> > + When both this option and <ref column="options" > + key="ic-route-filter-tag"/> are set, the blocklist behaviour of > + <ref column="options" key="ic-route-filter-tag"/> takes precedence: > + a route whose tag matches the filter tag is skipped even if the > + same tag is present in this allowlist. > + </p> > + </column> > + > <column name="options" key="requested-chassis"> > <p> > If set, identifies a specific chassis (by name or hostname) that > diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at > index f3bdc816f..a8827e198 100644 > --- a/tests/ovn-ic.at > +++ b/tests/ovn-ic.at > @@ -3322,6 +3322,41 @@ OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl > lr-route-list lr11 | grep 192.168 | > 192.168.1.0/24 169.254.103.22 > ]) > > +ovn_as az1 ovn-nbctl remove logical_router_port lrp-lr11-tspeer options > ic-route-filter-tag > +# Allowlist: only learn tspeer routes tagged vpc1 on lrp-lr11-tspeer. > +# The tspeer route from lr22 (169.254.103.22) carries no tag and is thus > +# filtered by the strict allowlist. Routes learned through ts11/ts12 are > +# not affected as the option is set on the tspeer LRP only. > +ovn_as az1 ovn-nbctl set logical_router_port lrp-lr11-tspeer > options:ic-route-allow-tag=vpc1 > +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.0.0/24 169.254.101.2 > +192.168.0.0/24 169.254.102.2 > +192.168.0.0/24 169.254.103.12 > +]) > + > +# Tag lr22's advertised route with vpc2 and allow a list of tags [vpc1,vpc2]. > +# The previously untagged route is now tagged vpc2 and learned again. > +ovn_as az2 ovn-nbctl set logical_router_port lrp-lr22-tspeer > options:ic-route-tag=vpc2 > +ovn_as az1 ovn-nbctl set logical_router_port lrp-lr11-tspeer > options:ic-route-allow-tag=vpc1,vpc2 > +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.0.0/24 169.254.101.2 > +192.168.0.0/24 169.254.102.2 > +192.168.0.0/24 169.254.103.12 > +192.168.1.0/24 169.254.103.22 > +]) > + > +# Blocklist takes precedence over the allowlist: filtering vpc1 drops the > +# vpc1 tspeer route (169.254.103.12) even though vpc1 is in the allow list. > +ovn_as az1 ovn-nbctl set logical_router_port lrp-lr11-tspeer > options:ic-route-filter-tag=vpc1 > +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.0.0/24 169.254.101.2 > +192.168.0.0/24 169.254.102.2 > +192.168.1.0/24 169.254.103.22 > +]) > + > OVN_CLEANUP_IC([az1], [az2]) > > AT_CLEANUP -- Rosemarie O'Riorden Lowell, MA, United States [email protected] _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
