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"? > > 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. >> >>> >>>> >>>>> >>>>>>> + table=OFTABLE_CHK_IN_PORT_SEC, >>>>> priority=90,reg14=0x1,metadata=0x1,dl_src=00:00:5e:00:01:00/ff:ff:ff:ff:ff:00 >>>>> actions=resubmit(,OFTABLE_CHK_IN_PORT_SEC_ND) >>>>>>> + table=OFTABLE_CHK_IN_PORT_SEC, >>>>> priority=90,reg14=0x1,metadata=0x1,dl_src=00:00:5e:00:02:00/ff:ff:ff:ff:ff:00 >>>>> actions=resubmit(,OFTABLE_CHK_IN_PORT_SEC_ND) >>>>> >>>>> These two rules are fine though, as they should allow routed ip traffic. >>>>> >>>>>>> + table=OFTABLE_CHK_IN_PORT_SEC, priority=95,arp,reg14=0x1,metadata=0x1 >>>>> actions=resubmit(,OFTABLE_CHK_IN_PORT_SEC_ND) >>>>>>> +]) >>>>>>> +AT_CHECK([ovs-ofctl dump-flows br-int table=OFTABLE_CHK_IN_PORT_SEC_ND >>>>> | ofctl_strip_all | sort | grep -v NXST_FLOW], [0], [dnl >>>>>>> + table=OFTABLE_CHK_IN_PORT_SEC_ND, >>>>> priority=80,arp,reg14=0x1,metadata=0x1 >>>>> actions=load:0x1->NXM_NX_REG10[[12]] >>>>>>> + table=OFTABLE_CHK_IN_PORT_SEC_ND, >>>>> priority=80,icmp6,reg14=0x1,metadata=0x1,nw_ttl=255,icmp_type=135 >>>>> actions=load:0x1->NXM_NX_REG10[[12]] >>>>>>> + table=OFTABLE_CHK_IN_PORT_SEC_ND, >>>>> priority=80,icmp6,reg14=0x1,metadata=0x1,nw_ttl=255,icmp_type=136 >>>>> actions=load:0x1->NXM_NX_REG10[[12]] >>>>>>> + table=OFTABLE_CHK_IN_PORT_SEC_ND, >>>>> priority=90,arp,reg14=0x1,metadata=0x1,dl_src=00:00:00:00:10:01,arp_sha=00:00:00:00:10:01 >>>>> actions=load:0->NXM_NX_REG10[[12]] >>>>> >>>>> Same here. This one allows ARP for the physical MAC and any SIP, so the >>>>> VRRP port security record allows for spoofing any IP address, even if we >>>>> have a regular port security that restricts it. >>>>> >>>>>>> + table=OFTABLE_CHK_IN_PORT_SEC_ND, >>>>> priority=90,arp,reg14=0x1,metadata=0x1,dl_src=00:00:00:00:10:01,arp_sha=00:00:5e:00:01:00/ff:ff:ff:ff:ff:00 >>>>> actions=load:0->NXM_NX_REG10[[12]] >>>>>>> + table=OFTABLE_CHK_IN_PORT_SEC_ND, >>>>> priority=90,icmp6,reg14=0x1,metadata=0x1,dl_src=00:00:00:00:10:01,nw_ttl=255,icmp_type=135,icmp_code=0,nd_sll=00:00:00:00:00:00 >>>>> actions=load:0->NXM_NX_REG10[[12]] >>>>>>> + table=OFTABLE_CHK_IN_PORT_SEC_ND, >>>>> priority=90,icmp6,reg14=0x1,metadata=0x1,dl_src=00:00:00:00:10:01,nw_ttl=255,icmp_type=135,icmp_code=0,nd_sll=00:00:00:00:10:01 >>>>> actions=load:0->NXM_NX_REG10[[12]] >>>>>>> + table=OFTABLE_CHK_IN_PORT_SEC_ND, >>>>> priority=90,icmp6,reg14=0x1,metadata=0x1,dl_src=00:00:00:00:10:01,nw_ttl=255,icmp_type=135,icmp_code=0,nd_sll=00:00:5e:00:02:00/ff:ff:ff:ff:ff:00 >>>>> actions=load:0->NXM_NX_REG10[[12]] >>>>>>> + table=OFTABLE_CHK_IN_PORT_SEC_ND, >>>>> priority=90,icmp6,reg14=0x1,metadata=0x1,dl_src=00:00:00:00:10:01,nw_ttl=255,icmp_type=136,icmp_code=0,nd_tll=00:00:00:00:00:00 >>>>> actions=load:0->NXM_NX_REG10[[12]] >>>>>>> + table=OFTABLE_CHK_IN_PORT_SEC_ND, >>>>> priority=90,icmp6,reg14=0x1,metadata=0x1,dl_src=00:00:00:00:10:01,nw_ttl=255,icmp_type=136,icmp_code=0,nd_tll=00:00:00:00:10:01 >>>>> actions=load:0->NXM_NX_REG10[[12]] >>>>>>> + table=OFTABLE_CHK_IN_PORT_SEC_ND, >>>>> priority=90,icmp6,reg14=0x1,metadata=0x1,dl_src=00:00:00:00:10:01,nw_ttl=255,icmp_type=136,icmp_code=0,nd_tll=00:00:5e:00:02:00/ff:ff:ff:ff:ff:00 >>>>> actions=load:0->NXM_NX_REG10[[12]] >>>>>>> +]) >>>>>>> +AT_CHECK([ovs-ofctl dump-flows br-int table=OFTABLE_CHK_OUT_PORT_SEC | >>>>> ofctl_strip_all | sort | grep -v NXST_FLOW], [0], [dnl >>>>>>> + table=OFTABLE_CHK_OUT_PORT_SEC, priority=80,reg15=0x1,metadata=0x1 >>>>> actions=load:0x1->NXM_NX_REG10[[12]] >>>>>>> + table=OFTABLE_CHK_OUT_PORT_SEC, >>>>> priority=85,reg15=0x1,metadata=0x1,dl_dst=00:00:00:00:10:01 >>>>> actions=load:0->NXM_NX_REG10[[12]] >>>>>>> + table=OFTABLE_CHK_OUT_PORT_SEC, >>>>> priority=85,reg15=0x1,metadata=0x1,dl_dst=00:00:5e:00:01:00/ff:ff:ff:ff:ff:00 >>>>> actions=load:0->NXM_NX_REG10[[12]] >>>>>>> + table=OFTABLE_CHK_OUT_PORT_SEC, >>>>> priority=85,reg15=0x1,metadata=0x1,dl_dst=00:00:5e:00:02:00/ff:ff:ff:ff:ff:00 >>>>> actions=load:0->NXM_NX_REG10[[12]] >>>>>>> +]) >>>>>>> + >>>>> >>>>> <snip> >>>>> >>>>> Best regards, Ilya Maximets. >>>>> >>>>> >>>> Regards, >>>> Ales >>>> >>> >>> Thanks, >>> Dumitru >>> >>> >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
