On 8/6/26 8:40 AM, Ales Musil via dev wrote:
> When always_learn_from_arp_request=false we still allow to learn from
> requests targeted to the LRP own address. This was working for IPv4,
> but not for IPv6. Add missing flows to make sure it works for both
> IP families.
>
> Fixes: 783337b24c3f ("Learn the mac binding only if required")
> Reported-at: https://redhat.atlassian.net/browse/FDP-4201
> Assisted-by: Claude Opus 4.6, OpenCode
> Signed-off-by: Ales Musil <[email protected]>
> ---
Hi Ales,
Thanks for the fix!
> northd/northd.c | 62 +++++++++++++++++----
> tests/ovn-northd.at | 10 +++-
> tests/ovn.at | 130 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 189 insertions(+), 13 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 21eb70141..8cd85a328 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -15017,15 +15017,6 @@ build_neigh_learning_flows_for_lrouter(
> ds_cstr(actions), lflow_ref);
> }
>
> - ds_clear(actions);
> - ds_put_format(actions, REGBIT_LOOKUP_NEIGHBOR_RESULT
> - " = lookup_nd(inport, ip6.src, nd.sll); %snext;",
> - learn_from_arp_request ? "" :
> - REGBIT_LOOKUP_NEIGHBOR_IP_RESULT
> - " = lookup_nd_ip(inport, ip6.src); ");
> - ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 100, "nd_ns",
> - ds_cstr(actions), lflow_ref);
> -
> /* For other packet types, we can skip neighbor learning.
> * So set REGBIT_LOOKUP_NEIGHBOR_RESULT to 1. */
> ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 0, "1",
> @@ -15089,7 +15080,7 @@ build_neigh_learning_flows_for_lrouter_port(
> "always_learn_from_arp_request", true);
>
> /* Check if we need to learn mac-binding from ARP requests. */
> - for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> + for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> if (!learn_from_arp_request) {
> /* ARP request to this address should always get learned,
> * so add a priority-110 flow to set
> @@ -15135,6 +15126,57 @@ build_neigh_learning_flows_for_lrouter_port(
> WITH_HINT(&op->nbrp->header_));
> }
>
> + /* Check if we need to learn mac-binding from ND NS. */
> + for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) {
> + if (in6_is_lla(&op->lrp_networks.ipv6_addrs[i].addr)) {
> + continue;
> + }
> +
> + if (!learn_from_arp_request) {
> + /* ND NS request to this address should always get learned,
> + * so add a priority-110 flow to set
> + * REGBIT_LOOKUP_NEIGHBOR_IP_RESULT to 1. */
> + ds_clear(match);
> + ds_put_format(match,
> + "inport == %s && ip6.src == %s/%u && "
> + "nd.target == %s && nd_ns",
> + op->json_key,
> + op->lrp_networks.ipv6_addrs[i].network_s,
> + op->lrp_networks.ipv6_addrs[i].plen,
> + op->lrp_networks.ipv6_addrs[i].addr_s);
> + if (lrp_is_l3dgw(op)) {
> + ds_put_format(match, " && is_chassis_resident(%s)",
> + op->cr_port->json_key);
> + }
> + const char *actions_s = REGBIT_LOOKUP_NEIGHBOR_RESULT
> + " = lookup_nd(inport, ip6.src, nd.sll); "
> + REGBIT_LOOKUP_NEIGHBOR_IP_RESULT" = 1;"
> + " next;";
Nit: the indentation is a bit weird here but it matches what we have for
IPv4, let's keep it I guess.
> + ovn_lflow_add(lflows, op->od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 110,
> + ds_cstr(match), actions_s, lflow_ref,
> + WITH_HINT(&op->nbrp->header_));
> + }
> + ds_clear(match);
> + ds_put_format(match,
> + "inport == %s && ip6.src == %s/%u && nd_ns",
> + op->json_key,
> + op->lrp_networks.ipv6_addrs[i].network_s,
> + op->lrp_networks.ipv6_addrs[i].plen);
> + if (lrp_is_l3dgw(op)) {
> + ds_put_format(match, " && is_chassis_resident(%s)",
> + op->cr_port->json_key);
> + }
> + ds_clear(actions);
> + ds_put_format(actions, REGBIT_LOOKUP_NEIGHBOR_RESULT
> + " = lookup_nd(inport, ip6.src, nd.sll); %snext;",
> + learn_from_arp_request ? "" :
> + REGBIT_LOOKUP_NEIGHBOR_IP_RESULT
> + " = lookup_nd_ip(inport, ip6.src); ");
> + ovn_lflow_add(lflows, op->od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 100,
> + ds_cstr(match), ds_cstr(actions), lflow_ref,
> + WITH_HINT(&op->nbrp->header_));
> + }
> +
> if (lrp_is_l3dgw(op)) {
> ds_clear(match);
> ds_put_format(match, "inport == %s && (nd_na || nd_ns) && "
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index d3d9de3b3..13400ff5e 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -9184,7 +9184,7 @@ AT_SETUP([LR neighbor lookup and learning flows])
> ovn_start
>
> # Create logical routers
> -check ovn-nbctl --wait=sb lr-add lr0
> +check ovn-nbctl --wait=sb lr-add lr0 -- lrp-add lr0 lrp 00:00:00:00:00:01
> 192.168.0.1/24 fd10::1/96
>
> ovn-sbctl dump-flows lr0 > lrflows
> AT_CAPTURE_FILE([lrflows])
> @@ -9199,8 +9199,9 @@ AT_CHECK([cat lrflows | grep -e lr_in_lookup_neighbor
> -e lr_in_learn_neighbor |
> table=??(lr_in_learn_neighbor), priority=95 , match=(nd_ns && (ip6.src
> == 0 || nd.sll == 0)), action=(next;)
> table=??(lr_in_lookup_neighbor), priority=0 , match=(1),
> action=(reg9[[2]] = 1; next;)
> table=??(lr_in_lookup_neighbor), priority=100 , match=(arp.op == 2),
> action=(reg9[[2]] = lookup_arp(inport, arp.spa, arp.sha); next;)
> + table=??(lr_in_lookup_neighbor), priority=100 , match=(inport == "lrp" &&
> arp.spa == 192.168.0.0/24 && arp.op == 1), action=(reg9[[2]] =
> lookup_arp(inport, arp.spa, arp.sha); next;)
> + table=??(lr_in_lookup_neighbor), priority=100 , match=(inport == "lrp" &&
> ip6.src == fd10::/96 && nd_ns), action=(reg9[[2]] = lookup_nd(inport,
> ip6.src, nd.sll); next;)
> table=??(lr_in_lookup_neighbor), priority=100 , match=(nd_na),
> action=(reg9[[2]] = lookup_nd(inport, nd.target, nd.tll); next;)
> - table=??(lr_in_lookup_neighbor), priority=100 , match=(nd_ns),
> action=(reg9[[2]] = lookup_nd(inport, ip6.src, nd.sll); next;)
> table=??(lr_in_lookup_neighbor), priority=105 , match=(nd_na && nd.tll ==
> 0), action=(reg9[[2]] = lookup_nd(inport, nd.target, eth.src); next;)
> ])
>
> @@ -9219,9 +9220,12 @@ AT_CHECK([cat lrflows | grep -e lr_in_lookup_neighbor
> -e lr_in_learn_neighbor |
> table=??(lr_in_learn_neighbor), priority=95 , match=(nd_ns && (ip6.src
> == 0 || nd.sll == 0)), action=(next;)
> table=??(lr_in_lookup_neighbor), priority=0 , match=(1),
> action=(reg9[[2]] = 1; next;)
> table=??(lr_in_lookup_neighbor), priority=100 , match=(arp.op == 2),
> action=(reg9[[2]] = lookup_arp(inport, arp.spa, arp.sha); reg9[[3]] = 1;
> next;)
> + table=??(lr_in_lookup_neighbor), priority=100 , match=(inport == "lrp" &&
> arp.spa == 192.168.0.0/24 && arp.op == 1), action=(reg9[[2]] =
> lookup_arp(inport, arp.spa, arp.sha); reg9[[3]] = lookup_arp_ip(inport,
> arp.spa); next;)
> + table=??(lr_in_lookup_neighbor), priority=100 , match=(inport == "lrp" &&
> ip6.src == fd10::/96 && nd_ns), action=(reg9[[2]] = lookup_nd(inport,
> ip6.src, nd.sll); reg9[[3]] = lookup_nd_ip(inport, ip6.src); next;)
> table=??(lr_in_lookup_neighbor), priority=100 , match=(nd_na),
> action=(reg9[[2]] = lookup_nd(inport, nd.target, nd.tll); reg9[[3]] = 1;
> next;)
> - table=??(lr_in_lookup_neighbor), priority=100 , match=(nd_ns),
> action=(reg9[[2]] = lookup_nd(inport, ip6.src, nd.sll); reg9[[3]] =
> lookup_nd_ip(inport, ip6.src); next;)
> table=??(lr_in_lookup_neighbor), priority=105 , match=(nd_na && nd.tll ==
> 0), action=(reg9[[2]] = lookup_nd(inport, nd.target, eth.src); reg9[[3]] = 1;
> next;)
> + table=??(lr_in_lookup_neighbor), priority=110 , match=(inport == "lrp" &&
> arp.spa == 192.168.0.0/24 && arp.tpa == 192.168.0.1 && arp.op == 1),
> action=(reg9[[2]] = lookup_arp(inport, arp.spa, arp.sha); reg9[[3]] = 1;
> next;)
> + table=??(lr_in_lookup_neighbor), priority=110 , match=(inport == "lrp" &&
> ip6.src == fd10::/96 && nd.target == fd10::1 && nd_ns), action=(reg9[[2]] =
> lookup_nd(inport, ip6.src, nd.sll); reg9[[3]] = 1; next;)
> table=??(lr_in_lookup_neighbor), priority=110 , match=(nd_na && ip6.src
> == fe80::/10 && ip6.dst == ff00::/8), action=(reg9[[2]] = lookup_nd(inport,
> nd.target, nd.tll); reg9[[3]] = lookup_nd_ip(inport, nd.target); next;)
> table=??(lr_in_lookup_neighbor), priority=115 , match=(nd_na && nd.tll ==
> 0 && ip6.src == fe80::/10 && ip6.dst == ff00::/8), action=(reg9[[2]] =
> lookup_nd(inport, nd.target, eth.src); reg9[[3]] = lookup_nd_ip(inport,
> nd.target); next;)
> ])
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 4324c2244..272a22db6 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -5740,6 +5740,136 @@ OVN_CLEANUP([hv1], [hv2])
> AT_CLEANUP
> ])
>
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([MAC binding learning from ARP request and ND NS])
> +CHECK_SCAPY
> +ovn_start
> +
> +dnl Logical network:
> +dnl
> +dnl One logical router lr0 connected to two logical switches.
> +dnl ls-ext has a localnet port ln0 (physnet0) and a router attachment
> +dnl for lrp0. ls-int has one VIF lsp1 on hv1.
> +dnl
> +dnl lrp0 is a distributed gateway port (172.18.0.3/16, fd00::3/64),
> +dnl resident on hv1. On hv2 the LS ARP/ND responder for lrp0 is
> +dnl conditioned with is_chassis_resident("cr-lrp0"), so ARP/NS
> +dnl injected via the localnet on hv2 bypasses the LS responder and
> +dnl reaches the router pipeline where lr_in_lookup_neighbor runs.
> +
> +check ovn-nbctl lr-add lr0
> +check ovn-nbctl ls-add ls-ext
> +check ovn-nbctl ls-add ls-int
> +
> +check ovn-nbctl lrp-add lr0 lrp0 00:00:00:00:ff:01 \
> + 172.18.0.3/16 fd00::3/64
> +check ovn-nbctl lsp-add-router-port ls-ext lrp0-attach lrp0
> +
> +check ovn-nbctl lrp-add lr0 lrp1 00:00:00:00:ff:02 192.168.1.1/24
> +check ovn-nbctl lsp-add-router-port ls-int lrp1-attach lrp1
> +
> +check ovn-nbctl lsp-add ls-int lsp1 \
> + -- lsp-set-addresses lsp1 "f0:00:00:00:00:01 192.168.1.10"
> +
> +check ovn-nbctl lsp-add-localnet-port ls-ext ln0 physnet0
> +check ovn-nbctl lrp-set-gateway-chassis lrp0 hv1 20
> +
> +net_add n1
> +for i in 1 2; do
> + sim_add hv$i
> + as hv$i
> + ovs-vsctl add-br br-phys
> + ovn_attach n1 br-phys 192.168.0.$i
> + as hv$i ovs-vsctl set open .
> external-ids:ovn-bridge-mappings=physnet0:br-phys
> +done
> +
> +as hv1 ovs-vsctl \
> + -- add-port br-int vif1 \
> + -- set Interface vif1 external_ids:iface-id=lsp1
Nit: missing check calls for a bunch of these.
> +
> +dnl Add a dummy port on hv2's br-phys to inject external packets.
> +as hv2 ovs-vsctl \
> + -- add-port br-phys ext1 \
> + -- set Interface ext1 type=internal
> +
> +OVN_POPULATE_ARP
> +wait_for_ports_up
> +check ovn-nbctl --wait=hv sync
> +
> +dnl External device's addresses.
> +sha=f0:00:00:00:10:01
> +spa=172.18.0.100
> +spa6=fd00::100
> +
> +dnl Router port addresses (target of ARP request / ND NS).
> +router_ip=172.18.0.3
> +router_ip6=fd00::3
> +
> +AS_BOX([always_learn_from_arp_request=false])
> +check ovn-nbctl --wait=hv set logical_router lr0 \
> + options:always_learn_from_arp_request=false
> +
> +AS_BOX([ARP request targeting router own IPv4])
> +packet=$(dump_arp 1 $sha ff:ff:ff:ff:ff:ff $spa $router_ip 00:00:00:00:00:00)
> +as hv2 ovs-appctl netdev-dummy/receive ext1 $packet
> +wait_row_count MAC_Binding 1 ip="$spa" mac=\"$sha\"
> +
> +check ovn-sbctl --all destroy mac_binding
> +check ovn-nbctl --wait=hv sync
> +
> +AS_BOX([ND NS targeting router own IPv6])
> +packet=$(dump_ns 33:33:ff:00:00:03 $sha ff02::1:ff00:3 $spa6 $router_ip6)
> +as hv2 ovs-appctl netdev-dummy/receive ext1 $packet
> +wait_row_count MAC_Binding 1 ip=\"$spa6\" mac=\"$sha\"
> +
> +check ovn-sbctl --all destroy mac_binding
> +check ovn-nbctl --wait=hv sync
> +
> +AS_BOX([GARP for unknown IP - no binding expected])
> +unknown_ip=172.18.0.200
> +packet=$(dump_arp 1 $sha ff:ff:ff:ff:ff:ff $unknown_ip $unknown_ip
> 00:00:00:00:00:00)
> +as hv2 ovs-appctl netdev-dummy/receive ext1 $packet
> +check ovn-nbctl --wait=hv sync
> +sleep 1
> +check_row_count MAC_Binding 0 ip="$unknown_ip"
> +
> +AS_BOX([ND NS for unknown IPv6 - no binding expected])
> +unknown_ip6=fd00::200
> +packet=$(dump_ns 33:33:ff:00:02:00 $sha ff02::1:ff00:200 $spa6 $unknown_ip6)
> +as hv2 ovs-appctl netdev-dummy/receive ext1 $packet
> +check ovn-nbctl --wait=hv sync
> +sleep 1
> +check_row_count MAC_Binding 0 ip=\"$spa6\"
> +
> +AS_BOX([Update existing ARP binding with new MAC])
> +check ovn-sbctl --all destroy mac_binding
> +check ovn-nbctl --wait=hv sync
> +
> +packet=$(dump_arp 1 $sha ff:ff:ff:ff:ff:ff $spa $router_ip 00:00:00:00:00:00)
> +as hv2 ovs-appctl netdev-dummy/receive ext1 $packet
> +wait_row_count MAC_Binding 1 ip="$spa" mac=\"$sha\"
> +check ovn-nbctl --wait=hv sync
> +
> +sha2=f0:00:00:00:10:02
> +packet=$(dump_arp 1 $sha2 ff:ff:ff:ff:ff:ff $spa $router_ip
> 00:00:00:00:00:00)
> +as hv2 ovs-appctl netdev-dummy/receive ext1 $packet
> +wait_row_count MAC_Binding 1 ip="$spa" mac=\"$sha2\"
> +
> +AS_BOX([Update existing ND binding with new MAC])
> +packet=$(dump_ns 33:33:ff:00:00:03 $sha ff02::1:ff00:3 $spa6 $router_ip6)
> +as hv2 ovs-appctl netdev-dummy/receive ext1 $packet
> +wait_row_count MAC_Binding 1 ip=\"$spa6\" mac=\"$sha\"
> +check ovn-nbctl --wait=hv sync
> +
> +packet=$(dump_ns 33:33:ff:00:00:03 $sha2 ff02::1:ff00:3 $spa6 $router_ip6)
> +as hv2 ovs-appctl netdev-dummy/receive ext1 $packet
> +wait_row_count MAC_Binding 1 ip=\"$spa6\" mac=\"$sha2\"
> +
> +OVN_CLEANUP([hv1], [hv2])
> +
> +AT_CLEANUP
> +])
> +
> # 3 hypervisors, one logical switch, 3 logical ports per hypervisor
> OVN_FOR_EACH_NORTHD([
> AT_SETUP([portsecurity : 3 HVs, 1 LS, 3 lports/HV])
I took care of the minor issues in the test and applied the patch to
main, 26.03 and 25.09.
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
