diff mbox

[ovs-dev,PATCHv2,3/6] ofproto-dpif: Shortcut common case in rule_check().

Message ID 1447270794-21103-4-git-send-email-joestringer@nicira.com
State Awaiting Upstream
Headers show

Commit Message

Joe Stringer Nov. 11, 2015, 7:39 p.m. UTC
Typically the datapath will support all available features, so check
that first before attempting to retrieve various values out of a
minimask as the latter doesn't need to be checked if all fields are
supported.

ct_state is an exception, because support for the bits in this field is
not binary; only some bits are defined so far, so they must still be
checked against the current known supported bits.

Suggested-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
 ofproto/ofproto-dpif.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Jarno Rajahalme Nov. 11, 2015, 10:21 p.m. UTC | #1
I was about to propose this for the patch 2/6,

Acked-by: Jarno Rajahalme <jarno@ovn.org>

> On Nov 11, 2015, at 11:39 AM, Joe Stringer <joestringer@nicira.com> wrote:
> 
> Typically the datapath will support all available features, so check
> that first before attempting to retrieve various values out of a
> minimask as the latter doesn't need to be checked if all fields are
> supported.
> 
> ct_state is an exception, because support for the bits in this field is
> not binary; only some bits are defined so far, so they must still be
> checked against the current known supported bits.
> 
> Suggested-by: Jarno Rajahalme <jrajahalme@nicira.com>
> Signed-off-by: Joe Stringer <joestringer@nicira.com>
> ---
> ofproto/ofproto-dpif.c | 5 +++++
> 1 file changed, 5 insertions(+)
> 
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index ee2d267ab7b8..f0a2ca59e2e8 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4022,6 +4022,11 @@ rule_check(struct rule *rule)
> 
>     support = &ofproto_dpif_get_support(ofproto)->odp;
>     ct_state = MINIFLOW_GET_U16(&rule->cr.match.mask->masks, ct_state);
> +    if (support->ct_state && support->ct_zone && support->ct_mark
> +        && support->ct_label) {
> +        return ct_state & CS_UNSUPPORTED_MASK ? OFPERR_OFPBMC_BAD_MASK : 0;
> +    }
> +
>     ct_zone = MINIFLOW_GET_U16(&rule->cr.match.mask->masks, ct_zone);
>     ct_mark = MINIFLOW_GET_U32(&rule->cr.match.mask->masks, ct_mark);
>     ct_label = MINIFLOW_GET_U128(&rule->cr.match.mask->masks, ct_label);
> -- 
> 2.1.4
>
diff mbox

Patch

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ee2d267ab7b8..f0a2ca59e2e8 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4022,6 +4022,11 @@  rule_check(struct rule *rule)
 
     support = &ofproto_dpif_get_support(ofproto)->odp;
     ct_state = MINIFLOW_GET_U16(&rule->cr.match.mask->masks, ct_state);
+    if (support->ct_state && support->ct_zone && support->ct_mark
+        && support->ct_label) {
+        return ct_state & CS_UNSUPPORTED_MASK ? OFPERR_OFPBMC_BAD_MASK : 0;
+    }
+
     ct_zone = MINIFLOW_GET_U16(&rule->cr.match.mask->masks, ct_zone);
     ct_mark = MINIFLOW_GET_U32(&rule->cr.match.mask->masks, ct_mark);
     ct_label = MINIFLOW_GET_U128(&rule->cr.match.mask->masks, ct_label);