Hi Rosemarie and Dumitru, It looks good to me.
Regards, Lucas Em sex., 21 de ago. de 2026 às 10:36, Rosemarie O'Riorden < [email protected]> escreveu: > 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] > > -- _‘Esta mensagem é direcionada apenas para os endereços constantes no cabeçalho inicial. Se você não está listado nos endereços constantes no cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão imediatamente anuladas e proibidas’._ * **‘Apesar do Magazine Luiza tomar todas as precauções razoáveis para assegurar que nenhum vírus esteja presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por quaisquer perdas ou danos causados por esse e-mail ou por seus anexos’.* _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
