| Message ID | 20261002142940.2556236-3-dceara@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ales Musil |
| Headers | show |
| Series | Fix FDB learning for VRRP scenarios. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Fri, Oct 2, 2026 at 8:34 PM Dumitru Ceara via dev < ovs-dev@openvswitch.org> wrote: > 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 <dceara@redhat.com> > --- > 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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Hi Dumitru, same as in the 1/2 aren't we missing update to ovn-logical-flows.7.rst? Other than that it looks good. Regards, Ales
On 10/5/26 10:31 AM, Ales Musil wrote: > On Fri, Oct 2, 2026 at 8:34 PM Dumitru Ceara via dev < > ovs-dev@openvswitch.org> wrote: > >> 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 <dceara@redhat.com> >> --- ... >> >> > Hi Dumitru, > Hi Ales, > same as in the 1/2 aren't we missing update to ovn-logical-flows.7.rst? Yeah we are, I'll do that in v2. > Other than that it looks good. > > Regards, > Ales > Thanks, Dumitru
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
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 <dceara@redhat.com> --- 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(-)