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? > >> >>> 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. > >> >>> 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.
Hmm, masked IPs are already supported. So having a masked MAC doesn't really make things that more complicated. >>> >>>> >>>>>> >>>>>> 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
