> On Jul 12, 2016, at 4:15 PM, Ben Pfaff <b...@ovn.org> wrote:
>
> On Mon, Jul 11, 2016 at 11:56:35PM -0700, Justin Pettit wrote:
>> These are needed to handle IPv6 addresses.
>>
>> Signed-off-by: Justin Pettit <jpet...@ovn.org>
>
> The comment in meta-flow.h talks about "Nicira extension" registers. I
> think this made sense a few years ago, but Nicira isn't very relevant
> anymore, so I tend to try to say "Open vSwitch extension" these days;
> Open vSwitch is still quite relevant!
True enough. I went ahead and changed the xreg definitions, too.
> I don't think that flow_get_xxreg() and flow_set_xxreg() are endian
> independent. That is, suppose that actions set reg0, reg1, reg2, and
> reg3 independently, and then read xxreg0. Will xxreg0 have the same
> value on big-endian and little-endian architectures? That is something
> that we guarantee for the xregs, so it would be bad to drop it for
> xxregs.
Ugh, good catch. I've appended a diff that should address this. Let me know
if you see any problems.
> Instead of this:
> + ovs_u128 u_zero = {.u64.hi = 0, .u64.lo = 0};
> I'd suggest adding an OVS_U128_ZERO to include/openvswitch/types.h to go
> along with OVS_U128_MAX.
Done.
> Acked-by: Ben Pfaff <b...@ovn.org>
Thanks!
--Justin
-=-=-=-=-=-=-=-=-
diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index 0a54afb..e2e9220 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -894,10 +894,10 @@ enum OVS_PACKED_ENUM mf_field_id {
/* "xreg<N>".
*
* OpenFlow 1.5 ``extended register". Each extended register
- * overlays two of the Nicira extension 32-bit registers: xreg0 overlays
- * reg0 and reg1, with reg0 supplying the most-significant bits of xreg0
- * and reg1 the least-significant. xreg1 similarly overlays reg2 and reg3,
- * and so on.
+ * overlays two of the Open vSwitch extension 32-bit registers:
+ * xreg0 overlays reg0 and reg1, with reg0 supplying the
+ * most-significant bits of xreg0 and reg1 the least-significant.
+ * xreg1 similarly overlays reg2 and reg3, and so on.
*
* These registers were introduced in OpenFlow 1.5, but EXT-244 in the ONF
* JIRA also publishes them as a (draft) OpenFlow extension to OpenFlow
@@ -927,8 +927,8 @@ enum OVS_PACKED_ENUM mf_field_id {
/* "xxreg<N>".
*
* ``extended-extended register". Each of these extended registers
- * overlays four of the Nicira extension 32-bit registers: xxreg0
- * overlays reg0 through reg3, with reg0 supplying the
+ * overlays four of the Open vSwitch extension 32-bit registers:
+ * xxreg0 overlays reg0 through reg3, with reg0 supplying the
* most-significant bits of xxreg0 and reg3 the least-significant.
* xxreg1 similarly overlays reg4 and reg7.
*
diff --git a/include/openvswitch/types.h b/include/openvswitch/types.h
index bc94145..da56d4b 100644
--- a/include/openvswitch/types.h
+++ b/include/openvswitch/types.h
@@ -103,6 +103,7 @@ typedef union {
/* MSVC2015 doesn't support designated initializers when compiling C++,
* and doesn't support ternary operators with non-designated initializers.
* So we use these static definitions rather than using initializer macros. */
+static const ovs_u128 OVS_U128_ZERO = { { 0, 0, 0, 0 } };
static const ovs_u128 OVS_U128_MAX = { { UINT32_MAX, UINT32_MAX,
UINT32_MAX, UINT32_MAX } };
static const ovs_be128 OVS_BE128_MAX OVS_UNUSED = { { OVS_BE32_MAX, OVS_BE32_MA
diff --git a/lib/flow.h b/lib/flow.h
index 7f373fe..4cff48c 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -124,10 +124,10 @@ flow_get_xxreg(const struct flow *flow, int idx)
{
ovs_u128 value;
- value.u32[3] = flow->regs[idx * 4];
- value.u32[2] = flow->regs[idx * 4 + 1];
- value.u32[1] = flow->regs[idx * 4 + 2];
- value.u32[0] = flow->regs[idx * 4 + 3];
+ value.u64.hi = (uint64_t) flow->regs[idx * 4] << 32;
+ value.u64.hi |= flow->regs[idx * 4 + 1];
+ value.u64.lo = (uint64_t) flow->regs[idx * 4 + 2] << 32;
+ value.u64.lo |= flow->regs[idx * 4 + 3];
return value;
}
@@ -135,10 +135,10 @@ flow_get_xxreg(const struct flow *flow, int idx)
static inline void
flow_set_xxreg(struct flow *flow, int idx, ovs_u128 value)
{
- flow->regs[idx * 4] = value.u32[3];
- flow->regs[idx * 4 + 1] = value.u32[2];
- flow->regs[idx * 4 + 2] = value.u32[1];
- flow->regs[idx * 4 + 3] = value.u32[0];
+ flow->regs[idx * 4] = value.u64.hi >> 32;
+ flow->regs[idx * 4 + 1] = value.u64.hi;
+ flow->regs[idx * 4 + 2] = value.u64.lo >> 32;
+ flow->regs[idx * 4 + 3] = value.u64.lo;
}
static inline int
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index b85ed97..af579a2 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -1616,8 +1616,8 @@ mf_set_wild(const struct mf_field *mf, struct match *match
break;
CASE_MFF_XXREGS: {
- ovs_u128 u_zero = {.u64.hi = 0, .u64.lo = 0};
- match_set_xxreg_masked(match, mf->id - MFF_XXREG0, u_zero, u_zero);
+ match_set_xxreg_masked(match, mf->id - MFF_XXREG0, OVS_U128_ZERO,
+ OVS_U128_ZERO);
break;
}
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
