[ovs-dev] [PATCHv2 2/6] ofproto-dpif: Validate ct_* field masks.

[Joe Stringer]

When inserting rules that match on connection tracking fields, datapath
support must be checked before allowing or denying the rule insertion.
Previously we only disallowed flows that had non-zero values for the
ct_* field, but allowed non-zero masks. This meant that, eg:

ct_state=-trk,...

Would be allowed, while

ct_state=+trk,...

Would be disallowed, due to lack of datapath support.

Fix this by performing the check on masks instead of the flows.

Reported-by: Ravindra Kenchappa <ravindra.kenchappa at hpe.com>
Signed-off-by: Joe Stringer <joestringer at nicira.com>
---
 ofproto/ofproto-dpif.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ab1b6a2f7d8e..ee2d267ab7b8 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4014,30 +4014,26 @@ rule_dealloc(struct rule *rule_)
 static enum ofperr
 rule_check(struct rule *rule)
 {
+    struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->ofproto);
+    const struct odp_support *support;
     uint16_t ct_state, ct_zone;
     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);
-    ct_label = MINIFLOW_GET_U128(rule->cr.match.flow, ct_label);
+    support = &ofproto_dpif_get_support(ofproto)->odp;
+    ct_state = MINIFLOW_GET_U16(&rule->cr.match.mask->masks, ct_state);
+    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);
 
-    if (ct_state || ct_zone || ct_mark
-        || !ovs_u128_is_zero(&ct_label)) {
-        struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->ofproto);
-        const struct odp_support *support = &ofproto_dpif_get_support(ofproto)->odp;
-
-        if ((ct_state && !support->ct_state)
-            || (ct_zone && !support->ct_zone)
-            || (ct_mark && !support->ct_mark)
-            || (!ovs_u128_is_zero(&ct_label) && !support->ct_label)) {
-            return OFPERR_OFPBMC_BAD_FIELD;
-        }
-        if (ct_state & CS_UNSUPPORTED_MASK) {
-            return OFPERR_OFPBMC_BAD_MASK;
-        }
+    if ((ct_state && !support->ct_state)
+        || (ct_state & CS_UNSUPPORTED_MASK)
+        || (ct_zone && !support->ct_zone)
+        || (ct_mark && !support->ct_mark)
+        || (!ovs_u128_is_zero(&ct_label) && !support->ct_label)) {
+        return OFPERR_OFPBMC_BAD_MASK;
     }
+
     return 0;
 }
 
-- 
2.1.4