Hi Lucas and Dumitru, With those minor edits you made Dumitru, it looks good to me to merge.
Acked-by: Rosemarie O'Riorden <[email protected]> On 8/21/26 9:30 AM, Dumitru Ceara wrote: > On 8/14/26 8:39 PM, Lucas Vargas Dias wrote: >> The "ic-route-filter-tag" option of a Logical_Router_Port connected to a >> transit switch blocks the routes carrying a single route tag. There is >> no way to express a list of tags, nor the opposite rule - "learn only >> the routes tagged with one of these tags" - which is useful when a >> router port has to import the routes of a known set of VPCs and drop >> everything else. >> >> Add an "ic-route-learn-tag-rules" option that takes a comma-separated >> list of route tags prefixed by the rule to apply to them: >> >> - "allow:<tags>": only the routes whose "ic-route-tag" is one of >> <tags> are learned. Every other route, the untagged ones included, >> is not learned. >> >> - "block:<tags>": the routes whose "ic-route-tag" is one of <tags> are >> not learned. Every other route, the untagged ones included, is >> learned. >> >> As commas separate the tags, a route tag cannot contain a comma. The >> two forms are mutually exclusive, so only the leading prefix is honored >> and everything after it is a tag name. Values that use neither prefix, >> as well as values with an empty tag list, are logged and ignored. >> >> This new option supersedes "ic-route-filter-tag", which is now >> deprecated and scheduled for removal in 28.09. The deprecated option is >> still honored on its own, but it is ignored whenever a valid >> "ic-route-learn-tag-rules" value is configured on the same port. >> >> Assisted-by: Claude Opus 5, Claude Code >> Signed-off-by: Lucas Vargas Dias <[email protected]> >> --- > > Hi Lucas, Rosemarie, > > Thanks for the new revision and for the previous discussions > and reviews! > >> NEWS | 7 ++ >> TODO.rst | 2 + >> ic/ovn-ic.c | 122 +++++++++++++++++++++++++++++++- >> ovn-nb.xml | 61 ++++++++++++++++ >> tests/ovn-ic.at | 180 ++++++++++++++++++++++++++++++++++++++++++++++++ >> 5 files changed, 370 insertions(+), 2 deletions(-) >> >> diff --git a/NEWS b/NEWS >> index 44f117807..16b016a69 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -94,6 +94,13 @@ Post v26.03.0 >> - Added a new "ovn-debug lflow-pipeline-oftable-start-list" command that >> prints the starting OpenFlow table number of the logical ingress and >> egress pipelines. >> + - Added the "ic-route-learn-tag-rules" option to Logical_Router_Port to >> + filter the routes learned through the port by route tag. It accepts >> + either "allow:<tags>", to learn only the routes tagged with one of the >> + comma-separated <tags>, or "block:<tags>", to learn every route but the >> + ones tagged with one of them. It supports more than one route tag and >> + supersedes "ic-route-filter-tag", which is now deprecated and is >> ignored >> + when "ic-route-learn-tag-rules" is set. > > This is a bit verbose. I would change it to: > > - Added the "ic-route-learn-tag-rules" option to Logical_Router_Port to > 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. > > The rest is documented in the nb man page. > >> >> OVN v26.03.0 - xxx xx xxxx >> -------------------------- >> diff --git a/TODO.rst b/TODO.rst >> index beca38daf..9341fe30c 100644 >> --- a/TODO.rst >> +++ b/TODO.rst >> @@ -214,6 +214,8 @@ when the feature/action will move from ``Deprecated`` to >> ``Removed``. >> * ``OVN_FEATURE_MAC_BINDING_TIMESTAMP`` feature, should be removed in >> 28.09. >> * ``OVN_FEATURE_FDB_TIMESTAMP`` feature, should be removed in 28.09. >> * ``OVN_FEATURE_LS_DPG_COLUMN`` feature, should be removed in 28.09. >> + * ``ic-route-filter-tag`` Logical_Router_Port option, superseded by >> + ``ic-route-learn-tag-rules``, should be removed in 28.09. >> >> * 26.03 Deprecated >> >> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c >> index f7cc41748..ea7625484 100644 >> --- a/ic/ovn-ic.c >> +++ b/ic/ovn-ic.c >> @@ -2526,6 +2526,91 @@ lrp_is_ts_port(struct ic_context *ctx, struct >> ic_router_info *ic_lr, >> return false; >> } >> >> +#define IC_ROUTE_LEARN_TAG_RULES "ic-route-learn-tag-rules" >> +#define IC_ROUTE_LEARN_TAG_ALLOW "allow:" >> +#define IC_ROUTE_LEARN_TAG_BLOCK "block:" >> +#define IC_ROUTE_FILTER_TAG "ic-route-filter-tag" >> + >> +/* Route tag rules of a TS LRP, as configured through its >> + * "ic-route-learn-tag-rules" option. */ >> +struct route_learn_tag_rules { >> + const char *config; /* Option value, for logging purposes. */ >> + bool configured; /* False if the option is unset or invalid, in >> + * which case no route is filtered. */ >> + bool allow; /* True: only routes tagged with one of 'tags' >> are >> + * learned. False: routes tagged with one of >> + * 'tags' are not learned. */ >> + struct sset tags; >> +}; >> + >> +/* Initializes 'rules' from the "ic-route-learn-tag-rules" option of 'lrp', >> + * which may be NULL. The option value must be either >> + * "allow:<comma-separated-tags>" or "block:<comma-separated-tags>"; any >> other >> + * value is logged and ignored. Route tags cannot contain commas. */ >> +static void >> +route_learn_tag_rules_init(struct route_learn_tag_rules *rules, >> + const struct nbrec_logical_router_port *lrp) >> +{ >> + static struct vlog_rate_limit bad_value_rl = VLOG_RATE_LIMIT_INIT(5, 1); >> + static struct vlog_rate_limit no_tag_rl = VLOG_RATE_LIMIT_INIT(5, 1); >> + const char *tags = ""; >> + >> + rules->config = lrp ? smap_get(&lrp->options, IC_ROUTE_LEARN_TAG_RULES) >> + : NULL; >> + rules->configured = false; >> + rules->allow = false; >> + >> + if (rules->config) { >> + if (!strncmp(rules->config, IC_ROUTE_LEARN_TAG_ALLOW, >> + strlen(IC_ROUTE_LEARN_TAG_ALLOW))) { >> + tags = rules->config + strlen(IC_ROUTE_LEARN_TAG_ALLOW); >> + rules->allow = true; >> + rules->configured = true; >> + } else if (!strncmp(rules->config, IC_ROUTE_LEARN_TAG_BLOCK, >> + strlen(IC_ROUTE_LEARN_TAG_BLOCK))) { >> + tags = rules->config + strlen(IC_ROUTE_LEARN_TAG_BLOCK); >> + rules->configured = true; >> + } else { >> + VLOG_WARN_RL(&bad_value_rl, >> + "Ignoring invalid %s value [%s] of logical " >> + "router port %s: expected \"%s<tags>\" or " >> + "\"%s<tags>\".", IC_ROUTE_LEARN_TAG_RULES, >> + rules->config, lrp->name, >> + IC_ROUTE_LEARN_TAG_ALLOW, >> IC_ROUTE_LEARN_TAG_BLOCK); >> + } >> + } >> + >> + sset_from_delimited_string(&rules->tags, tags, ","); >> + >> + if (rules->configured && sset_is_empty(&rules->tags)) { >> + VLOG_WARN_RL(&no_tag_rl, >> + "Ignoring %s value [%s] of logical router port %s: " >> + "no route tag specified.", IC_ROUTE_LEARN_TAG_RULES, >> + rules->config, lrp->name); >> + rules->configured = false; >> + } >> +} >> + >> +static void >> +route_learn_tag_rules_destroy(struct route_learn_tag_rules *rules) >> +{ >> + sset_destroy(&rules->tags); >> +} >> + >> +/* Returns true if a route tagged with 'route_tag' ('NULL' if the route > > Nit: s/'NULL'/NULL/ > >> + * carries no tag) can be learned according to 'rules'. */ >> +static bool >> +route_learn_tag_rules_allow(const struct route_learn_tag_rules *rules, >> + const char *route_tag) >> +{ >> + if (!rules->configured) { >> + return true; >> + } >> + >> + bool listed = route_tag && sset_contains(&rules->tags, route_tag); >> + return rules->allow ? listed : !listed; >> +} >> + >> static void >> sync_learned_routes(struct ic_context *ctx, >> struct ic_router_info *ic_lr) >> @@ -2549,12 +2634,31 @@ sync_learned_routes(struct ic_context *ctx, >> if (lrp) { >> ts_route_table = smap_get_def(&lrp->options, "route_table", ""); >> route_filter_tag = smap_get_def(&lrp->options, >> - "ic-route-filter-tag", ""); >> + IC_ROUTE_FILTER_TAG, ""); >> } else { >> ts_route_table = ""; >> route_filter_tag = ""; >> } >> >> + struct route_learn_tag_rules learn_tag_rules; >> + route_learn_tag_rules_init(&learn_tag_rules, lrp); >> + >> + if (route_filter_tag[0]) { >> + if (learn_tag_rules.configured) { >> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, >> 1); > > Nit: Max 1 log per second is probably fine. Also, I'd use a single rl for > both branches. Which also means we'd need a small change in the test. > >> + VLOG_WARN_RL(&rl, "The deprecated %s option of logical >> router " >> + "port %s is ignored as %s is also configured.", >> + IC_ROUTE_FILTER_TAG, lrp->name, >> + IC_ROUTE_LEARN_TAG_RULES); >> + } else { >> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, >> 1); >> + VLOG_WARN_RL(&rl, "The %s option of logical router port %s >> is " >> + "deprecated and will be removed in the 28.09 " >> + "release; use %s instead.", >> IC_ROUTE_FILTER_TAG, >> + lrp->name, IC_ROUTE_LEARN_TAG_RULES); >> + } >> + } >> + >> 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); >> @@ -2578,7 +2682,10 @@ sync_learned_routes(struct ic_context *ctx, >> >> const char *isb_route_tag = smap_get(&isb_route->external_ids, >> "ic-route-tag"); >> - if (isb_route_tag && !strcmp(isb_route_tag, route_filter_tag)) >> { >> + /* The deprecated filter tag is honored only when the route >> learn >> + * tag rules are not in effect. */ >> + if (!learn_tag_rules.configured && isb_route_tag && >> + !strcmp(isb_route_tag, route_filter_tag)) { >> VLOG_DBG("Skip learning route %s -> %s as its route tag " >> "[%s] is filtered by the filter tag [%s] of TS LRP >> ", >> isb_route->ip_prefix, isb_route->nexthop, >> @@ -2586,6 +2693,16 @@ sync_learned_routes(struct ic_context *ctx, >> continue; >> } >> >> + if (!route_learn_tag_rules_allow(&learn_tag_rules, >> + isb_route_tag)) { >> + VLOG_DBG("Skip learning route %s -> %s as its route tag " >> + "[%s] is filtered by the %s [%s] of TS LRP ", > > Trailing whitespace. Above too. > >> + isb_route->ip_prefix, isb_route->nexthop, >> + isb_route_tag ? isb_route_tag : "", >> + IC_ROUTE_LEARN_TAG_RULES, learn_tag_rules.config); >> + continue; >> + } >> + >> if (isb_route->route_table[0] && >> strcmp(isb_route->route_table, ts_route_table)) { >> if (VLOG_IS_DBG_ENABLED()) { >> @@ -2650,6 +2767,7 @@ sync_learned_routes(struct ic_context *ctx, >> } >> } >> icsbrec_route_index_destroy_row(isb_route_key); >> + route_learn_tag_rules_destroy(&learn_tag_rules); >> } >> >> /* Delete extra learned routes. */ >> diff --git a/ovn-nb.xml b/ovn-nb.xml >> index 8a9d19fa9..a71088e51 100644 >> --- a/ovn-nb.xml >> +++ b/ovn-nb.xml >> @@ -4697,6 +4697,67 @@ or >> <ref db="OVN_IC_Southbound"/> database, will be filtered and not >> learned by the <code>ovn-ic</code> daemon. >> </p> >> + >> + <p> >> + This option is deprecated and will be removed in the 28.09 >> + release. Use >> + <ref column="options" key="ic-route-learn-tag-rules"/> instead, >> + which supports more than one route tag and which, when set, causes >> + this option to be ignored. >> + </p> >> + </column> >> + >> + <column name="options" key="ic-route-learn-tag-rules" >> + type='{"type": "string"}'> >> + <p> >> + This option controls, based on route tags, which routes the >> + <code>ovn-ic</code> daemon learns through this Logical Router >> Port. >> + It expects a value in one of the following two forms, where >> + <code>tags</code> is a comma-separated list of route-tag names, >> for >> + example <code>allow:vpc1,vpc2</code>: >> + </p> >> + >> + <ul> >> + <li> >> + <code>allow:tags</code> - only routes whose >> + <code>ic-route-tag</code> matches one of the listed tags are >> + learned. Every other route, including routes that carry no >> + <code>ic-route-tag</code> at all, is not learned. >> + </li> >> + >> + <li> >> + <code>block:tags</code> - routes whose <code>ic-route-tag</code> >> + matches one of the listed tags are not learned. Every other >> + route, including routes that carry no <code>ic-route-tag</code> >> at >> + all, is learned. >> + </li> >> + </ul> >> + >> + <p> >> + The <code>ic-route-tag</code> of a route is the one 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. As commas separate the >> + tags, a route tag cannot contain a comma. Values that use neither >> + the <code>allow:</code> nor the <code>block:</code> prefix, as >> well >> + as values with an empty tag list, are invalid and are ignored (no >> + route is filtered by this option). >> + </p> >> + >> + <p> >> + The two forms are mutually exclusive: only the leading prefix is >> + honored and everything after it is a route tag name. In >> + <code>allow:vpc1,block:vpc2</code>, for example, the allowed tags >> + are <code>vpc1</code> and <code>block:vpc2</code>. >> + </p> >> + >> + <p> >> + This option supersedes the deprecated >> + <ref column="options" key="ic-route-filter-tag"/> option: >> whenever a >> + valid value is set here, the route tags to learn are the ones >> + defined by this option only and >> + <ref column="options" key="ic-route-filter-tag"/> is ignored. >> + </p> >> </column> >> >> <column name="options" key="requested-chassis"> >> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at >> index 1435a19a3..b23cbe185 100644 >> --- a/tests/ovn-ic.at >> +++ b/tests/ovn-ic.at >> @@ -3716,6 +3716,186 @@ OVN_CLEANUP_IC([az1], [az2]) >> AT_CLEANUP >> ]) >> >> +OVN_FOR_EACH_NORTHD([ >> +AT_SETUP([ovn-ic -- route tag -- learn tag rules]) >> + >> +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 >> + >> +# Test topology is next: >> +# >> +# / logical router (lr2) - 192.168.2.0/24 >> (vpc2) >> +# logical router (lr1) - ts1 - - logical router (lr3) - 192.168.3.0/24 >> (vpc3) >> +# \ logical router (lr4) - 192.168.4.0/24 (no >> tag) >> +# >> +# The learn tag rules are configured on lrp-lr1-ts1, the LRP through which >> lr1 >> +# learns the routes advertised by lr2, lr3 and lr4. >> + >> +ovn_as az1 >> +check ovn-nbctl lr-add lr1 >> +check ovn-nbctl lrp-add lr1 lrp-lr1-ts1 aa:aa:aa:aa:01:01 169.254.100.1/24 >> +check ovn-nbctl lsp-add-router-port ts1 lsp-ts1-lr1 lrp-lr1-ts1 >> + >> +ovn_as az2 >> +for i in 2 3 4; do >> + check ovn-nbctl lr-add lr$i >> + check ovn-nbctl lrp-add lr$i lrp-lr$i-ts1 aa:aa:aa:aa:0$i:01 \ >> + 169.254.100.$i/24 >> + check ovn-nbctl lsp-add-router-port ts1 lsp-ts1-lr$i lrp-lr$i-ts1 >> + # Create the directly-connected route advertised by lr$i >> + check ovn-nbctl lrp-add lr$i lrp-lr$i aa:aa:aa:aa:0$i:02 192.168.$i.1/24 >> +done >> + >> +# Tag the routes advertised by lr2 and lr3. lr4 routes stay untagged. >> +check ovn-nbctl set logical_router_port lrp-lr2-ts1 >> options:ic-route-tag="vpc2" >> +check ovn-nbctl --wait=sb set logical_router_port lrp-lr3-ts1 \ >> + options:ic-route-tag="vpc3" >> + >> +check ovn-ic-nbctl --wait=sb sync >> + >> +wait_row_count ic-sb:Route 1 ip_prefix=192.168.2.1/24 >> external_ids:ic-route-tag=vpc2 >> +wait_row_count ic-sb:Route 1 ip_prefix=192.168.3.1/24 >> external_ids:ic-route-tag=vpc3 >> + >> +# Without the learn tag rules option every route is learned. >> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep 192.168 | >> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl >> +192.168.2.0/24 169.254.100.2 >> +192.168.3.0/24 169.254.100.3 >> +192.168.4.0/24 169.254.100.4 >> +]) >> + >> +# Allow the vpc2 route tag only. The route tagged with vpc3 and the >> untagged >> +# one are not learned. >> +ovn_as az1 ovn-nbctl set logical_router_port lrp-lr1-ts1 \ >> + options:ic-route-learn-tag-rules="allow:vpc2" >> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep 192.168 | >> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl >> +192.168.2.0/24 169.254.100.2 >> +]) >> + >> +# Allow both route tags. Only the untagged route is not learned. >> +ovn_as az1 ovn-nbctl set logical_router_port lrp-lr1-ts1 \ >> + options:ic-route-learn-tag-rules="allow:vpc2,vpc3" >> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep 192.168 | >> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl >> +192.168.2.0/24 169.254.100.2 >> +192.168.3.0/24 169.254.100.3 >> +]) >> + >> +# Block the vpc2 route tag. Every other route, the untagged one included, >> is >> +# learned. >> +ovn_as az1 ovn-nbctl set logical_router_port lrp-lr1-ts1 \ >> + options:ic-route-learn-tag-rules="block:vpc2" >> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep 192.168 | >> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl >> +192.168.3.0/24 169.254.100.3 >> +192.168.4.0/24 169.254.100.4 >> +]) >> + >> +# Block both route tags. Only the untagged route is learned. >> +ovn_as az1 ovn-nbctl set logical_router_port lrp-lr1-ts1 \ >> + options:ic-route-learn-tag-rules="block:vpc2,vpc3" >> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep 192.168 | >> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl >> +192.168.4.0/24 169.254.100.4 >> +]) >> + >> +# The allow: and block: forms are mutually exclusive: only the leading >> prefix >> +# is honored and everything after it is a route tag name. So allowing vpc2 >> and >> +# blocking vpc3 in the same value allows vpc2 and the "block:vpc3" tag, and >> the >> +# route tagged with vpc3 is not learned because it is not allowed. >> +ovn_as az1 ovn-nbctl set logical_router_port lrp-lr1-ts1 \ >> + options:ic-route-learn-tag-rules="allow:vpc2,block:vpc3" >> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep 192.168 | >> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl >> +192.168.2.0/24 169.254.100.2 >> +]) >> + >> +# The same the other way around: vpc2 and the "allow:vpc3" tag are blocked, >> so >> +# the route tagged with vpc3 is learned. >> +ovn_as az1 ovn-nbctl set logical_router_port lrp-lr1-ts1 \ >> + options:ic-route-learn-tag-rules="block:vpc2,allow:vpc3" >> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep 192.168 | >> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl >> +192.168.3.0/24 169.254.100.3 >> +192.168.4.0/24 169.254.100.4 >> +]) >> + >> +# A value that uses neither the allow: nor the block: prefix is invalid and >> is >> +# ignored, so every route is learned again. >> +ovn_as az1 ovn-nbctl set logical_router_port lrp-lr1-ts1 \ >> + options:ic-route-learn-tag-rules="vpc2" >> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep 192.168 | >> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl >> +192.168.2.0/24 169.254.100.2 >> +192.168.3.0/24 169.254.100.3 >> +192.168.4.0/24 169.254.100.4 >> +]) >> +OVS_WAIT_UNTIL([grep -q "Ignoring invalid ic-route-learn-tag-rules value" \ >> + az1/ic/ovn-ic.log]) >> + >> +# A value with an empty route tag list is invalid and is ignored as well. >> +ovn_as az1 ovn-nbctl set logical_router_port lrp-lr1-ts1 \ >> + options:ic-route-learn-tag-rules="allow:" >> +OVS_WAIT_UNTIL([grep -q "no route tag specified" az1/ic/ovn-ic.log]) >> +AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep 192.168 | > > There's still a race here, there's no guarantee the AZ1 northd had a > chance to run. We should use OVS_WAIT_FOR_OUTPUT. > >> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl >> +192.168.2.0/24 169.254.100.2 >> +192.168.3.0/24 169.254.100.3 >> +192.168.4.0/24 169.254.100.4 >> +]) >> + >> +# The deprecated ic-route-filter-tag option is still honored on its own: the >> +# route tagged with vpc3 is not learned. >> +ovn_as az1 ovn-nbctl remove logical_router_port lrp-lr1-ts1 options \ >> + ic-route-learn-tag-rules >> +ovn_as az1 ovn-nbctl set logical_router_port lrp-lr1-ts1 \ >> + options:ic-route-filter-tag="vpc3" >> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep 192.168 | >> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl >> +192.168.2.0/24 169.254.100.2 >> +192.168.4.0/24 169.254.100.4 >> +]) >> +OVS_WAIT_UNTIL([grep -q "The ic-route-filter-tag option of logical router >> port lrp-lr1-ts1 is deprecated" az1/ic/ovn-ic.log]) >> + >> +# The learn tag rules take precedence over the deprecated option: the route >> +# tagged with vpc3 is learned again as ic-route-filter-tag is now ignored. >> +ovn_as az1 ovn-nbctl set logical_router_port lrp-lr1-ts1 \ >> + options:ic-route-learn-tag-rules="allow:vpc2,vpc3" >> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep 192.168 | >> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl >> +192.168.2.0/24 169.254.100.2 >> +192.168.3.0/24 169.254.100.3 >> +]) >> +OVS_WAIT_UNTIL([grep -q "The deprecated ic-route-filter-tag option of >> logical router port lrp-lr1-ts1 is ignored" az1/ic/ovn-ic.log]) >> + >> +# Remove both filtering options. Every route is learned again. >> +ovn_as az1 ovn-nbctl remove logical_router_port lrp-lr1-ts1 options \ >> + ic-route-learn-tag-rules >> +ovn_as az1 ovn-nbctl remove logical_router_port lrp-lr1-ts1 options \ >> + ic-route-filter-tag >> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep 192.168 | >> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl >> +192.168.2.0/24 169.254.100.2 >> +192.168.3.0/24 169.254.100.3 >> +192.168.4.0/24 169.254.100.4 >> +]) >> + >> +OVN_CLEANUP_IC([az1], [az2]) >> + >> +AT_CLEANUP >> +]) >> + >> OVN_FOR_EACH_NORTHD([ >> AT_SETUP([spine-leaf: 3 AZs, 3 HVs, 3 LSs, connected via transit spine >> switch]) >> AT_KEYWORDS([spine leaf]) > > I can take care of the minor comments above before applying the patch to the > main branch. Here's an incremental patch addressing those, if this looks OK > to you guys, I can squash it in and merge everything to main. > > diff --git a/NEWS b/NEWS > index 78be5f861a..19bebc5c7a 100644 > --- a/NEWS > +++ b/NEWS > @@ -97,12 +97,9 @@ Post v26.03.0 > - The support for co-hosting multiple controller instances is now > considered stable. Its "experimental" tag has been removed. > - Added the "ic-route-learn-tag-rules" option to Logical_Router_Port to > - filter the routes learned through the port by route tag. It accepts > - either "allow:<tags>", to learn only the routes tagged with one of the > - comma-separated <tags>, or "block:<tags>", to learn every route but the > - ones tagged with one of them. It supports more than one route tag and > - supersedes "ic-route-filter-tag", which is now deprecated and is ignored > - when "ic-route-learn-tag-rules" is set. > + 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. > > OVN v26.03.0 - xxx xx xxxx > -------------------------- > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > index ea76254846..79d4685dfa 100644 > --- a/ic/ovn-ic.c > +++ b/ic/ovn-ic.c > @@ -2597,7 +2597,7 @@ route_learn_tag_rules_destroy(struct > route_learn_tag_rules *rules) > sset_destroy(&rules->tags); > } > > -/* Returns true if a route tagged with 'route_tag' ('NULL' if the route > +/* Returns true if a route tagged with 'route_tag' (NULL if the route > * carries no tag) can be learned according to 'rules'. */ > static bool > route_learn_tag_rules_allow(const struct route_learn_tag_rules *rules, > @@ -2644,14 +2644,13 @@ sync_learned_routes(struct ic_context *ctx, > route_learn_tag_rules_init(&learn_tag_rules, lrp); > > if (route_filter_tag[0]) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > if (learn_tag_rules.configured) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, > 1); > VLOG_WARN_RL(&rl, "The deprecated %s option of logical > router " > "port %s is ignored as %s is also configured.", > IC_ROUTE_FILTER_TAG, lrp->name, > IC_ROUTE_LEARN_TAG_RULES); > } else { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, > 1); > VLOG_WARN_RL(&rl, "The %s option of logical router port %s > is " > "deprecated and will be removed in the 28.09 " > "release; use %s instead.", IC_ROUTE_FILTER_TAG, > @@ -2687,7 +2686,7 @@ sync_learned_routes(struct ic_context *ctx, > if (!learn_tag_rules.configured && isb_route_tag && > !strcmp(isb_route_tag, route_filter_tag)) { > VLOG_DBG("Skip learning route %s -> %s as its route tag " > - "[%s] is filtered by the filter tag [%s] of TS LRP > ", > + "[%s] is filtered by the filter tag [%s] of TS LRP", > isb_route->ip_prefix, isb_route->nexthop, > isb_route_tag, route_filter_tag); > continue; > @@ -2696,7 +2695,7 @@ sync_learned_routes(struct ic_context *ctx, > if (!route_learn_tag_rules_allow(&learn_tag_rules, > isb_route_tag)) { > VLOG_DBG("Skip learning route %s -> %s as its route tag " > - "[%s] is filtered by the %s [%s] of TS LRP ", > + "[%s] is filtered by the %s [%s] of TS LRP", > isb_route->ip_prefix, isb_route->nexthop, > isb_route_tag ? isb_route_tag : "", > IC_ROUTE_LEARN_TAG_RULES, learn_tag_rules.config); > diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at > index b23cbe185a..a5588c74d9 100644 > --- a/tests/ovn-ic.at > +++ b/tests/ovn-ic.at > @@ -3732,6 +3732,8 @@ for i in 1 2; do > check ovn-nbctl set nb_global . options:ic-route-adv=true > done > > +as az1 check ovn-appctl -t ic/ovn-ic vlog/disable-rate-limit > + > # Test topology is next: > # > # / logical router (lr2) - 192.168.2.0/24 (vpc2) > @@ -3848,7 +3850,7 @@ OVS_WAIT_UNTIL([grep -q "Ignoring invalid > ic-route-learn-tag-rules value" \ > ovn_as az1 ovn-nbctl set logical_router_port lrp-lr1-ts1 \ > options:ic-route-learn-tag-rules="allow:" > OVS_WAIT_UNTIL([grep -q "no route tag specified" az1/ic/ovn-ic.log]) > -AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep 192.168 | > +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep 192.168 | > grep learned | awk '{print $1, $2}' | sort ], [0], [dnl > 192.168.2.0/24 169.254.100.2 > 192.168.3.0/24 169.254.100.3 > > > Regards, > Dumitru > -- Rosemarie O'Riorden Lowell, MA, United States [email protected] _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
