Hi Ales, Ilya, On 1/27/26 7:27 AM, Ales Musil via dev wrote: > On Mon, Jan 26, 2026 at 5:56 PM Ilya Maximets <[email protected]> wrote: > >> On 1/26/26 3:00 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 >>> compliant add an option which when enabled will add extra flows >>> that match on the MAC specified by the option (within the range) >>> or any MACs. >>> >>> [0] https://datatracker.ietf.org/doc/html/rfc5798 >>> Reported-at: https://issues.redhat.com/browse/FDP-2979 >>> Signed-off-by: Ales Musil <[email protected]> >>> --- >>> v2: Rebase on top of latest main. >>> Add missing checks in the test. >>> Rename the option to "port-security-allow-vrrpv3-arp-nd". >>> Allow the list of MACs to be specified in the option. >> >> I don't think we finished the discussion on v1, and it seems like
Just for tracking, the v1 discussion: https://www.mail-archive.com/[email protected]/msg100992.html >> v2 is taking the "worst of both worlds" approach when it comes to >> user experience, i.e. having a very long option name and also >> forcing to specify all the MAC addresses twice. Why can't we just >> allow all the specified MAC addresses and not require listing them >> again in the port_security column? >> One thing we didn't discuss on-list until now, which makes me favor Ales' v2 proposal is: If we allow all the specified MAC addresses and don't require listing them again in the port_security column, then with the following configuration: - port_security=[<physical-MAC>] - port-security-allow-vrrpv3=[<VRRP-MAC1>, <VRRP-MAC2>] we would: a. allow traffic from <physical-MAC> b. allow ARPs with eth.src=<physical-MAC> && arp.sha=<physical-MAC> (same for ND) c. allow traffic from <VRRP-MAC1> and <VRRP-MAC2> d. allow ARPs with eth.src=<physical-MAC> && arp.sha=<VRRP-MAC1> and ARPs with eth.src=<physical-MAC> && arp.sha=<VRRP-MAC2> (same for ND) However, if we look at the non-VRRP case, users can rely on port_security today to further restrict the traffic they allow on a logical port by also specifying a list of allowed IP addresses (or CIDRs) for each mac. That is, if: port_security=["<physical-MAC1> IP1 IP2", "<physical-MAC2> IP3 IP4"] Then for that LSP traffic is allowed only if it's from/for <physical-MAC1> _and_ one of IP1 or IP2 OR if it's from/for <physical-MAC2> _and_ one of IP3 or IP4. Back to the VRRP case, if we go with Ilya's suggestion, if users also want to further restrict port security to only allow the VRRP VIP for a given VRID the only way to achieve that would be: - port_security=["<physical-MAC>", "<VRRP-MAC1> VIP1", "<VRRP-MAC2> VIP2"] - port-security-allow-vrrpv3=[<VRRP-MAC1>, <VRRP-MAC2>] So users would still have to duplicate the VRRP MACs in some of the cases. I don't have stats about it but my guess is in most deployments port security usually includes both the MACs and the IPs of the workloads. So while for the MAC-only case we avoid duplication we can't avoid it for MAC+IP. An alternative would be to essentially duplicate the port_security column internal implementation for all the values listed in the new config option, e.g.: - port_security=["<physical-MAC>"] - port-security-allow-vrrpv3=["<VRRP-MAC1> VIP1", "<VRRP-MAC2> VIP2"] And make under the hood port-security-allow-vrrpv3 behave the same as port_security but relaxing the inner ARP checks. That sounds even worse, IMO. It also seems to me that would make it hard to mix "any" with VRRP IPs. Another argument, albeit maybe not the most convincing one, is that OVN requires similar port_security related config duplication for other features already, i.e., virtual ports: <group title="Virtual port Options"> <p> These options apply when <ref column="type"/> is <code>virtual</code>. </p> <column name="options" key="virtual-ip"> This option represents the virtual IPv4 address. </column> <column name="options" key="virtual-parents"> This options represents a set of logical port names (with in the same logical switch) which can own the <code>virtual ip</code> configured in the <ref column="options:virtual-ip"/>. All these virtual parents should add the <code>virtual ip</code> in the <ref column="port_security"/> if port security addressed are enabled. </column> </group> > > I disagree, as stated on v1 there is a way for user to define which MAC > is allowed for regular traffic via port_security. The only case where it > doesn't work is mixing port_security MAC and VRRPv3 inner MAC of > arp/nd. Adding another option that does the same thing as port_security > and in addition to that allows the inner MAC to be different doesn't seem > any better. I would argue that it makes it even more confusing. So it > seems we have reached an impasse. > As mentioned above, I currently favor the v2 approach. On the patch itself, I didn't review it in depth yet but I do think we should have explicit tests that ensure all the semantics we agree upon. For example, I see no tests for: port_security=[<phys-MAC>, <VRRP-MAC1>, <VRRP-MAC2>] port-security-allow-vrrpv3-arp-nd=[<VRRP-MAC1>, <VRRP-MAC2>] or negative tests (VRRP routed IP traffic blocked) for: port_security=[<phys-MAC>] port-security-allow-vrrpv3-arp-nd=[<VRRP-MAC1>, <VRRP-MAC2>] If I'm not wrong we're also missing some tests for VIPs as part of port_security. Regards, Dumitru > >> Best regards, Ilya Maximets. >> >> > Regards, > Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
