We already have the MAC address within the MAC binding row. There
is no need to flood the ARP request as this serves as liveness
check. Send the ARP probe as unicast instead.

The test wait_row_count checks are narrowed to specific IP/port
combinations because unicast ARP is not intercepted by the
pipeline and instead it travels to the target. That will result
in additional MAC bindings which will remain through out the test.

Acked-by: Ilya Maximets <[email protected]>
Signed-off-by: Ales Musil <[email protected]>
---
v2: Rebase on top of latest main.
    Mention the justification for the test update.
    Add Ilya's ack.
---
 controller/mac-cache.c |  3 ++-
 controller/pinctrl.c   | 20 +++++++++++++-------
 controller/pinctrl.h   |  3 ++-
 tests/ovn.at           | 29 +++++++++++++++--------------
 4 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/controller/mac-cache.c b/controller/mac-cache.c
index 58f42f2ea..8f100a29d 100644
--- a/controller/mac-cache.c
+++ b/controller/mac-cache.c
@@ -961,7 +961,8 @@ mac_binding_probe_stats_run(struct vector *stats_vec, 
uint64_t *req_delay,
             send_self_originated_neigh_packet(probe_data->swconn,
                                               sbrec->datapath->tunnel_key,
                                               pb->tunnel_key, laddr.ea,
-                                              &local, &mb->data.ip,
+                                              mb->data.mac, &local,
+                                              &mb->data.ip,
                                               OFTABLE_LOCAL_OUTPUT);
         }
 
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 83db495b5..216831e6e 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -5199,7 +5199,8 @@ send_arp_nd_update(const struct sbrec_port_binding *pb, 
const char *nexthop,
 void
 send_self_originated_neigh_packet(struct rconn *swconn,
                                   uint32_t dp_key, uint32_t port_key,
-                                  struct eth_addr eth,
+                                  struct eth_addr eth_src,
+                                  struct eth_addr eth_dst,
                                   struct in6_addr *local,
                                   struct in6_addr *target,
                                   uint8_t table_id)
@@ -5208,13 +5209,15 @@ send_self_originated_neigh_packet(struct rconn *swconn,
     struct dp_packet packet;
     dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
     if (!local) {
-        compose_rarp(&packet, eth);
+        compose_rarp(&packet, eth_src);
     } else if (IN6_IS_ADDR_V4MAPPED(local)) {
-        compose_arp(&packet, ARP_OP_REQUEST, eth, eth_addr_zero, true,
+        compose_arp(&packet, ARP_OP_REQUEST, eth_src, eth_dst,
+                    eth_addr_is_zero(eth_dst),
                     in6_addr_get_mapped_ipv4(local),
                     in6_addr_get_mapped_ipv4(target));
     } else {
-        compose_nd_ns(&packet, true, eth, eth_addr_zero, local, target);
+        compose_nd_ns(&packet, eth_addr_is_zero(eth_dst), eth_src,
+                      eth_dst, local, target);
     }
 
     /* Inject GARP request. */
@@ -5269,13 +5272,15 @@ send_garp_rarp(struct rconn *swconn, struct 
garp_rarp_node *garp_rarp,
         send_self_originated_neigh_packet(swconn,
                                           garp_rarp->dp_key,
                                           garp_rarp->port_key,
-                                          garp_rarp->ea, &addr, &addr,
+                                          garp_rarp->ea, eth_addr_zero,
+                                          &addr, &addr,
                                           OFTABLE_LOG_INGRESS_PIPELINE);
     } else {
         send_self_originated_neigh_packet(swconn,
                                           garp_rarp->dp_key,
                                           garp_rarp->port_key,
-                                          garp_rarp->ea, NULL, NULL,
+                                          garp_rarp->ea, eth_addr_zero,
+                                          NULL, NULL,
                                           OFTABLE_LOG_INGRESS_PIPELINE);
     }
 
@@ -6387,7 +6392,8 @@ send_arp_nd(struct rconn *swconn, struct arp_nd_data *e,
     /* Compose a ARP request packet. */
     send_self_originated_neigh_packet(swconn,
                                       e->dp_key, e->port_key,
-                                      e->ea, &e->src_ip, &e->dst_ip,
+                                      e->ea, eth_addr_zero,
+                                      &e->src_ip, &e->dst_ip,
                                       OFTABLE_LOCAL_OUTPUT);
 
     /* Set the next announcement.  At most 5 announcements are sent for a
diff --git a/controller/pinctrl.h b/controller/pinctrl.h
index 80384ac9b..0667ac34e 100644
--- a/controller/pinctrl.h
+++ b/controller/pinctrl.h
@@ -79,7 +79,8 @@ struct ovs_list *get_ports_to_activate_in_engine(void);
 bool pinctrl_is_port_activated(int64_t dp_key, int64_t port_key);
 void send_self_originated_neigh_packet(struct rconn *swconn,
                                        uint32_t dp_key, uint32_t port_key,
-                                       struct eth_addr eth,
+                                       struct eth_addr eth_src,
+                                       struct eth_addr eth_dst,
                                        struct in6_addr *local,
                                        struct in6_addr *target,
                                        uint8_t table_id);
diff --git a/tests/ovn.at b/tests/ovn.at
index 3ff535227..fd3be2adb 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -37201,8 +37201,8 @@ OVS_WAIT_UNTIL([test $(ovs-ofctl dump-flows br-int 
table=OFTABLE_MAC_CACHE_USE |
 send_udp hv1 vif1 00:00:00:00:10:00 00:00:00:00:10:2a 192.168.20.100 
192.168.10.100
 send_udp hv1 vif2 00:00:00:00:20:00 00:00:00:00:10:2b 192.168.10.100 
192.168.20.100
 # Wait for ARP requests to be generated.
-dump_arp 1 00:00:00:00:20:00 ff:ff:ff:ff:ff:ff 192.168.20.1 192.168.20.100 
00:00:00:00:00:00 > expected1
-dump_arp 1 00:00:00:00:10:00 ff:ff:ff:ff:ff:ff 192.168.10.1 192.168.10.100 
00:00:00:00:00:00 > expected2
+dump_arp 1 00:00:00:00:20:00 00:00:00:00:10:1b 192.168.20.1 192.168.20.100 
00:00:00:00:10:1b > expected1
+dump_arp 1 00:00:00:00:10:00 00:00:00:00:10:1a 192.168.10.1 192.168.10.100 
00:00:00:00:10:1a > expected2
 OVN_CHECK_PACKETS_CONTAIN([hv1/vif1-tx.pcap], [expected2])
 
 # Check MAC_Binding timestamp is updated receiving the ARP replay.
@@ -37211,8 +37211,8 @@ send_garp hv1 vif2 2 00:00:00:00:10:1b 
00:00:00:00:20:00 192.168.20.100 192.168.
 OVS_WAIT_UNTIL([test $(fetch_column Mac_Binding timestamp ip=192.168.10.100) 
-gt $ts0])
 
 # Refresh OFTABLE_MAC_BINDING entires.
-dump_arp 1 00:00:00:00:20:00 ff:ff:ff:ff:ff:ff 192.168.20.1 192.168.20.100 
00:00:00:00:00:00 >> expected1
-dump_arp 1 00:00:00:00:10:00 ff:ff:ff:ff:ff:ff 192.168.10.1 192.168.10.100 
00:00:00:00:00:00 >> expected2
+dump_arp 1 00:00:00:00:20:00 00:00:00:00:10:1b 192.168.20.1 192.168.20.100 
00:00:00:00:10:1b >> expected1
+dump_arp 1 00:00:00:00:10:00 00:00:00:00:10:1a 192.168.10.1 192.168.10.100 
00:00:00:00:10:1a >> expected2
 OVS_WAIT_UNTIL([test $(ovs-ofctl dump-flows br-int table=OFTABLE_MAC_CACHE_USE 
| \
                        awk '/arp_spa=192.168.20.100/{print substr($6,10,1)}') 
-ge $((aging_th/2))])
 send_udp hv1 vif1 00:00:00:00:10:00 00:00:00:00:10:2a 192.168.20.100 
192.168.10.100
@@ -37239,8 +37239,8 @@ OVS_WAIT_UNTIL([test $(ovs-ofctl dump-flows br-int 
table=OFTABLE_MAC_CACHE_USE |
 send_udp6 hv1 vif1 00:00:00:00:10:00 00:00:00:00:10:2a fd12::64 fd11::64
 send_udp6 hv1 vif2 00:00:00:00:20:00 00:00:00:00:10:2b fd11::64 fd12::64
 
-dump_ns 33:33:ff:00:00:64 00:00:00:00:10:00 ff02::1:ff00:64 fd11::1 fd11::64 > 
expected2
-dump_ns 33:33:ff:00:00:64 00:00:00:00:10:00 ff02::1:ff00:64 fd11::1 fd11::64 
>> expected2
+dump_ns 00:00:00:00:10:1a 00:00:00:00:10:00 fd11::64 fd11::1 fd11::64 > 
expected2
+dump_ns 00:00:00:00:10:1a 00:00:00:00:10:00 fd11::64 fd11::1 fd11::64 >> 
expected2
 OVN_CHECK_PACKETS_CONTAIN([hv1/vif1-tx.pcap], [expected2])
 
 send_na hv1 vif1 00:00:00:00:10:1a 00:00:00:00:10:00 fd11::64 fd11::1
@@ -37341,16 +37341,16 @@ send_udp6 hv1 vif1 00:00:00:00:10:00 
00:00:00:00:10:2b fd11::64 fd12::64
 
 # Verify ARP probes use the correct source IPs from matching subnets.
 # ARP for 10.10.10.100 must use source IP 10.10.10.1 (first subnet).
-dump_arp 1 00:00:00:00:10:00 ff:ff:ff:ff:ff:ff 10.10.10.1 10.10.10.100 
00:00:00:00:00:00 > expected
+dump_arp 1 00:00:00:00:10:00 00:00:00:00:10:1a 10.10.10.1 10.10.10.100 
00:00:00:00:10:1a > expected
 # ARP for 42.42.42.253 must use source IP 42.42.42.1 (second subnet).
-dump_arp 1 00:00:00:00:10:00 ff:ff:ff:ff:ff:ff 42.42.42.1 42.42.42.253 
00:00:00:00:00:00 >> expected
+dump_arp 1 00:00:00:00:10:00 00:00:00:00:10:1b 42.42.42.1 42.42.42.253 
00:00:00:00:10:1b >> expected
 OVN_CHECK_PACKETS_CONTAIN([hv1/vif1-tx.pcap], [expected])
 
 # Verify NS probes use the correct source IPs from matching subnets.
 # NS for fd11::64 must use source IP fd11::1 (first IPv6 subnet).
-dump_ns 33:33:ff:00:00:64 00:00:00:00:10:00 ff02::1:ff00:64 fd11::1 fd11::64 > 
expected_v6
+dump_ns 00:00:00:00:10:1a 00:00:00:00:10:00 fd11::64 fd11::1 fd11::64 > 
expected_v6
 # NS for fd12::64 must use source IP fd12::1 (second IPv6 subnet).
-dump_ns 33:33:ff:00:00:64 00:00:00:00:10:00 ff02::1:ff00:64 fd12::1 fd12::64 
>> expected_v6
+dump_ns 00:00:00:00:10:1b 00:00:00:00:10:00 fd12::64 fd12::1 fd12::64 >> 
expected_v6
 OVN_CHECK_PACKETS_CONTAIN([hv1/vif1-tx.pcap], [expected_v6])
 
 # Send ARP/NA replies and check MAC_Binding UUIDs remain consistent.
@@ -37427,8 +37427,8 @@ wait_row_count mac_binding 0 ip="$ext_ip" 
logical_port="lr-ls"
 
 # hv1 should have sent out arps for mac binding refresh
 arp_req=$(fmt_pkt \
-    "Ether(dst='ff:ff:ff:ff:ff:ff', src='$lrp_mac')/ \
-     ARP(hwsrc='$lrp_mac', hwdst='00:00:00:00:00:00', psrc='$lrp_ip', 
pdst='$ext_ip')")
+    "Ether(dst='$ext_mac', src='$lrp_mac')/ \
+     ARP(hwsrc='$lrp_mac', hwdst='$ext_mac', psrc='$lrp_ip', pdst='$ext_ip')")
 echo $arp_req >> hv1_snoopvif.expected
 OVN_CHECK_PACKETS_CONTAIN([hv1/snoopvif-tx.pcap], [hv1_snoopvif.expected])
 
@@ -37538,7 +37538,8 @@ send_imcp_echo_req hv1 public 00:00:00:00:10:00 
00:00:00:00:10:1a 192.168.20.2 1
 OVS_WAIT_UNTIL([test $(grep -c "Sending ARP/ND.*ip: 192.168.20.2" 
hv1/ovn-controller.log) -eq 2])
 
 check ovn-nbctl --wait=hv acl-del join
-wait_row_count mac_binding 0
+wait_row_count mac_binding 0 ip="192.168.10.100" logical_port="gw-public"
+wait_row_count mac_binding 0 ip="192.168.20.2" logical_port="gw-join"
 
 send_icmp6_echo_req hv1 public 00:00:00:00:10:00 00:00:00:00:50:01 fd12::2 
fd11::64
 wait_row_count mac_binding 1 mac=\"00:00:00:00:30:00\" ip=\"fd12::2\"
@@ -37555,7 +37556,7 @@ sleep $((aging_th / 2))
 send_icmp6_echo_req hv1 public 00:00:00:00:10:00 00:00:00:00:50:01 fd12::2 
fd11::64
 OVS_WAIT_UNTIL([test $(grep -c "Sending ARP/ND.*ip: fd12::2" 
hv1/ovn-controller.log) -eq 2])
 
-wait_row_count mac_binding 0
+wait_row_count mac_binding 0 ip=\"fd12::2\" logical_port="gw-join"
 
 OVN_CLEANUP([hv1])
 AT_CLEANUP
-- 
2.55.0

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

Reply via email to