On 1/23/26 1:08 PM, Dumitru Ceara wrote: > On 1/23/26 12:53 PM, Ilya Maximets wrote: >> On 1/23/26 11:38 AM, Dumitru Ceara wrote: >>> Hi all, >>> >>> Thanks for the patch and discussion! I'll add my own opinion below. >>> >>> On 1/23/26 7:30 AM, Ales Musil wrote: >>>> On Thu, Jan 22, 2026 at 7:40 PM Mark Michelson <[email protected]> wrote: >>>> >>>>> In the commit title and description, "compliant" is misspelled as >>>>> "complaint". It is spelled correctly in the code, though. >>>>> >>>>> I have comments below, both regarding the code submission and to >>>>> respond to Ilya's remarks. >>>>> >>>>> On Thu, Jan 22, 2026 at 12:18 PM Ilya Maximets <[email protected]> wrote: >>>>>> >>>>>> On 1/22/26 3:27 PM, 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 >>>>>>> complaint add an option which when enabled will add extra flows >>>>>>> that match on the MAC address range specified by the RFC. >>>>>>> >>>>>>> [0] https://datatracker.ietf.org/doc/html/rfc5798 >>>>>>> Reported-at: https://issues.redhat.com/browse/FDP-2979 >>>>>>> Signed-off-by: Ales Musil <[email protected]> >>>>>>> --- >>>>>>> NEWS | 2 + >>>>>>> controller/lflow.c | 67 ++++++++++++++++++++ >>>>>>> ovn-nb.xml | 9 +++ >>>>>>> tests/ovn.at | 149 >>>>> +++++++++++++++++++++++++++++++++++++++++++++ >>>>>>> 4 files changed, 227 insertions(+) >>>>>>> >>>>>>> diff --git a/NEWS b/NEWS >>>>>>> index dc9d28f2c..e58f8ee1e 100644 >>>>>>> --- a/NEWS >>>>>>> +++ b/NEWS >>>>>>> @@ -87,6 +87,8 @@ Post v25.09.0 >>>>>>> other_config column. >>>>>>> - Introduce the capability to specify multiple ips for >>>>> ovn-evpn-local-ip >>>>>>> option. >>>>>>> + - Add an LSP option called "port-security-rfc5798-compliant", this >>>>>>> + allows VRRP v3 (RFC 5798) to work with port security enabled. >>>>>> >>>>>> Hi, Ales. >>>>>> >>>>>> This option name seems a bit misleading, as someone seeing it may assume >>>>>> that just enbaling it will allow the port to participate in VRRPv3. But >>>>>> it's not the case as port security will drop all the actual VRRP traffic, >>>>>> unless the virtual router mac address is also added to the port security, >>>>>> as all the actual VRRP packets with have this address as a source. As >>>>> per >>>>>> RFC itself: >>>>>> >>>>>> Note: VRRP packets are transmitted with the virtual router MAC >>>>>> address as the source MAC address to ensure that learning bridges >>>>>> correctly determine the LAN segment the virtual router is >>>>>> attached to. >>>>>> >>>>>> We should either add explicit indication that this option only affects >>>>>> ARP/ND packets, which would still be a bit confusing and will make the >>>>>> name even longer, or just have one knob like port-security-allow-vrrpv3 >>>>>> that also allows all the actual VRRP traffic, so users don't have to >>>>>> add all the possible router IDs into port security. This option may >>>>>> potentially accept a list of allowed router IDs or "yes" for all, but >>>>>> that can be added later if necessary. >>>>> >>>>> If this is the case, then does this mean that non-ARP and non-ND VRRP >>>>> traffic will still be dropped by port security flows when this option >>>>> is enabled? If so, then it seems like allowing the ARPs and NDs is >>>>> only getting rid of the surface issue. The ARPs/NDs will be allowed >>>>> through, but then other VRRP traffic will still be dropped. Is that >>>>> OK? It seems like we're going to immediately get a complaint about >>>>> this patch not actually fixing the problem if that's the case. I think >>>>> Ilya is on the right track by suggesting that we allow all VRRP >>>>> traffic with this option, not just the ARP/ND packets. >>>>> >>> >>> The bug report was about ARP replies not being allowed. VRRP ARP >>> replies for the virtual IP are special: >>> >>> https://datatracker.ietf.org/doc/html/rfc5798#section-8.1.2 >>> >>> When a host sends an ARP request for one of the virtual router IPv4 >>> addresses, the Virtual Router Master MUST respond to the ARP request >>> with an ARP response that indicates the virtual MAC address for the >>> virtual router. Note that the source address of the Ethernet frame >>> of this ARP response is the physical MAC address of the physical >>> router. >>> >>> The traffic the virtual router actually forwards, IP traffic, is sent >>> using the _virtual_ MAC as source, i.e., from 00-00-5E-00-01-{VRID} (for >>> IPv4). >>> >>> But as the RFC states, the ARP replies are sent from with eth.src == >>> _physical_ MAC of the VRRP member and arp.sha == 00-00-5E-00-01-{VRID}. >>> >>> Actual VRRP protocol packets are multicast and use the _physical_ MAC >>> address as source. >> >> I suppose, you meant "virtual" here. See the note at the end of >> https://datatracker.ietf.org/doc/html/rfc5798#section-7.2 that I >> cited before. >> > > You're right, it should be "virtual". But that doesn't my argument > above much. > >>> >>> So, in order for ALL this to work, the CMS must _anyway_ configure two >>> MACs (physical and 00-00-5E-00-01-{VRID}) in the port_security field of >>> the LSP. >> >> That is true. But why not save users from extra configuration and >> just allow the new knob to allow both the ARP/ND and the VRRP and >> the routed traffic? AFAIU, anyone setting the option will always >> have to set the MACs in the port_security column as well. There is >> no point in setting the option and not adding virtual MAC to the >> port_security list as well. So I'm not sure why we're forcing users >> to do that when we already adding a special vrrp knob. >> > > Your suggestion was: > >>>>>> We should either add explicit indication that this option >>>>>> only affects ARP/ND packets, which would still be a bit >>>>>> confusing and will make the name even longer, or just have >>>>>> one knob like port-security-allow-vrrpv3 that also allows >>>>>> all the actual VRRP traffic, so users don't have to add all >>>>>> the possible router IDs into port security. This option may >>>>>> potentially accept a list of allowed router IDs or "yes" for >>>>>> all, but that can be added later if necessary. > I read it as: > > 1. for now support "yes/no" for this option. > 2. if "yes" then the user needs to configure the physical MAC in the > port_security field. > > In that case OVN will allow: > - all traffic from/to the physical MAC, including VRRP ARPs with arp.sha > == VRRP-MAC (all VRIDs) > > but also > > - all traffic from/to any VRRP VID mac, i.e, any 00-00-5E-00-01-{VRID} > > This is less "secure" than the additional knob added by Ales in this > patch which, in combination with explicitly adding the user VRID MAC to > port_security along with the physical MAC, would allow routed IP traffic > only if it's routed by that explicitly configured VRRP VRID.
If we're allowing to spoof any of the virtual MAC addresses, what's the point of not allowing to send packets from subset of them? Not sure what the use case would be for that. > > Now, if we assume in the future we allow a list of VRRP MACs instead of > "yes" as value to the option, the user still needs to configure: > - port_security=[<physical-MAC>] > - port-security-allow-vrrpv3=[<VRRP-MAC1>, <VRRP-MAC2>] This may be a list of IDs or ID ranges, don't really need to be full MAC addresses. > > While that's one config less, it does mean that we're not as restrictive > as we could be in 26.03 (or until we backport the extended config support). If you don't like the "yes" option, the list of IDs can be implemented from the beginning. I don't see why that would be complicated to do in v2 of this patch. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
