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. >> >>> >>>>> >>>>> 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
