Setting tag sliver-openvswitch-2.2.90-1
[sliver-openvswitch.git] / lib / learn.c
index d749ce2..8727a55 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2012 Nicira Networks.
+ * Copyright (c) 2011, 2012, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
 
 #include "byte-order.h"
 #include "dynamic-string.h"
+#include "match.h"
 #include "meta-flow.h"
 #include "nx-match.h"
+#include "ofp-actions.h"
 #include "ofp-errors.h"
 #include "ofp-util.h"
 #include "ofpbuf.h"
@@ -46,19 +48,6 @@ get_be32(const void **pp)
     return value;
 }
 
-static uint64_t
-get_bits(int n_bits, const void **p)
-{
-    int n_segs = DIV_ROUND_UP(n_bits, 16);
-    uint64_t value;
-
-    value = 0;
-    while (n_segs-- > 0) {
-        value = (value << 16) | ntohs(get_be16(p));
-    }
-    return value;
-}
-
 static void
 get_subfield(int n_bits, const void **p, struct mf_subfield *sf)
 {
@@ -90,97 +79,96 @@ learn_min_len(uint16_t header)
     return min_len;
 }
 
-static enum ofperr
-learn_check_header(uint16_t header, size_t len)
+/* Converts 'nal' into a "struct ofpact_learn" and appends that struct to
+ * 'ofpacts'.  Returns 0 if successful, otherwise an OFPERR_*. */
+enum ofperr
+learn_from_openflow(const struct nx_action_learn *nal, struct ofpbuf *ofpacts)
 {
-    int src_type = header & NX_LEARN_SRC_MASK;
-    int dst_type = header & NX_LEARN_DST_MASK;
+    struct ofpact_learn *learn;
+    const void *p, *end;
 
-    /* Check for valid src and dst type combination. */
-    if (dst_type == NX_LEARN_DST_MATCH ||
-        dst_type == NX_LEARN_DST_LOAD ||
-        (dst_type == NX_LEARN_DST_OUTPUT &&
-         src_type == NX_LEARN_SRC_FIELD)) {
-        /* OK. */
-    } else {
+    if (nal->pad) {
         return OFPERR_OFPBAC_BAD_ARGUMENT;
     }
 
-    /* Check that the arguments don't overrun the end of the action. */
-    if (len < learn_min_len(header)) {
-        return OFPERR_OFPBAC_BAD_LEN;
+    learn = ofpact_put_LEARN(ofpacts);
+
+    learn->idle_timeout = ntohs(nal->idle_timeout);
+    learn->hard_timeout = ntohs(nal->hard_timeout);
+    learn->priority = ntohs(nal->priority);
+    learn->cookie = ntohll(nal->cookie);
+    learn->table_id = nal->table_id;
+    learn->fin_idle_timeout = ntohs(nal->fin_idle_timeout);
+    learn->fin_hard_timeout = ntohs(nal->fin_hard_timeout);
+
+    /* We only support "send-flow-removed" for now. */
+    switch (ntohs(nal->flags)) {
+    case 0:
+        learn->flags = 0;
+        break;
+    case OFPFF_SEND_FLOW_REM:
+        learn->flags = OFPUTIL_FF_SEND_FLOW_REM;
+        break;
+    default:
+        return OFPERR_OFPBAC_BAD_ARGUMENT;
     }
 
-    return 0;
-}
-
-/* Checks that 'learn' (which must be at least 'sizeof *learn' bytes long) is a
- * valid action on 'flow'. */
-enum ofperr
-learn_check(const struct nx_action_learn *learn, const struct flow *flow)
-{
-    struct cls_rule rule;
-    const void *p, *end;
-
-    cls_rule_init_catchall(&rule, 0);
-
-    if (learn->flags & ~htons(OFPFF_SEND_FLOW_REM)
-        || learn->pad
-        || learn->table_id == 0xff) {
+    if (learn->table_id == 0xff) {
         return OFPERR_OFPBAC_BAD_ARGUMENT;
     }
 
-    end = (char *) learn + ntohs(learn->len);
-    for (p = learn + 1; p != end; ) {
+    end = (char *) nal + ntohs(nal->len);
+    for (p = nal + 1; p != end; ) {
+        struct ofpact_learn_spec *spec;
         uint16_t header = ntohs(get_be16(&p));
-        int n_bits = header & NX_LEARN_N_BITS_MASK;
-        int src_type = header & NX_LEARN_SRC_MASK;
-        int dst_type = header & NX_LEARN_DST_MASK;
-
-        enum ofperr error;
-        uint64_t value;
 
         if (!header) {
             break;
         }
 
-        error = learn_check_header(header, (char *) end - (char *) p);
-        if (error) {
-            return error;
-        }
+        spec = ofpbuf_put_zeros(ofpacts, sizeof *spec);
+        learn = ofpacts->frame;
+        learn->n_specs++;
 
-        /* Check the source. */
-        if (src_type == NX_LEARN_SRC_FIELD) {
-            struct mf_subfield src;
+        spec->src_type = header & NX_LEARN_SRC_MASK;
+        spec->dst_type = header & NX_LEARN_DST_MASK;
+        spec->n_bits = header & NX_LEARN_N_BITS_MASK;
 
-            get_subfield(n_bits, &p, &src);
-            error = mf_check_src(&src, flow);
-            if (error) {
-                return error;
-            }
-            value = 0;
+        /* Check for valid src and dst type combination. */
+        if (spec->dst_type == NX_LEARN_DST_MATCH ||
+            spec->dst_type == NX_LEARN_DST_LOAD ||
+            (spec->dst_type == NX_LEARN_DST_OUTPUT &&
+             spec->src_type == NX_LEARN_SRC_FIELD)) {
+            /* OK. */
         } else {
-            value = get_bits(n_bits, &p);
+            return OFPERR_OFPBAC_BAD_ARGUMENT;
         }
 
-        /* Check the destination. */
-        if (dst_type == NX_LEARN_DST_MATCH || dst_type == NX_LEARN_DST_LOAD) {
-            struct mf_subfield dst;
+        /* Check that the arguments don't overrun the end of the action. */
+        if ((char *) end - (char *) p < learn_min_len(header)) {
+            return OFPERR_OFPBAC_BAD_LEN;
+        }
 
-            get_subfield(n_bits, &p, &dst);
-            error = (dst_type == NX_LEARN_DST_LOAD
-                     ? mf_check_dst(&dst, &rule.flow)
-                     : mf_check_src(&dst, &rule.flow));
-            if (error) {
-                return error;
-            }
+        /* Get the source. */
+        if (spec->src_type == NX_LEARN_SRC_FIELD) {
+            get_subfield(spec->n_bits, &p, &spec->src);
+        } else {
+            int p_bytes = 2 * DIV_ROUND_UP(spec->n_bits, 16);
 
-            if (dst_type == NX_LEARN_DST_MATCH
-                && src_type == NX_LEARN_SRC_IMMEDIATE) {
-                mf_set_subfield(&dst, value, &rule);
-            }
+            bitwise_copy(p, p_bytes, 0,
+                         &spec->src_imm, sizeof spec->src_imm, 0,
+                         spec->n_bits);
+            p = (const uint8_t *) p + p_bytes;
+        }
+
+        /* Get the destination. */
+        if (spec->dst_type == NX_LEARN_DST_MATCH ||
+            spec->dst_type == NX_LEARN_DST_LOAD) {
+            get_subfield(spec->n_bits, &p, &spec->dst);
         }
     }
+    ofpact_update_len(ofpacts, &learn->ofpact);
+
     if (!is_all_zeros(p, (char *) end - (char *) p)) {
         return OFPERR_OFPBAC_BAD_ARGUMENT;
     }
@@ -188,80 +176,50 @@ learn_check(const struct nx_action_learn *learn, const struct flow *flow)
     return 0;
 }
 
-void
-learn_execute(const struct nx_action_learn *learn, const struct flow *flow,
-              struct ofputil_flow_mod *fm)
+/* Checks that 'learn' is a valid action on 'flow'.  Returns 0 if it is valid,
+ * otherwise an OFPERR_*. */
+enum ofperr
+learn_check(const struct ofpact_learn *learn, const struct flow *flow)
 {
-    const void *p, *end;
-    struct ofpbuf actions;
-
-    cls_rule_init_catchall(&fm->cr, ntohs(learn->priority));
-    fm->cookie = learn->cookie;
-    fm->table_id = learn->table_id;
-    fm->command = OFPFC_MODIFY_STRICT;
-    fm->idle_timeout = ntohs(learn->idle_timeout);
-    fm->hard_timeout = ntohs(learn->hard_timeout);
-    fm->buffer_id = UINT32_MAX;
-    fm->out_port = OFPP_NONE;
-    fm->flags = ntohs(learn->flags) & OFPFF_SEND_FLOW_REM;
-    fm->actions = NULL;
-    fm->n_actions = 0;
-
-    ofpbuf_init(&actions, 64);
-
-    if (learn->fin_idle_timeout || learn->fin_hard_timeout) {
-        struct nx_action_fin_timeout *naft;
-
-        naft = ofputil_put_NXAST_FIN_TIMEOUT(&actions);
-        naft->fin_idle_timeout = learn->fin_idle_timeout;
-        naft->fin_hard_timeout = learn->fin_hard_timeout;
-    }
-
-    for (p = learn + 1, end = (char *) learn + ntohs(learn->len); p != end; ) {
-        uint16_t header = ntohs(get_be16(&p));
-        int n_bits = header & NX_LEARN_N_BITS_MASK;
-        int src_type = header & NX_LEARN_SRC_MASK;
-        int dst_type = header & NX_LEARN_DST_MASK;
-        uint64_t value;
+    const struct ofpact_learn_spec *spec;
+    struct match match;
 
-        struct nx_action_reg_load *load;
-        struct mf_subfield dst;
-
-        if (!header) {
-            break;
-        }
-
-        if (src_type == NX_LEARN_SRC_FIELD) {
-            struct mf_subfield src;
+    match_init_catchall(&match);
+    for (spec = learn->specs; spec < &learn->specs[learn->n_specs]; spec++) {
+        enum ofperr error;
 
-            get_subfield(n_bits, &p, &src);
-            value = mf_get_subfield(&src, flow);
-        } else {
-            value = get_bits(n_bits, &p);
+        /* Check the source. */
+        if (spec->src_type == NX_LEARN_SRC_FIELD) {
+            error = mf_check_src(&spec->src, flow);
+            if (error) {
+                return error;
+            }
         }
 
-        switch (dst_type) {
+        /* Check the destination. */
+        switch (spec->dst_type) {
         case NX_LEARN_DST_MATCH:
-            get_subfield(n_bits, &p, &dst);
-            mf_set_subfield(&dst, value, &fm->cr);
+            error = mf_check_src(&spec->dst, &match.flow);
+            if (error) {
+                return error;
+            }
+
+            mf_write_subfield(&spec->dst, &spec->src_imm, &match);
             break;
 
         case NX_LEARN_DST_LOAD:
-            get_subfield(n_bits, &p, &dst);
-            load = ofputil_put_NXAST_REG_LOAD(&actions);
-            load->ofs_nbits = nxm_encode_ofs_nbits(dst.ofs, dst.n_bits);
-            load->dst = htonl(dst.field->nxm_header);
-            load->value = htonll(value);
+            error = mf_check_dst(&spec->dst, &match.flow);
+            if (error) {
+                return error;
+            }
             break;
 
         case NX_LEARN_DST_OUTPUT:
-            ofputil_put_OFPAT_OUTPUT(&actions)->port = htons(value);
+            /* Nothing to do. */
             break;
         }
     }
-
-    fm->actions = ofpbuf_steal_data(&actions);
-    fm->n_actions = actions.size / sizeof(struct ofp_action_header);
+    return 0;
 }
 
 static void
@@ -288,22 +246,222 @@ put_u32(struct ofpbuf *b, uint32_t x)
     put_be32(b, htonl(x));
 }
 
-struct learn_spec {
-    int n_bits;
+/* Converts 'learn' into a "struct nx_action_learn" and appends that action to
+ * 'ofpacts'. */
+void
+learn_to_nxast(const struct ofpact_learn *learn, struct ofpbuf *openflow)
+{
+    const struct ofpact_learn_spec *spec;
+    struct nx_action_learn *nal;
+    size_t start_ofs;
+
+    start_ofs = ofpbuf_size(openflow);
+    nal = ofputil_put_NXAST_LEARN(openflow);
+    nal->idle_timeout = htons(learn->idle_timeout);
+    nal->hard_timeout = htons(learn->hard_timeout);
+    nal->fin_idle_timeout = htons(learn->fin_idle_timeout);
+    nal->fin_hard_timeout = htons(learn->fin_hard_timeout);
+    nal->priority = htons(learn->priority);
+    nal->cookie = htonll(learn->cookie);
+    nal->flags = htons(learn->flags);
+    nal->table_id = learn->table_id;
+
+    for (spec = learn->specs; spec < &learn->specs[learn->n_specs]; spec++) {
+        put_u16(openflow, spec->n_bits | spec->dst_type | spec->src_type);
+
+        if (spec->src_type == NX_LEARN_SRC_FIELD) {
+            put_u32(openflow, spec->src.field->nxm_header);
+            put_u16(openflow, spec->src.ofs);
+        } else {
+            size_t n_dst_bytes = 2 * DIV_ROUND_UP(spec->n_bits, 16);
+            uint8_t *bits = ofpbuf_put_zeros(openflow, n_dst_bytes);
+            bitwise_copy(&spec->src_imm, sizeof spec->src_imm, 0,
+                         bits, n_dst_bytes, 0,
+                         spec->n_bits);
+        }
+
+        if (spec->dst_type == NX_LEARN_DST_MATCH ||
+            spec->dst_type == NX_LEARN_DST_LOAD) {
+            put_u32(openflow, spec->dst.field->nxm_header);
+            put_u16(openflow, spec->dst.ofs);
+        }
+    }
+
+    if ((ofpbuf_size(openflow) - start_ofs) % 8) {
+        ofpbuf_put_zeros(openflow, 8 - (ofpbuf_size(openflow) - start_ofs) % 8);
+    }
+
+    nal = ofpbuf_at_assert(openflow, start_ofs, sizeof *nal);
+    nal->len = htons(ofpbuf_size(openflow) - start_ofs);
+}
+
+/* Composes 'fm' so that executing it will implement 'learn' given that the
+ * packet being processed has 'flow' as its flow.
+ *
+ * Uses 'ofpacts' to store the flow mod's actions.  The caller must initialize
+ * 'ofpacts' and retains ownership of it.  'fm->ofpacts' will point into the
+ * 'ofpacts' buffer.
+ *
+ * The caller has to actually execute 'fm'. */
+void
+learn_execute(const struct ofpact_learn *learn, const struct flow *flow,
+              struct ofputil_flow_mod *fm, struct ofpbuf *ofpacts)
+{
+    const struct ofpact_learn_spec *spec;
+
+    match_init_catchall(&fm->match);
+    fm->priority = learn->priority;
+    fm->cookie = htonll(0);
+    fm->cookie_mask = htonll(0);
+    fm->new_cookie = htonll(learn->cookie);
+    fm->modify_cookie = fm->new_cookie != OVS_BE64_MAX;
+    fm->table_id = learn->table_id;
+    fm->command = OFPFC_MODIFY_STRICT;
+    fm->idle_timeout = learn->idle_timeout;
+    fm->hard_timeout = learn->hard_timeout;
+    fm->buffer_id = UINT32_MAX;
+    fm->out_port = OFPP_NONE;
+    fm->flags = learn->flags;
+    fm->ofpacts = NULL;
+    fm->ofpacts_len = 0;
+
+    if (learn->fin_idle_timeout || learn->fin_hard_timeout) {
+        struct ofpact_fin_timeout *oft;
+
+        oft = ofpact_put_FIN_TIMEOUT(ofpacts);
+        oft->fin_idle_timeout = learn->fin_idle_timeout;
+        oft->fin_hard_timeout = learn->fin_hard_timeout;
+    }
+
+    for (spec = learn->specs; spec < &learn->specs[learn->n_specs]; spec++) {
+        union mf_subvalue value;
+        int chunk, ofs;
+
+        if (spec->src_type == NX_LEARN_SRC_FIELD) {
+            mf_read_subfield(&spec->src, flow, &value);
+        } else {
+            value = spec->src_imm;
+        }
 
-    int src_type;
-    struct mf_subfield src;
-    uint8_t src_imm[sizeof(union mf_value)];
+        switch (spec->dst_type) {
+        case NX_LEARN_DST_MATCH:
+            mf_write_subfield(&spec->dst, &value, &fm->match);
+            break;
 
-    int dst_type;
+        case NX_LEARN_DST_LOAD:
+            for (ofs = 0; ofs < spec->n_bits; ofs += chunk) {
+                struct ofpact_reg_load *load;
+
+                chunk = MIN(spec->n_bits - ofs, 64);
+
+                load = ofpact_put_REG_LOAD(ofpacts);
+                load->dst.field = spec->dst.field;
+                load->dst.ofs = spec->dst.ofs + ofs;
+                load->dst.n_bits = chunk;
+                bitwise_copy(&value, sizeof value, ofs,
+                             &load->subvalue, sizeof load->subvalue, 0,
+                             chunk);
+            }
+            break;
+
+        case NX_LEARN_DST_OUTPUT:
+            if (spec->n_bits <= 16
+                || is_all_zeros(value.u8, sizeof value - 2)) {
+                ofp_port_t port = u16_to_ofp(ntohs(value.be16[7]));
+
+                if (ofp_to_u16(port) < ofp_to_u16(OFPP_MAX)
+                    || port == OFPP_IN_PORT
+                    || port == OFPP_FLOOD
+                    || port == OFPP_LOCAL
+                    || port == OFPP_ALL) {
+                    ofpact_put_OUTPUT(ofpacts)->port = port;
+                }
+            }
+            break;
+        }
+    }
+    ofpact_pad(ofpacts);
+
+    fm->ofpacts = ofpbuf_data(ofpacts);
+    fm->ofpacts_len = ofpbuf_size(ofpacts);
+}
+
+/* Perform a bitwise-OR on 'wc''s fields that are relevant as sources in
+ * the learn action 'learn'. */
+void
+learn_mask(const struct ofpact_learn *learn, struct flow_wildcards *wc)
+{
+    const struct ofpact_learn_spec *spec;
+    union mf_subvalue value;
+
+    memset(&value, 0xff, sizeof value);
+    for (spec = learn->specs; spec < &learn->specs[learn->n_specs]; spec++) {
+        if (spec->src_type == NX_LEARN_SRC_FIELD) {
+            mf_write_subfield_flow(&spec->src, &value, &wc->masks);
+        }
+    }
+}
+
+/* Returns NULL if successful, otherwise a malloc()'d string describing the
+ * error.  The caller is responsible for freeing the returned string. */
+static char * WARN_UNUSED_RESULT
+learn_parse_load_immediate(const char *s, struct ofpact_learn_spec *spec)
+{
+    const char *full_s = s;
+    const char *arrow = strstr(s, "->");
     struct mf_subfield dst;
-};
+    union mf_subvalue imm;
+    char *error;
+
+    memset(&imm, 0, sizeof imm);
+    if (s[0] == '0' && (s[1] == 'x' || s[1] == 'X') && arrow) {
+        const char *in = arrow - 1;
+        uint8_t *out = imm.u8 + sizeof imm.u8 - 1;
+        int n = arrow - (s + 2);
+        int i;
 
-static void
+        for (i = 0; i < n; i++) {
+            int hexit = hexit_value(in[-i]);
+            if (hexit < 0) {
+                return xasprintf("%s: bad hex digit in value", full_s);
+            }
+            out[-(i / 2)] |= i % 2 ? hexit << 4 : hexit;
+        }
+        s = arrow;
+    } else {
+        imm.be64[1] = htonll(strtoull(s, (char **) &s, 0));
+    }
+
+    if (strncmp(s, "->", 2)) {
+        return xasprintf("%s: missing `->' following value", full_s);
+    }
+    s += 2;
+
+    error = mf_parse_subfield(&dst, s);
+    if (error) {
+        return error;
+    }
+
+    if (!bitwise_is_all_zeros(&imm, sizeof imm, dst.n_bits,
+                              (8 * sizeof imm) - dst.n_bits)) {
+        return xasprintf("%s: value does not fit into %u bits",
+                         full_s, dst.n_bits);
+    }
+
+    spec->n_bits = dst.n_bits;
+    spec->src_type = NX_LEARN_SRC_IMMEDIATE;
+    spec->src_imm = imm;
+    spec->dst_type = NX_LEARN_DST_LOAD;
+    spec->dst = dst;
+    return NULL;
+}
+
+/* Returns NULL if successful, otherwise a malloc()'d string describing the
+ * error.  The caller is responsible for freeing the returned string. */
+static char * WARN_UNUSED_RESULT
 learn_parse_spec(const char *orig, char *name, char *value,
-                 struct learn_spec *spec)
+                 struct ofpact_learn_spec *spec)
 {
-    memset(spec, 0, sizeof *spec);
     if (mf_from_name(name)) {
         const struct mf_field *dst = mf_from_name(name);
         union mf_value imm;
@@ -311,33 +469,37 @@ learn_parse_spec(const char *orig, char *name, char *value,
 
         error = mf_parse_value(dst, value, &imm);
         if (error) {
-            ovs_fatal(0, "%s", error);
+            return error;
         }
 
         spec->n_bits = dst->n_bits;
         spec->src_type = NX_LEARN_SRC_IMMEDIATE;
-        memcpy(spec->src_imm, &imm, dst->n_bytes);
+        memset(&spec->src_imm, 0, sizeof spec->src_imm);
+        memcpy(&spec->src_imm.u8[sizeof spec->src_imm - dst->n_bytes],
+               &imm, dst->n_bytes);
         spec->dst_type = NX_LEARN_DST_MATCH;
         spec->dst.field = dst;
         spec->dst.ofs = 0;
         spec->dst.n_bits = dst->n_bits;
     } else if (strchr(name, '[')) {
         /* Parse destination and check prerequisites. */
-        if (mf_parse_subfield(&spec->dst, name)[0] != '\0') {
-            ovs_fatal(0, "%s: syntax error after NXM field name `%s'",
-                      orig, name);
+        char *error;
+
+        error = mf_parse_subfield(&spec->dst, name);
+        if (error) {
+            return error;
         }
 
         /* Parse source and check prerequisites. */
         if (value[0] != '\0') {
-            if (mf_parse_subfield(&spec->src, value)[0] != '\0') {
-                ovs_fatal(0, "%s: syntax error after NXM field name `%s'",
-                          orig, value);
+            error = mf_parse_subfield(&spec->src, value);
+            if (error) {
+                return error;
             }
             if (spec->src.n_bits != spec->dst.n_bits) {
-                ovs_fatal(0, "%s: bit widths of %s (%u) and %s (%u) differ",
-                          orig, name, spec->src.n_bits, value,
-                          spec->dst.n_bits);
+                return xasprintf("%s: bit widths of %s (%u) and %s (%u) "
+                                 "differ", orig, name, spec->src.n_bits, value,
+                                 spec->dst.n_bits);
             }
         } else {
             spec->src = spec->dst;
@@ -348,340 +510,206 @@ learn_parse_spec(const char *orig, char *name, char *value,
         spec->dst_type = NX_LEARN_DST_MATCH;
     } else if (!strcmp(name, "load")) {
         if (value[strcspn(value, "[-")] == '-') {
-            struct nx_action_reg_load load;
-            int nbits, imm_bytes;
-            uint64_t imm;
-            int i;
-
-            nxm_parse_reg_load(&load, value);
-            nbits = nxm_decode_n_bits(load.ofs_nbits);
-            imm_bytes = DIV_ROUND_UP(nbits, 8);
-            imm = ntohll(load.value);
-
-            spec->n_bits = nbits;
-            spec->src_type = NX_LEARN_SRC_IMMEDIATE;
-            for (i = 0; i < imm_bytes; i++) {
-                spec->src_imm[i] = imm >> ((imm_bytes - i - 1) * 8);
+            char *error = learn_parse_load_immediate(value, spec);
+            if (error) {
+                return error;
             }
-            spec->dst_type = NX_LEARN_DST_LOAD;
-            nxm_decode(&spec->dst, load.dst, load.ofs_nbits);
         } else {
-            struct nx_action_reg_move move;
+            struct ofpact_reg_move move;
+            char *error;
 
-            nxm_parse_reg_move(&move, value);
+            error = nxm_parse_reg_move(&move, value);
+            if (error) {
+                return error;
+            }
 
-            spec->n_bits = ntohs(move.n_bits);
+            spec->n_bits = move.src.n_bits;
             spec->src_type = NX_LEARN_SRC_FIELD;
-            nxm_decode_discrete(&spec->src,
-                                move.src, move.src_ofs, move.n_bits);
+            spec->src = move.src;
             spec->dst_type = NX_LEARN_DST_LOAD;
-            nxm_decode_discrete(&spec->dst,
-                                move.dst, move.dst_ofs, move.n_bits);
+            spec->dst = move.dst;
         }
     } else if (!strcmp(name, "output")) {
-        if (mf_parse_subfield(&spec->src, value)[0] != '\0') {
-            ovs_fatal(0, "%s: syntax error after NXM field name `%s'",
-                      orig, name);
+        char *error = mf_parse_subfield(&spec->src, value);
+        if (error) {
+            return error;
         }
 
         spec->n_bits = spec->src.n_bits;
         spec->src_type = NX_LEARN_SRC_FIELD;
         spec->dst_type = NX_LEARN_DST_OUTPUT;
     } else {
-        ovs_fatal(0, "%s: unknown keyword %s", orig, name);
+        return xasprintf("%s: unknown keyword %s", orig, name);
     }
+
+    return NULL;
 }
 
-/* Parses 'arg' as a set of arguments to the "learn" action and appends a
- * matching NXAST_LEARN action to 'b'.  The format parsed is described in
- * ovs-ofctl(8).
- *
- * Prints an error on stderr and aborts the program if 'arg' syntax is invalid.
- *
- * If 'flow' is nonnull, then it should be the flow from a cls_rule that is
- * the matching rule for the learning action.  This helps to better validate
- * the action's arguments.
- *
- * Modifies 'arg'. */
-void
-learn_parse(struct ofpbuf *b, char *arg, const struct flow *flow)
+/* Returns NULL if successful, otherwise a malloc()'d string describing the
+ * error.  The caller is responsible for freeing the returned string. */
+static char * WARN_UNUSED_RESULT
+learn_parse__(char *orig, char *arg, struct ofpbuf *ofpacts)
 {
-    char *orig = xstrdup(arg);
+    struct ofpact_learn *learn;
+    struct match match;
     char *name, *value;
-    enum ofperr error;
-    size_t learn_ofs;
-    size_t len;
-
-    struct nx_action_learn *learn;
-    struct cls_rule rule;
-
-    learn_ofs = b->size;
-    learn = ofputil_put_NXAST_LEARN(b);
-    learn->idle_timeout = htons(OFP_FLOW_PERMANENT);
-    learn->hard_timeout = htons(OFP_FLOW_PERMANENT);
-    learn->priority = htons(OFP_DEFAULT_PRIORITY);
-    learn->cookie = htonll(0);
-    learn->flags = htons(0);
+
+    learn = ofpact_put_LEARN(ofpacts);
+    learn->idle_timeout = OFP_FLOW_PERMANENT;
+    learn->hard_timeout = OFP_FLOW_PERMANENT;
+    learn->priority = OFP_DEFAULT_PRIORITY;
     learn->table_id = 1;
 
-    cls_rule_init_catchall(&rule, 0);
+    match_init_catchall(&match);
     while (ofputil_parse_key_value(&arg, &name, &value)) {
-        learn = ofpbuf_at_assert(b, learn_ofs, sizeof *learn);
         if (!strcmp(name, "table")) {
             learn->table_id = atoi(value);
             if (learn->table_id == 255) {
-                ovs_fatal(0, "%s: table id 255 not valid for `learn' action",
-                          orig);
+                return xasprintf("%s: table id 255 not valid for `learn' "
+                                 "action", orig);
             }
         } else if (!strcmp(name, "priority")) {
-            learn->priority = htons(atoi(value));
+            learn->priority = atoi(value);
         } else if (!strcmp(name, "idle_timeout")) {
-            learn->idle_timeout = htons(atoi(value));
+            learn->idle_timeout = atoi(value);
         } else if (!strcmp(name, "hard_timeout")) {
-            learn->hard_timeout = htons(atoi(value));
+            learn->hard_timeout = atoi(value);
         } else if (!strcmp(name, "fin_idle_timeout")) {
-            learn->fin_idle_timeout = htons(atoi(value));
+            learn->fin_idle_timeout = atoi(value);
         } else if (!strcmp(name, "fin_hard_timeout")) {
-            learn->fin_hard_timeout = htons(atoi(value));
+            learn->fin_hard_timeout = atoi(value);
         } else if (!strcmp(name, "cookie")) {
-            learn->cookie = htonll(strtoull(value, NULL, 0));
+            learn->cookie = strtoull(value, NULL, 0);
         } else {
-            struct learn_spec spec;
+            struct ofpact_learn_spec *spec;
+            char *error;
 
-            learn_parse_spec(orig, name, value, &spec);
+            spec = ofpbuf_put_zeros(ofpacts, sizeof *spec);
+            learn = ofpacts->frame;
+            learn->n_specs++;
 
-            /* Check prerequisites. */
-            if (spec.src_type == NX_LEARN_SRC_FIELD
-                && flow && !mf_are_prereqs_ok(spec.src.field, flow)) {
-                ovs_fatal(0, "%s: cannot specify source field %s because "
-                          "prerequisites are not satisfied",
-                          orig, spec.src.field->name);
-            }
-            if ((spec.dst_type == NX_LEARN_DST_MATCH
-                 || spec.dst_type == NX_LEARN_DST_LOAD)
-                && !mf_are_prereqs_ok(spec.dst.field, &rule.flow)) {
-                ovs_fatal(0, "%s: cannot specify destination field %s because "
-                          "prerequisites are not satisfied",
-                          orig, spec.dst.field->name);
+            error = learn_parse_spec(orig, name, value, spec);
+            if (error) {
+                return error;
             }
 
-            /* Update 'rule' to allow for satisfying destination
+            /* Update 'match' to allow for satisfying destination
              * prerequisites. */
-            if (spec.src_type == NX_LEARN_SRC_IMMEDIATE
-                && spec.dst_type == NX_LEARN_DST_MATCH
-                && spec.dst.ofs == 0
-                && spec.n_bits == spec.dst.field->n_bytes * 8) {
-                union mf_value imm;
-
-                memcpy(&imm, spec.src_imm, spec.dst.field->n_bytes);
-                mf_set_value(spec.dst.field, &imm, &rule);
-            }
-
-            /* Output the flow_mod_spec. */
-            put_u16(b, spec.n_bits | spec.src_type | spec.dst_type);
-            if (spec.src_type == NX_LEARN_SRC_IMMEDIATE) {
-                int n_bytes = DIV_ROUND_UP(spec.n_bits, 8);
-                if (n_bytes % 2) {
-                    ofpbuf_put_zeros(b, 1);
-                }
-                ofpbuf_put(b, spec.src_imm, n_bytes);
-            } else {
-                put_u32(b, spec.src.field->nxm_header);
-                put_u16(b, spec.src.ofs);
-            }
-            if (spec.dst_type == NX_LEARN_DST_MATCH ||
-                spec.dst_type == NX_LEARN_DST_LOAD) {
-                put_u32(b, spec.dst.field->nxm_header);
-                put_u16(b, spec.dst.ofs);
-            } else {
-                assert(spec.dst_type == NX_LEARN_DST_OUTPUT);
+            if (spec->src_type == NX_LEARN_SRC_IMMEDIATE
+                && spec->dst_type == NX_LEARN_DST_MATCH) {
+                mf_write_subfield(&spec->dst, &spec->src_imm, &match);
             }
         }
     }
+    ofpact_update_len(ofpacts, &learn->ofpact);
 
-    put_u16(b, 0);
-
-    len = b->size - learn_ofs;
-    if (len % 8) {
-        ofpbuf_put_zeros(b, 8 - len % 8);
-    }
-
-    learn = ofpbuf_at_assert(b, learn_ofs, sizeof *learn);
-    learn->len = htons(b->size - learn_ofs);
+    return NULL;
+}
 
-    /* In theory the above should have caught any errors, but... */
-    if (flow) {
-        error = learn_check(learn, flow);
-        if (error) {
-            ovs_fatal(0, "%s: %s", orig, ofperr_to_string(error));
-        }
-    }
+/* Parses 'arg' as a set of arguments to the "learn" action and appends a
+ * matching OFPACT_LEARN action to 'ofpacts'.  ovs-ofctl(8) describes the
+ * format parsed.
+ *
+ * Returns NULL if successful, otherwise a malloc()'d string describing the
+ * error.  The caller is responsible for freeing the returned string.
+ *
+ * If 'flow' is nonnull, then it should be the flow from a struct match that is
+ * the matching rule for the learning action.  This helps to better validate
+ * the action's arguments.
+ *
+ * Modifies 'arg'. */
+char * WARN_UNUSED_RESULT
+learn_parse(char *arg, struct ofpbuf *ofpacts)
+{
+    char *orig = xstrdup(arg);
+    char *error = learn_parse__(orig, arg, ofpacts);
     free(orig);
+    return error;
 }
 
+/* Appends a description of 'learn' to 's', in the format that ovs-ofctl(8)
+ * describes. */
 void
-learn_format(const struct nx_action_learn *learn, struct ds *s)
+learn_format(const struct ofpact_learn *learn, struct ds *s)
 {
-    struct cls_rule rule;
-    const void *p, *end;
+    const struct ofpact_learn_spec *spec;
+    struct match match;
 
-    cls_rule_init_catchall(&rule, 0);
+    match_init_catchall(&match);
 
     ds_put_format(s, "learn(table=%"PRIu8, learn->table_id);
-    if (learn->idle_timeout != htons(OFP_FLOW_PERMANENT)) {
-        ds_put_format(s, ",idle_timeout=%"PRIu16, ntohs(learn->idle_timeout));
+    if (learn->idle_timeout != OFP_FLOW_PERMANENT) {
+        ds_put_format(s, ",idle_timeout=%"PRIu16, learn->idle_timeout);
     }
-    if (learn->hard_timeout != htons(OFP_FLOW_PERMANENT)) {
-        ds_put_format(s, ",hard_timeout=%"PRIu16, ntohs(learn->hard_timeout));
+    if (learn->hard_timeout != OFP_FLOW_PERMANENT) {
+        ds_put_format(s, ",hard_timeout=%"PRIu16, learn->hard_timeout);
     }
     if (learn->fin_idle_timeout) {
-        ds_put_format(s, ",fin_idle_timeout=%"PRIu16,
-                      ntohs(learn->fin_idle_timeout));
+        ds_put_format(s, ",fin_idle_timeout=%"PRIu16, learn->fin_idle_timeout);
     }
     if (learn->fin_hard_timeout) {
-        ds_put_format(s, ",fin_hard_timeout=%"PRIu16,
-                      ntohs(learn->fin_hard_timeout));
+        ds_put_format(s, ",fin_hard_timeout=%"PRIu16, learn->fin_hard_timeout);
     }
-    if (learn->priority != htons(OFP_DEFAULT_PRIORITY)) {
-        ds_put_format(s, ",priority=%"PRIu16, ntohs(learn->priority));
+    if (learn->priority != OFP_DEFAULT_PRIORITY) {
+        ds_put_format(s, ",priority=%"PRIu16, learn->priority);
     }
-    if (learn->flags & htons(OFPFF_SEND_FLOW_REM)) {
+    if (learn->flags & OFPFF_SEND_FLOW_REM) {
         ds_put_cstr(s, ",OFPFF_SEND_FLOW_REM");
     }
-    if (learn->flags & htons(~OFPFF_SEND_FLOW_REM)) {
-        ds_put_format(s, ",***flags=%"PRIu16"***",
-                      ntohs(learn->flags) & ~OFPFF_SEND_FLOW_REM);
-    }
-    if (learn->cookie != htonll(0)) {
-        ds_put_format(s, ",cookie=0x%"PRIx64, ntohll(learn->cookie));
+    if (learn->cookie != 0) {
+        ds_put_format(s, ",cookie=%#"PRIx64, learn->cookie);
     }
-    if (learn->pad != 0) {
-        ds_put_cstr(s, ",***nonzero pad***");
-    }
-
-    end = (char *) learn + ntohs(learn->len);
-    for (p = learn + 1; p != end; ) {
-        uint16_t header = ntohs(get_be16(&p));
-        int n_bits = header & NX_LEARN_N_BITS_MASK;
-
-        int src_type = header & NX_LEARN_SRC_MASK;
-        struct mf_subfield src;
-        const uint8_t *src_value;
-        int src_value_bytes;
-
-        int dst_type = header & NX_LEARN_DST_MASK;
-        struct mf_subfield dst;
-
-        enum ofperr error;
-        int i;
-
-        if (!header) {
-            break;
-        }
-
-        error = learn_check_header(header, (char *) end - (char *) p);
-        if (error == OFPERR_OFPBAC_BAD_ARGUMENT) {
-            ds_put_format(s, ",***bad flow_mod_spec header %"PRIx16"***)",
-                          header);
-            return;
-        } else if (error == OFPERR_OFPBAC_BAD_LEN) {
-            ds_put_format(s, ",***flow_mod_spec at offset %td is %u bytes "
-                          "long but only %td bytes are left***)",
-                          (char *) p - (char *) (learn + 1) - 2,
-                          learn_min_len(header) + 2,
-                          (char *) end - (char *) p + 2);
-            return;
-        }
-        assert(!error);
-
-        /* Get the source. */
-        if (src_type == NX_LEARN_SRC_FIELD) {
-            get_subfield(n_bits, &p, &src);
-            src_value_bytes = 0;
-            src_value = NULL;
-        } else {
-            src.field = NULL;
-            src.ofs = 0;
-            src.n_bits = 0;
-            src_value_bytes = 2 * DIV_ROUND_UP(n_bits, 16);
-            src_value = p;
-            p = (const void *) ((const uint8_t *) p + src_value_bytes);
-        }
-
-        /* Get the destination. */
-        if (dst_type == NX_LEARN_DST_MATCH || dst_type == NX_LEARN_DST_LOAD) {
-            get_subfield(n_bits, &p, &dst);
-        } else {
-            dst.field = NULL;
-            dst.ofs = 0;
-            dst.n_bits = 0;
-        }
 
+    for (spec = learn->specs; spec < &learn->specs[learn->n_specs]; spec++) {
         ds_put_char(s, ',');
 
-        switch (src_type | dst_type) {
+        switch (spec->src_type | spec->dst_type) {
         case NX_LEARN_SRC_IMMEDIATE | NX_LEARN_DST_MATCH:
-            if (dst.field && dst.ofs == 0 && n_bits == dst.field->n_bits) {
+            if (spec->dst.ofs == 0
+                && spec->dst.n_bits == spec->dst.field->n_bits) {
                 union mf_value value;
-                uint8_t *bytes = (uint8_t *) &value;
-
-                if (src_value_bytes > dst.field->n_bytes) {
-                    /* The destination field is an odd number of bytes, which
-                     * got rounded up to a multiple of 2 to be put into the
-                     * learning action.  Skip over the leading byte, which
-                     * should be zero anyway.  Otherwise the memcpy() below
-                     * will overrun the start of 'value'. */
-                    int diff = src_value_bytes - dst.field->n_bytes;
-                    src_value += diff;
-                    src_value_bytes -= diff;
-                }
 
                 memset(&value, 0, sizeof value);
-                memcpy(&bytes[dst.field->n_bytes - src_value_bytes],
-                       src_value, src_value_bytes);
-                ds_put_format(s, "%s=", dst.field->name);
-                mf_format(dst.field, &value, NULL, s);
+                bitwise_copy(&spec->src_imm, sizeof spec->src_imm, 0,
+                             &value, spec->dst.field->n_bytes, 0,
+                             spec->dst.field->n_bits);
+                ds_put_format(s, "%s=", spec->dst.field->name);
+                mf_format(spec->dst.field, &value, NULL, s);
             } else {
-                mf_format_subfield(&dst, s);
-                ds_put_cstr(s, "=0x");
-                for (i = 0; i < src_value_bytes; i++) {
-                    ds_put_format(s, "%02"PRIx8, src_value[i]);
-                }
+                mf_format_subfield(&spec->dst, s);
+                ds_put_char(s, '=');
+                mf_format_subvalue(&spec->src_imm, s);
             }
             break;
 
         case NX_LEARN_SRC_FIELD | NX_LEARN_DST_MATCH:
-            mf_format_subfield(&dst, s);
-            if (src.field != dst.field || src.ofs != dst.ofs) {
+            mf_format_subfield(&spec->dst, s);
+            if (spec->src.field != spec->dst.field ||
+                spec->src.ofs != spec->dst.ofs) {
                 ds_put_char(s, '=');
-                mf_format_subfield(&src, s);
+                mf_format_subfield(&spec->src, s);
             }
             break;
 
         case NX_LEARN_SRC_IMMEDIATE | NX_LEARN_DST_LOAD:
-            ds_put_cstr(s, "load:0x");
-            for (i = 0; i < src_value_bytes; i++) {
-                ds_put_format(s, "%02"PRIx8, src_value[i]);
-            }
+            ds_put_format(s, "load:");
+            mf_format_subvalue(&spec->src_imm, s);
             ds_put_cstr(s, "->");
-            mf_format_subfield(&dst, s);
+            mf_format_subfield(&spec->dst, s);
             break;
 
         case NX_LEARN_SRC_FIELD | NX_LEARN_DST_LOAD:
             ds_put_cstr(s, "load:");
-            mf_format_subfield(&src, s);
+            mf_format_subfield(&spec->src, s);
             ds_put_cstr(s, "->");
-            mf_format_subfield(&dst, s);
+            mf_format_subfield(&spec->dst, s);
             break;
 
         case NX_LEARN_SRC_FIELD | NX_LEARN_DST_OUTPUT:
             ds_put_cstr(s, "output:");
-            mf_format_subfield(&src, s);
+            mf_format_subfield(&spec->src, s);
             break;
         }
     }
-    if (!is_all_zeros(p, (char *) end - (char *) p)) {
-        ds_put_cstr(s, ",***nonzero trailer***");
-    }
     ds_put_char(s, ')');
 }