ofproto-dpif: Validate ct_* field masks.

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@hpe.com>
Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Jarno Rajahalme <jarno@ovn.org>

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 37c5d5d..8d6865e 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4015,30 +4015,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;
 }