It is technically correct to send the entire maximum length of
a field when it is variable length. However, it is awkward to
do so and not what one would naively expect. Since receivers will
internally zero-extend fields, we can do the opposite and trim
off leading zeros. This results in encodings that are generally
sensible without specific knowledge of what is being transmitted.
(Of course, other implementations, such as controllers, may know
exactly the expected length of the field and are free to encode
it that way even if it has leading zeros.)

Signed-off-by: Jesse Gross <je...@nicira.com>
---
 lib/meta-flow.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 lib/meta-flow.h |  2 ++
 lib/nx-match.c  | 20 +++++++++++++-------
 3 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 5916244..526f88e 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -1013,6 +1013,49 @@ mf_mask_field(const struct mf_field *mf, struct flow 
*mask)
     }
 }
 
+static int
+field_len(const struct mf_field *mf, const union mf_value *value_)
+{
+    const uint8_t *value = &value_->u8;
+    int i;
+
+    if (!mf->variable_len) {
+        return mf->n_bytes;
+    }
+
+    if (!value) {
+        return 0;
+    }
+
+    for (i = 0; i < mf->n_bytes; i++) {
+        if (value[i] != 0) {
+            break;
+        }
+    }
+
+    return mf->n_bytes - i;
+}
+
+/* Returns the effective length of the field. For fixed length fields,
+ * this is just the defined length. For variable length fields, it is
+ * the minimum size encoding that retains the same meaning (i.e.
+ * discarding leading zeros). */
+int
+mf_field_len(const struct mf_field *mf, const union mf_value *value,
+             const union mf_value *mask)
+{
+    bool masked = mask && !is_all_ones(mask, mf->n_bytes);
+    int len, mask_len;
+
+    len = field_len(mf, value);
+    if (masked) {
+        mask_len = field_len(mf, mask);
+        len = MAX(len, mask_len);
+    }
+
+    return len;
+}
+
 /* Sets 'flow' member field described by 'mf' to 'value'.  The caller is
  * responsible for ensuring that 'flow' meets 'mf''s prerequisites.*/
 void
diff --git a/lib/meta-flow.h b/lib/meta-flow.h
index ce50b16..b04344b 100644
--- a/lib/meta-flow.h
+++ b/lib/meta-flow.h
@@ -1651,6 +1651,8 @@ void mf_set_flow_value_masked(const struct mf_field *,
                               struct flow *);
 bool mf_is_zero(const struct mf_field *, const struct flow *);
 void mf_mask_field(const struct mf_field *, struct flow *);
+int mf_field_len(const struct mf_field *, const union mf_value *value,
+                 const union mf_value *mask);
 
 void mf_get(const struct mf_field *, const struct match *,
             union mf_value *value, union mf_value *mask);
diff --git a/lib/nx-match.c b/lib/nx-match.c
index c92f12d..3dcee5d 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -1164,8 +1164,10 @@ oxm_put_field_array(struct ofpbuf *b, const struct 
field_array *fa,
 
     for (i = 0; i < MFF_N_IDS; i++) {
         if (bitmap_is_set(fa->used.bm, i)) {
-            nxm_put_unmasked(b, i, version, &fa->value[i],
-                             mf_from_id(i)->n_bytes);
+            int len = mf_field_len(mf_from_id(i), &fa->value[i], NULL);
+            nxm_put_unmasked(b, i, version,
+                             &fa->value[i].u8 + mf_from_id(i)->n_bytes - len,
+                             len);
         }
     }
 
@@ -1206,13 +1208,17 @@ nx_put_entry(struct ofpbuf *b,
              enum mf_field_id field, enum ofp_version version,
              const union mf_value *value, const union mf_value *mask)
 {
-    int n_bytes = mf_from_id(field)->n_bytes;
-    bool masked = mask && !is_all_ones(mask, n_bytes);
+    const struct mf_field *mf = mf_from_id(field);
+    bool masked = mask && !is_all_ones(mask, mf->n_bytes);
+    int len, offset;
 
-    nx_put_header(b, field, version, masked);
-    ofpbuf_put(b, value, n_bytes);
+    len = mf_field_len(mf, value, mask);
+    offset = mf->n_bytes - len;
+
+    nx_put_header_len(b, field, version, masked, len);
+    ofpbuf_put(b, &value->u8 + offset, len);
     if (masked) {
-        ofpbuf_put(b, mask, n_bytes);
+        ofpbuf_put(b, &mask->u8 + offset, len);
     }
 }
 
-- 
2.1.0

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to