On Tue, Oct 07, 2014 at 10:58:55AM +0900, YAMAMOTO Takashi wrote:
> > -#define NXM_HEADER(CLASS, FIELD, HASMASK, LENGTH) \
> > - (((CLASS) << 16) | ((FIELD) << 9) | ((HASMASK) << 8) | (LENGTH))
> > +#define NXM_HEADER(VENDOR, CLASS, HASMASK, FIELD, LENGTH) \
>
> why did you swap FIELD and HASMASK?
> it's easier for me to remember if it's in the on-wire order.
> ie. CLASS, FIELD, HASMASK, LENGTH, VENDOR.
I don't know why this happened. I agree that the on-wire order is
better. I made that change.
> > static void
> > -nx_put_header__(struct ofpbuf *b, uint32_t header, bool masked)
> > +nx_put_header__(struct ofpbuf *b, uint64_t header, bool masked)
> > {
> > - uint32_t masked_header = masked ? nxm_make_wild_header(header) :
> > header;
> > - ovs_be32 network_header = htonl(masked_header);
> > + uint64_t masked_header = masked ? nxm_make_wild_header(header) :
> > header;
> > + ovs_be64 network_header = htonll(masked_header);
> >
> > - ofpbuf_put(b, &network_header, sizeof network_header);
> > + ofpbuf_put(b, &network_header, 4 + nxm_experimenter_len(header));
>
> better to use nxm_header_len?
I agree. I made that change too. Here is the incremental that I
folded in. Because Jarno acked this, and because you did not see any
serious problems, I applied this to master.
Thanks,
ben.
diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index 88eb6d2..95714ee 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -143,7 +143,7 @@ def parse_oxm(s, prefix, n_bytes):
if oxm_class == 0xffff:
oxm_length += 4
- header = ("NXM_HEADER(0x%x,0x%x,0,%s,%d)"
+ header = ("NXM_HEADER(0x%x,0x%x,%s,0,%d)"
% (oxm_vendor, oxm_class, oxm_type, oxm_length))
if of_version:
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 13e758b..82b472c 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -148,7 +148,7 @@ is_nxm_header(uint64_t header)
return nxm_class(header) <= 1;
}
-#define NXM_HEADER(VENDOR, CLASS, HASMASK, FIELD, LENGTH) \
+#define NXM_HEADER(VENDOR, CLASS, FIELD, HASMASK, LENGTH) \
(((uint64_t) (CLASS) << 48) | \
((uint64_t) (FIELD) << 41) | \
((uint64_t) (HASMASK) << 40) | \
@@ -166,15 +166,15 @@ static uint64_t
nxm_make_exact_header(uint64_t header)
{
int new_len = nxm_payload_len(header) / 2 + nxm_experimenter_len(header);
- return NXM_HEADER(nxm_vendor(header), nxm_class(header), 0,
- nxm_field(header), new_len);
+ return NXM_HEADER(nxm_vendor(header), nxm_class(header),
+ nxm_field(header), 0, new_len);
}
static uint64_t
nxm_make_wild_header(uint64_t header)
{
int new_len = nxm_payload_len(header) * 2 + nxm_experimenter_len(header);
- return NXM_HEADER(nxm_vendor(header), nxm_class(header), 1,
- nxm_field(header), new_len);
+ return NXM_HEADER(nxm_vendor(header), nxm_class(header),
+ nxm_field(header), 1, new_len);
}
/* Flow cookie.
@@ -184,7 +184,7 @@ nxm_make_wild_header(uint64_t header)
* with specific cookies. See the "nx_flow_mod" and "nx_flow_stats_request"
* structure definitions for more details. This match is otherwise not
* allowed. */
-#define NXM_NX_COOKIE NXM_HEADER (0, 0x0001, 0, 30, 8)
+#define NXM_NX_COOKIE NXM_HEADER (0, 0x0001, 30, 0, 8)
#define NXM_NX_COOKIE_W nxm_make_wild_header(NXM_NX_COOKIE)
struct nxm_field {
@@ -1040,7 +1040,7 @@ nx_put_header__(struct ofpbuf *b, uint64_t header, bool
masked)
uint64_t masked_header = masked ? nxm_make_wild_header(header) : header;
ovs_be64 network_header = htonll(masked_header);
- ofpbuf_put(b, &network_header, 4 + nxm_experimenter_len(header));
+ ofpbuf_put(b, &network_header, nxm_header_len(header));
}
void
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
