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 denying nonzero masks whenever there is no datapath support
for connection tracking.

Reported-by: Ravindra Kenchappa <[email protected]>
Signed-off-by: Joe Stringer <[email protected]>
---
 ofproto/ofproto-dpif.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 5cc64cbca1f5..2f75b93d9694 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4012,40 +4012,55 @@ rule_dealloc(struct rule *rule_)
 }
 
 static enum ofperr
-rule_check(struct rule *rule)
+check_flow(const struct ofproto_dpif *ofproto, const struct miniflow *flow,
+           bool mask)
 {
+    ovs_u128 ct_label = { { 0, 0 } };
     uint16_t ct_state, ct_zone;
     const ovs_u128 *labelp;
-    ovs_u128 ct_label = { { 0, 0 } };
     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);
+    ct_state = MINIFLOW_GET_U16(flow, ct_state);
+    ct_zone = MINIFLOW_GET_U16(flow, ct_zone);
+    ct_mark = MINIFLOW_GET_U32(flow, ct_mark);
+    labelp = MINIFLOW_GET_U128_PTR(flow, ct_label);
     if (labelp) {
         ct_label = *labelp;
     }
 
     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;
+        const struct odp_support *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) {
+        if (mask && ct_state & CS_UNSUPPORTED_MASK) {
             return OFPERR_OFPBMC_BAD_MASK;
         }
     }
+
     return 0;
 }
 
 static enum ofperr
+rule_check(struct rule *rule)
+{
+    struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->ofproto);
+    enum ofperr err;
+
+    err = check_flow(ofproto, rule->cr.match.flow, false);
+    if (err) {
+        return err;
+    }
+    return check_flow(ofproto, &rule->cr.match.mask->masks, true);
+}
+
+static enum ofperr
 rule_construct(struct rule *rule_)
     OVS_NO_THREAD_SAFETY_ANALYSIS
 {
-- 
2.1.4

_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to