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