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. > > 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"] > > > 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. Thanks, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
