[ovs-dev] [PATCH v2 07/12] nx-match: Use nx_put_header() internally for encoding flow matches.

Ben Pfaff blp at nicira.com
Wed Oct 8 21:38:40 UTC 2014


On Mon, Oct 06, 2014 at 12:52:59PM -0700, Jarno Rajahalme wrote:
> One small matter of opinion below,
> 
>   Jarno
> 
> Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>
> 
> On Sep 30, 2014, at 5:47 PM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > This will make it easier to support 64-bit OXM experimenter fields.
> > 
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > Acked-by: YAMAMOTO Takashi <yamamoto at valinux.co.jp>
> > ---
> > v1->v2: Use ->s6_addr for IPv6 raw bytes in nxm_put_ipv6().
> > ---
> > lib/nx-match.c |  313 +++++++++++++++++++++-----------------------------------
> > 1 file changed, 117 insertions(+), 196 deletions(-)
> > 
> > diff --git a/lib/nx-match.c b/lib/nx-match.c
> > index 238bfdb..da99f13 100644
> > --- a/lib/nx-match.c
> > +++ b/lib/nx-match.c
> 
> (snip)
> 
> > -        nxm_put_16w(b, nxm_make_wild_header(header), value, mask);
> > -        break;
> > +nxm_put(struct ofpbuf *b, enum mf_field_id field, enum ofp_version version,
> > +        const void *value, const void *mask, size_t n_bytes)
> > +{
> > +    if (!is_all_zeros(mask, n_bytes)) {
> > +        bool masked = !is_all_ones(mask, n_bytes);
> > +        nx_put_header(b, field, version, masked);
> > +        ofpbuf_put(b, value, n_bytes);
> > +        if (masked) {
> > +            ofpbuf_put(b, mask, n_bytes);
> > +        }
> >     }
> > }
> > 
> 
> I would be inclined to define mxm_put_unmasked() like this and use it in appropriate places below, instead of calling nxm_put() with all-ones mask:
> 
> static void
> nxm_put_unmasked(struct ofpbuf *b, enum mf_field_id field, enum ofp_version version,
>         const void *value, size_t n_bytes)
> {
>     nx_put_header(b, field, version, false);
>     ofpbuf_put(b, value, n_bytes);
> }

Thanks for the review.  I made that change by folding in the following
incremental change.  I'm going to push patches 6 and 7 in a minute.  I
know that you had one concern on patch 6, but I've fixed up the issue
that I saw there and it is something that we can easily adjust later.

diff --git a/lib/nx-match.c b/lib/nx-match.c
index da99f13..6f10d14 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -503,6 +503,14 @@ oxm_pull_match_loose(struct ofpbuf *b, struct match *match)
  */
 
 static void
+nxm_put_unmasked(struct ofpbuf *b, enum mf_field_id field,
+                 enum ofp_version version, const void *value, size_t n_bytes)
+{
+    nx_put_header(b, field, version, false);
+    ofpbuf_put(b, value, n_bytes);
+}
+
+static void
 nxm_put(struct ofpbuf *b, enum mf_field_id field, enum ofp_version version,
         const void *value, const void *mask, size_t n_bytes)
 {
@@ -527,7 +535,7 @@ static void
 nxm_put_8(struct ofpbuf *b, enum mf_field_id field, enum ofp_version version,
           uint8_t value)
 {
-    nxm_put_8m(b, field, version, value, UINT8_MAX);
+    nxm_put_unmasked(b, field, version, &value, sizeof value);
 }
 
 static void
@@ -541,7 +549,7 @@ static void
 nxm_put_16(struct ofpbuf *b, enum mf_field_id field, enum ofp_version version,
            ovs_be16 value)
 {
-    nxm_put_16m(b, field, version, value, OVS_BE16_MAX);
+    nxm_put_unmasked(b, field, version, &value, sizeof value);
 }
 
 static void
@@ -555,7 +563,7 @@ static void
 nxm_put_32(struct ofpbuf *b, enum mf_field_id field, enum ofp_version version,
            ovs_be32 value)
 {
-    nxm_put_32m(b, field, version, value, OVS_BE32_MAX);
+    nxm_put_unmasked(b, field, version, &value, sizeof value);
 }
 
 static void



More information about the dev mailing list