By default, northd does not install any responder flows for unicast ARP. These are intended to be forwarded to the destination so that the destination can respond appropriately. For VIF ports, this tends to work as expected in most scenarios.
When proxy ARP is configured on a logical switch port conntected to a logical router, then we install low priority flows to ensure that ARPs for the configured proxy addresses is responded to by the logical switch. These proxy ARP flows are hit when unicast ARP requests are sent for the VIF ports on the logical switch. We therefore end up responding incorrectly to unicast ARP requests with the proxy ARP MAC instead of forwarding the ARP request to the proper VIF port. This commit fixes the issue by installing explicit "next;" actions for unicast ARP requests directed towards VIFs so that the proxy ARP flows will not be hit. These flows are only installed if proxy ARP is configured, since they are unnecessary otherwise. Reported-at: https://issues.redhat.com/browse/FDP-1646 Signed-off-by: Mark Michelson <[email protected]> --- v4: * Changed the logic in northd to be similar to what was in v1. That is, instead of only installing flows for known overlaps of LSP and proxy ARP addresses, we install flows for all LSP addresses if proxy ARP is configured at all. This makes the logic in northd simpler, at the possible expense of adding more logical flows. * Added a check for scapy in the ovn.at test. v3: * Split the new build_lswitch_arp_nd_unicast_flows() function into smaller pieces. This has several benefits: * v2 had several checkpatch warnings about the lines exceeding 80 characters. This made it easier to stay under the line length limit. * v2 had a subtle indexing error that caused a crash in the proxy_arp test. Splitting this up helps make the indexing easier to follow. * This makes the logic much easier to understand, visually. * Fixed some typos in the commit message. v2: * This version also fixes the problem with IPv6 ND_NS packets. The tests have been updated to test for IPv6. * This version only installs unicast ARP/ND_NS flows if the LSP address overlaps with the configured proxy_arp addresses. The ovn-northd test has been updated to ensure that this is correct. The code that does this is pretty gnarly, since it's four levels of for loops. But it works! * Fixed nits from Xavier (removed ofport request, removed unnecessary lflow-list, etc.). --- northd/northd.c | 51 ++++++++++++++++++++++++----- northd/northd.h | 7 ++++ tests/ovn-northd.at | 79 +++++++++++++++++++++++++++++++++++++++++++++ tests/ovn.at | 77 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 206 insertions(+), 8 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index 3b1d3eba7..cdcb8550b 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -9956,15 +9956,32 @@ build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op, for (size_t i = 0; i < op->n_lsp_addrs; i++) { for (size_t j = 0; j < op->lsp_addrs[i].n_ipv4_addrs; j++) { ds_clear(match); - /* Do not reply on unicast ARPs, forward them to the target - * to have ability to monitor target liveness via unicast - * ARP requests. - */ ds_put_format(match, "arp.tpa == %s && " - "arp.op == 1 && " - "eth.dst == ff:ff:ff:ff:ff:ff", - op->lsp_addrs[i].ipv4_addrs[j].addr_s); + "arp.op == 1", op->lsp_addrs[i].ipv4_addrs[j].addr_s); + + /* Do not reply on unicast ARPs, forward them to the target + * to have ability to monitor target liveness via unicast + * ARP requests. If proxy arp is configured, then we need + * to set up flows to forward the packets. Otherwise, we + * could end up replying with the proxy ARP erroneously. + * Without proxy arp configured, these flows are + * unnecessary since the packets will hit the default + * "next" flow at priority 0. + */ + if (op->od->has_arp_proxy_port) { + size_t match_len = match->length; + ds_put_format(match, " && eth.dst == %s", + op->lsp_addrs[i].ea_s); + ovn_lflow_add_with_hint(lflows, op->od, + S_SWITCH_IN_ARP_ND_RSP, 50, + ds_cstr(match), + "next;", &op->nbsp->header_, + op->lflow_ref); + ds_truncate(match, match_len); + } + ds_put_cstr(match, " && eth.dst == ff:ff:ff:ff:ff:ff"); + ds_clear(actions); ds_put_format(actions, "eth.dst = eth.src; " @@ -10013,9 +10030,27 @@ build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op, * * - Do not reply for unicast ND solicitations. Let the target * reply to it, so that the sender has the ability to monitor - * the target liveness via the unicast ND solicitations. + * the target liveness via the unicast ND solicitations. Like + * with IPv4, if proxy ARP is configured, then we need to + * install unicast nd_ns flows that allow the packet to reach + * its target as intended. */ for (size_t j = 0; j < op->lsp_addrs[i].n_ipv6_addrs; j++) { + if (op->od->has_arp_proxy_port) { + ds_clear(match); + ds_put_format(match, + "nd_ns && ip6.dst == %s && nd.target == %s", + op->lsp_addrs[i].ipv6_addrs[j].addr_s, + op->lsp_addrs[i].ipv6_addrs[j].addr_s); + ovn_lflow_add_with_hint__(lflows, op->od, + S_SWITCH_IN_ARP_ND_RSP, 50, + ds_cstr(match), "next;", NULL, + copp_meter_get(COPP_ND_NA, + op->od->nbs->copp, + meter_groups), + &op->nbsp->header_, + op->lflow_ref); + } ds_clear(match); ds_put_format( match, diff --git a/northd/northd.h b/northd/northd.h index cdc532954..dc863ec48 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -471,6 +471,13 @@ struct ovn_datapath { /* Reference to the lflows belonging to this datapath currently router * only lflows. */ struct lflow_ref *datapath_lflows; + + /* This vector contains pointers to struct lport_addresses. These are + * the configured "arp_proxy" addresses of all logical switch ports on + * this datapath. The ovn_ports own these addresses, so we should not + * free them when destroying the ovn_datapath. + */ + struct vector proxy_arp_addrs; }; const struct ovn_datapath *ovn_datapath_find(const struct hmap *datapaths, diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 07fb57bd6..d6f2470c6 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -18716,3 +18716,82 @@ AT_CHECK( AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([Unicast ARP flows]) +ovn_start + +# Typically on logical switches, the ARP responder stage installs nothing +# for unicast ARPs towards VIF MACs. Instead, we rely on the default priority +# 1 "next;" action to move these ARPs through the pipeline, eventually resulting +# in the ARP reaching the destination. When proxy ARP is configured, we need +# to install explicit flows for the unicast ARPs at a higher priority. Otherwise +# the proxy ARP responder may respond with incorrect information. +# +# In this test, we are ensuring that the unicast flows in the ARP responder stage +# are only installed when proxy ARP is enabled. + +check ovn-nbctl ls-add ls1 +check ovn-nbctl lsp-add ls1 vm1 -- \ + lsp-set-addresses vm1 "00:00:00:00:00:02 192.168.0.2 fd01::2" + +check ovn-nbctl lr-add lr1 +check ovn-nbctl lrp-add lr1 lr1-ls1 00:00:00:00:00:01 192.168.0.1 fd01::1 +check ovn-nbctl lsp-add ls1 ls1-lr1 -- \ + lsp-set-addresses ls1-lr1 router -- \ + lsp-set-type ls1-lr1 router -- \ + lsp-set-options ls1-lr1 router-port=lr1-ls1 + +check ovn-nbctl --wait=sb sync + +# If we check the ARP responder flows in ls1, we should not see any unicast +# flows for vm1 (00:00:00:00:00:02). We also should not see any unicast +# IPv6 ND flows for vm1 (fd01::2) +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_arp_rsp | grep "eth.dst == 00:00:00:00:00:02" | ovn_strip_lflows], [0], [dnl +]) +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_arp_rsp | grep "ip6.dst == fd01::2" | ovn_strip_lflows], [0], [dnl +]) + +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_arp_rsp | grep "eth.dst == 00:00:00:00:00:01" | ovn_strip_lflows], [0], [dnl +]) +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_arp_rsp | grep "ip6.dst == fd01::1" | ovn_strip_lflows], [0], [dnl +]) + +# Add ARP proxy configuration on the router port. +check ovn-nbctl set logical_switch_port ls1-lr1 options:arp_proxy="192.168.0.0/24 fd01::/64" +check ovn-nbctl --wait=sb sync + +# Now that we have ARP proxy configured, we should see flows for +# vm1's MAC. +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_arp_rsp | grep "eth.dst == 00:00:00:00:00:02" | ovn_strip_lflows], [0], [dnl + table=??(ls_in_arp_rsp ), priority=50 , match=(arp.tpa == 192.168.0.2 && arp.op == 1 && eth.dst == 00:00:00:00:00:02), action=(next;) +]) +# And we should see unicast ND flows for vm1's IP address +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_arp_rsp | grep "ip6.dst == fd01::2" | ovn_strip_lflows], [0], [dnl + table=??(ls_in_arp_rsp ), priority=50 , match=(nd_ns && ip6.dst == fd01::2 && nd.target == fd01::2), action=(next;) +]) + +# We should also see an ARP flow for ls1-lr1's MAC. +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_arp_rsp | grep "eth.dst == 00:00:00:00:00:01" | ovn_strip_lflows], [0], [dnl + table=??(ls_in_arp_rsp ), priority=50 , match=(arp.tpa == 192.168.0.1 && arp.op == 1 && eth.dst == 00:00:00:00:00:01), action=(next;) +]) +# And we should see an ND flow for ls1-lr1's IPv6 address. +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_arp_rsp | grep "ip6.dst == fd01::1" | ovn_strip_lflows], [0], [dnl + table=??(ls_in_arp_rsp ), priority=50 , match=(nd_ns && ip6.dst == fd01::1 && nd.target == fd01::1), action=(next;) +]) + +check ovn-nbctl remove logical_switch_port ls1-lr1 options arp_proxy +check ovn-nbctl --wait=sb sync + +# We have removed ARP proxy, so we should no longer see a unicast ARP or ND flow. +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_arp_rsp | grep "eth.dst == 00:00:00:00:00:02" | ovn_strip_lflows], [0], [dnl +]) +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_arp_rsp | grep "ip6.dst == fd01::2" | ovn_strip_lflows], [0], [dnl +]) +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_arp_rsp | grep "eth.dst == 00:00:00:00:00:01" | ovn_strip_lflows], [0], [dnl +]) +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_arp_rsp | grep "ip6.dst == fd01::1" | ovn_strip_lflows], [0], [dnl +]) + +AT_CLEANUP +]) diff --git a/tests/ovn.at b/tests/ovn.at index 3f340e7b3..f8ad98917 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -44371,3 +44371,80 @@ check ovn-nbctl --wait=hv sync OVN_CLEANUP([hv1],[hv2],[hv3]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([Unicast ARP when proxy ARP is configured]) +AT_SKIP_IF([test $HAVE_SCAPY = no]) +ovn_start + +check ovn-nbctl ls-add ls1 +check ovn-nbctl lsp-add ls1 vm1 -- \ + lsp-set-addresses vm1 "00:00:00:00:00:02 10.0.0.2 fd01::2" +check ovn-nbctl lsp-add ls1 vm2 -- \ + lsp-set-addresses vm2 "00:00:00:00:00:03 10.0.0.3 fd01::3" + +check ovn-nbctl lr-add lr1 +check ovn-nbctl lrp-add lr1 lr1-ls1 00:00:00:00:00:01 10.0.0.1 fd01::1 +check ovn-nbctl lsp-add ls1 ls1-lr1 -- \ + lsp-set-addresses ls1-lr1 router -- \ + lsp-set-type ls1-lr1 router -- \ + lsp-set-options ls1-lr1 router-port=lr1-ls1 + +check ovn-nbctl --wait=hv sync + +net_add n1 +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +ovs-vsctl add-port br-int vif1 -- set Interface vif1 external-ids:iface-id=vm1 \ + options:tx_pcap=hv1/vif1-tx.pcap \ + options:rxq_pcap=hv1/vif1-rx.pcap +ovs-vsctl add-port br-int vif2 -- set Interface vif2 external-ids:iface-id=vm2 \ + options:tx_pcap=hv1/vif2-tx.pcap \ + options:rxq_pcap=hv1/vif2-rx.pcap + +OVN_POPULATE_ARP + +wait_for_ports_up + +arp=$(fmt_pkt "Ether(dst='00:00:00:00:00:02', src='00:00:00:00:00:03')/ \ + ARP(hwsrc='00:00:00:00:00:03', hwdst='00:00:00:00:00:02', + psrc='10.0.0.3', pdst='10.0.0.2')") +nd_ns=$(fmt_pkt "Ether(dst='00:00:00:00:00:02', src='00:00:00:00:00:03')/ \ + IPv6(src='fd01::3', dst='fd01::2')/ \ + ICMPv6ND_NS(tgt='fd01::2')") + +# If we send a unicast ARP from vm2 towards vm1, the ARP should be forwarded +# to vm1 by ls1. +as hv1 ovs-appctl netdev-dummy/receive vif2 $arp +echo $arp > expected +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], expected) + +# And if we send a unicast ND_NS from vm2 towards vm1, the ND_NS should be +# forwarded to vm1 by ls1. +as hv1 ovs-appctl netdev-dummy/receive vif2 $nd_ns +echo $nd_ns >> expected +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], expected) + + +# Add ARP proxy configuration on the router port. The subnet for the ARP proxy +# addresses overlaps with the VIF addresses for vm1 and vm2. +check ovn-nbctl set logical_switch_port ls1-lr1 options:arp_proxy="10.0.0.0/8 fd01::/64" +check ovn-nbctl --wait=hv sync + +# Sending a unicast ARP from vm2 towards vm1 should still result in the ARP being +# forwarded to vm1 by ls1. +as hv1 ovs-appctl netdev-dummy/receive vif2 $arp +echo $arp >> expected +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], expected) + +# Sending a unicast ND_NS from vm2 towards vm1 should still result in the ND_NS being +# forwarded to vm1 by ls1. +as hv1 ovs-appctl netdev-dummy/receive vif2 $nd_ns +echo $nd_ns >> expected +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], expected) + +OVN_CLEANUP([hv1]) +AT_CLEANUP +]) -- 2.50.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
