[ovs-dev,RFC,5/7] controller: Keep the content of route notifications.

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 931fb95dccb3..7fa8f2adb563 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -94,6 +94,7 @@ 
 #include "acl-ids.h"
 #include "route.h"
 #include "route-exchange.h"
+#include "route-exchange-netlink.h"
 #include "route-table.h"
 #include "garp_rarp.h"
 #include "host-if-monitor.h"
@@ -5712,8 +5713,21 @@  route_table_notify_update(struct vector *watches)
 struct ed_type_route_table_notify {
     /* Vector of ordered 'uint32_t' representing table_ids. */
     struct vector watches;
+    /* Routes ('struct ovn_route_msg *', owned) the last run was told about,
+     * limited to the tables in 'watches'. */
+    struct vector changed_routes;
 };
 
+static void
+route_table_notify_clear_changes(struct ed_type_route_table_notify *rtn)
+{
+    struct ovn_route_msg *msg;
+    VECTOR_FOR_EACH (&rtn->changed_routes, msg) {
+        free(msg);
+    }
+    vector_clear(&rtn->changed_routes);
+}
+
 /* The kernel nexthop table is shared by the features below, each of them
  * independently declares whether it needs it to be tracked. */
 enum nexthop_exchange_user {
@@ -5888,40 +5902,35 @@  en_route_exchange_cleanup(void *data)
 static enum engine_node_state
 en_route_table_notify_run(struct engine_node *node OVS_UNUSED, void *data)
 {
+    static const enum ovn_netlink_notifier_type route_notifiers[] = {
+        OVN_NL_NOTIFIER_ROUTE_V4, OVN_NL_NOTIFIER_ROUTE_V6,
+    };
     enum engine_node_state state = EN_UNCHANGED;
     struct ed_type_route_table_notify *rtn = data;
-    struct vector *msgs;
-    uint32_t *table_id;
 
-    /* We cannot tell whether a table we watch was among the changes we
-     * missed, so assume it was. */
-    if (ovn_netlink_notifier_lost(OVN_NL_NOTIFIER_ROUTE_V4) ||
-        ovn_netlink_notifier_lost(OVN_NL_NOTIFIER_ROUTE_V6)) {
-        state = EN_UPDATED;
-    }
+    route_table_notify_clear_changes(rtn);
 
-    if (state != EN_UPDATED) {
-        msgs = ovn_netlink_get_msgs(OVN_NL_NOTIFIER_ROUTE_V4);
-        VECTOR_FOR_EACH_PTR (msgs, table_id) {
-            if (vector_bsearch(&rtn->watches, table_id, table_id_cmp)) {
-                state = EN_UPDATED;
-                break;
-            }
+    for (size_t i = 0; i < ARRAY_SIZE(route_notifiers); i++) {
+        /* We cannot tell whether a table we watch was among the changes we
+         * missed, so assume it was. */
+        if (ovn_netlink_notifier_lost(route_notifiers[i])) {
+            state = EN_UPDATED;
         }
-    }
 
-    if (state != EN_UPDATED) {
-        msgs = ovn_netlink_get_msgs(OVN_NL_NOTIFIER_ROUTE_V6);
-        VECTOR_FOR_EACH_PTR (msgs, table_id) {
-            if (vector_bsearch(&rtn->watches, table_id, table_id_cmp)) {
-                state = EN_UPDATED;
-                break;
+        struct vector *msgs = ovn_netlink_get_msgs(route_notifiers[i]);
+        struct ovn_route_msg *msg;
+        VECTOR_FOR_EACH (msgs, msg) {
+            if (!vector_bsearch(&rtn->watches, &msg->table_id, table_id_cmp)) {
+                continue;
             }
+
+            struct ovn_route_msg *changed_route = ovn_route_msg_clone(msg);
+            vector_push(&rtn->changed_routes, &changed_route);
+            state = EN_UPDATED;
         }
-    }
 
-    ovn_netlink_notifier_flush(OVN_NL_NOTIFIER_ROUTE_V4);
-    ovn_netlink_notifier_flush(OVN_NL_NOTIFIER_ROUTE_V6);
+        ovn_netlink_notifier_flush(route_notifiers[i]);
+    }
 
     return state;
 }
@@ -5935,6 +5944,7 @@  en_route_table_notify_init(struct engine_node *node OVS_UNUSED,
 
     *rtn = (struct ed_type_route_table_notify) {
         .watches = VECTOR_EMPTY_INITIALIZER(uint32_t),
+        .changed_routes = VECTOR_EMPTY_INITIALIZER(struct ovn_route_msg *),
     };
     return rtn;
 }
@@ -5943,6 +5953,8 @@  static void
 en_route_table_notify_cleanup(void *data)
 {
     struct ed_type_route_table_notify *rtn = data;
+    route_table_notify_clear_changes(rtn);
+    vector_destroy(&rtn->changed_routes);
     vector_destroy(&rtn->watches);
 }
 
diff --git a/controller/ovn-netlink-notifier.c b/controller/ovn-netlink-notifier.c
index 04db42d0e109..c772bdb7ca2c 100644
--- a/controller/ovn-netlink-notifier.c
+++ b/controller/ovn-netlink-notifier.c
@@ -66,13 +66,13 @@  static void ovn_netlink_notifier_report_lost(struct ovn_netlink_notifier *);
 static struct ovn_netlink_notifier notifiers[OVN_NL_NOTIFIER_MAX] = {
     [OVN_NL_NOTIFIER_ROUTE_V4] = {
         .group = RTNLGRP_IPV4_ROUTE,
-        .msgs = VECTOR_EMPTY_INITIALIZER(uint32_t),
+        .msgs = VECTOR_EMPTY_INITIALIZER(struct ovn_route_msg *),
         .change_handler = ovn_netlink_route_change_handler,
         .name = "route-ipv4",
     },
     [OVN_NL_NOTIFIER_ROUTE_V6] = {
         .group = RTNLGRP_IPV6_ROUTE,
-        .msgs = VECTOR_EMPTY_INITIALIZER(uint32_t),
+        .msgs = VECTOR_EMPTY_INITIALIZER(struct ovn_route_msg *),
         .change_handler = ovn_netlink_route_change_handler,
         .name = "route-ipv6",
     },
@@ -149,9 +149,9 @@  ovn_netlink_route_change_handler(const void *change_, void *aux)
 
     struct route_data *rd = &change->route.rd;
     if (rd->rtm_protocol != RTPROT_OVN) {
-        /* We just cannot copy the whole route_data because it has reference
-         * to self for the nexthop list. */
-        vector_push(&notifier->msgs, &rd->rta_table_id);
+        struct ovn_route_msg *msg =
+            ovn_route_msg_from_route_data(change->route.nlmsg_type, rd);
+        vector_push(&notifier->msgs, &msg);
     }
 
     route_data_destroy(rd);
@@ -288,7 +288,13 @@  ovn_netlink_notifier_flush(enum ovn_netlink_notifier_type type)
         break;
     }
     case OVN_NL_NOTIFIER_ROUTE_V4:
-    case OVN_NL_NOTIFIER_ROUTE_V6:
+    case OVN_NL_NOTIFIER_ROUTE_V6: {
+        struct ovn_route_msg *msg;
+        VECTOR_FOR_EACH (&notifier->msgs, msg) {
+            free(msg);
+        }
+        break;
+    }
     case OVN_NL_NOTIFIER_NEIGHBOR:
     case OVN_NL_NOTIFIER_MAX:
         break;
diff --git a/controller/route-exchange-netlink.c b/controller/route-exchange-netlink.c
index 501a78e69164..56c998d3b3b6 100644
--- a/controller/route-exchange-netlink.c
+++ b/controller/route-exchange-netlink.c
@@ -200,6 +200,72 @@  re_nl_delete_route(uint32_t table_id, const struct advertise_route_entry *re)
     return modify_route(RTM_DELROUTE, 0, table_id, re);
 }
 
+static size_t
+ovn_route_msg_size(const struct ovn_route_msg *msg)
+{
+    return sizeof *msg + msg->n_nexthops * sizeof msg->nexthops[0];
+}
+
+/* Returns a self contained copy of the route 'rd' reported by a message of
+ * type 'nlmsg_type'.  The caller takes ownership of it. */
+struct ovn_route_msg *
+ovn_route_msg_from_route_data(uint16_t nlmsg_type,
+                              const struct route_data *rd)
+{
+    size_t n_nexthops = ovs_list_size(&rd->nexthops);
+    struct ovn_route_msg *msg =
+        xzalloc(sizeof *msg + n_nexthops * sizeof msg->nexthops[0]);
+
+    msg->nlmsg_type = nlmsg_type;
+    msg->table_id = rd->rta_table_id;
+    msg->prefix = rd->rta_dst;
+    msg->plen = rd->rtm_dst_len;
+    msg->protocol = rd->rtm_protocol;
+    msg->priority = rd->rta_priority;
+    msg->nhid = rd->rta_nhid;
+    msg->n_nexthops = n_nexthops;
+
+    struct ovn_route_nexthop *nh = msg->nexthops;
+    const struct route_data_nexthop *rdnh;
+    LIST_FOR_EACH (rdnh, nexthop_node, &rd->nexthops) {
+        nh->addr = rdnh->addr;
+        memcpy(nh->ifname, rdnh->ifname, IFNAMSIZ);
+        nh++;
+    }
+
+    return msg;
+}
+
+struct ovn_route_msg *
+ovn_route_msg_clone(const struct ovn_route_msg *msg)
+{
+    return xmemdup(msg, ovn_route_msg_size(msg));
+}
+
+void
+ovn_route_msg_format(struct ds *ds, const struct ovn_route_msg *msg)
+{
+    ds_put_format(ds, "table_id=%"PRIu32" dst=", msg->table_id);
+    ipv6_format_mapped(&msg->prefix, ds);
+    ds_put_format(ds, " plen=%u proto=%u priority=%"PRIu32,
+                  msg->plen, msg->protocol, msg->priority);
+
+    if (msg->nhid) {
+        ds_put_format(ds, " nhid=%"PRIu32, msg->nhid);
+        return;
+    }
+
+    for (size_t i = 0; i < msg->n_nexthops; i++) {
+        const struct ovn_route_nexthop *nh = &msg->nexthops[i];
+
+        ds_put_cstr(ds, " nexthop=");
+        ipv6_format_mapped(&nh->addr, ds);
+        if (nh->ifname[0]) {
+            ds_put_format(ds, ",dev=%s", nh->ifname);
+        }
+    }
+}
+
 /* Appends a learned route for the prefix in 'rd' reachable through the leaf
  * nexthop object 'nhe' to 'learned_routes'. */
 static void
diff --git a/controller/route-exchange-netlink.h b/controller/route-exchange-netlink.h
index be0496d5e0ab..26aa8de9328e 100644
--- a/controller/route-exchange-netlink.h
+++ b/controller/route-exchange-netlink.h
@@ -36,9 +36,47 @@ 
 
 struct in6_addr;
 struct hmap;
+struct route_data;
 struct vector;
 struct advertise_route_entry;
 
+/* One of the next hops of a route as reported by the kernel. */
+struct ovn_route_nexthop {
+    struct in6_addr addr;
+    /* Adding 1 to this to be sure we actually have a terminating '\0' */
+    char ifname[IFNAMSIZ + 1];
+};
+
+/* A digested version of a route message sent down by the kernel to indicate
+ * that a route has changed.  Unlike 'struct route_data', which points into
+ * itself to describe the next hops, this is self contained, so it stays valid
+ * after the message it was built from is gone. */
+struct ovn_route_msg {
+    /* E.g. RTM_NEWROUTE, RTM_DELROUTE. */
+    uint16_t nlmsg_type;
+    /* Routing table the route belongs to. */
+    uint32_t table_id;
+    /* Prefix the route is for. */
+    struct in6_addr prefix;
+    unsigned int plen;
+    /* Routing protocol that installed the route, e.g. RTPROT_BGP. */
+    unsigned char protocol;
+    /* Metric of the route.  The kernel allows several routes for one prefix
+     * that differ only by this, so it is part of a route's identity. */
+    uint32_t priority;
+    /* Id of the kernel nexthop object the route resolves through, 0 if the
+     * next hops are described by 'nexthops' instead. */
+    uint32_t nhid;
+    /* Number of next hops described by the route itself. */
+    size_t n_nexthops;
+    struct ovn_route_nexthop nexthops[];
+};
+
+struct ovn_route_msg *ovn_route_msg_from_route_data(
+    uint16_t nlmsg_type, const struct route_data *);
+struct ovn_route_msg *ovn_route_msg_clone(const struct ovn_route_msg *);
+void ovn_route_msg_format(struct ds *, const struct ovn_route_msg *);
+
 struct re_nl_received_route_node {
     struct in6_addr prefix;
     unsigned int plen;
diff --git a/tests/system-ovn-netlink.at b/tests/system-ovn-netlink.at
index 1c2e2995a7ce..5b6772da38ad 100644
--- a/tests/system-ovn-netlink.at
+++ b/tests/system-ovn-netlink.at
@@ -702,13 +702,19 @@  check ip link set up lo-test
 dnl Should notify if an IPv4 route is added to a table monitored by OVN.
 AT_CHECK_UNQUOTED([ovstest test-ovn-netlink route-table-notify \
     "ip route add 10.10.10.0/24 via 20.0.0.1 vrf vrf-$table_id"], [0], [dnl
-Notification v4 table_id=$table_id
+Notification v4 add route table_id=$table_id dst=10.10.10.0 plen=24 proto=3 priority=0 nexthop=20.0.0.1,dev=lo-test
 ])
 
 dnl Should notify if an IPv6 route is added to a table monitored by OVN.
 AT_CHECK_UNQUOTED([ovstest test-ovn-netlink route-table-notify \
     "ip -6 route add fd10::/64 via fd20::1 vrf vrf-$table_id"], [0], [dnl
-Notification v6 table_id=$table_id
+Notification v6 add route table_id=$table_id dst=fd10:: plen=64 proto=3 priority=1024 nexthop=fd20::1,dev=lo-test
+])
+
+dnl Should report which route was removed, not just that something changed.
+AT_CHECK_UNQUOTED([ovstest test-ovn-netlink route-table-notify \
+    "ip route del 10.10.10.0/24 via 20.0.0.1 vrf vrf-$table_id"], [0], [dnl
+Notification v4 delete route table_id=$table_id dst=10.10.10.0 plen=24 proto=3 priority=0 nexthop=20.0.0.1,dev=lo-test
 ])
 
 dnl Should NOT notify if a route with proto ovn is added.
diff --git a/tests/test-ovn-netlink.c b/tests/test-ovn-netlink.c
index aff9ba1d0474..ba779938028f 100644
--- a/tests/test-ovn-netlink.c
+++ b/tests/test-ovn-netlink.c
@@ -277,18 +277,26 @@  test_route_table_notify(struct ovs_cmdl_context *ctx)
     ovn_netlink_update_notifier(OVN_NL_NOTIFIER_ROUTE_V6, true);
     run_command_under_notifier(cmd);
 
-    uint32_t table_id;
+    static const char *families[] = {"v4", "v6"};
+    static const enum ovn_netlink_notifier_type types[] = {
+        OVN_NL_NOTIFIER_ROUTE_V4, OVN_NL_NOTIFIER_ROUTE_V6,
+    };
+    struct ds ds = DS_EMPTY_INITIALIZER;
 
-    struct vector *msgs = ovn_netlink_get_msgs(OVN_NL_NOTIFIER_ROUTE_V4);
-    VECTOR_FOR_EACH (msgs, table_id) {
-        printf("Notification v4 table_id=%"PRIu32"\n", table_id);
-    }
+    for (size_t i = 0; i < ARRAY_SIZE(types); i++) {
+        struct vector *msgs = ovn_netlink_get_msgs(types[i]);
+        struct ovn_route_msg *msg;
 
-    msgs = ovn_netlink_get_msgs(OVN_NL_NOTIFIER_ROUTE_V6);
-    VECTOR_FOR_EACH (msgs, table_id) {
-        printf("Notification v6 table_id=%"PRIu32"\n", table_id);
+        VECTOR_FOR_EACH (msgs, msg) {
+            ds_clear(&ds);
+            ovn_route_msg_format(&ds, msg);
+            printf("Notification %s %s route %s\n", families[i],
+                   msg->nlmsg_type == RTM_NEWROUTE ? "add" : "delete",
+                   ds_cstr(&ds));
+        }
     }
 
+    ds_destroy(&ds);
     ovn_netlink_notifiers_destroy();
 }