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