meta-flow: New functions mf_subfield_copy() and mf_subfield_swap().

The function nxm_execute_reg_move() was almost a general-purpose function
for manipulating subfields, except for its awkward interface that took a
struct ofpact_reg_move instead of a plain source and destination.  This
commit introduces a general-purpose function in meta-flow that corrects
this flaw, and updates the callers.  An upcoming commit will introduce a
new user of the function.

This commit also introduces a related function mf_subfield_swap() to swap
the contents of subfields.  An upcoming commit will introduce the first
user.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Ryan Moats <rmoats@us.ibm.com>
Acked-by: Justin Pettit <jpettit@ovn.org>

5 files changed, 82 insertions, 34 deletions

diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index 76d915ce3..f209fc2f8 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -2116,6 +2116,12 @@ void mf_read_subfield(const struct mf_subfield *, const struct flow *,
                       union mf_subvalue *);
 uint64_t mf_get_subfield(const struct mf_subfield *, const struct flow *);
 
+void mf_subfield_copy(const struct mf_subfield *src,
+                      const struct mf_subfield *dst,
+                      struct flow *, struct flow_wildcards *);
+void mf_subfield_swap(const struct mf_subfield *,
+                      const struct mf_subfield *,
+                      struct flow *flow, struct flow_wildcards *);
 
 enum ofperr mf_check_src(const struct mf_subfield *, const struct flow *);
 enum ofperr mf_check_dst(const struct mf_subfield *, const struct flow *);
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 2c896134e..3dc27707b 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -1962,6 +1962,80 @@ mf_check__(const struct mf_subfield *sf, const struct flow *flow,
     }
 }
 
+/* Sets all the bits in 'sf' to 1 within 'wc', if 'wc' is nonnull. */
+static void
+unwildcard_subfield(const struct mf_subfield *sf, struct flow_wildcards *wc)
+{
+    if (wc) {
+        union mf_value mask;
+
+        memset(&mask, 0, sizeof mask);
+        bitwise_one(&mask, sf->field->n_bytes, sf->ofs, sf->n_bits);
+        mf_mask_field_masked(sf->field, &mask, wc);
+    }
+}
+
+/* Copies 'src' into 'dst' within 'flow', and sets all the bits in 'src' and
+ * 'dst' to 1s in 'wc', if 'wc' is nonnull.
+ *
+ * 'src' and 'dst' may overlap. */
+void
+mf_subfield_copy(const struct mf_subfield *src,
+                 const struct mf_subfield *dst,
+                 struct flow *flow, struct flow_wildcards *wc)
+{
+    ovs_assert(src->n_bits == dst->n_bits);
+    if (mf_are_prereqs_ok(dst->field, flow, wc)
+        && mf_are_prereqs_ok(src->field, flow, wc)) {
+        unwildcard_subfield(src, wc);
+        unwildcard_subfield(dst, wc);
+
+        union mf_value src_value;
+        union mf_value dst_value;
+        mf_get_value(dst->field, flow, &dst_value);
+        mf_get_value(src->field, flow, &src_value);
+        bitwise_copy(&src_value, src->field->n_bytes, src->ofs,
+                     &dst_value, dst->field->n_bytes, dst->ofs,
+                     src->n_bits);
+        mf_set_flow_value(dst->field, &dst_value, flow);
+    }
+}
+
+/* Swaps the bits in 'src' and 'dst' within 'flow', and sets all the bits in
+ * 'src' and 'dst' to 1s in 'wc', if 'wc' is nonnull.
+ *
+ * 'src' and 'dst' may overlap. */
+void
+mf_subfield_swap(const struct mf_subfield *a,
+                 const struct mf_subfield *b,
+                 struct flow *flow, struct flow_wildcards *wc)
+{
+    ovs_assert(a->n_bits == b->n_bits);
+    if (mf_are_prereqs_ok(a->field, flow, wc)
+        && mf_are_prereqs_ok(b->field, flow, wc)) {
+        unwildcard_subfield(a, wc);
+        unwildcard_subfield(b, wc);
+
+        union mf_value a_value;
+        union mf_value b_value;
+        mf_get_value(a->field, flow, &a_value);
+        mf_get_value(b->field, flow, &b_value);
+        union mf_value b2_value = b_value;
+
+        /* Copy 'a' into 'b'. */
+        bitwise_copy(&a_value, a->field->n_bytes, a->ofs,
+                     &b_value, b->field->n_bytes, b->ofs,
+                     a->n_bits);
+        mf_set_flow_value(b->field, &b_value, flow);
+
+        /* Copy original 'b' into 'a'. */
+        bitwise_copy(&b2_value, b->field->n_bytes, b->ofs,
+                     &a_value, a->field->n_bytes, a->ofs,
+                     b->n_bits);
+        mf_set_flow_value(a->field, &a_value, flow);
+    }
+}
+
 /* Checks whether 'sf' is valid for reading a subfield out of 'flow'.  Returns
  * 0 if so, otherwise an OpenFlow error code (e.g. as returned by
  * ofp_mkerr()).  */
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 89c6e3eac..b03ccf260 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -1623,37 +1623,6 @@ nxm_reg_move_check(const struct ofpact_reg_move *move, const struct flow *flow)
 /* nxm_execute_reg_move(). */
 
 void
-nxm_execute_reg_move(const struct ofpact_reg_move *move,
-                     struct flow *flow, struct flow_wildcards *wc)
-{
-    /* Check that the fields exist. */
-    if (mf_are_prereqs_ok(move->dst.field, flow, wc)
-        && mf_are_prereqs_ok(move->src.field, flow, wc)) {
-        union mf_value src_value;
-        union mf_value dst_value;
-        union mf_value mask;
-
-        /* Should only mask the bits affected. */
-        memset(&mask, 0, sizeof mask);
-        bitwise_one(&mask, move->dst.field->n_bytes, move->dst.ofs,
-                    move->src.n_bits);
-        mf_mask_field_masked(move->dst.field, &mask, wc);
-
-        memset(&mask, 0, sizeof mask);
-        bitwise_one(&mask, move->src.field->n_bytes, move->src.ofs,
-                    move->src.n_bits);
-        mf_mask_field_masked(move->src.field, &mask, wc);
-
-        mf_get_value(move->dst.field, flow, &dst_value);
-        mf_get_value(move->src.field, flow, &src_value);
-        bitwise_copy(&src_value, move->src.field->n_bytes, move->src.ofs,
-                     &dst_value, move->dst.field->n_bytes, move->dst.ofs,
-                     move->src.n_bits);
-        mf_set_flow_value(move->dst.field, &dst_value, flow);
-    }
-}
-
-void
 nxm_reg_load(const struct mf_subfield *dst, uint64_t src_data,
              struct flow *flow, struct flow_wildcards *wc)
 {
diff --git a/lib/nx-match.h b/lib/nx-match.h
index 51d64148a..c366a04cc 100644
--- a/lib/nx-match.h
+++ b/lib/nx-match.h
@@ -107,8 +107,6 @@ void nxm_format_reg_move(const struct ofpact_reg_move *, struct ds *);
 enum ofperr nxm_reg_move_check(const struct ofpact_reg_move *,
                                const struct flow *);
 
-void nxm_execute_reg_move(const struct ofpact_reg_move *, struct flow *,
-                          struct flow_wildcards *);
 void nxm_reg_load(const struct mf_subfield *, uint64_t src_data,
                   struct flow *, struct flow_wildcards *);
 
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index bec599b4b..893c0335e 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4914,7 +4914,8 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_REG_MOVE:
-            nxm_execute_reg_move(ofpact_get_REG_MOVE(a), flow, wc);
+            mf_subfield_copy(&ofpact_get_REG_MOVE(a)->src,
+                             &ofpact_get_REG_MOVE(a)->dst, flow, wc);
             break;
 
         case OFPACT_SET_FIELD: