On Tue, Jan 27, 2026 at 2:49 PM Dumitru Ceara <[email protected]> wrote:
> On 1/27/26 2:28 PM, Ilya Maximets wrote: > > On 1/27/26 2:21 PM, Dumitru Ceara wrote: > >> On 1/27/26 2:07 PM, Ilya Maximets wrote: > >>> On 1/27/26 1:29 PM, Dumitru Ceara wrote: > >>>> On 1/27/26 12:54 PM, Ilya Maximets wrote: > >>>>> On 1/27/26 10:44 AM, Dumitru Ceara wrote: > >>>>>> Hi Ales, Ilya, > >>>>>> > >>>>>> On 1/27/26 7:27 AM, Ales Musil via dev wrote: > >>>>>>> On Mon, Jan 26, 2026 at 5:56 PM Ilya Maximets <[email protected]> > wrote: > >>>>>>> > >>>>>>>> On 1/26/26 3:00 PM, Ales Musil via dev wrote: > >>>>>>>>> The RFC defines a Virtual Router Redundancy Protocol [0], in > order > >>>>>>>>> for that protocol to work the workload might "spoof" MAC address > >>>>>>>>> within ARP or ND request/response. This wasn't allowed as the > port > >>>>>>>>> security is specifically designed against spoofing and checks if > >>>>>>>>> the port security MAC address is the same for source of ARP/ND > >>>>>>>>> and the inner source/target address. To make the port security > >>>>>>>>> compliant add an option which when enabled will add extra flows > >>>>>>>>> that match on the MAC specified by the option (within the range) > >>>>>>>>> or any MACs. > >>>>>>>>> > >>>>>>>>> [0] https://datatracker.ietf.org/doc/html/rfc5798 > >>>>>>>>> Reported-at: https://issues.redhat.com/browse/FDP-2979 > >>>>>>>>> Signed-off-by: Ales Musil <[email protected]> > >>>>>>>>> --- > >>>>>>>>> v2: Rebase on top of latest main. > >>>>>>>>> Add missing checks in the test. > >>>>>>>>> Rename the option to "port-security-allow-vrrpv3-arp-nd". > >>>>>>>>> Allow the list of MACs to be specified in the option. > >>>>>>>> > >>>>>>>> I don't think we finished the discussion on v1, and it seems like > >>>>>> > >>>>>> Just for tracking, the v1 discussion: > >>>>>> > >>>>>> https://www.mail-archive.com/[email protected]/msg100992.html > >>>>>> > >>>>>>>> v2 is taking the "worst of both worlds" approach when it comes to > >>>>>>>> user experience, i.e. having a very long option name and also > >>>>>>>> forcing to specify all the MAC addresses twice. Why can't we just > >>>>>>>> allow all the specified MAC addresses and not require listing them > >>>>>>>> again in the port_security column? > >>>>>>>> > >>>>>> > >>>>>> One thing we didn't discuss on-list until now, which makes me favor > >>>>>> Ales' v2 proposal is: > >>>>>> > >>>>>> If we allow all the specified MAC addresses and don't require > listing > >>>>>> them again in the port_security column, then with the following > >>>>>> configuration: > >>>>>> > >>>>>> - port_security=[<physical-MAC>] > >>>>>> - port-security-allow-vrrpv3=[<VRRP-MAC1>, <VRRP-MAC2>] > >>>>>> > >>>>>> we would: > >>>>>> a. allow traffic from <physical-MAC> > >>>>>> b. allow ARPs with eth.src=<physical-MAC> && arp.sha=<physical-MAC> > >>>>>> (same for ND) > >>>>>> c. allow traffic from <VRRP-MAC1> and <VRRP-MAC2> > >>>>>> d. allow ARPs with eth.src=<physical-MAC> && arp.sha=<VRRP-MAC1> > >>>>>> and ARPs with eth.src=<physical-MAC> && arp.sha=<VRRP-MAC2> > >>>>>> (same for ND) > >>>>>> > >>>>>> However, if we look at the non-VRRP case, users can rely on > >>>>>> port_security today to further restrict the traffic they allow on > >>>>>> a logical port by also specifying a list of allowed IP addresses > >>>>>> (or CIDRs) for each mac. That is, if: > >>>>>> > >>>>>> port_security=["<physical-MAC1> IP1 IP2", "<physical-MAC2> IP3 IP4"] > >>>>>> > >>>>>> Then for that LSP traffic is allowed only if it's from/for > >>>>>> <physical-MAC1> _and_ one of IP1 or IP2 OR if it's from/for > >>>>>> <physical-MAC2> _and_ one of IP3 or IP4. > >>>>>> > >>>>>> Back to the VRRP case, if we go with Ilya's suggestion, if users > >>>>>> also want to further restrict port security to only allow the VRRP > >>>>>> VIP for a given VRID the only way to achieve that would be: > >>>>>> > >>>>>> - port_security=["<physical-MAC>", "<VRRP-MAC1> VIP1", "<VRRP-MAC2> > VIP2"] > >>>>>> - port-security-allow-vrrpv3=[<VRRP-MAC1>, <VRRP-MAC2>] > >>>>> > >>>>> But this will not allow actual routing, i.e. this will only allow the > >>>>> virtual router to route packets between VIP1 and VIP2. Is that a > common > >>>>> or desired configuration? It's practically a router for two IPs that > >>>>> belong to that same router...? > >>>>> > >>>> > >>>> That's a good point, I was stuck in my "traditional" OVN view of > things. > >>>> > >>>> In this case the VRRP VR is probably the gateway (for some of the > other > >>>> LSPs in the switch) so it wouldn't really make sense to configure the > >>>> VIP into port_security, just the VRRP mac, as it's valid to accept > >>>> packets with destIP == "external" and dmac == "VRRP-MAC". > >>>> > >>>>>> So users would still have to duplicate the VRRP MACs in some of the > >>>>>> cases. I don't have stats about it but my guess is in most > deployments > >>>>>> port security usually includes both the MACs and the IPs of the > workloads. > >>>>> > >>>>> If that's the case then we should not introduce the new option at > all, > >>>>> but allow multiple MAC addresses within the port_security record, so > >>>>> OVN can generate rules for permutations of these MAC addresses with > the > >>>>> corresponding IP addresses. For simplicity, we may restrict the > number > >>>>> of addresses to some fairly small number or allow masking. This > >>>>> will be the most versatile and user-friendly configuration as no new > >>>>> knobs will be required, no duplication, and the option will also not > >>>>> be tied to VRRP, so can be re-purposed for other things, potentially. > >>>>> > >>>> > >>>> So, to clarify, your suggestion is to change allowed values for > >>>> port_security to be a list of: > >>>> "MAC1 MAC2 .. MAC_N IP1 IP2 .. IP_M" > >>>> > >>>> vs the current: > >>>> > >>>> "MAC1 IP1 IP2 .. IP_M" > >>>> > >>>> right? > >>> > >>> Right. > >>> > >>>> > >>>> And generate port security flows that allow: > >>>> - IP packets to/from MAC-X,IP-Y where X=[1..N], Y=[1..M] > >>>> - ARP packets with eth.src=MAC-X,ARP.sha=MAC-Y,ARP.spa=IP-Z where > >>>> X=[1..N], Y=[1..N], Z=[1..M] > >>>> - ND packets too as above > >>>> > >>>> So 2 x M x N ^ 2 + M x N flows. > >>>> > >>>> I guess that's doable and up to the user to not add too many mac > addresses. > >>> > >>> With the port-security-allow-vrrpv3-arp-nd="MAC1 MAC2 .. MAC_N" > >>> we also need the port-security=["MACX IP1 IP2 .. IP_M", > >>> "MAC1 IP1 IP2 .. IP_M", > >>> "MAC2 IP1 IP2 .. IP_M", > >>> ... > >>> "MAC_N IP1 IP2 .. IP_M"] > >>> > >>> in order to allow the actual routed traffic, which is > >>> > >>> - N x M just for port security itself. > >>> - M x N ^ 2 for the ARP/ND > >>> > >>> Which is exactly the same as with the multiple MACs in the same > >> > >> True. > >> > >>> port security record, but with a huge pile of extra repeated > >>> configuration in the database. We also have no control over > >>> the MACs in different port security records, so it's harder to > >>> find duplicates, which is important as only the half of these > >>> flows will be unique. > >>> > >>> Note: these IPs are IPs of the other LSPs from which this VR > >>> is routing the traffic, i.e. the traffic source. If we consider > >> > >> Maybe I'm misunderstanding what you're saying here but that's not how > >> port_security works, it's not an ACL, it should contain addresses (MACs > >> and IPs) owned by the LSP it's applied on. > > > > Routers route traffic from someone else. It means that traffic > > that enters OVN from this LSP will have someone else's source IP. > > If this IP is not in the port security configuration, the packet > > will be dropped. Or am I missing something? > > > > Ah, so it's actually traffic coming from "outside" the vrouter, towards > other LSPs that would have this issue. Because from OVN perspective > it's traffic originating from the vrouter LSP. > > But in that case it would be a wide range of IPs that would have to be > allowed as "source". So probably in practice we'd no see this being used. > > >> > >>> that source traffic IPs do not overlap between VRIDs, then the > >>> config becomes simpler, but the same is true for the multiple > >>> MAC in the same port-security option, it will become: > >>> "MAC_PHY MAC_V1 IP_SET_1", "MAC_PHY MAC_V1 IP_SET_2". We'll > >>> be repeating the MAC_PHY here, but it doesn't seem too bad in > >>> comparison with the v2 option. > >>> > >>>> > >>>> But it will also be hard to implement the "any VRRP MAC" semantics and > >>>> users will have to add 512 MACs if they want that behavior. Which > >>>> would mean that, in order to have this generic solution, we'd probably > >>>> need to allow masked MACs, something like: > >>>> > >>>> "MAC1/mask1 MAC2/mask2 .. MAC_N/mask_N IP1 IP2 .. IP_M" > >>>> > >>>> Which becomes very complex very quickly IMO (if I try to put myself in > >>>> the shoes of a user). > >>> > >>> If we use prefixes instead of arbitrary masks, /40 doesn't seem > >>> too complicated. > >>> > >> > >> Sure, for VRRP MACs it's a relatively easy to read: > >> 00:00:5E:00:00:00/40 > >> > >> But in general, this can be any MAC and prefix, e.g.: > >> 00:00:12:34:56:78/33 > >> > >> which is incorrect actually, it should be (I think): > >> 00:00:12:34:00:00/33 > >> > >> Which makes me step back and think about what we're trying to fix here: > >> the problem that triggered this long discussion is the fact that VRRP > >> ARPs use the physical MAC as ethernet source and the virtual mac as ARP > >> source and port security didn't allow that. > >> > >> So it really feels to me like we're over-engineering at this point. > >> > >> I know new knobs are not nice, and it would be a very specific VRRP knob > >> but it really feels to me like the alternatives are a bit of > >> over-engineering at this point. > > > > That's why the original proposal was to just have a simple knob that > > allows all VRRP and the routed traffic, i.e. port-security-allow-vrrpv3. > > That is simple, targeted, and doesn't force users to put a ton of > > duplicated addresses into the port security. > > > > But that did mean, adding implicit port_security rules for VRRP routed > traffic and VRRP ARPs, duplicating what port_security does for the > former, just that it would be for all VRRP macs (or for a subset > specified by MAC/ID). > > So it still feels to me like the config duplication (most likely in > practice we won't have more than a handful of VRRP MACs) is not such a > huge compromise as it doesn't "hide" some port security rules. > > I know internally we had a discussion about the statement in our docs > saying that port security is "a convenience to cloud management systems, > but all of the features that it implements can be implemented as ACLs". > > Implementing your suggestion would create another (in lack of a better > term) layer of abstraction. I.e., the users reading the config: > > port_security=[MAC1 MAC2] > port-security-allow-vrrpv3=[VRRP-MAC1/yes] > > would have to interpret it as: > > "allow traffic from this LSP only if: > - it originates from MAC1 or MAC2 or VRRP-MAC1 if it's plain IP > - it originates from MAC1 or MAC2 and it's an ARP with SHA=VRRP-MAC1 > > allow traffic to this LSP only if > - it's destined to MAC1 or MAC2 or VRRP-MAC2 if it's plain IP" > > Which is, aside from the duplication of config, exactly the same > behavior as proposed by this v2 patch. > > I would like to add two things, the proposed multiple MACs per entry (with or without mask, that doesn't that much) would be a complex change so other things to consider here are how long it would take for this change to happen. But also potential of backporting this change. The option has a big advantage of being backwards compatible without anything extra. > >> > >>> We may also consider masking on the IPs, as the variant here in > >>> v2 requires a lot of duplication of IPs, if they are configured. > >>> > >>>> > >>>>>> > >>>>>> So while for the MAC-only case we avoid duplication we can't avoid > >>>>>> it for MAC+IP. An alternative would be to essentially duplicate the > >>>>>> port_security column internal implementation for all the values > listed > >>>>>> in the new config option, e.g.: > >>>>>> > >>>>>> - port_security=["<physical-MAC>"] > >>>>>> - port-security-allow-vrrpv3=["<VRRP-MAC1> VIP1", "<VRRP-MAC2> > VIP2"] > >>>>>> > >>>>>> And make under the hood port-security-allow-vrrpv3 behave the > >>>>>> same as port_security but relaxing the inner ARP checks. > >>>>>> > >>>>>> That sounds even worse, IMO. It also seems to me that would make > it hard > >>>>>> to mix "any" with VRRP IPs. > >>>>>> > >>>>>> Another argument, albeit maybe not the most convincing one, is that > >>>>>> OVN requires similar port_security related config duplication for > >>>>>> other features already, i.e., virtual ports: > >>>>>> > >>>>>> <group title="Virtual port Options"> > >>>>>> <p> > >>>>>> These options apply when <ref column="type"/> is > >>>>>> <code>virtual</code>. > >>>>>> </p> > >>>>>> > >>>>>> <column name="options" key="virtual-ip"> > >>>>>> This option represents the virtual IPv4 address. > >>>>>> </column> > >>>>>> > >>>>>> <column name="options" key="virtual-parents"> > >>>>>> This options represents a set of logical port names (with > in the same > >>>>>> logical switch) which can own the <code>virtual ip</code> > configured > >>>>>> in the <ref column="options:virtual-ip"/>. All these > virtual parents > >>>>>> should add the <code>virtual ip</code> in the > >>>>>> <ref column="port_security"/> if port security addressed > are enabled. > >>>>>> </column> > >>>>>> </group> > >>>>>> > >>>>>>> > >>>>>>> I disagree, as stated on v1 there is a way for user to define > which MAC > >>>>>>> is allowed for regular traffic via port_security. The only case > where it > >>>>>>> doesn't work is mixing port_security MAC and VRRPv3 inner MAC of > >>>>>>> arp/nd. Adding another option that does the same thing as > port_security > >>>>>>> and in addition to that allows the inner MAC to be different > doesn't seem > >>>>>>> any better. I would argue that it makes it even more confusing. So > it > >>>>>>> seems we have reached an impasse. > >>>>>>> > >>>>>> > >>>>>> As mentioned above, I currently favor the v2 approach. > >>>>>> > >>>> > >>>> I think I still stand behind this. > >>>> > >>>>>> On the patch itself, I didn't review it in depth yet but I do > >>>>>> think we should have explicit tests that ensure all the > >>>>>> semantics we agree upon. For example, I see no tests for: > >>>>>> > >>>>>> port_security=[<phys-MAC>, <VRRP-MAC1>, <VRRP-MAC2>] > >>>>>> port-security-allow-vrrpv3-arp-nd=[<VRRP-MAC1>, <VRRP-MAC2>] > >>>>>> > >>>>>> or negative tests (VRRP routed IP traffic blocked) for: > >>>>>> > >>>>>> port_security=[<phys-MAC>] > >>>>>> port-security-allow-vrrpv3-arp-nd=[<VRRP-MAC1>, <VRRP-MAC2>] > >>>>>> > >>>>>> If I'm not wrong we're also missing some tests for VIPs as part of > >>>>>> port_security. > >>>>>> > >>>>>> Regards, > >>>>>> Dumitru > >>>>>> > >>>>>>> > >>>>>>>> Best regards, Ilya Maximets. > >>>>>>>> > >>>>>>>> > >>>>>>> Regards, > >>>>>>> Ales > >>>>>> > >>>>> > >>>> > >>> > >> > > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
