Acked-by: Jarno Rajahalme <ja...@ovn.org>
> On Nov 11, 2015, at 11:39 AM, Joe Stringer <joestrin...@nicira.com> wrote:
>
> If only half of a ct_label is present in a miniflow/minimask (eg, only
> matching on one specific bit), then rule_check() would allow the flow
> even if ct_label was unsupported, because it required both 64-bit fields
> that comprise the ct_label to be present in the miniflow before
> performing the check.
>
> Fix this by populating the stack copy of the label directly from the
> miniflow fields if available (or zero each 64-bit word if unavailable).
>
> Suggested-by: Jarno Rajahalme <jrajaha...@nicira.com>
> Signed-off-by: Joe Stringer <joestrin...@nicira.com>
> ---
> lib/flow.h | 14 ++++++--------
> ofproto/ofproto-dpif.c | 8 ++------
> 2 files changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/lib/flow.h b/lib/flow.h
> index efd34a0da1a4..e7f851ea3dd1 100644
> --- a/lib/flow.h
> +++ b/lib/flow.h
> @@ -788,14 +788,12 @@ miniflow_get__(const struct miniflow *mf, size_t idx)
> [FLOW_U64_OFFREM(FIELD) / sizeof(TYPE)] \
> : 0)
>
> -/* Get a pointer to the ovs_u128 value of struct flow 'FIELD' from miniflow
> - * 'FLOW'. */
> -#define MINIFLOW_GET_U128_PTR(FLOW, FIELD) \
> - ((MINIFLOW_IN_MAP(FLOW, FLOW_U64_OFFSET(FIELD)) \
> - && (MINIFLOW_IN_MAP(FLOW, FLOW_U64_OFFSET(FIELD) + 1))) \
> - ? &((OVS_FORCE const ovs_u128 *)miniflow_get__(FLOW,
> FLOW_U64_OFFSET(FIELD))) \
> - [FLOW_U64_OFFREM(FIELD) / sizeof(ovs_u128)] \
> - : NULL)
> +#define MINIFLOW_GET_U128(FLOW, FIELD) \
> + (ovs_u128) { .u64 = { \
> + (MINIFLOW_IN_MAP(FLOW, FLOW_U64_OFFSET(FIELD)) ? \
> + *miniflow_get__(FLOW, FLOW_U64_OFFSET(FIELD)) : 0), \
> + (MINIFLOW_IN_MAP(FLOW, FLOW_U64_OFFSET(FIELD) + 1) ? \
> + *miniflow_get__(FLOW, FLOW_U64_OFFSET(FIELD) + 1) : 0) } }
>
> #define MINIFLOW_GET_U8(FLOW, FIELD) \
> MINIFLOW_GET_TYPE(FLOW, uint8_t, FIELD)
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 5cc64cbca1f5..ab1b6a2f7d8e 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4015,17 +4015,13 @@ static enum ofperr
> rule_check(struct rule *rule)
> {
> uint16_t ct_state, ct_zone;
> - const ovs_u128 *labelp;
> - ovs_u128 ct_label = { { 0, 0 } };
> + ovs_u128 ct_label;
> uint32_t ct_mark;
>
> ct_state = MINIFLOW_GET_U16(rule->cr.match.flow, ct_state);
> ct_zone = MINIFLOW_GET_U16(rule->cr.match.flow, ct_zone);
> ct_mark = MINIFLOW_GET_U32(rule->cr.match.flow, ct_mark);
> - labelp = MINIFLOW_GET_U128_PTR(rule->cr.match.flow, ct_label);
> - if (labelp) {
> - ct_label = *labelp;
> - }
> + ct_label = MINIFLOW_GET_U128(rule->cr.match.flow, ct_label);
>
> if (ct_state || ct_zone || ct_mark
> || !ovs_u128_is_zero(&ct_label)) {
> --
> 2.1.4
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
