When a logical switch port has both port_security and "unknown"
in its addresses, build_lswitch_learn_fdb_op() skipped FDB
learning flow generation because of an early-return guard that
checked op->lsp_has_port_sec.

When multiple logical switch ports share the same MAC address
(e.g., a VRRP virtual MAC) in their LSP.addresses, northd
generates duplicate L2 lookup flows at the same priority with
the same match but different outport actions.  Since
ovn-controller can only install one, traffic to the shared MAC
is nondeterministically sent to only one of the ports.

The recommended configuration to avoid this is to omit the shared
MAC from LSP.addresses (placing it only in port_security) and
include "unknown" in addresses, so the shared MAC is resolved
dynamically via FDB learning.  However, the early-return guard
blocked FDB learning on ports with port_security, preventing
this configuration from working.

The original guard was added in commit dd94f1266 ("northd: MAC
learning: Add logical flows for fdb") under the assumption that
ports with port security should not participate in FDB learning.
However, ingress port security (ls_in_check_port_sec) validates
source MACs before the FDB learning stages (ls_in_lookup_fdb,
ls_in_put_fdb), so only MACs that pass port security are
recorded.  Egress port security also validates packets after L2
lookup.  Removing the lsp_has_port_sec check is therefore safe.

Reported-at: https://redhat.atlassian.net/browse/FDP-4286
Assisted-by: Claude Opus 4.6, Claude Code
Signed-off-by: Dumitru Ceara <[email protected]>
---
 NEWS                      |   2 +
 northd/northd.c           |  16 +---
 northd/northd.h           |   3 -
 tests/ovn-northd.at       |  74 +++++++++++++++++
 tests/ovn.at              | 169 ++++++++++++++++++++++++++++++++++++++
 utilities/ovn-nbctl.8.xml |   2 +-
 6 files changed, 248 insertions(+), 18 deletions(-)

diff --git a/NEWS b/NEWS
index d025581c9a..b72fba011a 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,8 @@ Post v26.09.0
      table in addition to eth.src.  This allows VRRP virtual MACs to
      be learned from gratuitous ARP packets and unsolicited Neighbor
      Advertisements.
+   - Allow FDB learning on logical switch ports that have both
+     port_security and "unknown" addresses configured.
    - Added a new "options:ttl" key on the NB DNS table to make the TTL of
      DNS replies from OVN's native DNS resolver configurable per row.
    - Removed implementations of the commit_ecmp_nh, chk_ecmp_nh, and
diff --git a/northd/northd.c b/northd/northd.c
index d33b60cd76..cbfce95fea 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -1175,8 +1175,6 @@ ovn_port_cleanup(struct ovn_port *port)
         port->peer->peer = NULL;
     }
 
-    port->lsp_has_port_sec = false;
-
     destroy_lport_addresses(&port->lrp_networks);
     destroy_lport_addresses(&port->proxy_arp_addrs);
 }
@@ -1569,16 +1567,6 @@ parse_lsp_addrs(struct ovn_port *op)
     if (lsp_is_switch(nbsp)) {
         op->has_unknown = true;
     }
-
-    struct eth_addr mac;
-    for (size_t j = 0; j < nbsp->n_port_security; j++) {
-        int n = !strncmp(nbsp->port_security[j], "VRRPv3", 6) ? 7 : 0;
-        if (ovs_scan_len(nbsp->port_security[j], &n, ETH_ADDR_SCAN_FMT,
-                         ETH_ADDR_SCAN_ARGS(mac))) {
-            op->lsp_has_port_sec = true;
-            break;
-        }
-    }
 }
 
 static void
@@ -1675,7 +1663,7 @@ join_logical_ports_lsp(struct hmap *ports,
 
             /* This port exists due to a SB binding, but should
              * not have been initialized fully. */
-            ovs_assert(!op->n_lsp_addrs && !op->lsp_has_port_sec);
+            ovs_assert(!op->n_lsp_addrs);
         }
     } else {
         op = ovn_port_create(ports, name, nbsp, NULL, NULL);
@@ -6632,7 +6620,7 @@ build_lswitch_learn_fdb_op(
 {
     ovs_assert(op->nbsp);
 
-    if (op->lsp_has_port_sec || !op->has_unknown) {
+    if (!op->has_unknown) {
         return;
     }
 
diff --git a/northd/northd.h b/northd/northd.h
index 4150157b00..a2b8a0c933 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -740,9 +740,6 @@ struct ovn_port {
                                           * beginning of 'lsp_addrs' extracted
                                           * directly from LSP 'addresses'. */
 
-    /* LSP has at least one valid MAC address in the port security column. */
-    bool lsp_has_port_sec;
-
     bool lsp_can_be_inc_processed; /* If it can be incrementally processed when
                                       the port changes. */
 
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 11d181e6bb..9145c8dc27 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -8269,6 +8269,80 @@ AT_CLEANUP
 ])
 
 
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([FDB learning with port security and shared VMAC])
+AT_KEYWORDS([ovn])
+ovn_start
+
+check ovn-nbctl ls-add ls0
+
+check ovn-nbctl lsp-add ls0 p1
+check ovn-nbctl lsp-add ls0 p2
+check ovn-nbctl set Logical_Switch_Port p1 \
+    'addresses=["fa:16:3e:00:00:01 10.0.0.1", "unknown"]' \
+    'port_security=["fa:16:3e:00:00:01 10.0.0.1", "VRRPv3 fa:16:3e:00:00:01 
00:00:5e:00:01:33/40"]'
+check ovn-nbctl set Logical_Switch_Port p2 \
+    'addresses=["fa:16:3e:00:00:02 10.0.0.2", "unknown"]' \
+    'port_security=["fa:16:3e:00:00:02 10.0.0.2", "VRRPv3 fa:16:3e:00:00:02 
00:00:5e:00:01:33/40"]'
+check ovn-nbctl --wait=sb sync
+
+ovn-sbctl dump-flows ls0 > ls0flows
+AT_CAPTURE_FILE([ls0flows])
+
+dnl Static L2 lookup flows at priority 50 for each port's primary MAC.
+AT_CHECK([grep -e "ls_in_l2_lkup" ls0flows | grep -q "eth.dst == 
fa:16:3e:00:00:01"])
+AT_CHECK([grep -e "ls_in_l2_lkup" ls0flows | grep -q "eth.dst == 
fa:16:3e:00:00:02"])
+
+dnl No static L2 lookup flow for the VMAC (it is not in addresses).
+AT_CHECK([grep -e "ls_in_l2_lkup" ls0flows | grep -c "00:00:5e:00:01:33"], 
[1], [0
+])
+
+dnl FDB learning flows generated for both ports despite port_security.
+AT_CHECK([grep -e "ls_in_lookup_fdb" ls0flows | grep -q "p1"])
+AT_CHECK([grep -e "ls_in_lookup_fdb" ls0flows | grep -q "p2"])
+AT_CHECK([grep -e "ls_in_put_fdb" ls0flows | grep -q "p1"])
+AT_CHECK([grep -e "ls_in_put_fdb" ls0flows | grep -q "p2"])
+
+dnl FDB-based L2 lookup at priority 0 is present.
+AT_CHECK([grep -e "ls_in_l2_lkup" ls0flows | grep -q "get_fdb"])
+
+dnl Port security flows still present.
+AT_CHECK([grep -e "ls_in_check_port_sec" ls0flows | grep -q "p1"])
+AT_CHECK([grep -e "ls_in_check_port_sec" ls0flows | grep -q "p2"])
+
+dnl A port without port_security + unknown still gets FDB
+dnl learning flows (existing behavior preserved).
+check ovn-nbctl lsp-add ls0 p3
+check ovn-nbctl lsp-set-addresses p3 "fa:16:3e:00:00:03 10.0.0.3" unknown
+check ovn-nbctl --wait=sb sync
+
+ovn-sbctl dump-flows ls0 > ls0flows2
+AT_CAPTURE_FILE([ls0flows2])
+
+AT_CHECK([grep -e "ls_in_lookup_fdb" ls0flows2 | grep -q "p3"])
+AT_CHECK([grep -e "ls_in_put_fdb" ls0flows2 | grep -q "p3"])
+
+dnl A port with port_security but WITHOUT "unknown" does NOT
+dnl get FDB learning flows.
+check ovn-nbctl lsp-add ls0 p4
+check ovn-nbctl set Logical_Switch_Port p4 \
+    'addresses=["fa:16:3e:00:00:04 10.0.0.4"]' \
+    'port_security=["fa:16:3e:00:00:04 10.0.0.4"]'
+check ovn-nbctl --wait=sb sync
+
+ovn-sbctl dump-flows ls0 > ls0flows3
+AT_CAPTURE_FILE([ls0flows3])
+
+AT_CHECK([grep -e "ls_in_lookup_fdb" ls0flows3 | grep -c "p4"], [1], [0
+])
+AT_CHECK([grep -e "ls_in_put_fdb" ls0flows3 | grep -c "p4"], [1], [0
+])
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+])
+
+
 OVN_FOR_EACH_NORTHD_NO_HV([
 AT_SETUP([check options:pkt_clone_type for LSP])
 ovn_start
diff --git a/tests/ovn.at b/tests/ovn.at
index e095e50bff..6ad7541f11 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -34570,6 +34570,175 @@ OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn -- FDB learning with port security and VRRP VMAC])
+AT_KEYWORDS([fdb port-security vrrp])
+CHECK_SCAPY
+ovn_start
+
+dnl Topology: one logical switch, one hypervisor, three ports.
+dnl p1 and p2 are VRRP members sharing a virtual MAC (VMAC).
+dnl p3 is a regular client that sends traffic to the VMAC.
+dnl
+dnl The VMAC is NOT in LSP.addresses (only in port_security).
+dnl "unknown" IS in addresses so FDB learning is enabled.
+dnl
+dnl FDB learning is triggered by gratuitous ARPs with
+dnl eth.src=physical MAC and arp.sha=VMAC.  The put_fdb action
+dnl records arp.sha (the VMAC) into the FDB table.
+
+vmac=00:00:5e:00:01:33
+vip=10.0.0.100
+p1_mac=fa:16:3e:00:00:01
+p1_ip=10.0.0.1
+p2_mac=fa:16:3e:00:00:02
+p2_ip=10.0.0.2
+p3_mac=fa:16:3e:00:00:03
+p3_ip=10.0.0.3
+
+check ovn-nbctl ls-add ls0
+
+check ovn-nbctl \
+    -- lsp-add ls0 p1 \
+    -- lsp-set-addresses p1 "fa:16:3e:00:00:01 10.0.0.1" unknown \
+    -- lsp-set-port-security p1 "fa:16:3e:00:00:01 10.0.0.1" \
+       "VRRPv3 fa:16:3e:00:00:01 00:00:5e:00:01:33"
+
+check ovn-nbctl \
+    -- lsp-add ls0 p2 \
+    -- lsp-set-addresses p2 "fa:16:3e:00:00:02 10.0.0.2" unknown \
+    -- lsp-set-port-security p2 "fa:16:3e:00:00:02 10.0.0.2" \
+       "VRRPv3 fa:16:3e:00:00:02 00:00:5e:00:01:33"
+
+check ovn-nbctl lsp-add ls0 p3
+check ovn-nbctl lsp-set-addresses p3 "fa:16:3e:00:00:03 10.0.0.3"
+
+net_add n1
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+
+check ovs-vsctl -- add-port br-int vif1 -- \
+    set interface vif1 external-ids:iface-id=p1 \
+    options:tx_pcap=hv1/vif1-tx.pcap \
+    options:rxq_pcap=hv1/vif1-rx.pcap
+check ovs-vsctl -- add-port br-int vif2 -- \
+    set interface vif2 external-ids:iface-id=p2 \
+    options:tx_pcap=hv1/vif2-tx.pcap \
+    options:rxq_pcap=hv1/vif2-rx.pcap
+check ovs-vsctl -- add-port br-int vif3 -- \
+    set interface vif3 external-ids:iface-id=p3 \
+    options:tx_pcap=hv1/vif3-tx.pcap \
+    options:rxq_pcap=hv1/vif3-rx.pcap
+
+wait_for_ports_up
+check ovn-nbctl --wait=hv sync
+
+AS_BOX([p1 sends a GARP to trigger FDB learning of the VMAC])
+
+dnl p1 sends a gratuitous ARP: eth.src=p1's physical MAC,
+dnl arp.sha=VMAC, arp.spa=arp.tpa=VIP.  The put_fdb action
+dnl records both eth.src (p1_mac) and arp.sha (VMAC) into FDB.
+trigger=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', src='${p1_mac}')/ \
+                   ARP(op=1, hwsrc='${vmac}', psrc='${vip}', \
+                       hwdst='00:00:00:00:00:00', pdst='${vip}')")
+as hv1 ovs-appctl netdev-dummy/receive vif1 $trigger
+
+dnl The broadcast reaches p2 and p3.
+echo $trigger > p2.expected
+OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [p2.expected])
+
+dnl Wait for the VMAC FDB entry to be created.
+wait_row_count fdb 1 mac='"00:00:5e:00:01:33"'
+
+AS_BOX([After FDB learned: traffic to VMAC goes only to p1])
+
+as hv1 reset_pcap_file vif1 hv1/vif1
+as hv1 reset_pcap_file vif2 hv1/vif2
+as hv1 reset_pcap_file vif3 hv1/vif3
+
+dnl Send a UDP packet from p3 to the VMAC.  The FDB should direct
+dnl it to p1 only.
+packet=$(fmt_pkt "Ether(dst='${vmac}', src='${p3_mac}')/ \
+                  IP(src='${p3_ip}', dst='${vip}')/ \
+                  UDP(sport=12345, dport=5678)/ \
+                  Raw(b'to_vrrp_master')")
+as hv1 ovs-appctl netdev-dummy/receive vif3 $packet
+
+echo $packet > p1.expected
+OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [p1.expected])
+
+dnl p2 should NOT receive the packet.
+: > p2.expected
+OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [p2.expected])
+
+AS_BOX([VRRP failover: p2 claims VMAC via GARP])
+
+as hv1 reset_pcap_file vif1 hv1/vif1
+as hv1 reset_pcap_file vif2 hv1/vif2
+as hv1 reset_pcap_file vif3 hv1/vif3
+
+dnl p2 sends a GARP: eth.src=p2's physical MAC, arp.sha=VMAC.
+dnl FDB updates: VMAC -> p2.
+trigger2=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', src='${p2_mac}')/ \
+                    ARP(op=1, hwsrc='${vmac}', psrc='${vip}', \
+                        hwdst='00:00:00:00:00:00', pdst='${vip}')")
+as hv1 ovs-appctl netdev-dummy/receive vif2 $trigger2
+
+echo $trigger2 > p1.expected
+OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [p1.expected])
+
+dnl Wait for the FDB entry to update.
+p2_key=$(fetch_column port_binding tunnel_key logical_port=p2)
+wait_column "$p2_key" fdb port_key mac='"00:00:5e:00:01:33"'
+
+AS_BOX([After failover: traffic to VMAC goes only to p2])
+
+as hv1 reset_pcap_file vif1 hv1/vif1
+as hv1 reset_pcap_file vif2 hv1/vif2
+as hv1 reset_pcap_file vif3 hv1/vif3
+
+dnl Send a UDP packet from p3 to the VMAC.  The FDB should now
+dnl direct it to p2 only.
+packet=$(fmt_pkt "Ether(dst='${vmac}', src='${p3_mac}')/ \
+                  IP(src='${p3_ip}', dst='${vip}')/ \
+                  UDP(sport=12345, dport=5678)/ \
+                  Raw(b'after_failover')")
+as hv1 ovs-appctl netdev-dummy/receive vif3 $packet
+
+echo $packet > p2.expected
+OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [p2.expected])
+
+dnl p1 should NOT receive the packet.
+: > p1.expected
+OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [p1.expected])
+
+AS_BOX([Port security: disallowed MAC is dropped])
+
+as hv1 reset_pcap_file vif1 hv1/vif1
+as hv1 reset_pcap_file vif2 hv1/vif2
+as hv1 reset_pcap_file vif3 hv1/vif3
+
+dnl Send a packet from p1 with a MAC not in its port_security.
+dnl It should be dropped by ingress port security.
+rogue_mac=de:ad:be:ef:00:01
+packet=$(fmt_pkt "Ether(dst='${p3_mac}', src='${rogue_mac}')/ \
+                  IP(src='${p1_ip}', dst='${p3_ip}')/ \
+                  UDP(sport=1111, dport=2222)/ \
+                  Raw(b'rogue')")
+as hv1 ovs-appctl netdev-dummy/receive vif1 $packet
+
+dnl Neither p2 nor p3 should receive the rogue packet.
+: > p2.expected
+: > p3.expected
+OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [p2.expected])
+OVN_CHECK_PACKETS([hv1/vif3-tx.pcap], [p3.expected])
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([container port changed to normal port and then deleted])
 ovn_start
diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
index ce09bdce5d..ffc706dcef 100644
--- a/utilities/ovn-nbctl.8.xml
+++ b/utilities/ovn-nbctl.8.xml
@@ -916,7 +916,7 @@
             </p>
             <p>
               All logical switch ports of that type have implicit 'unknown'
-              addresses and FDB learning enabled (unless port security is set).
+              addresses and FDB learning enabled.
               This comes with all the positive and negative sides of the
               'unknown' address.  Static addresses-to-port mappings don't need
               to be maintained on each node and only the actually used mappings
-- 
2.55.0

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to