Implement new "learn" action.

There are a few loose ends here.  First, learning actions cause too much
flow revalidation.  Upcoming commits will fix that problem.  The following
additional issues have not yet been addressed:

    * Resource limits: nothing yet limits the maximum number of flows that
      can be learned.  It is possible to exhaust all system memory.

    * Age reporting: there is no way to find out how soon a learned table
      entry is due to be evicted.

To try this action out, here's a recipe for a very simple-minded MAC
learning switch.  It uses a 10-second MAC expiration time to make it easier
to see what's going on:

ovs-vsctl del-controller br0
ovs-ofctl del-flows br0
ovs-ofctl add-flow br0 "table=0 actions=learn(table=1, hard_timeout=10, \
	NXM_OF_VLAN_TCI[0..11], NXM_OF_ETH_DST[]=NXM_OF_ETH_SRC[], \
	output:NXM_OF_IN_PORT[]), resubmit(,1)"
ovs-ofctl add-flow br0 "table=1 priority=0 actions=flood"

You can then dump the MAC learning table with:

ovs-ofctl dump-flows br0 table=1

3 files changed, 112 insertions, 42 deletions

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 93920452a..ac196eeb6 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -32,6 +32,7 @@
 #include "fail-open.h"
 #include "hmapx.h"
 #include "lacp.h"
+#include "learn.h"
 #include "mac-learning.h"
 #include "multipath.h"
 #include "netdev.h"
@@ -166,6 +167,12 @@ struct action_xlate_ctx {
      * revalidating without a packet to refer to. */
     const struct ofpbuf *packet;
 
+    /* Should OFPP_NORMAL MAC learning and NXAST_LEARN actions execute?  We
+     * want to execute them if we are actually processing a packet, or if we
+     * are accounting for packets that the datapath has processed, but not if
+     * we are just revalidating. */
+    bool may_learn;
+
     /* If nonnull, called just before executing a resubmit action.
      *
      * This is normally null so the client has to set it manually after
@@ -176,9 +183,11 @@ struct action_xlate_ctx {
  * to look at them after it returns. */
 
     struct ofpbuf *odp_actions; /* Datapath actions. */
-    tag_type tags;              /* Tags associated with OFPP_NORMAL actions. */
+    tag_type tags;              /* Tags associated with actions. */
     bool may_set_up_flow;       /* True ordinarily; false if the actions must
                                  * be reassessed for every packet. */
+    bool has_learn;             /* Actions include NXAST_LEARN? */
+    bool has_normal;            /* Actions output to OFPP_NORMAL? */
     uint16_t nf_output_iface;   /* Output interface index for NetFlow. */
 
 /* xlate_actions() initializes and uses these members, but the client has no
@@ -229,6 +238,8 @@ struct facet {
     bool installed;              /* Installed in datapath? */
     bool may_install;            /* True ordinarily; false if actions must
                                   * be reassessed for every packet. */
+    bool has_learn;              /* Actions include NXAST_LEARN? */
+    bool has_normal;             /* Actions output to OFPP_NORMAL? */
     size_t actions_len;          /* Number of bytes in actions[]. */
     struct nlattr *actions;      /* Datapath actions. */
     tag_type tags;               /* Tags. */
@@ -2168,6 +2179,8 @@ facet_make_actions(struct ofproto_dpif *p, struct facet *facet,
     odp_actions = xlate_actions(&ctx, rule->up.actions, rule->up.n_actions);
     facet->tags = ctx.tags;
     facet->may_install = ctx.may_set_up_flow;
+    facet->has_learn = ctx.has_learn;
+    facet->has_normal = ctx.has_normal;
     facet->nf_flow.output_iface = ctx.nf_output_iface;
 
     if (facet->actions_len != odp_actions->size
@@ -2239,12 +2252,9 @@ static void
 facet_account(struct ofproto_dpif *ofproto, struct facet *facet)
 {
     uint64_t n_bytes;
-    struct ofbundle *in_bundle;
     const struct nlattr *a;
-    tag_type dummy = 0;
     unsigned int left;
     ovs_be16 vlan_tci;
-    int vlan;
 
     if (facet->byte_count <= facet->accounted_bytes) {
         return;
@@ -2252,22 +2262,19 @@ facet_account(struct ofproto_dpif *ofproto, struct facet *facet)
     n_bytes = facet->byte_count - facet->accounted_bytes;
     facet->accounted_bytes = facet->byte_count;
 
-    /* Test that 'tags' is nonzero to ensure that only flows that include an
-     * OFPP_NORMAL action are used for learning and bond slave rebalancing.
-     * This works because OFPP_NORMAL always sets a nonzero tag value.
-     *
-     * Feed information from the active flows back into the learning table to
+    /* Feed information from the active flows back into the learning table to
      * ensure that table is always in sync with what is actually flowing
      * through the datapath. */
-    if (!facet->tags
-        || !is_admissible(ofproto, &facet->flow, false, &dummy,
-                          &vlan, &in_bundle)) {
-        return;
-    }
+    if (facet->has_learn || facet->has_normal) {
+        struct action_xlate_ctx ctx;
 
-    update_learning_table(ofproto, &facet->flow, vlan, in_bundle);
+        action_xlate_ctx_init(&ctx, ofproto, &facet->flow, NULL);
+        ctx.may_learn = true;
+        ofpbuf_delete(xlate_actions(&ctx, facet->rule->up.actions,
+                                    facet->rule->up.n_actions));
+    }
 
-    if (!ofproto->has_bonded_bundles) {
+    if (!facet->has_normal || !ofproto->has_bonded_bundles) {
         return;
     }
 
@@ -2491,6 +2498,8 @@ facet_revalidate(struct ofproto_dpif *ofproto, struct facet *facet)
     facet->tags = ctx.tags;
     facet->nf_flow.output_iface = ctx.nf_output_iface;
     facet->may_install = ctx.may_set_up_flow;
+    facet->has_learn = ctx.has_learn;
+    facet->has_normal = ctx.has_normal;
     if (actions_changed) {
         free(facet->actions);
         facet->actions_len = odp_actions->size;
@@ -3169,6 +3178,26 @@ slave_enabled_cb(uint16_t ofp_port, void *ofproto_)
 }
 
 static void
+xlate_learn_action(struct action_xlate_ctx *ctx,
+                   const struct nx_action_learn *learn)
+{
+    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+    struct ofputil_flow_mod fm;
+    int error;
+
+    learn_execute(learn, &ctx->flow, &fm);
+
+    error = ofproto_flow_mod(&ctx->ofproto->up, &fm);
+    if (error && !VLOG_DROP_WARN(&rl)) {
+        char *msg = ofputil_error_to_string(error);
+        VLOG_WARN("learning action failed to modify flow table (%s)", msg);
+        free(msg);
+    }
+
+    free(fm.actions);
+}
+
+static void
 do_xlate_actions(const union ofp_action *in, size_t n_in,
                  struct action_xlate_ctx *ctx)
 {
@@ -3325,6 +3354,13 @@ do_xlate_actions(const union ofp_action *in, size_t n_in,
             naor = (const struct nx_action_output_reg *) ia;
             xlate_output_reg_action(ctx, naor);
             break;
+
+        case OFPUTIL_NXAST_LEARN:
+            ctx->has_learn = true;
+            if (ctx->may_learn) {
+                xlate_learn_action(ctx, (const struct nx_action_learn *) ia);
+            }
+            break;
         }
     }
 }
@@ -3337,6 +3373,7 @@ action_xlate_ctx_init(struct action_xlate_ctx *ctx,
     ctx->ofproto = ofproto;
     ctx->flow = *flow;
     ctx->packet = packet;
+    ctx->may_learn = packet != NULL;
     ctx->resubmit_hook = NULL;
 }
 
@@ -3349,6 +3386,8 @@ xlate_actions(struct action_xlate_ctx *ctx,
     ctx->odp_actions = ofpbuf_new(512);
     ctx->tags = 0;
     ctx->may_set_up_flow = true;
+    ctx->has_learn = false;
+    ctx->has_normal = false;
     ctx->nf_output_iface = NF_OUT_DROP;
     ctx->recurse = 0;
     ctx->priority = 0;
@@ -3821,6 +3860,7 @@ is_admissible(struct ofproto_dpif *ofproto, const struct flow *flow,
                          "port %"PRIu16,
                          ofproto->up.name, flow->in_port);
         }
+        *vlanp = -1;
         return false;
     }
     *vlanp = vlan = flow_get_vlan(ofproto, flow, in_bundle, have_packet);
@@ -3879,6 +3919,8 @@ xlate_normal(struct action_xlate_ctx *ctx)
     struct mac_entry *mac;
     int vlan;
 
+    ctx->has_normal = true;
+
     /* Check whether we should drop packets in this flow. */
     if (!is_admissible(ctx->ofproto, &ctx->flow, ctx->packet != NULL,
                        &ctx->tags, &vlan, &in_bundle)) {
@@ -3886,8 +3928,8 @@ xlate_normal(struct action_xlate_ctx *ctx)
         goto done;
     }
 
-    /* Learn source MAC (but don't try to learn from revalidation). */
-    if (ctx->packet) {
+    /* Learn source MAC. */
+    if (ctx->may_learn) {
         update_learning_table(ctx->ofproto, &ctx->flow, vlan, in_bundle);
     }
 
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index b1c697223..3037bb2e9 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -26,6 +26,8 @@
 #include "shash.h"
 #include "timeval.h"
 
+struct ofputil_flow_mod;
+
 /* An OpenFlow switch.
  *
  * With few exceptions, ofproto implementations may look at these fields but
@@ -957,6 +959,18 @@ extern const struct ofproto_class ofproto_dpif_class;
 int ofproto_class_register(const struct ofproto_class *);
 int ofproto_class_unregister(const struct ofproto_class *);
 
+/* ofproto_flow_mod() returns this value if the flow_mod could not be processed
+ * because it overlaps with an ongoing flow table operation that has not yet
+ * completed.  The caller should retry the operation later.
+ *
+ * ofproto.c also uses this value internally for additional (similar) purposes.
+ *
+ * This particular value is a good choice because it is negative (so it won't
+ * collide with any errno value or any value returned by ofp_mkerr()) and large
+ * (so it won't accidentally collide with EOF or a negative errno value). */
+enum { OFPROTO_POSTPONE = -100000 };
+
+int ofproto_flow_mod(struct ofproto *, const struct ofputil_flow_mod *);
 void ofproto_add_flow(struct ofproto *, const struct cls_rule *,
                       const union ofp_action *, size_t n_actions);
 bool ofproto_delete_flow(struct ofproto *, const struct cls_rule *);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index b96fa6e67..979335543 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -139,16 +139,10 @@ static int add_flow(struct ofproto *, struct ofconn *,
                     const struct ofputil_flow_mod *,
                     const struct ofp_header *);
 
-/* This return value tells handle_openflow() that processing of the current
- * OpenFlow message must be postponed until some ongoing operations have
- * completed.
- *
- * This particular value is a good choice because it is negative (so it won't
- * collide with any errno value or any value returned by ofp_mkerr()) and large
- * (so it won't accidentally collide with EOF or a negative errno value). */
-enum { OFPROTO_POSTPONE = -100000 };
-
 static bool handle_openflow(struct ofconn *, struct ofpbuf *);
+static int handle_flow_mod__(struct ofproto *, struct ofconn *,
+                             const struct ofputil_flow_mod *,
+                             const struct ofp_header *);
 
 static void update_port(struct ofproto *, const char *devname);
 static int init_ports(struct ofproto *);
@@ -1062,6 +1056,18 @@ ofproto_add_flow(struct ofproto *ofproto, const struct cls_rule *cls_rule,
     }
 }
 
+/* Executes the flow modification specified in 'fm'.  Returns 0 on success, an
+ * OpenFlow error code as encoded by ofp_mkerr() on failure, or
+ * OFPROTO_POSTPONE if the operation cannot be initiated now but may be retried
+ * later.
+ *
+ * This is a helper function for in-band control and fail-open. */
+int
+ofproto_flow_mod(struct ofproto *ofproto, const struct ofputil_flow_mod *fm)
+{
+    return handle_flow_mod__(ofproto, NULL, fm, NULL);
+}
+
 /* Searches for a rule with matching criteria exactly equal to 'target' in
  * ofproto's table 0 and, if it finds one, deletes it.
  *
@@ -2190,8 +2196,9 @@ is_flow_deletion_pending(const struct ofproto *ofproto,
  * in which no matching flow already exists in the flow table.
  *
  * Adds the flow specified by 'ofm', which is followed by 'n_actions'
- * ofp_actions, to the ofproto's flow table.  Returns 0 on success or an
- * OpenFlow error code as encoded by ofp_mkerr() on failure.
+ * ofp_actions, to the ofproto's flow table.  Returns 0 on success, an OpenFlow
+ * error code as encoded by ofp_mkerr() on failure, or OFPROTO_POSTPONE if the
+ * operation cannot be initiated now but may be retried later.
  *
  * 'ofconn' is used to retrieve the packet buffer specified in ofm->buffer_id,
  * if any. */
@@ -2472,7 +2479,6 @@ ofproto_rule_expire(struct rule *rule, uint8_t reason)
 static int
 handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh)
 {
-    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
     struct ofputil_flow_mod fm;
     int error;
 
@@ -2481,11 +2487,6 @@ handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh)
         return error;
     }
 
-    if (ofproto->n_pending >= 50) {
-        assert(!list_is_empty(&ofproto->pending));
-        return OFPROTO_POSTPONE;
-    }
-
     error = ofputil_decode_flow_mod(&fm, oh,
                                     ofconn_get_flow_mod_table_id(ofconn));
     if (error) {
@@ -2500,24 +2501,37 @@ handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh)
         return ofp_mkerr(OFPET_FLOW_MOD_FAILED, OFPFMFC_ALL_TABLES_FULL);
     }
 
-    switch (fm.command) {
+    return handle_flow_mod__(ofconn_get_ofproto(ofconn), ofconn, &fm, oh);
+}
+
+static int
+handle_flow_mod__(struct ofproto *ofproto, struct ofconn *ofconn,
+                  const struct ofputil_flow_mod *fm,
+                  const struct ofp_header *oh)
+{
+    if (ofproto->n_pending >= 50) {
+        assert(!list_is_empty(&ofproto->pending));
+        return OFPROTO_POSTPONE;
+    }
+
+    switch (fm->command) {
     case OFPFC_ADD:
-        return add_flow(ofproto, ofconn, &fm, oh);
+        return add_flow(ofproto, ofconn, fm, oh);
 
     case OFPFC_MODIFY:
-        return modify_flows_loose(ofproto, ofconn, &fm, oh);
+        return modify_flows_loose(ofproto, ofconn, fm, oh);
 
     case OFPFC_MODIFY_STRICT:
-        return modify_flow_strict(ofproto, ofconn, &fm, oh);
+        return modify_flow_strict(ofproto, ofconn, fm, oh);
 
     case OFPFC_DELETE:
-        return delete_flows_loose(ofproto, ofconn, &fm, oh);
+        return delete_flows_loose(ofproto, ofconn, fm, oh);
 
     case OFPFC_DELETE_STRICT:
-        return delete_flow_strict(ofproto, ofconn, &fm, oh);
+        return delete_flow_strict(ofproto, ofconn, fm, oh);
 
     default:
-        if (fm.command > 0xff) {
+        if (fm->command > 0xff) {
             VLOG_WARN_RL(&rl, "flow_mod has explicit table_id but "
                          "flow_mod_table_id extension is not enabled");
         }