netdev-offload: Implement terse dump support

In order to improve revalidator performance by minimizing unnecessary
copying of data, extend netdev-offloads to support terse dump mode. Extend
netdev_flow_api->flow_dump_create() with 'terse' bool argument. Implement
support for terse dump in functions that convert netlink to flower and
flower to match. Set flow stats "used" value based on difference in number
of flow packets because lastuse timestamp is not included in TC terse dump.

Kernel API support is implemented in following patch.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>

5 files changed, 100 insertions, 56 deletions

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index a19ed7e53..8e08b3c1c 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1445,7 +1445,8 @@ start_netdev_dump(const struct dpif *dpif_,
     dump->netdev_current_dump = 0;
     dump->netdev_dumps
         = netdev_ports_flow_dump_create(dpif_->dpif_class,
-                                        &dump->netdev_dumps_num);
+                                        &dump->netdev_dumps_num,
+                                        dump->up.terse);
     ovs_mutex_unlock(&dump->netdev_lock);
 }
 
@@ -1640,41 +1641,42 @@ dpif_netlink_netdev_match_to_dpif_flow(struct match *match,
                                        struct dpif_flow_attrs *attrs,
                                        ovs_u128 *ufid,
                                        struct dpif_flow *flow,
-                                       bool terse OVS_UNUSED)
-{
-
-    struct odp_flow_key_parms odp_parms = {
-        .flow = &match->flow,
-        .mask = &match->wc.masks,
-        .support = {
-            .max_vlan_headers = 2,
-            .recirc = true,
-            .ct_state = true,
-            .ct_zone = true,
-            .ct_mark = true,
-            .ct_label = true,
-        },
-    };
-    size_t offset;
-
+                                       bool terse)
+{
     memset(flow, 0, sizeof *flow);
 
-    /* Key */
-    offset = key_buf->size;
-    flow->key = ofpbuf_tail(key_buf);
-    odp_flow_key_from_flow(&odp_parms, key_buf);
-    flow->key_len = key_buf->size - offset;
-
-    /* Mask */
-    offset = mask_buf->size;
-    flow->mask = ofpbuf_tail(mask_buf);
-    odp_parms.key_buf = key_buf;
-    odp_flow_key_from_mask(&odp_parms, mask_buf);
-    flow->mask_len = mask_buf->size - offset;
-
-    /* Actions */
-    flow->actions = nl_attr_get(actions);
-    flow->actions_len = nl_attr_get_size(actions);
+    if (!terse) {
+        struct odp_flow_key_parms odp_parms = {
+            .flow = &match->flow,
+            .mask = &match->wc.masks,
+            .support = {
+                .max_vlan_headers = 2,
+                .recirc = true,
+                .ct_state = true,
+                .ct_zone = true,
+                .ct_mark = true,
+                .ct_label = true,
+            },
+        };
+        size_t offset;
+
+        /* Key */
+        offset = key_buf->size;
+        flow->key = ofpbuf_tail(key_buf);
+        odp_flow_key_from_flow(&odp_parms, key_buf);
+        flow->key_len = key_buf->size - offset;
+
+        /* Mask */
+        offset = mask_buf->size;
+        flow->mask = ofpbuf_tail(mask_buf);
+        odp_parms.key_buf = key_buf;
+        odp_flow_key_from_mask(&odp_parms, mask_buf);
+        flow->mask_len = mask_buf->size - offset;
+
+        /* Actions */
+        flow->actions = nl_attr_get(actions);
+        flow->actions_len = nl_attr_get_size(actions);
+    }
 
     /* Stats */
     memcpy(&flow->stats, stats, sizeof *stats);
diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
index 5a809c0cd..0bed7bf61 100644
--- a/lib/netdev-offload-provider.h
+++ b/lib/netdev-offload-provider.h
@@ -42,7 +42,8 @@ struct netdev_flow_api {
      *
      * On success returns 0 and allocates data, on failure returns
      * positive errno. */
-    int (*flow_dump_create)(struct netdev *, struct netdev_flow_dump **dump);
+    int (*flow_dump_create)(struct netdev *, struct netdev_flow_dump **dump,
+                            bool terse);
     int (*flow_dump_destroy)(struct netdev_flow_dump *);
 
     /* Returns true if there are more flows to dump.
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 8b43be52e..18e1ec835 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -366,7 +366,8 @@ netdev_tc_flow_flush(struct netdev *netdev)
 
 static int
 netdev_tc_flow_dump_create(struct netdev *netdev,
-                           struct netdev_flow_dump **dump_out)
+                           struct netdev_flow_dump **dump_out,
+                           bool terse)
 {
     enum tc_qdisc_hook hook = get_tc_qdisc_hook(netdev);
     struct netdev_flow_dump *dump;
@@ -386,6 +387,7 @@ netdev_tc_flow_dump_create(struct netdev *netdev,
     dump = xzalloc(sizeof *dump);
     dump->nl_dump = xzalloc(sizeof *dump->nl_dump);
     dump->netdev = netdev_ref(netdev);
+    dump->terse = terse;
 
     id = tc_make_tcf_id(ifindex, block_id, prio, hook);
     tc_dump_flower_start(&id, dump->nl_dump);
@@ -502,13 +504,53 @@ flower_tun_opt_to_match(struct match *match, struct tc_flower *flower)
     match->wc.masks.tunnel.flags |= FLOW_TNL_F_UDPIF;
 }
 
+static void
+parse_tc_flower_to_stats(struct tc_flower *flower,
+                         struct dpif_flow_stats *stats)
+{
+    if (!stats) {
+        return;
+    }
+
+    memset(stats, 0, sizeof *stats);
+    stats->n_packets = get_32aligned_u64(&flower->stats.n_packets);
+    stats->n_bytes = get_32aligned_u64(&flower->stats.n_bytes);
+    stats->used = flower->lastused;
+}
+
+static void
+parse_tc_flower_to_attrs(struct tc_flower *flower,
+                         struct dpif_flow_attrs *attrs)
+{
+    attrs->offloaded = (flower->offloaded_state == TC_OFFLOADED_STATE_IN_HW ||
+                        flower->offloaded_state ==
+                        TC_OFFLOADED_STATE_UNDEFINED);
+    attrs->dp_layer = "tc";
+    attrs->dp_extra_info = NULL;
+}
+
+static int
+parse_tc_flower_terse_to_match(struct tc_flower *flower,
+                               struct match *match,
+                               struct dpif_flow_stats *stats,
+                               struct dpif_flow_attrs *attrs)
+{
+    match_init_catchall(match);
+
+    parse_tc_flower_to_stats(flower, stats);
+    parse_tc_flower_to_attrs(flower, attrs);
+
+    return 0;
+}
+
 static int
 parse_tc_flower_to_match(struct tc_flower *flower,
                          struct match *match,
                          struct nlattr **actions,
                          struct dpif_flow_stats *stats,
                          struct dpif_flow_attrs *attrs,
-                         struct ofpbuf *buf)
+                         struct ofpbuf *buf,
+                         bool terse)
 {
     size_t act_off;
     struct tc_flower_key *key = &flower->key;
@@ -517,6 +559,10 @@ parse_tc_flower_to_match(struct tc_flower *flower,
     struct tc_action *action;
     int i;
 
+    if (terse) {
+        return parse_tc_flower_terse_to_match(flower, match, stats, attrs);
+    }
+
     ofpbuf_clear(buf);
 
     match_init_catchall(match);
@@ -877,17 +923,8 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 
     *actions = ofpbuf_at_assert(buf, act_off, sizeof(struct nlattr));
 
-    if (stats) {
-        memset(stats, 0, sizeof *stats);
-        stats->n_packets = get_32aligned_u64(&flower->stats.n_packets);
-        stats->n_bytes = get_32aligned_u64(&flower->stats.n_bytes);
-        stats->used = flower->lastused;
-    }
-
-    attrs->offloaded = (flower->offloaded_state == TC_OFFLOADED_STATE_IN_HW)
-                       || (flower->offloaded_state == TC_OFFLOADED_STATE_UNDEFINED);
-    attrs->dp_layer = "tc";
-    attrs->dp_extra_info = NULL;
+    parse_tc_flower_to_stats(flower, stats);
+    parse_tc_flower_to_attrs(flower, attrs);
 
     return 0;
 }
@@ -919,7 +956,7 @@ netdev_tc_flow_dump_next(struct netdev_flow_dump *dump,
         }
 
         if (parse_tc_flower_to_match(&flower, match, actions, stats, attrs,
-                                     wbuffer)) {
+                                     wbuffer, dump->terse)) {
             continue;
         }
 
@@ -1805,7 +1842,7 @@ netdev_tc_flow_get(struct netdev *netdev,
     }
 
     in_port = netdev_ifindex_to_odp_port(id.ifindex);
-    parse_tc_flower_to_match(&flower, match, actions, stats, attrs, buf);
+    parse_tc_flower_to_match(&flower, match, actions, stats, attrs, buf, false);
 
     match->wc.masks.in_port.odp_port = u32_to_odp(UINT32_MAX);
     match->flow.in_port.odp_port = in_port;
diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
index 32eab5910..ab97a292e 100644
--- a/lib/netdev-offload.c
+++ b/lib/netdev-offload.c
@@ -201,13 +201,14 @@ netdev_flow_flush(struct netdev *netdev)
 }
 
 int
-netdev_flow_dump_create(struct netdev *netdev, struct netdev_flow_dump **dump)
+netdev_flow_dump_create(struct netdev *netdev, struct netdev_flow_dump **dump,
+                        bool terse)
 {
     const struct netdev_flow_api *flow_api =
         ovsrcu_get(const struct netdev_flow_api *, &netdev->flow_api);
 
     return (flow_api && flow_api->flow_dump_create)
-           ? flow_api->flow_dump_create(netdev, dump)
+           ? flow_api->flow_dump_create(netdev, dump, terse)
            : EOPNOTSUPP;
 }
 
@@ -436,7 +437,8 @@ netdev_ports_flow_flush(const struct dpif_class *dpif_class)
 }
 
 struct netdev_flow_dump **
-netdev_ports_flow_dump_create(const struct dpif_class *dpif_class, int *ports)
+netdev_ports_flow_dump_create(const struct dpif_class *dpif_class, int *ports,
+                              bool terse)
 {
     struct port_to_netdev_data *data;
     struct netdev_flow_dump **dumps;
@@ -454,7 +456,7 @@ netdev_ports_flow_dump_create(const struct dpif_class *dpif_class, int *ports)
 
     HMAP_FOR_EACH (data, portno_node, &port_to_netdev) {
         if (data->dpif_class == dpif_class) {
-            if (netdev_flow_dump_create(data->netdev, &dumps[i])) {
+            if (netdev_flow_dump_create(data->netdev, &dumps[i], terse)) {
                 continue;
             }
 
diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h
index b4b882a56..87f5852c8 100644
--- a/lib/netdev-offload.h
+++ b/lib/netdev-offload.h
@@ -80,7 +80,8 @@ struct offload_info {
 };
 
 int netdev_flow_flush(struct netdev *);
-int netdev_flow_dump_create(struct netdev *, struct netdev_flow_dump **dump);
+int netdev_flow_dump_create(struct netdev *, struct netdev_flow_dump **dump,
+                            bool terse);
 int netdev_flow_dump_destroy(struct netdev_flow_dump *);
 bool netdev_flow_dump_next(struct netdev_flow_dump *, struct match *,
                           struct nlattr **actions, struct dpif_flow_stats *,
@@ -114,7 +115,8 @@ odp_port_t netdev_ifindex_to_odp_port(int ifindex);
 
 struct netdev_flow_dump **netdev_ports_flow_dump_create(
                                         const struct dpif_class *,
-                                        int *ports);
+                                        int *ports,
+                                        bool terse);
 void netdev_ports_flow_flush(const struct dpif_class *);
 int netdev_ports_flow_del(const struct dpif_class *, const ovs_u128 *ufid,
                           struct dpif_flow_stats *stats);