controller: Send ARP/ND for stale mac_bindings entries.

Introduce the capability to actively refresh mac_binding entries available
in OFTABLE_MAC_BINDING table when they are inactive for more than the
configured threshold. Add the OFTABLE_MAC_BINDING table stats
monitoring. This feature avoids possible unnecessary mac_entry removals
if the destination is still alive but inactive for more than the configured
value.
Reduce threshold dump_period and cooldown_period to 3/16 of threshold
value in order to not miss any re-arping time slot.

Acked-by: Ales Musil <[email protected]>
Acked-by: Mark Michelson <[email protected]>
Reported-at: https://issues.redhat.com/browse/FDP-1135
Signed-off-by: Lorenzo Bianconi <[email protected]>
Signed-off-by: Mark Michelson <[email protected]>

Author: LorenzoBianconi

controller/mac-cache.c

@@ -16,6 +16,7 @@
 #include <config.h>
 #include <stdbool.h>
 
+#include "lflow.h"
 #include "lib/mac-binding-index.h"
 #include "local_data.h"
 #include "lport.h"
@@ -24,6 +25,7 @@
 #include "openvswitch/vlog.h"
 #include "ovn/logical-fields.h"
 #include "ovn-sb-idl.h"
+#include "pinctrl.h"
 
 VLOG_DEFINE_THIS_MODULE(mac_cache);
 
@@ -90,15 +92,20 @@ mac_cache_threshold_add(struct mac_cache_data *data,
     threshold = xmalloc(sizeof *threshold);
     threshold->dp_key = dp->tunnel_key;
     threshold->value = value;
-    threshold->dump_period = value / 2;
-    threshold->cooldown_period = value / 4;
+    threshold->dump_period = (3 * value) / 16;
+    threshold->cooldown_period = (3 * value) / 16;
 
     /* (cooldown_period + dump_period) is the maximum time the timestamp may
-     * be not updated.  So, the sum of those times must be lower than the
-     * threshold, otherwise we may fail to update an active MAC binding in
+     * be not updated for an entry with IP + MAC combination from which we see
+     * incoming traffic.  For the entry that is used only in Tx direction
+     * (e.g., an entry for a default gateway of the chassis) this time is
+     * doubled, because an ARP/ND probe will need to be sent first and the
+     * (cooldown_period + dump_period) will be the maximum time between such
+     * probes.  Hence, 2 * (cooldown_period + dump_period) should be less than
+     * a threshold, otherwise we may fail to update an active MAC binding in
      * time and risk it being removed.  Giving it an extra 1/10 of the time
      * for all the processing that needs to happen. */
-    ovs_assert(threshold->cooldown_period + threshold->dump_period
+    ovs_assert(2 * (threshold->cooldown_period + threshold->dump_period)
                < (9 * value) / 10);
 
     hmap_insert(&data->thresholds, &threshold->hmap_node, dp->tunnel_key);
@@ -399,8 +406,10 @@ mac_binding_update_log(const char *action,
 }
 
 void
-mac_binding_stats_run(struct ovs_list *stats_list, uint64_t *req_delay,
-                      void *data)
+mac_binding_stats_run(
+        struct rconn *swconn OVS_UNUSED,
+        struct ovsdb_idl_index *sbrec_port_binding_by_name OVS_UNUSED,
+        struct ovs_list *stats_list, uint64_t *req_delay, void *data)
 {
     struct mac_cache_data *cache_data = data;
     long long timewall_now = time_wall_msec();
@@ -495,8 +504,10 @@ fdb_update_log(const char *action,
 }
 
 void
-fdb_stats_run(struct ovs_list *stats_list, uint64_t *req_delay,
-              void *data)
+fdb_stats_run(struct rconn *swconn OVS_UNUSED,
+              struct ovsdb_idl_index *sbrec_port_binding_by_name OVS_UNUSED,
+              struct ovs_list *stats_list,
+              uint64_t *req_delay, void *data)
 {
     struct mac_cache_data *cache_data = data;
     long long timewall_now = time_wall_msec();
@@ -847,3 +858,112 @@ buffered_packets_db_lookup(struct buffered_packets *bp, struct ds *ip,
 
     eth_addr_from_string(smb->mac, mac);
 }
+
+void
+mac_binding_probe_stats_process_flow_stats(
+        struct ovs_list *stats_list,
+        struct ofputil_flow_stats *ofp_stats)
+{
+    struct mac_cache_stats *stats = xmalloc(sizeof *stats);
+
+    stats->idle_age_ms = ofp_stats->idle_age * 1000;
+    stats->data.mb = (struct mac_binding_data) {
+        .cookie = ntohll(ofp_stats->cookie),
+        /* The port_key must be zero to match mac_binding_data_from_sbrec. */
+        .port_key = 0,
+        .dp_key = ntohll(ofp_stats->match.flow.metadata),
+    };
+
+    if (ofp_stats->match.flow.regs[0]) {
+        stats->data.mb.ip =
+            in6_addr_mapped_ipv4(htonl(ofp_stats->match.flow.regs[0]));
+    } else {
+        ovs_be128 ip6 = hton128(flow_get_xxreg(&ofp_stats->match.flow, 1));
+        memcpy(&stats->data.mb.ip, &ip6, sizeof stats->data.mb.ip);
+    }
+
+    ovs_list_push_back(stats_list, &stats->list_node);
+}
+
+void
+mac_binding_probe_stats_run(
+        struct rconn *swconn,
+        struct ovsdb_idl_index *sbrec_port_binding_by_name,
+        struct ovs_list *stats_list,
+        uint64_t *req_delay, void *data)
+{
+    long long timewall_now = time_wall_msec();
+    struct mac_cache_data *cache_data = data;
+
+    struct mac_cache_stats *stats;
+    LIST_FOR_EACH_POP (stats, list_node, stats_list) {
+        struct mac_binding *mb = mac_binding_find(&cache_data->mac_bindings,
+                                                  &stats->data.mb);
+        if (!mb) {
+            mac_binding_update_log("Probe: not found in the cache:",
+                                   &stats->data.mb, false, NULL, 0, 0);
+            free(stats);
+            continue;
+        }
+
+        struct mac_cache_threshold *threshold =
+                mac_cache_threshold_find(cache_data, mb->data.dp_key);
+        uint64_t since_updated_ms = timewall_now - mb->sbrec->timestamp;
+        const struct sbrec_mac_binding *sbrec = mb->sbrec;
+
+        if (stats->idle_age_ms > threshold->value) {
+            mac_binding_update_log("Not sending ARP/ND request for non-active",
+                                   &mb->data, true, threshold,
+                                   stats->idle_age_ms, since_updated_ms);
+            free(stats);
+            continue;
+        }
+
+        if (since_updated_ms < threshold->cooldown_period) {
+            mac_binding_update_log(
+                    "Not sending ARP/ND request for recently updated",
+                    &mb->data, true, threshold, stats->idle_age_ms,
+                    since_updated_ms);
+            free(stats);
+            continue;
+        }
+
+        const struct sbrec_port_binding *pb =
+            lport_lookup_by_name(sbrec_port_binding_by_name,
+                                 sbrec->logical_port);
+        if (!pb) {
+            free(stats);
+            continue;
+        }
+
+        struct lport_addresses laddr;
+        if (!extract_lsp_addresses(pb->mac[0], &laddr)) {
+            free(stats);
+            continue;
+        }
+
+        if (laddr.n_ipv4_addrs || laddr.n_ipv6_addrs) {
+            struct in6_addr local = laddr.n_ipv4_addrs
+                ? in6_addr_mapped_ipv4(laddr.ipv4_addrs[0].addr)
+                : laddr.ipv6_addrs[0].addr;
+
+            mac_binding_update_log("Sending ARP/ND request for active",
+                                   &mb->data, true, threshold,
+                                   stats->idle_age_ms, since_updated_ms);
+
+            send_self_originated_neigh_packet(swconn,
+                                              sbrec->datapath->tunnel_key,
+                                              pb->tunnel_key, laddr.ea,
+                                              &local, &mb->data.ip,
+                                              OFTABLE_LOCAL_OUTPUT);
+        }
+
+        free(stats);
+        destroy_lport_addresses(&laddr);
+    }
+
+    mac_cache_update_req_delay(&cache_data->thresholds, req_delay);
+    if (*req_delay) {
+        VLOG_DBG("MAC probe binding statistics delay: %"PRIu64, *req_delay);
+    }
+}

controller/mac-cache.h

@@ -180,15 +180,19 @@ void
 mac_binding_stats_process_flow_stats(struct ovs_list *stats_list,
                                      struct ofputil_flow_stats *ofp_stats);
 
-void mac_binding_stats_run(struct ovs_list *stats_list, uint64_t *req_delay,
-                           void *data);
+void mac_binding_stats_run(
+        struct rconn *swconn OVS_UNUSED,
+        struct ovsdb_idl_index *sbrec_port_binding_by_name OVS_UNUSED,
+        struct ovs_list *stats_list, uint64_t *req_delay, void *data);
 
 /* FDB stat processing. */
 void fdb_stats_process_flow_stats(struct ovs_list *stats_list,
                                   struct ofputil_flow_stats *ofp_stats);
 
-void fdb_stats_run(struct ovs_list *stats_list, uint64_t *req_delay,
-                   void *data);
+void fdb_stats_run(
+        struct rconn *swconn OVS_UNUSED,
+        struct ovsdb_idl_index *sbrec_port_binding_by_name OVS_UNUSED,
+        struct ovs_list *stats_list, uint64_t *req_delay, void *data);
 
 void mac_cache_stats_destroy(struct ovs_list *stats_list);
 
@@ -221,4 +225,13 @@ bool buffered_packets_ctx_is_ready_to_send(struct buffered_packets_ctx *ctx);
 
 bool buffered_packets_ctx_has_packets(struct buffered_packets_ctx *ctx);
 
+void mac_binding_probe_stats_process_flow_stats(
+        struct ovs_list *stats_list,
+        struct ofputil_flow_stats *ofp_stats);
+
+void mac_binding_probe_stats_run(
+        struct rconn *swconn,
+        struct ovsdb_idl_index *sbrec_port_binding_by_name,
+        struct ovs_list *stats_list, uint64_t *req_delay, void *data);
+
 #endif /* controller/mac-cache.h */

controller/ovn-controller.c

@@ -6342,7 +6342,8 @@ main(int argc, char *argv[])
 
                     mac_cache_data = engine_get_data(&en_mac_cache);
                     if (mac_cache_data) {
-                        statctrl_run(ovnsb_idl_txn, mac_cache_data);
+                        statctrl_run(ovnsb_idl_txn, sbrec_port_binding_by_name,
+                                     mac_cache_data);
                     }
 
                     ofctrl_seqno_update_create(

controller/pinctrl.c

@@ -5303,7 +5303,7 @@ send_garp_rarp_delete(const char *lport)
     notify_pinctrl_handler();
 }
 
-static void
+void
 send_self_originated_neigh_packet(struct rconn *swconn,
                                   uint32_t dp_key, uint32_t port_key,
                                   struct eth_addr eth,

controller/pinctrl.h

@@ -31,6 +31,7 @@ struct ovsdb_idl_index;
 struct ovsdb_idl_txn;
 struct ovsrec_bridge;
 struct ovsrec_open_vswitch_table;
+struct rconn;
 struct sbrec_chassis;
 struct sbrec_dns_table;
 struct sbrec_controller_event_table;
@@ -77,4 +78,10 @@ struct activated_port {
 void tag_port_as_activated_in_engine(struct activated_port *ap);
 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 in6_addr *local,
+                                       struct in6_addr *target,
+                                       uint8_t table_id);
 #endif /* controller/pinctrl.h */

controller/statctrl.c

@@ -39,6 +39,7 @@ VLOG_DEFINE_THIS_MODULE(statctrl);
 enum stat_type {
     STATS_MAC_BINDING = 0,
     STATS_FDB,
+    STATS_MAC_BINDING_PROBE,
     STATS_MAX,
 };
 
@@ -62,7 +63,10 @@ struct stats_node {
                                struct ofputil_flow_stats *ofp_stats);
     /* Function to process the parsed stats.
      * This function runs in main thread locked behind mutex. */
-    void (*run)(struct ovs_list *stats_list, uint64_t *req_delay, void *data);
+    void (*run)(struct rconn *swconn,
+                struct ovsdb_idl_index *sbrec_port_binding_by_name,
+                struct ovs_list *stats_list,
+                uint64_t *req_delay, void *data);
 };
 
 #define STATS_NODE(NAME, REQUEST, DESTROY, PROCESS, RUN)                   \
@@ -145,20 +149,37 @@ statctrl_init(void)
     STATS_NODE(FDB, fdb_request, mac_cache_stats_destroy,
                fdb_stats_process_flow_stats, fdb_stats_run);
 
+    struct ofputil_flow_stats_request mac_binding_probe_request = {
+            .cookie = htonll(0),
+            .cookie_mask = htonll(0),
+            .out_port = OFPP_ANY,
+            .out_group = OFPG_ANY,
+            .table_id = OFTABLE_MAC_BINDING,
+    };
+    STATS_NODE(MAC_BINDING_PROBE, mac_binding_probe_request,
+               mac_cache_stats_destroy,
+               mac_binding_probe_stats_process_flow_stats,
+               mac_binding_probe_stats_run);
+
     statctrl_ctx.thread = ovs_thread_create("ovn_statctrl",
                                             statctrl_thread_handler,
                                             &statctrl_ctx);
 }
 
 void
 statctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
+             struct ovsdb_idl_index *sbrec_port_binding_by_name,
              struct mac_cache_data *mac_cache_data)
 {
     if (!ovnsb_idl_txn) {
         return;
     }
 
-    void *node_data[STATS_MAX] = {mac_cache_data, mac_cache_data};
+    void *node_data[STATS_MAX] = {
+        mac_cache_data,
+        mac_cache_data,
+        mac_cache_data
+    };
 
     bool schedule_updated = false;
     long long now = time_msec();
@@ -168,7 +189,9 @@ statctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
         struct stats_node *node = &statctrl_ctx.nodes[i];
         uint64_t prev_delay = node->request_delay;
 
-        node->run(&node->stats_list, &node->request_delay, node_data[i]);
+        node->run(statctrl_ctx.swconn,
+                  sbrec_port_binding_by_name, &node->stats_list,
+                  &node->request_delay, node_data[i]);
 
         schedule_updated |=
                 statctrl_update_next_request_timestamp(node, now, prev_delay);

controller/statctrl.h

@@ -20,6 +20,7 @@
 
 void statctrl_init(void);
 void statctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
+                  struct ovsdb_idl_index *sbrec_port_binding_by_name,
                   struct mac_cache_data *mac_cache_data);
 
 void statctrl_update_swconn(const char *target, int probe_interval);