Do you think ovn-ic could automatically replace ic-router-filter-tag by ic-route-learn-tag-rules=block?
Regards, Lucas Em sex., 14 de ago. de 2026 às 09:31, Lucas Vargas Dias <[email protected]> escreveu: > > > Em sex., 14 de ago. de 2026 às 09:24, Dumitru Ceara <[email protected]> > escreveu: > >> On 8/14/26 1:57 PM, Lucas Vargas Dias wrote: >> > Hi Dumitru and Rosemaria, >> > Thanks for your review. >> > >> > Em sex., 14 de ago. de 2026 às 07:22, Dumitru Ceara <[email protected]> >> > escreveu: >> > >> >> On 8/14/26 12:09 AM, Rosemarie O'Riorden wrote: >> >>> Hi Lucas, >> >>> >> >> >> >> Hi Lucas, Rosemarie, >> >> >> >> Thanks for the new version and for the reviews! >> >> >> >>> 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. >> >>> >> >> >> >> It should be this one: >> >> >> >> >> https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/#3734125 >> >> >> >> Copied inline here: >> >> >> >>>> 1. The naming isn't ideal IMHO. "Filter" implies a block-list >> already, >> >>>> because with how this option already works, we are filtering tags out >> >>>> (rather than filtering FOR certain tags), so adding a "filter-allow" >> >>>> option is kind of paradoxical. >> >>>> >> >>>> Do you think ic-route-allow-tag would be better? >> >>> >> >>> >> >>> >> >>>> 2. You could combine your other patch converting the >> ic-route-filter-tag >> >>>> to accept lists with this one into one patch set. >> >>>> >> >>>> >> >>>> I agree >> >>> >> >>> >> >>>> 3. I think it's too complex to have two dependent lists configured in >> >>>> separate options. It just seems like this could lead to confusion for >> >>>> users, and adding another option unnecessarily. >> >>>> >> >>>> We already have something like that in ovn-ic (ic-route-deny-adv and >> >>> ic-route-filter-adv). >> >>> >> >>> >> >>> The thing is allow and block do not make sense be used at the same >> time, >> >>>> because allowing any tag(s) blocks all other tags, and blocking any >> >>>> tag(s) allows all other tags. So this means that only one list >> should be >> >>>> in use at any given time, meaning that maybe it makes sense to just >> have >> >>>> one, and also all options in said list can be interpreted as either >> >>>> allow or block. >> >>>> >> >>>> >> >>> I agree. It doesn't make sense at the same time, but I think scenarios >> >>> where it could be more easy to configure with allow >> >>> and others with block. >> >>> >> >>> Let me explain my scenario: >> >>> I have 5 VPCs, lets say: vpc1, vpc2, vpc3, vpc4, vpc5 >> >>> vpc1 must connect with all. (No options configure) >> >>> vpc2 must connect with vpc1,vpc3. (allow vpc1,vpc3) >> >>> vpc3 must connect with vpc1,vpc2. (allow vpc1,vpc2) >> >>> vpc4 must connect with vpc1,vpc5. (allow vpc1,vpc5) >> >>> vpc5 must connect with vpc1,vpc4. (allow vpc1,vpc4) >> >>> >> >>> I know that I can block all except some VPCs , however for each new >> VPC >> >>> connected in TS, I must update all >> >>> existing configurations to deny the new VPC. >> >>>> One idea is to do this with a list like "*vpc1,*vpc2,*vpc3" to allow >> all >> >>>> of those tags, and then "vpc1,vpc2,vpc3" to block all of them. >> >>>> >> >>>> Similarly, "*vpc1,vpc2,vpc3" could mean the whole list is allow, >> since >> >>>> it's only used for just block or allow at one time. >> >>>> >> >>>> Or to be more explicit, "allow:vpc1,vpc2,vpc3" or >> "block:vpc1,vpc2,vpc3" >> >> >> >> Lucas, with Rosemarie's explicit single list alternative above you >> could >> >> do: >> >> >> >> vpc1: no ic-route-filter-tag >> >> vpc2: ic-route-filter-tag="allow:vpc1,vpc3" >> >> vpc3: ic-route-filter-tag="allow:vpc1,vpc2" >> >> vpc4: ic-route-filter-tag="allow:vpc1,vpc5" >> >> vpc5: ic-route-filter-tag="allow:vpc1,vpc4" >> >> >> >> Now if you add a vpc6 you don't need to change the configuration of the >> >> existing vpc2-6, they won't be importing routes from vpc6 because it's >> >> not listed in their allow list. >> >> >> >> So if we would be implementing this from scratch I think I'd go for >> this >> >> explicit option that Rosemarie suggested. >> >> >> >> There's a catch though: >> >> >> >> The current ic-route-filter-tag used to be a single tag, now patch 1/2 >> >> changes it to accept a list which means that if users used to have a >> >> configuration with a single tag that happens to include a comma >> >> character we now break their configuration. That is backwards >> >> incompatible but I guess that might be considered acceptable. >> >> >> >> Now, if we go for this single allow/block list option we add even more >> >> restrictions, i.e., tags cannot contain semicolons either (or at least >> >> they can't start with allow:/block:). >> >> >> >> With all that in mind maybe it's good to take a step back and consider >> >> Rosemarie's original suggestion of adding a new configuration key (I >> >> know I'm contradicting my reply on the v1 of patch 1/2 but I didn't >> >> realize all of the above until now). >> >> >> >> So what if we: >> >> - keep the existing behavior >> >> - add a new key 'ic-route-learn-tag-rules' which only supports the >> >> following types of values: >> >> >> >> EITHER >> >> a. "allow:<comma-separated-tags>" >> >> >> >> OR >> >> b. "block:<comma-separated-tags>" >> >> >> >> with the restriction that tags cannot contain commas. >> >> >> >> >> > I agree about this. So, you can ignore the first patch where I add the >> > support to multiple tags in ic-route-filter-tag. >> > I'll send a new version with the config 'ic-route-learn-tag-rules' with >> > possibility of allow, block. >> >> Cool, thank you! I'd also like to hear inputs from Rosemarie but I >> guess we can move the discussion on the new version when you post it for >> review. >> >> > ic-route-filter-tag will take precedence over this config. >> > >> >> Why? Why not give precedence to the new config you're adding, instead >> of preferring the one we're essentially deprecating? >> >> > OK, I'll take the precedence to the new config. > > >> On that front, I guess your new patch version should probably also add a >> deprecation note for the old ic-route-filter-tag (see the "OVN >> Deprecation plan" section in TODO.rst). >> > > I'll add a deprecation note. > > Regards, > Lucas > > >> >> Regards, >> Dumitru >> >> > Regards, >> > Lucas >> > >> >>> >> >>>> >> >>>> 4. The "precedence" that the filter-tag takes is kind of confusing, >> >>>> because what happens for non-specified tags if you have both the >> >>>> filter-tag and allow-tag configured at once? >> >>>> >> >>>> For example: vpc1 allowed, vpc2 blocked, what happens to vpc3? I >> guess >> >>>> it's blocked because of the blocklist's precedence only applying to >> >>>> specified tags, but this isn't easy to understand. >> >>>> >> >> >> >> >> >> Regards, >> >> Dumitru >> >> >> >> >> > >> >> -- _‘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
