From: Daniele Di Proietto <diproiettod@vmware.com>
Date: Fri, 4 Dec 2015 23:01:37 +0000 (-0800)
Subject: ofproto-dpif: Validate NAT action support.
X-Git-Url: http://git.cascardo.eti.br/?p=cascardo%2Fovs.git;a=commitdiff_plain;h=7b27258c4e0821ade73e0e0a9bd5339328489523

ofproto-dpif: Validate NAT action support.

The NAT validation is similar (and based on) the existing conntrack
validation: when a dpif backer is created, we try to install a flow with
the ct_state NAT bits set.  If the flow setup fails we assume that the
backer doesn't support NAT and we reject OpenFlow flows with a NAT
action or a match on the ct_state NAT bits.

Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
---

diff --git a/lib/odp-util.h b/lib/odp-util.h
index a9364164f..35849ae24 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -178,6 +178,11 @@ struct odp_support {
     bool ct_zone;
     bool ct_mark;
     bool ct_label;
+
+    /* If true, it means that the datapath supports the NAT bits in
+     * 'ct_state'.  The above 'ct_state' member must be true for this
+     * to make sense */
+    bool ct_state_nat;
 };
 
 struct odp_flow_key_parms {
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index e76d8fe9a..d920ed064 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1236,7 +1236,7 @@ check_masked_set_action(struct dpif_backer *backer)
     return !error;
 }
 
-#define CHECK_FEATURE__(NAME, FIELD)                                        \
+#define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE)                        \
 static bool                                                                 \
 check_##NAME(struct dpif_backer *backer)                                    \
 {                                                                           \
@@ -1247,12 +1247,12 @@ check_##NAME(struct dpif_backer *backer)                                    \
     struct odp_flow_key_parms odp_parms = {                                 \
         .flow = &flow,                                                      \
         .support = {                                                        \
-            .NAME = true,                                                   \
+            .SUPPORT = true,                                                \
         },                                                                  \
     };                                                                      \
                                                                             \
     memset(&flow, 0, sizeof flow);                                          \
-    flow.FIELD = 1;                                                         \
+    flow.FIELD = VALUE;                                                     \
                                                                             \
     ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);                         \
     odp_flow_key_from_flow(&odp_parms, &key);                               \
@@ -1267,12 +1267,13 @@ check_##NAME(struct dpif_backer *backer)                                    \
                                                                             \
     return enable;                                                          \
 }
-#define CHECK_FEATURE(FIELD) CHECK_FEATURE__(FIELD, FIELD)
+#define CHECK_FEATURE(FIELD) CHECK_FEATURE__(FIELD, FIELD, FIELD, 1)
 
 CHECK_FEATURE(ct_state)
 CHECK_FEATURE(ct_zone)
 CHECK_FEATURE(ct_mark)
-CHECK_FEATURE__(ct_label, ct_label.u64.lo)
+CHECK_FEATURE__(ct_label, ct_label, ct_label.u64.lo, 1)
+CHECK_FEATURE__(ct_state_nat, ct_state, ct_state, CS_TRACKED|CS_SRC_NAT)
 
 #undef CHECK_FEATURE
 #undef CHECK_FEATURE__
@@ -1293,6 +1294,8 @@ check_support(struct dpif_backer *backer)
     backer->support.odp.ct_zone = check_ct_zone(backer);
     backer->support.odp.ct_mark = check_ct_mark(backer);
     backer->support.odp.ct_label = check_ct_label(backer);
+
+    backer->support.odp.ct_state_nat = check_ct_state_nat(backer);
 }
 
 static int
@@ -4023,7 +4026,7 @@ check_mask(struct ofproto_dpif *ofproto, const struct miniflow *flow)
     support = &ofproto_dpif_get_support(ofproto)->odp;
     ct_state = MINIFLOW_GET_U16(flow, ct_state);
     if (support->ct_state && support->ct_zone && support->ct_mark
-        && support->ct_label) {
+        && support->ct_label && support->ct_state_nat) {
         return ct_state & CS_UNSUPPORTED_MASK ? OFPERR_OFPBMC_BAD_MASK : 0;
     }
 
@@ -4033,6 +4036,7 @@ check_mask(struct ofproto_dpif *ofproto, const struct miniflow *flow)
 
     if ((ct_state && !support->ct_state)
         || (ct_state & CS_UNSUPPORTED_MASK)
+        || ((ct_state & (CS_SRC_NAT | CS_DST_NAT)) && !support->ct_state_nat)
         || (ct_zone && !support->ct_zone)
         || (ct_mark && !support->ct_mark)
         || (!ovs_u128_is_zero(&ct_label) && !support->ct_label)) {
@@ -4070,6 +4074,12 @@ check_actions(const struct ofproto_dpif *ofproto,
         OFPACT_FOR_EACH(a, ct->actions, ofpact_ct_get_action_len(ct)) {
             const struct mf_field *dst = ofpact_get_mf_dst(a);
 
+            if (a->type == OFPACT_NAT && !support->ct_state_nat) {
+                /* The backer doesn't seem to support the NAT bits in
+                 * 'ct_state': assume that it doesn't support the NAT
+                 * action. */
+                return OFPERR_OFPBAC_BAD_TYPE;
+            }
             if (dst && ((dst->id == MFF_CT_MARK && !support->ct_mark)
                         || (dst->id == MFF_CT_LABEL && !support->ct_label))) {
                 return OFPERR_OFPBAC_BAD_SET_ARGUMENT;