On 1/29/26 1:43 PM, Dumitru Ceara wrote: > On 1/29/26 12:43 PM, Ilya Maximets wrote: >> On 1/29/26 9:29 AM, Dumitru Ceara wrote: >>> On 1/29/26 7:36 AM, Ales Musil wrote: >>>> On Tue, Jan 27, 2026 at 7:47 PM Ilya Maximets <[email protected]> wrote: >>>> >>>>> On 1/27/26 2:49 PM, Dumitru Ceara 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. >>>>> >>>>> The behavior is not the argument from my side. I'm just trying to argue >>>>> for a better UX, i.e. easier to understand configuration. >>>>> >>>>> Both 'port-security-allow-vrrpv3' and the >>>>> 'port-security-allow-vrrpv3-arp-nd' >>>>> are modifiers applied to the port security, which require extra >>>>> understanding. >>>>> In other words, just looking at the port security, user can't have a full >>>>> picture of what is allowed and what is not and will have to do the mental >>>>> math to map things onto each other in different combinations, as both >>>>> options >>>>> will affect every port security entry separately. >>>>> >>>>> The mutli-MAC port security syntax approach ("MAC1 MACN IP1 IPN") has an >>>>> advantage of keeping everything in the same place, so if you look at port >>>>> security, you know what it allows. >>>>> >>>>> The exact syntax may be a bit convoluted and may use some work, I agree. >>>>> >>>>> For example, in practice, I'm not sure we'll need more than one "physical" >>>>> MAC, so having an option to allow multiple MACs generically is probably >>>>> not >>>>> necessary. We could come up with a distinct syntax for VRRP-allowed port >>>>> security, e.g. >>>>> "VRRPv3 <MAC_PHY> <VRRP_MAC1> <VRRP_NET/plen> <IP_1> <IP_NET/plen>". >>>>> This would mean: >>>>> >>>>> 1. Allow IP traffic from VRRP_MAC1 or VRRP_NET/plen + IP_1 or IP_NET/plen. >>>>> 2. Allow ARP from MAC_PHY with VRRP_MAC1 or VRRP_NET/plen as SHA and >>>>> IP_1 or IP_NET/plen as SIP. >> >> This may not be fully correct. I wonder if the syntax should be: >> >> "VRRPv3 PHY-MAC PHY-IPs <delimiter?> VRRP-MACs ROUTABLE-IPs" >> or >> "PHY-MAC PHY-IPs VRRPv3 VRRP-MACs ROUTABLE-IPs" >> > > The "<delimiter>" doesn't sound too nice to me to be honest. At that > point I'd consider different columns or a different way of storing the > port_security config. > > Also, on VRRP-MACs, these have a fixed prefix, it's easy for us to > determine the physical mac (only one) and the set of VRRP macs when > parsing the port_security column. > > So we don't have to enforce any order between the MACs (at least). We > should however validate that there's at most one physical MAC I guess. > > Unless we need to separate "PHY-IPs" and "ROUTABLE-IPs" as you did above > but.. > >> So, allowed SIP would be limited to PHY-IPs, not the ROUTABLE-IPs. >> And if not provided, then any SIP would be permitted, like with the >> normal port security. > > I'm trying to understand what we win by doing this. > > What if the workload running the VRRP instance actually sends traffic > from the VR IP? IIRC, when keepalived runs it actually configures the > VIP on a linux interface on the member that was elected VRRP master. So > in theory it could send traffic with VIP as source IP. And that should > likely be allowed. > > So aren't we just making it too strict? And also less readable?
Yeah, you're right. We need to allow traffic from the VR IP, which should be one of the ROUTABLE-IPs. I suppose, for simplicity, we can just allow any provided IPs as an ARP SIP or a source IP in regular IP traffic. And it's out of scope for the VRRP port security to allow PHY-MAC + PHY-IP traffic, that should be handled by a normal port security entry. In which case, the original proposal should be fine. > >> >>>>> >>>>> No inter-operation or side effects on other entries within port-security >>>>> column on the same port. >>>>> >>>>> The "any v4" config would look like: >>>>> >>>>> port-security=["02:12:34:56:78:9a 192.168.1.2", >>>>> "VRRPv3 02:12:34:56:78:9a 00:00:5e:00:01:00/40"] >>>>> >>>>> We may also omit the masked MAC address meaning "any", e.g. >>>>> >>>>> port-security=["02:12:34:56:78:9a 192.168.1.2", >>>>> "VRRPv3 02:12:34:56:78:9a"] >>>>> >>>>> IMO, seems more clear what it does, and no mental math required. >>>>> >>>>> If we want to restrict the VR to only some particular networks: >>>>> >>>>> port-security=[ >>>>> "02:12:34:56:78:9a 192.168.1.2", >>>>> "VRRPv3 02:12:34:56:78:9a 00:00:5e:00:01:00/40 192.168.1.0/24 >>>>> 10.10.0.0/16"] >>>>> >>>>> This would allow forwarding between internal subnet 192.168.1.0/24 and >>>>> some external subnet 10.10.0.0/16. Though, as you mentioned, it seems >>>>> like not a particularly useful restriction to apply as the external >>>>> traffic subnet would likely need to be pretty large. >>>>> >>>>> WDYT? >>>>> >>>>> >>>>> Summarizing all proposals with configuration that allows all the >>>>> required types of traffic, including ARP, normal IP, and routed IP: >>>>> >>>>> 1. "arp-nd option" (I added /40 syntax to avoid listing all 255 MACs). >>>>> >>>>> - Any VRID: >>>>> port-security-allow-vrrpv3-arp-nd="any" >>>>> port-security=["MAC-PHY", "VRRP-v4MAC/40", "VRRP-v6MAC/40"] >>>>> >>>>> - 2 VRID, Any IP: >>>>> port-security-allow-vrrpv3-arp-nd="VRRP-MAC1 VRRP-MAC2" >>>>> port-security=["MAC-PHY", "VRRP-MAC1", "VRRP-MAC2"] >>>>> >>>>> - 2 VRID, Specific IPs: >>>>> port-security-allow-vrrpv3-arp-nd="VRRP-MAC1 VRRP-MAC2" >>>>> port-security=["MAC-PHY", "VRRP-MAC1 IPs-1", "VRRP-MAC2 IPs-2"] >>>>> >>>>> >>>>> 2. "full allow-vrrp". >>>>> >>>>> - Any VRID: >>>>> port-security-allow-vrrpv3=["any"] >>>>> port-security=["MAC-PHY"] >>>>> >>>>> - 2 VRID, Any IP: >>>>> port-security-allow-vrrpv3=["VRRP-MAC1", "VRRP-MAC2"] >>>>> port-security=["MAC-PHY"] >>>>> >>>>> - 2 VRID, Specific IPs: >>>>> port-security-allow-vrrpv3=["VRRP-MAC1 IPs-1", "VRRP-MAC2 IPs-2"] >>>>> port-security=["MAC-PHY"] >>>>> >>>>> >>>>> 3. "multi-MAC port-security". >>>>> >>>>> - Any VRID: >>>>> port-security=["MAC-PHY VRRP-v4MAC/40 VRRP-v6MAC/40"] >>>>> >>>>> - 2 VRID, Any IP: >>>>> port-security=["MAC-PHY VRRP-MAC1", "MAC-PHY VRRP-MAC2"] >>>>> >>>>> - 2 VRID, Specific IPs: >>>>> port-security=["MAC-PHY VRRP-MAC1 IPs-1", "MAC-PHY VRRP-MAC2 IPs-2"] >>>>> >>>>> >>>>> 4. "VRRP-specific port-security". >>>>> >>>>> - Any VRID: >>>>> port-security=["MAC-PHY", "VRRPv3 MAC-PHY"] >>>>> >>>>> - 2 VRID, Any IP: >>>>> port-security=["MAC-PHY", >>>>> "VRRPv3 MAC-PHY VRRP-MAC1 VRRP-MAC2"] >>>>> >>>>> - 2 VRID, Specific IPs: >>>>> port-security=["MAC-PHY", >>>>> "VRRPv3 MAC-PHY VRRP-MAC1 IPs-1", >>>>> "VRRPv3 MAC-PHY VRRP-MAC2 IPs-2"] >>>>> >>> >>> Thanks for summarizing this Ilya! >>> >>>>> >>>>> My preference here is 4, as it is explicit and the least verbose >>>>> for a simple config and still fairly simple for the complex case. >>>>> It also tells the full store in a single place with no mental math >>>>> required. >>>>> >>>>> Next in priotity would be 2, as it is the most concise. Then 3 >>>>> and then 1. Note that 1 is not really what this patch is proposing >>>>> as it also adds prefix matches on MAC addresses. >>>>> >>>>> For the implementation complexity point raised by Ales, it feels >>>>> like the option 4 should not be hard to implement as a fairly >>>>> standalone thing, as it doesn't impose side effects on any other >>>>> port security entries. >>>>> >>>>> Best regards, Ilya Maximets. >>>>> >>>>> >>>> Based on the discussion I will attempt to make the version 4 >>>> work in v3. Let's see how it goes. >>>> >>> >>> I agree, version 4 seems the way forward now. >>> >>>> Thanks, >>>> Ales >>>> >>> >>> Regards, >>> Dumitru >>> >>> >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
