[ovs-dev] [PATCH v3 08/13] ofproto-dpif-xlate: Add xlate cache type XC_TABLE.

Jarno Rajahalme jarno at ovn.org
Mon Sep 12 20:52:38 UTC 2016


Xlate cache entry type XC_TABLE is required for the table stats
(number of misses and matches) to be correctly attributed.

It appears that table stats have been off ever since xlate cache was
introduced.  This was now revealed by a PACKET_OUT unit test case in a
later patch that checks for table stats explicitly.

Fixes: b256dc52 ("ofproto-dpif-xlate: Cache xlate_actions() effects.")
Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
---
v3: Add the NEWS flash.

 NEWS                               |  3 +++
 ofproto/ofproto-dpif-xlate-cache.c | 10 ++++++++++
 ofproto/ofproto-dpif-xlate-cache.h |  6 ++++++
 ofproto/ofproto-dpif-xlate.c       |  6 +++---
 ofproto/ofproto-dpif.c             | 35 ++++++++++++++++++++++++++++++++++-
 ofproto/ofproto-dpif.h             |  8 +++++++-
 6 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 9e8034a..7f501d6 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,8 @@
 Post-v2.6.0
 ---------------------
+   - Fixed regression in table stats maintenance introduced in OVS
+     2.3.0, wherein the number of OpenFlow table hits and misses was
+     not accurate.
    - OpenFlow:
      * Bundles now expire after 10 seconds since the last time the
        bundle was either opened, modified, or closed.
diff --git a/ofproto/ofproto-dpif-xlate-cache.c b/ofproto/ofproto-dpif-xlate-cache.c
index ebcfa8a..ff32a99 100644
--- a/ofproto/ofproto-dpif-xlate-cache.c
+++ b/ofproto/ofproto-dpif-xlate-cache.c
@@ -88,6 +88,14 @@ xlate_push_stats_entry(struct xc_entry *entry,
     struct eth_addr dmac;
 
     switch (entry->type) {
+    case XC_TABLE:
+        ofproto_dpif_credit_table_stats(entry->u.table.ofproto,
+                                        entry->u.table.id,
+                                        entry->u.table.match
+                                        ? stats->n_packets : 0,
+                                        entry->u.table.match
+                                        ? 0 : stats->n_packets);
+        break;
     case XC_RULE:
         rule_dpif_credit_stats(entry->u.rule, stats);
         break;
@@ -178,6 +186,8 @@ void
 xlate_cache_clear_entry(struct xc_entry *entry)
 {
     switch (entry->type) {
+    case XC_TABLE:
+        break;
     case XC_RULE:
         rule_dpif_unref(entry->u.rule);
         break;
diff --git a/ofproto/ofproto-dpif-xlate-cache.h b/ofproto/ofproto-dpif-xlate-cache.h
index 30e5c75..d9e0e52 100644
--- a/ofproto/ofproto-dpif-xlate-cache.h
+++ b/ofproto/ofproto-dpif-xlate-cache.h
@@ -40,6 +40,7 @@ struct group_dpif;
 struct ofputil_bucket;
 
 enum xc_type {
+    XC_TABLE,
     XC_RULE,
     XC_BOND,
     XC_NETDEV,
@@ -64,6 +65,11 @@ enum xc_type {
 struct xc_entry {
     enum xc_type type;
     union {
+        struct {
+            struct ofproto_dpif *ofproto;
+            uint8_t id;
+            bool    match; /* or miss. */
+        } table;
         struct rule_dpif *rule;
         struct {
             struct netdev *tx;
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 11d3732..cf8d1fc 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3188,8 +3188,8 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t in_port, uint8_t table_id,
                                            &ctx->xin->flow, ctx->wc,
                                            ctx->xin->resubmit_stats,
                                            &ctx->table_id, in_port,
-                                           may_packet_in, honor_table_miss);
-
+                                           may_packet_in, honor_table_miss,
+                                           ctx->xin->xcache);
         if (OVS_UNLIKELY(ctx->xin->resubmit_hook)) {
             ctx->xin->resubmit_hook(ctx->xin, rule, ctx->indentation + 1);
         }
@@ -5336,7 +5336,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
         ctx.rule = rule_dpif_lookup_from_table(
             ctx.xbridge->ofproto, ctx.tables_version, flow, ctx.wc,
             ctx.xin->resubmit_stats, &ctx.table_id,
-            flow->in_port.ofp_port, true, true);
+            flow->in_port.ofp_port, true, true, ctx.xin->xcache);
         if (ctx.xin->resubmit_stats) {
             rule_dpif_credit_stats(ctx.rule, ctx.xin->resubmit_stats);
         }
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index d928f01..b8171a7 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -49,6 +49,7 @@
 #include "ofproto-dpif-sflow.h"
 #include "ofproto-dpif-upcall.h"
 #include "ofproto-dpif-xlate.h"
+#include "ofproto-dpif-xlate-cache.h"
 #include "openvswitch/ofp-actions.h"
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/meta-flow.h"
@@ -3914,6 +3915,21 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, ovs_version_t version,
                                                                flow, wc)));
 }
 
+void
+ofproto_dpif_credit_table_stats(struct ofproto_dpif *ofproto, uint8_t table_id,
+                                uint64_t n_matches, uint64_t n_misses)
+{
+    struct oftable *tbl = &ofproto->up.tables[table_id];
+    unsigned long orig;
+
+    if (n_matches) {
+        atomic_add_relaxed(&tbl->n_matched, n_matches, &orig);
+    }
+    if (n_misses) {
+        atomic_add_relaxed(&tbl->n_missed, n_misses, &orig);
+    }
+}
+
 /* Look up 'flow' in 'ofproto''s classifier version 'version', starting from
  * table '*table_id'.  Returns the rule that was found, which may be one of the
  * special rules according to packet miss hadling.  If 'may_packet_in' is
@@ -3945,7 +3961,8 @@ rule_dpif_lookup_from_table(struct ofproto_dpif *ofproto,
                             struct flow_wildcards *wc,
                             const struct dpif_flow_stats *stats,
                             uint8_t *table_id, ofp_port_t in_port,
-                            bool may_packet_in, bool honor_table_miss)
+                            bool may_packet_in, bool honor_table_miss,
+                            struct xlate_cache *xcache)
 {
     ovs_be16 old_tp_src = flow->tp_src, old_tp_dst = flow->tp_dst;
     ofp_port_t old_in_port = flow->in_port.ofp_port;
@@ -3971,6 +3988,14 @@ rule_dpif_lookup_from_table(struct ofproto_dpif *ofproto,
 
                 atomic_add_relaxed(&tbl->n_matched, stats->n_packets, &orig);
             }
+            if (xcache) {
+                struct xc_entry *entry;
+
+                entry = xlate_cache_add_entry(xcache, XC_TABLE);
+                entry->u.table.ofproto = ofproto;
+                entry->u.table.id = *table_id;
+                entry->u.table.match = true;
+            }
             return rule;
         }
     }
@@ -3999,6 +4024,14 @@ rule_dpif_lookup_from_table(struct ofproto_dpif *ofproto,
             atomic_add_relaxed(rule ? &tbl->n_matched : &tbl->n_missed,
                                stats->n_packets, &orig);
         }
+        if (xcache) {
+            struct xc_entry *entry;
+
+            entry = xlate_cache_add_entry(xcache, XC_TABLE);
+            entry->u.table.ofproto = ofproto;
+            entry->u.table.id = next_id;
+            entry->u.table.match = (rule != NULL);
+        }
         if (rule) {
             goto out;   /* Match. */
         }
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index f912f6e..c156795 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -102,6 +102,11 @@ struct dpif_backer_support *ofproto_dpif_get_support(const struct ofproto_dpif *
 
 ovs_version_t ofproto_dpif_get_tables_version(struct ofproto_dpif *);
 
+void ofproto_dpif_credit_table_stats(struct ofproto_dpif *, uint8_t table_id,
+                                     uint64_t n_matches, uint64_t n_misses);
+
+struct xlate_cache;
+
 struct rule_dpif *rule_dpif_lookup_from_table(struct ofproto_dpif *,
                                               ovs_version_t, struct flow *,
                                               struct flow_wildcards *,
@@ -109,7 +114,8 @@ struct rule_dpif *rule_dpif_lookup_from_table(struct ofproto_dpif *,
                                               uint8_t *table_id,
                                               ofp_port_t in_port,
                                               bool may_packet_in,
-                                              bool honor_table_miss);
+                                              bool honor_table_miss,
+                                              struct xlate_cache *xcache);
 
 static inline void rule_dpif_ref(struct rule_dpif *);
 static inline void rule_dpif_unref(struct rule_dpif *);
-- 
2.1.4




More information about the dev mailing list