[ovs-dev] [RFC 9/9] ofproto-dpif: Revalidate flows more promptly.

Joe Stringer joestringer at nicira.com
Fri Mar 7 01:20:32 UTC 2014


Previously, the main thread would set ofproto->backer->need_revalidate
when it performed a change that required revalidation. Later in the main
loop, it would check this flag and call udpif_revalidate() to notify the
udpif that it needs to revalidate. The flow dumper then, before starting
a flow dump, checks the udpif->reval_seq to determine whether full
revalidation needs to happen and sets the per-udump flag.

Two delays here:
* Time for main loop to reconfigure and "Get around to" setting the
  udpif revalidate sequence.
* Flow dumper only sets udump revalidation flag if the sequence was
  different at the start of the flow dump.

Finally, when revalidator threads come to credit the stats, we translate
the datapath flow then check whether the actions and mask need comparing.

This patch is about reducing the delays; if the main thread effects
a change, it immediately notifies the udpif. This should make it less
likely for revalidators to attribute statistics to the wrong module.
When revalidators go to credit the stats, they compare the udpif->seq
against a per-ukey seq first. Each ukey will only be checked once within
a revalidation cycle.

* If the sequences are the same, use the 'xlate_cache'.
* Else if (it's too costly to revalidate), delete the flow
* Else revalidate

Previously, the main thread would limit the churn/work for revalidators
by only notifying the seq periodically; this patch limits the churn by
how often the revalidators run.

XXX: This patch causes flows to be revalidated immediately after they
are first dumped. I think this can be fixed by pushing the ukey creation
into the handler threads, and fetching the udpif->reval_seq at that
time.

Signed-off-by: Joe Stringer <joestringer at nicira.com>
---
 ofproto/ofproto-dpif-upcall.c |   28 +++++++++++++++-------------
 ofproto/ofproto-dpif.c        |    5 +----
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index f72c3fd..31483de 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -166,6 +166,7 @@ struct udpif_key {
     struct dpif_flow_stats stats;  /* Stats at most recent flow dump. */
     long long int created;         /* Estimation of creation time. */
 
+    uint64_t reval_seq;            /* udpif->reval_seq at last revalidation. */
     bool mark;                     /* Used by mark and sweep GC algorithm. */
 
     struct odputil_keybuf key_buf; /* Memory for 'key'. */
@@ -189,8 +190,6 @@ struct udpif_flow_dump {
 
     struct dpif_flow_stats stats;  /* Stats pulled from the datapath. */
 
-    bool need_revalidate;          /* Key needs revalidation? */
-
     struct odputil_keybuf key_buf;
 };
 
@@ -552,14 +551,12 @@ udpif_flow_dumper(void *arg)
         struct dpif_flow_dump dump;
         size_t key_len, mask_len;
         unsigned int flow_limit;
-        bool need_revalidate;
         uint64_t reval_seq;
         size_t n_flows, i;
         int error;
         void *state = NULL;
 
         reval_seq = seq_read(udpif->reval_seq);
-        need_revalidate = udpif->last_reval_seq != reval_seq;
         udpif->last_reval_seq = reval_seq;
 
         n_flows = udpif_get_n_flows(udpif);
@@ -589,7 +586,6 @@ udpif_flow_dumper(void *arg)
             udump->mask_len = mask_len;
 
             udump->stats = *stats;
-            udump->need_revalidate = need_revalidate;
 
             revalidator = &udpif->revalidators[udump->key_hash
                 % udpif->n_revalidators];
@@ -1290,6 +1286,9 @@ ukey_create(const struct nlattr *key, size_t key_len, long long int used)
     ukey->key_len = key_len;
 
     ukey->mark = false;
+
+    /* XXX: This always results in flows being revalidated upon first dump. */
+    ukey->reval_seq = 0;
     ukey->created = used ? used : time_msec();
     memset(&ukey->stats, 0, sizeof ukey->stats);
     ukey->xc = NULL;
@@ -1320,7 +1319,7 @@ revalidate_ukey(struct udpif *udpif, struct udpif_flow_dump *udump,
     struct xlate_in xin;
     int error;
     size_t i;
-    bool ok;
+    bool ok, need_revalidate;
 
     ok = false;
     xoutp = NULL;
@@ -1329,7 +1328,8 @@ revalidate_ukey(struct udpif *udpif, struct udpif_flow_dump *udump,
     /* If we don't need to revalidate, we can simply push the stats contained
      * in the udump, otherwise we'll have to get the actions so we can check
      * them. */
-    if (udump->need_revalidate) {
+    need_revalidate = ukey->reval_seq != udpif->last_reval_seq;
+    if (need_revalidate) {
         if (dpif_flow_get(udpif->dpif, ukey->key, ukey->key_len, &actions,
                           &udump->stats)) {
             goto exit;
@@ -1346,12 +1346,12 @@ revalidate_ukey(struct udpif *udpif, struct udpif_flow_dump *udump,
         : 0;
     ukey->stats = udump->stats;
 
-    if (!push.n_packets && !udump->need_revalidate) {
+    if (!push.n_packets && !need_revalidate) {
         ok = true;
         goto exit;
     }
 
-    if (ukey->xc && !udump->need_revalidate) {
+    if (ukey->xc && !need_revalidate) {
         xlate_from_cache(ukey->xc, &push);
         ok = true;
         goto exit;
@@ -1363,7 +1363,7 @@ revalidate_ukey(struct udpif *udpif, struct udpif_flow_dump *udump,
         goto exit;
     }
 
-    if (udump->need_revalidate) {
+    if (need_revalidate) {
         xlate_cache_clear(ukey->xc);
     }
     if (!ukey->xc) {
@@ -1374,11 +1374,11 @@ revalidate_ukey(struct udpif *udpif, struct udpif_flow_dump *udump,
     xin.resubmit_stats = push.n_packets ? &push : NULL;
     xin.xc = ukey->xc;
     xin.may_learn = push.n_packets > 0;
-    xin.skip_wildcards = !udump->need_revalidate;
+    xin.skip_wildcards = !need_revalidate;
     xlate_actions(&xin, &xout);
     xoutp = &xout;
 
-    if (!udump->need_revalidate) {
+    if (!need_revalidate) {
         ok = true;
         goto exit;
     }
@@ -1545,6 +1545,7 @@ revalidate_udumps(struct revalidator *revalidator, struct list *udumps)
         long long int used, now;
         unsigned threshold;
         struct udpif_key *ukey;
+        bool need_revalidate;
 
         now = time_msec();
         ukey = ukey_lookup(revalidator, udump);
@@ -1570,7 +1571,8 @@ revalidate_udumps(struct revalidator *revalidator, struct list *udumps)
 
         /* Only perform revalidation if the flow has high throughput. */
         threshold = REVALIDATION_THRESHOLD;
-        if ((udump->need_revalidate && udump->stats.n_bytes < threshold)
+        need_revalidate = ukey->reval_seq != udpif->last_reval_seq;
+        if ((need_revalidate && udump->stats.n_bytes < threshold)
             || !revalidate_ukey(udpif, udump, ukey)) {
             dpif_flow_del(udpif->dpif, udump->key, udump->key_len, NULL);
             ukey_delete(revalidator, ukey);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index c7a3f98..e1f2017 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -226,14 +226,12 @@ enum revalidate_reason {
     REV_STP,                   /* Spanning tree protocol port status change. */
     REV_BOND,                  /* Bonding changed. */
     REV_PORT_TOGGLED,          /* Port enabled or disabled by CFM, LACP, ...*/
-    REV_FLOW_TABLE,            /* Flow table changed. */
     REV_MAC_LEARNING,          /* Mac learning changed. */
 };
 COVERAGE_DEFINE(rev_reconfigure);
 COVERAGE_DEFINE(rev_stp);
 COVERAGE_DEFINE(rev_bond);
 COVERAGE_DEFINE(rev_port_toggled);
-COVERAGE_DEFINE(rev_flow_table);
 COVERAGE_DEFINE(rev_mac_learning);
 
 /* All datapaths of a given type share a single dpif backer instance. */
@@ -558,7 +556,6 @@ type_run(const char *type)
         case REV_STP:           COVERAGE_INC(rev_stp);           break;
         case REV_BOND:          COVERAGE_INC(rev_bond);          break;
         case REV_PORT_TOGGLED:  COVERAGE_INC(rev_port_toggled);  break;
-        case REV_FLOW_TABLE:    COVERAGE_INC(rev_flow_table);    break;
         case REV_MAC_LEARNING:  COVERAGE_INC(rev_mac_learning);  break;
         }
         backer->need_revalidate = 0;
@@ -3161,7 +3158,7 @@ complete_operation(struct rule_dpif *rule)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
 
-    ofproto->backer->need_revalidate = REV_FLOW_TABLE;
+    udpif_revalidate(ofproto->backer->udpif);
     ofoperation_complete(rule->up.pending, 0);
 }
 
-- 
1.7.9.5




More information about the dev mailing list