On 2/5/26 5:00 PM, Ilya Maximets wrote: > On 2/5/26 3:58 PM, Dumitru Ceara wrote: >> On 2/5/26 3:16 PM, Ilya Maximets wrote: >>> On 2/5/26 2:58 PM, Dumitru Ceara wrote: >>>> On 2/5/26 2:35 PM, Ilya Maximets wrote: >>>>> On 2/5/26 2:29 PM, Dumitru Ceara wrote: >>>>>> On 2/5/26 11:55 AM, Ilya Maximets wrote: >>>>>>> On 2/5/26 10:29 AM, Dumitru Ceara wrote: >>>>>>>> On 2/5/26 7:35 AM, Ales Musil wrote: >>>>>>>>> On Wed, Feb 4, 2026 at 11:40 PM Ilya Maximets <[email protected]> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> On 2/4/26 4:20 PM, Dumitru Ceara wrote: >>>>>>>>>>> On 2/3/26 8:34 AM, 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 special literal which when specified will allow >>>>>>>>>>>> user to add any/all MAC addresses defined by VRRPv3. The traffic >>>>>>>>>>>> from and to those additional MAC addresses will be allowed as >>>>>>>>>>>> well as permutations of ARP/ND inner MACs combined with the >>>>>>>>>>>> physical MAC as a source. >>>>>>>>>>>> >>>>>>>>>>>> [0] https://datatracker.ietf.org/doc/html/rfc9568 >>>>>>>>>>>> Reported-at: https://issues.redhat.com/browse/FDP-2979 >>>>>>>>>>>> Signed-off-by: Ales Musil <[email protected]> >>>>>>>>>>>> Acked-by: Jacob Tanenbaum <[email protected]> >>>>>>>>>>>> --- >>>>>>>>>>>> v2: Rebase on top of latest main. >>>>>>>>>>>> Update the RFC url. >>>>>>>>>>>> Add Jacob's ack. >>>>>>>>>>> >>>>>>>>>>> Hi Ales, >>>>>>>>>>> >>>>>>>>>>> Thanks for the patch! It seems correct to me, I just had some minor >>>>>>>>>>> findings below. >>>>>>>>>>> >>>>>>>>>>> I'm fine if you address them while merging the patch if there are no >>>>>>>>>>> further comments. It would be nice to also hear Ilya's opinion >>>>>>>>>>> first >>>>>>>>>>> though as he was part of the discussions on the previous versions. >>>>>>>>>> >>>>>>>>>> Thanks, Ales and Dumitru! >>>>>>>>>> >>>>>>>>>> I think, this change looks good in general, I didn't read the code >>>>>>>>>> too >>>>>>>>>> closely, but I have just one comment below. >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> Thank you Dumitru and Ilya for the review, >>>>>>>>> see my answer below. >>>>>>>>> >>>>>>>> >>>>>>>> Hi Ales, Ilya, >>>>>>>> >>>>>>>>> >>>>>>>>>> >>>>>>>>>> <snip> >>>>>>>>>> >>>>>>>>>>>> diff --git a/ovn-nb.xml b/ovn-nb.xml >>>>>>>>>>>> index e9ca27413..a0a465be4 100644 >>>>>>>>>>>> --- a/ovn-nb.xml >>>>>>>>>>>> +++ b/ovn-nb.xml >>>>>>>>>>>> @@ -2057,6 +2057,15 @@ >>>>>>>>>>>> addresses within an element may be space or comma >>>>>>>>>>>> separated. >>>>>>>>>>>> </p> >>>>>>>>>>>> >>>>>>>>>>>> + <p> >>>>>>>>>>>> + There is also special prefix "VRRPv3" that allows >>>>>>>>>> specification of >>>>>>>>>>> >>>>>>>>>>> Nit: "This also supports the special prefix ..." >>>>>>>>>>> >>>>>>>>>>>> + single physical MAC and multiple VRRPv3 MAC addresses. >>>>>>>>>>>> The >>>>>>>>>> format >>>>>>>>>>>> + allows the same definition of multiple IP addresses >>>>>>>>>> associated with >>>>>>>>>>> >>>>>>>>>>> Nit: "same" sounds weird here. Maybe "As for non VRRPv3 entries, >>>>>>>>>>> multiple IP addresses can be associated with the specified MACs"? >>>>>>>>>> >>>>>>>>>> May also need to qualify which MACs, as it is not clear how the >>>>>>>>>> physical >>>>>>>>>> MAC plays here. More on that below... >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> + the MACs specified. "VRRPv3" without single physical MAC >>>>>>>>>> translates >>>>>>>>>>>> + allowing traffic for the whole "VRRPv3" range of MACs. >>>>>>>>>>>> See >>>>>>>>>> more in >>>>>>>>>>>> + the examples. >>>>>>>>>>>> + </p> >>>>>>>>>>>> + >>>>>>>>>>>> <p> >>>>>>>>>>>> This column is provided as a convenience to cloud >>>>>>>>>>>> management >>>>>>>>>> systems, >>>>>>>>>>>> but all of the features that it implements can be >>>>>>>>>> implemented as ACLs >>>>>>>>>>>> @@ -2102,6 +2111,30 @@ >>>>>>>>>>>> 255.255.255.255, and any address in 224.0.0.0/4. The >>>>>>>>>> host may not >>>>>>>>>>>> send or receive any IPv6 (including IPv6 Neighbor >>>>>>>>>> Discovery) traffic. >>>>>>>>>>>> </dd> >>>>>>>>>>>> + >>>>>>>>>>>> + <dt><code>"VRRPv3 <PHYSICAL_MAC>"</code></dt> >>>>>>>>>>>> + <dd> >>>>>>>>>>>> + The host may send traffic from and receive traffic to >>>>>>>>>>>> the >>>>>>>>>>>> + specified physical MAC addresses, and to all VRRPv3 >>>>>>>>>>>> MAC >>>>>>>>>> addresses. >>>>>>>>>>> >>>>>>>>>>> I'd specify the VRRPv3 MAC formats here explicitly, i.e., >>>>>>>>>>> 00-00-5E-00-01-XX and 00-00-5E-00-02-XX. >>>>>>>>>>> >>>>>>>>>>>> + This also affects the permutations of inner MAC for >>>>>>>>>> ARP/ND. The >>>>>>>>>>> >>>>>>>>>>> "the permutations" doesn't sound explicit enough to me. I'd just >>>>>>>>>>> skip >>>>>>>>>>> it and only leave the sentence below, I think. >>>>>>>>>>> >>>>>>>>>>>> + ARP/ND can be sent with physical MAC as a source and >>>>>>>>>>>> the >>>>>>>>>> inner >>>>>>>>>>>> + SHA/TLL/SLL can be the physical MAC or any VRRPv3 MAC. >>>>>>>>>>>> + </dd> >>>>>>>>>>>> + >>>>>>>>>>>> + <dt><code>"VRRPv3 <PHYSICAL_MAC> >>>>>>>>>>>> + <VRRPV3_MACv4_1>/[<MASK1>] >>>>>>>>>>>> + <VRRPV3_MACv4_N> >>>>>>>>>>>> + <VRRPV3_MACv6_1>/[<MASK2>] >>>>>>>>>>>> + <VRRPV3_MACv6_N>"</code></dt> >>>>>>>>>>>> + <dd> >>>>>>>>>>>> + The host may send traffic from and receive traffic to >>>>>>>>>>>> the >>>>>>>>>>>> + specified physical MAC addresses, and to all VRRPv3 >>>>>>>>>>>> MAC >>>>>>>>>> addresses >>>>>>>>>>>> + specified in the option. Also note that the MAC >>>>>>>>>>>> addresses >>>>>>>>>> can >>>>>>>>>>>> + be provided with a mask between /40 and /48 prefix. >>>>>>>>>>>> + This also affects the permutations of inner MAC for >>>>>>>>>> ARP/ND. The >>>>>>>>>>> >>>>>>>>>>> Same here. >>>>>>>>>>> >>>>>>>>>>>> + ARP/ND can be sent with physical MAC as a source and >>>>>>>>>>>> the >>>>>>>>>> inner >>>>>>>>>>>> + SHA/TLL/SLL can be the physical MAC or provided >>>>>>>>>>>> VRRPv3 MAC. >>>>>>>>>>>> + </dd> >>>>>>>>>>>> </dl> >>>>>>>>>>>> </column> >>>>>>>>>>>> </group> >>>>>>>>>> >>>>>>>>>> <snip> >>>>>>>>>> >>>>>>>>>>>> +AS_BOX([With MAC only port security, VRRPv3=any]) >>>>>>>>>>>> +reset_pcap_and_expected >>>>>>>>>>>> +check ovn-nbctl --wait=hv lsp-set-port-security lsp1 "VRRPv3 >>>>>>>>>> 00:00:00:00:10:01" >>>>>>>>>>>> + >>>>>>>>>>>> +test_arp no >>>>>>>>>>>> +test_nd no >>>>>>>>>>>> + >>>>>>>>>>>> +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected]) >>>>>>>>>>>> +OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [vif2.expected]) >>>>>>>>>>>> + >>>>>>>>>>>> +AT_CHECK([ovs-ofctl dump-flows br-int >>>>>>>>>>>> table=OFTABLE_CHK_IN_PORT_SEC | >>>>>>>>>> ofctl_strip_all | sort | grep -v NXST_FLOW], [0], [dnl >>>>>>>>>>>> + table=OFTABLE_CHK_IN_PORT_SEC, priority=80,reg14=0x1,metadata=0x1 >>>>>>>>>> actions=load:0x1->NXM_NX_REG10[[12]] >>>>>>>>>>>> + table=OFTABLE_CHK_IN_PORT_SEC, >>>>>>>>>> priority=90,reg14=0x1,metadata=0x1,dl_src=00:00:00:00:10:01 >>>>>>>>>> actions=resubmit(,OFTABLE_CHK_IN_PORT_SEC_ND) >>>>>>>>>> >>>>>>>>>> So, this allows any traffic from the physical MAC, even though a >>>>>>>>>> regular >>>>>>>>>> port security is not configured, only the VRRP one. So, if we add >>>>>>>>>> the >>>>>>>>>> regular port security "00:00:00:00:10:01", we'll get a duplicate OF >>>>>>>>>> rule. >>>>>>>>>> Not sure if it's a problem, but just something to note. >>>>>>>>>> >>>>>>>>>> But also, port security "00:00:00:00:10:01 192.168.0.1" will be >>>>>>>>>> ineffective >>>>>>>>>> in combination with "VRRPv3 00:00:00:00:10:01", as the vrrp one will >>>>>>>>>> allow >>>>>>>>>> any traffic from 00:00:00:00:10:01, not only vrrp-related. >>>>>>>>>> >>>>>>>>>> Should this rule be restricted to ARP/ND and have all the regular >>>>>>>>>> traffic >>>>>>>>>> handled by standard port security? >>>>>>>>>> >>>>>>>> >>>>>>>> I guess we might have a slight misunderstanding here. I thought that >>>>>>>> in >>>>>>>> order to make the user experience smoother we'd avoid having to >>>>>>>> configure the physical MAC twice in port security. >>>>>>>> >>>>>>>> The way I read port_security="VRRPv3 00:00:00:00:10:01 [VRRP_MAC1]" is: >>>>>>>> >>>>>>>> a. enforce exactly the same rules as port_security="00:00:00:00:10:01" >>>>>>>> does >>>>>>>> >>>>>>>> _additionally_ >>>>>>>> >>>>>>>> b. allow ARP/ND that use VRRP_MAC1 as arp.sha >>>>>>>> c. allow traffic routed by VRRP_MAC1 >>>>>>> >>>>>>> The idea was a separation of concerns and a principle of least required >>>>>>> privileges. Port doesn't need to be able to send IP traffic from the >>>>>>> physical MAC in order to be a fully functional virtual router. A >>>>>>> combined >>>>>>> configuration where vrrp entry acts as a regular one as well hinders the >>>>>>> ability for the fine control. >>>>>>> >>>>>>> It is also, IMO, less confusing for the user that vrrp entry only >>>>>>> handles >>>>>>> vrrp stuff and nothing more. That's why I defined the "option 4" this >>>>>>> way >>>>>>> originally. We can add extensive docuemntation about how this actually >>>>>> >>>>>> I see, I had slightly misunderstood "option 4". But that's fine. >>>>>> >>>>>>> works and call it a day, but we do need to document the behavior in much >>>>>>> more details. >>>>>>> >>>>>>> There are still problmes though with the meaning of the IP addresses >>>>>>> on that entry, see below... >>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> While this is true doesn't that also apply to port_security without >>>>>>>>> VRRPv3? >>>>>>>>> You can have just a MAC rule and second rule with the same MAC + IP. >>>>>>>>> The result is the same, does that configuration make sense? Not >>>>>>>>> really, >>>>>>>>> but it is allowed. So maybe some emphasize in the documentation that >>>>>>>>> VRRPv3 replaces "traditional" port security in cases you need VRRPv3 >>>>>>>>> might be enough? WDYT? >>>>>>>>> >>>>>>>> >>>>>>>> I agree, we should somehow emphasize this. Maybe the following is a >>>>>>>> bit >>>>>>>> verbose but I couldn't come up with anything better. >>>>>>>> >>>>>>>> "When the port security configuration entry contains the VRRPv3 label, >>>>>>>> the behavior for the regular MAC (and IPs) listed in the entry doesn't >>>>>>>> change, traffic to and from that MAC (and IPs) is allowed as usual. >>>>>>>> However, additional rules are added to also allow traffic to/from the >>>>>>>> extra VRRP MACs (if none explicitly listed all 512 VRRP MACs are >>>>>>>> allowed) and ARP/ND traffic using the VRRP MAC. >>>>>>>> >>>>>>>> In essence, VRRPv3 labeled port security entries are a more relaxed >>>>>>>> version of the same non-VRRPv3 rules and users should not configure >>>>>>>> both >>>>>>>> simulateneously for the same non-VRRP MAC." >>>>>>> >>>>>>> When IP is listed in the entry, the code in this patch only allows for >>>>>>> the virtual router to route this one IP. So, if user wants IP port >>>>>>> security for the physical MAC, they need to also add all the routeable >>>>>>> IPs for that VR to the port security entry, otherwise the router will >>>>>>> not be functional. But that also likely means that the IP port security >>>>>>> for the actual port IP doesn't make sense, as a wide range of IPs will >>>>>>> be allowed anyway. Again, the port doesn't need to be able to send >>>>>>> traffic to/from all these IPs using the physical MAC, but there is no >>>>>>> way to restrict that if we define vrrp entry as a superset of a regular >>>>>>> one. >>>>>>> >>>>>>> Not sure how much of a problem that is, but I would imagine users would >>>>>>> like to have a finer control where all IPs can be routed by the VR, but >>>>>>> the port itself could only use its own IP. >>>>>>> >>>>>>> Either way we'll need an extensive documentation with examples, >>>>>>> especially >>>>>>> for the part where adding one IP makes no practical sense for the vrrp >>>>>>> port security entry. >>>>>>> >>>>>> >>>>>> It might be better to do what you suggest instead of trying to "fix the >>>>>> behavior through documentation". >>>>>> >>>>>> Just to make sure that I correctly understand this time. Your view of >>>>>> how port security should be configured for VRRPv3 capable workloads on >>>>>> LSPs is: >>>>>> >>>>>> LSP.port_security = [ >>>>>> (1) "00:00:00:00:00:01 10.0.0.1 20.0.0.1", >>>>>> (2) "VRRPv3 00:00:00:00:00:01 00:00:5E:00:01:01 30.0.0.1 30.0.0.2", >>>>>> ] >>>>>> >>>>>> (1) is classic port security allowing traffic to/from 00:00:00:00:00:01 >>>>>> and IPs 10.0.0.1 and 20.0.0.1, likely workload originated traffic. >>>>>> >>>>>> (2) >>>>>> a. allows ARPs/ND with eth.src == 00:00:00:00:00:01 && arp.sha == >>>>>> 00:00:5E:00:01:01 && arp.spa == {30.0.0.1, 30.0.0.2} >>>>>> b. allows routed traffic, i.e., eth.src == 00:00:5E:00:01:01 && ip >>>>>> (or in the other direction) >>>>>> >>>>>> Now, if users want to further restrict the IPs from which/to which >>>>>> traffic is routed they could either add the IPs to (2) but that leads >>>>>> to awkward ARP allow rules or try to add: >>>>>> >>>>>> (3) "00:00:5E:00:01:01 40.0.0.1 40.0.0.2" >>>>>> >>>>>> in the hope that this classic port security rule will allow only IP >>>>>> traffic >>>>>> from 40.0.0.1 and 40.0.0.2 routed by 00:00:5E:00:01:01. >>>>> >>>>> I do not understand this. The routing is already restricted to only the >>>>> 30.0.0.1 and 30.0.0.2. Did you mean "extend the list of IPs" instead of >>>>> "further restrict"? >>>>> >>>> >>>> In my example above I had: >>>> - 10.0.0.1, 20.0.0.1 are workload owned IPs >>>> - 30.0.0.1, 30.0.0.2 are VRRP virtual IPs >>>> - 40.0.0.1, 40.0.0.2 are "external" source IPs sending traffic through >>>> the VRRP router. >>>> >>>> If (2)b would match on 30.0.0.1 and 30.0.0.2 then it would not allow any >>>> traffic from "external" to be routed through the VRRP router. So it >>>> can't match on them. That's why I had: >>>> >>>>>> b. allows routed traffic, i.e., eth.src == 00:00:5E:00:01:01 && ip >>>>>> (or in the other direction) >>>> >>>> So I really meant "further restrict routed traffic to also match on IP". >>>> >>>> Does that make it a bit more clear? >>> >>> Yeah, I see. Though that was not what my design for the "option 4" >>> meant with the IP definitions. This ties back to my concern about >>> VR IP vs ROUTABLE IPs that we discussed here: >>> https://mail.openvswitch.org/pipermail/ovs-dev/2026-January/429825.html >>> And the conclusion was that we just specify all the ROUTABLE IPs in >>> the VRRPv3 port security entry, as the VR IP is likely one of the >>> routable IPs anyway. >>> >> >> That's true, and I did realize while replying earlier that I'm >> essentially suggesting to move back to the approach from v2 (just >> without an explicit knob). >> >>> In your exmaple, if we're not treating 30.0.0.1, 30.0.0.2 as routable >>> IPs and only allow to use them in ARP/ND, then router will not be able >>> to use them as a source. And as you mentioned in the previous discussion >>> keepalived, for exmample, will require to be able to send traffic from >>> these IPs. It may also need to send soem icmp errors from these IPs(?). >>> So, we'll have to create a separate regular port security entry for them >>> as well. Instead, we can just allow to use any IPs specified in the >>> VRRPv3 entry for ARP/ND and IP traffic. >>> This will allow ARp/ND with routable IPs as SIP, but I'm really not >>> sure if this is a problem and I thought we concluded that it's not. >>> >> >> My suggestion had similar issues too, allowing weird ARPs due to the >> last rule to restrict "routed" IPs. So there's a small compromise in >> both cases. Let's drop my suggestion though. > > OK. > >> >> So, to summarize, v5 should: >> - remove the flows that allow the "physical MAC" as ethernet source and >> destination for IP traffic (currently generated from the "VRRPv3" rule) >> - improve documentation >> - address the minor comments that were raised in the beginning of this >> thread >> >> Anything else I missed? > > Need to also remove flows that allow dl_src=PHY-MAC,arp_sha=PHY-MAC, > and the same for ND.
Maybe also add a few tests where regular and VRRP entries are used at the same time. > >> >>>> >>>>>> >>>>>> But that won't work because all routed traffic was already allowed by >>>>>> (2)b above. >>>>>> >>>>>> So maybe we should just do what you said in your previous email, and have >>>>>> the VRRPv3 rules only deal with ARP/ND (not even virtual router routed >>>>>> traffic). Then the config would be (sorry for the long lines): >>>>>> >>>>>> LSP.port_security = [ >>>>>> (1) "00:00:00:00:00:01 10.0.0.1 20.0.0.1", >>>>>> --- regular MAC+IP port security >>>>>> (2) "VRRPv3 00:00:00:00:00:01 00:00:5E:00:01:01 30.0.0.1 30.0.0.2", >>>>>> --- VRRPv3 rule (2)a above >>>>>> (3) "00:00:5E:00:01:01 40.0.0.1 40.0.0.2" >>>>>> --- regular MAC+IP port security (for routed traffic) >>>>>> ] >>>>>> >>>>>> What do you think? The changes to this patch should be minimal and docs >>>>>> will be clearer IMO. >>>>>> >>>>>> Thanks, >>>>>> Dumitru >>>>>> >>>>>>> Best regards, Ilya Maximets. >>>>>>> >> >> Thanks, >> Dumitru >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
