Hi Dumitru,
Em seg., 17 de ago. de 2026 às 07:42, Dumitru Ceara <[email protected]> escreveu: > Hi Lucas, > > On 8/14/26 2:52 PM, Lucas Vargas Dias wrote: > > > > Do you think ovn-ic could automatically replace ic-router-filter-tag by > > ic-route-learn-tag-rules=block? > > > > I'm not sure that will be easy to do and might impact upgrades. > Wouldn't it mean that all ovn-ic's should be upgraded at the same time? > > You're right. It'll add other complexity. Regards, Lucas > Regards, > Dumitru > > > > > 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] <mailto:[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] <mailto:[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 > > <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’./ > > > > -- _‘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
