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>

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index a19ed7e535664ea91fe7207ac16b35a4c460936c..8e08b3c1cf45e1cca2e6cc871f79324ab77ef0e7 100644 (file)
--- 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 5a809c0cdf1fbe8197763fc3aeb265942a5db24b..0bed7bf61edba2931cb1c9ad6af1b5204e07120a 100644 (file)
--- 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 8b43be52e95e30adf5fe745d02eb8b4bd947e69c..18e1ec83532da9e5f409942a3d8efe9b32bd673f 100644 (file)
--- 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 32eab5910760f1123d8809c0ac3d370f4499c436..ab97a292ebac42cee2ab0dbcccf27db29dd9277d 100644 (file)
--- 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 b4b882a56aacecb784650ecabb5a34313e8a8bfd..87f5852c8d600d9ab6b15d7337808b5258e0338f 100644 (file)
--- 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);

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 5e08ef10dad657b883914aea49f43681db161131..920f29a6f36a57f5bdfbf0f67040120bbe7a3d1e 100644 (file)
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2576,6 +2576,25 @@ udpif_update_flow_pps(struct udpif *udpif, struct udpif_key *ukey,
     ukey->flow_time = udpif->dpif->current_ms;
 }
 
+static long long int
+udpif_update_used(struct udpif *udpif, struct udpif_key *ukey,
+                  struct dpif_flow_stats *stats)
+    OVS_REQUIRES(ukey->mutex)
+{
+    if (!udpif->dump->terse) {
+        return ukey->created;
+    }
+
+    if (stats->n_packets > ukey->stats.n_packets) {
+        stats->used = udpif->dpif->current_ms;
+    } else if (ukey->stats.used) {
+        stats->used = ukey->stats.used;
+    } else {
+        stats->used = ukey->created;
+    }
+    return stats->used;
+}
+
 static void
 revalidate(struct revalidator *revalidator)
 {
@@ -2631,6 +2650,7 @@ revalidate(struct revalidator *revalidator)
         for (f = flows; f < &flows[n_dumped]; f++) {
             long long int used = f->stats.used;
             struct recirc_refs recircs = RECIRC_REFS_EMPTY_INITIALIZER;
+            struct dpif_flow_stats stats = f->stats;
             enum reval_result result;
             struct udpif_key *ukey;
             bool already_dumped;
@@ -2675,12 +2695,12 @@ revalidate(struct revalidator *revalidator)
             }
 
             if (!used) {
-                used = ukey->created;
+                used = udpif_update_used(udpif, ukey, &stats);
             }
             if (kill_them_all || (used && used < now - max_idle)) {
                 result = UKEY_DELETE;
             } else {
-                result = revalidate_ukey(udpif, ukey, &f->stats, &odp_actions,
+                result = revalidate_ukey(udpif, ukey, &stats, &odp_actions,
                                          reval_seq, &recircs,
                                          f->attrs.offloaded);
             }