ofproto: Use ofproto_flow_mod for learn execution from xlate cache.

Use ofproto_flow_mod with a reference to an existing or new rule
instead of ofputil_flow_mod for learn action execution from xlate
cache

Typically we would find that when a learn xlate cache entry is
created, a preceding upcall has already created the learned flow.  In
this case the xlate cache entry takes a reference to that flow and
keeps refreshing it without needing to perform any flow table lookups.
Otherwise the creation of the xlate cache entry creates the new rule,
which is then subsequently added to the classifier.  In both cases
this is both faster and shrinks the memory cost of each learn cache
entry from ~3.5kb to about 0.3kb.

If the learned rule does not yet exist, it is created and attached to
the ofproto_flow_mod, from which it is then added.  If the referred
rule happens to expire, or is modified in any way and is thus removed
from the classifier tables, we create a new rule using the old rule as
a template, so that we can avoid storing the ofputil_flow_mod in all
cases.

Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>

ofproto/ofproto-dpif-xlate-cache.c

@@ -115,9 +115,15 @@ xlate_push_stats_entry(struct xc_entry *entry,
                             entry->mirror.mirrors,
                             stats->n_packets, stats->n_bytes);
         break;
-    case XC_LEARN:
-        ofproto_dpif_flow_mod(entry->learn.ofproto, entry->learn.fm);
+    case XC_LEARN: {
+        enum ofperr error;
+        error = ofproto_flow_mod_learn(entry->learn.ofm, true);
+        if (error) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+            VLOG_WARN_RL(&rl, "xcache LEARN action execution failed.");
+        }
         break;
+    }
     case XC_NORMAL:
         xlate_mac_learning_update(entry->normal.ofproto,
                                   entry->normal.in_port,
@@ -205,8 +211,8 @@ xlate_cache_clear_entry(struct xc_entry *entry)
         mbridge_unref(entry->mirror.mbridge);
         break;
     case XC_LEARN:
-        free(entry->learn.fm);
-        ofpbuf_delete(entry->learn.ofpacts);
+        ofproto_flow_mod_uninit(entry->learn.ofm);
+        free(entry->learn.ofm);
         break;
     case XC_NORMAL:
         break;

ofproto/ofproto-dpif-xlate-cache.h

@@ -91,9 +91,7 @@ struct xc_entry {
             uint16_t vid;
         } bond;
         struct {
-            struct ofproto_dpif *ofproto;
-            struct ofputil_flow_mod *fm;
-            struct ofpbuf *ofpacts;
+            struct ofproto_flow_mod *ofm;
         } learn;
         struct {
             struct ofproto_dpif *ofproto;

ofproto/ofproto-dpif-xlate.c

@@ -3980,38 +3980,43 @@ xlate_bundle_action(struct xlate_ctx *ctx,
     }
 }
 
-static void
-xlate_learn_action__(struct xlate_ctx *ctx, const struct ofpact_learn *learn,
-                     struct ofputil_flow_mod *fm, struct ofpbuf *ofpacts)
-{
-    learn_execute(learn, &ctx->xin->flow, fm, ofpacts);
-    if (ctx->xin->may_learn) {
-        ofproto_dpif_flow_mod(ctx->xbridge->ofproto, fm);
-    }
-}
-
 static void
 xlate_learn_action(struct xlate_ctx *ctx, const struct ofpact_learn *learn)
 {
     learn_mask(learn, ctx->wc);
 
-    if (ctx->xin->xcache) {
-        struct xc_entry *entry;
-
-        entry = xlate_cache_add_entry(ctx->xin->xcache, XC_LEARN);
-        entry->learn.ofproto = ctx->xbridge->ofproto;
-        entry->learn.fm = xmalloc(sizeof *entry->learn.fm);
-        entry->learn.ofpacts = ofpbuf_new(64);
-        xlate_learn_action__(ctx, learn, entry->learn.fm,
-                             entry->learn.ofpacts);
-    } else if (ctx->xin->may_learn) {
+    if (ctx->xin->xcache || ctx->xin->may_learn) {
         uint64_t ofpacts_stub[1024 / 8];
         struct ofputil_flow_mod fm;
+        struct ofproto_flow_mod ofm__, *ofm;
         struct ofpbuf ofpacts;
+        enum ofperr error;
+
+        if (ctx->xin->xcache) {
+            struct xc_entry *entry;
+
+            entry = xlate_cache_add_entry(ctx->xin->xcache, XC_LEARN);
+            entry->learn.ofm = xmalloc(sizeof *entry->learn.ofm);
+            ofm = entry->learn.ofm;
+        } else {
+            ofm = &ofm__;
+        }
 
         ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
-        xlate_learn_action__(ctx, learn, &fm, &ofpacts);
+        learn_execute(learn, &ctx->xin->flow, &fm, &ofpacts);
+        error = ofproto_dpif_flow_mod_init_for_learn(ctx->xbridge->ofproto,
+                                                     &fm, ofm);
         ofpbuf_uninit(&ofpacts);
+
+        if (!error && ctx->xin->may_learn) {
+            error = ofproto_flow_mod_learn(ofm, ctx->xin->xcache != NULL);
+        }
+
+        if (error) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+            VLOG_WARN_RL(&rl, "%s: LEARN action execution failed (%s).",
+                         ctx->xbridge->name, ofperr_to_string(error));
+        }
     }
 }
 

ofproto/ofproto-dpif.c

@@ -357,13 +357,16 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 /* Initial mappings of port to bridge mappings. */
 static struct shash init_ofp_ports = SHASH_INITIALIZER(&init_ofp_ports);
 
-/* Executes 'fm'.  The caller retains ownership of 'fm' and everything in
- * it. */
-void
-ofproto_dpif_flow_mod(struct ofproto_dpif *ofproto,
-                      const struct ofputil_flow_mod *fm)
-{
-    ofproto_flow_mod(&ofproto->up, fm);
+/* Initialize 'ofm' for a learn action.  If the rule already existed, reference
+ * to that rule is taken, otherwise a new rule is created.  'ofm' keeps the
+ * rule reference in both cases. */
+enum ofperr
+ofproto_dpif_flow_mod_init_for_learn(struct ofproto_dpif *ofproto,
+                                     const struct ofputil_flow_mod *fm,
+                                     struct ofproto_flow_mod *ofm)
+{
+    /* This will not take the global 'ofproto_mutex'. */
+    return ofproto_flow_mod_init_for_learn(&ofproto->up, fm, ofm);
 }
 
 /* Appends 'am' to the queue of asynchronous messages to be sent to the

ofproto/ofproto-dpif.h

@@ -164,8 +164,9 @@ void ofproto_dpif_send_async_msg(struct ofproto_dpif *,
                                  struct ofproto_async_msg *);
 int ofproto_dpif_send_packet(const struct ofport_dpif *, bool oam,
                              struct dp_packet *);
-void ofproto_dpif_flow_mod(struct ofproto_dpif *,
-                           const struct ofputil_flow_mod *);
+enum ofperr ofproto_dpif_flow_mod_init_for_learn(struct ofproto_dpif *,
+                                                 const struct ofputil_flow_mod *,
+                                                 struct ofproto_flow_mod *);
 
 struct ofport_dpif *odp_port_to_ofport(const struct dpif_backer *, odp_port_t);
 struct ofport_dpif *ofp_port_to_ofport(const struct ofproto_dpif *,

ofproto/ofproto-provider.h

@@ -1899,6 +1899,12 @@ struct ofproto_group_mod {
 
 enum ofperr ofproto_flow_mod(struct ofproto *, const struct ofputil_flow_mod *)
     OVS_EXCLUDED(ofproto_mutex);
+enum ofperr ofproto_flow_mod_init_for_learn(struct ofproto *,
+                                            const struct ofputil_flow_mod *,
+                                            struct ofproto_flow_mod *)
+    OVS_EXCLUDED(ofproto_mutex);
+enum ofperr ofproto_flow_mod_learn(struct ofproto_flow_mod *, bool keep_ref)
+    OVS_EXCLUDED(ofproto_mutex);
 void ofproto_add_flow(struct ofproto *, const struct match *, int priority,
                       const struct ofpact *ofpacts, size_t ofpacts_len)
     OVS_EXCLUDED(ofproto_mutex);