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