On Mon, Oct 06, 2014 at 12:52:59PM -0700, Jarno Rajahalme wrote:
> One small matter of opinion below,
>
> Jarno
>
> Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
>
> On Sep 30, 2014, at 5:47 PM, Ben Pfaff <b...@nicira.com> wrote:
>
> > This will make it easier to support 64-bit OXM experimenter fields.
> >
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > Acked-by: YAMAMOTO Takashi <yamam...@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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
