dpif: Add support for OVS_ACTION_ATTR_CT_CLEAR

This supports using the ct_clear action in the kernel datapath. To
preserve compatibility with current ct_clear behavior on old kernels, we
only pass this action down to the datapath if a probe reveals the
datapath actually supports it.

Signed-off-by: Eric Garver <e@erig.me>
Acked-by: William Tu <u9012063@gmail.com>
Acked-by: Flavio Leitner <fbl@sysclose.org>
Signed-off-by: Justin Pettit <jpettit@ovn.org>

diff --git a/NEWS b/NEWS
index 59f3587b790d40749032047975495d70587886de..101ac1bbb6f890b1ffa4417f32dbee66f94676d8 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,9 @@ v2.9.0 - xx xxx xxxx
          action 'nd_ns' to resolve unknown next hop MAC addresses for the
          IPv6 packets.
      * ovn-ctl: New commands run_nb_ovsdb and run_sb_ovsdb.
+   - OpenFlow:
+     * ct_clear action is now backed by kernel datapath. Support is probed for
+       when OVS starts.
    - Linux kernel 4.13
      * Add support for compiling OVS with the latest Linux 4.13 kernel
    - ovs-dpctl and related ovs-appctl commands:

diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
index c7142b604c21ac70a29fb7a2465e02ffd2273660..3deaba6f98a72e78b46d8ab5fe45bc7db0308386 100644 (file)
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -883,6 +883,7 @@ enum ovs_nat_attr {
  * @OVS_ACTION_ATTR_PUSH_ETH: Push a new outermost Ethernet header onto the
  * packet.
  * @OVS_ACTION_ATTR_POP_ETH: Pop the outermost Ethernet header off the packet.
+ * @OVS_ACTION_ATTR_CT_CLEAR: Clear conntrack state from the packet.
  * @OVS_ACTION_ATTR_PUSH_NSH: push NSH header to the packet.
  * @OVS_ACTION_ATTR_POP_NSH: pop the outermost NSH header off the packet.
  *
@@ -920,6 +921,7 @@ enum ovs_action_attr {
        OVS_ACTION_ATTR_TRUNC,        /* u32 struct ovs_action_trunc. */
        OVS_ACTION_ATTR_PUSH_ETH,     /* struct ovs_action_push_eth. */
        OVS_ACTION_ATTR_POP_ETH,      /* No argument. */
+       OVS_ACTION_ATTR_CT_CLEAR,     /* No argument. */
 
 #ifndef __KERNEL__
        OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/

diff --git a/lib/conntrack.c b/lib/conntrack.c
index c89ac43ad1e6ba5a46b3c67a2821f5636c0d2954..562e767833d1bdeb3006f3def3d3b87d01ac5b36 100644 (file)
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1325,6 +1325,14 @@ conntrack_execute(struct conntrack *ct, struct dp_packet_batch *pkt_batch,
     return 0;
 }
 
+void
+conntrack_clear(struct dp_packet *packet)
+{
+    /* According to pkt_metadata_init(), ct_state == 0 is enough to make all of
+     * the conntrack fields invalid. */
+    packet->md.ct_state = 0;
+}
+
 static void
 set_mark(struct dp_packet *pkt, struct conn *conn, uint32_t val, uint32_t mask)
 {

diff --git a/lib/conntrack.h b/lib/conntrack.h
index 419bcc3e3996d347a7052c683fbf09c8c5e5c1ba..ac650304a41f9844404bf75c005d754c0c098570 100644 (file)
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -98,6 +98,7 @@ int conntrack_execute(struct conntrack *ct, struct dp_packet_batch *pkt_batch,
                       ovs_be16 tp_src, ovs_be16 tp_dst, const char *helper,
                       const struct nat_action_info_t *nat_action_info,
                       long long now);
+void conntrack_clear(struct dp_packet *packet);
 
 struct conntrack_dump {
     struct conntrack *ct;

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index f94b189e0403c6a2caa3fa356e8a1f57cd59acc7..ba62128c758c48794392ccbb5eb38407da84aa57 100644 (file)
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -5762,6 +5762,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
     case OVS_ACTION_ATTR_CLONE:
     case OVS_ACTION_ATTR_PUSH_NSH:
     case OVS_ACTION_ATTR_POP_NSH:
+    case OVS_ACTION_ATTR_CT_CLEAR:
     case __OVS_ACTION_ATTR_MAX:
         OVS_NOT_REACHED();
     }

diff --git a/lib/dpif.c b/lib/dpif.c
index ab2e2329b536e41b898e589a4c8a801264b78c40..d3ea4613fd2125b2a09747f21bd83be6211a0964 100644 (file)
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1275,6 +1275,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
     case OVS_ACTION_ATTR_CLONE:
     case OVS_ACTION_ATTR_PUSH_NSH:
     case OVS_ACTION_ATTR_POP_NSH:
+    case OVS_ACTION_ATTR_CT_CLEAR:
     case OVS_ACTION_ATTR_UNSPEC:
     case __OVS_ACTION_ATTR_MAX:
         OVS_NOT_REACHED();

diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index ebaea3182c85d79efc0498f64238fbf2b0ab44ea..20f33eb57acb21e5e1cd5d6f8fbe5955a68c3680 100644 (file)
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -35,6 +35,7 @@
 #include "unaligned.h"
 #include "util.h"
 #include "csum.h"
+#include "conntrack.h"
 
 /* Masked copy of an ethernet address. 'src' is already properly masked. */
 static void
@@ -674,6 +675,7 @@ requires_datapath_assistance(const struct nlattr *a)
     case OVS_ACTION_ATTR_CLONE:
     case OVS_ACTION_ATTR_PUSH_NSH:
     case OVS_ACTION_ATTR_POP_NSH:
+    case OVS_ACTION_ATTR_CT_CLEAR:
         return false;
 
     case OVS_ACTION_ATTR_UNSPEC:
@@ -874,6 +876,11 @@ odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal,
             }
             break;
         }
+        case OVS_ACTION_ATTR_CT_CLEAR:
+            DP_PACKET_BATCH_FOR_EACH (packet, batch) {
+                conntrack_clear(packet);
+            }
+            break;
 
         case OVS_ACTION_ATTR_OUTPUT:
         case OVS_ACTION_ATTR_TUNNEL_PUSH:

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 8a5e9e82eaa8555d5f854d83e807eb79610310e8..6a29a76de5cdf6f46392f9a7be2014ae1b6720dd 100644 (file)
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -118,6 +118,7 @@ odp_action_len(uint16_t type)
     case OVS_ACTION_ATTR_SET_MASKED: return ATTR_LEN_VARIABLE;
     case OVS_ACTION_ATTR_SAMPLE: return ATTR_LEN_VARIABLE;
     case OVS_ACTION_ATTR_CT: return ATTR_LEN_VARIABLE;
+    case OVS_ACTION_ATTR_CT_CLEAR: return 0;
     case OVS_ACTION_ATTR_PUSH_ETH: return sizeof(struct ovs_action_push_eth);
     case OVS_ACTION_ATTR_POP_ETH: return 0;
     case OVS_ACTION_ATTR_CLONE: return ATTR_LEN_VARIABLE;
@@ -1131,6 +1132,9 @@ format_odp_action(struct ds *ds, const struct nlattr *a,
     case OVS_ACTION_ATTR_CT:
         format_odp_conntrack_action(ds, a);
         break;
+    case OVS_ACTION_ATTR_CT_CLEAR:
+        ds_put_cstr(ds, "ct_clear");
+        break;
     case OVS_ACTION_ATTR_CLONE:
         format_odp_clone_action(ds, a, portno_names);
         break;
@@ -2284,6 +2288,13 @@ parse_odp_action(const char *s, const struct simap *port_names,
         }
     }
 
+    {
+        if (!strncmp(s, "ct_clear", 8)) {
+            nl_msg_put_flag(actions, OVS_ACTION_ATTR_CT_CLEAR);
+            return 8;
+        }
+    }
+
     {
         int retval;
 

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index db933634bf8b96e6d69caaf32a124b891f6cf2c2..01249497660078ed40b6dba03aef2d6e96ce0aa8 100644 (file)
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -7400,6 +7400,7 @@ ofpacts_execute_action_set(struct ofpbuf *action_list,
     if (!ofpacts_copy_last(action_list, action_set, OFPACT_GROUP) &&
         !ofpacts_copy_last(action_list, action_set, OFPACT_OUTPUT) &&
         !ofpacts_copy_last(action_list, action_set, OFPACT_RESUBMIT) &&
+        !ofpacts_copy_last(action_list, action_set, OFPACT_CT_CLEAR) &&
         !ofpacts_copy_last(action_list, action_set, OFPACT_CT)) {
         ofpbuf_clear(action_list);
     }

diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
index a420903e9dd448df152688a5881a3d092e6e8f7e..97dbc3d29fe1a6e5e9b05c2f2ab9dad6f2a69f05 100644 (file)
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -2983,6 +2983,7 @@ dpif_ipfix_read_actions(const struct flow *flow,
         case OVS_ACTION_ATTR_TRUNC:
         case OVS_ACTION_ATTR_HASH:
         case OVS_ACTION_ATTR_CT:
+        case OVS_ACTION_ATTR_CT_CLEAR:
         case OVS_ACTION_ATTR_METER:
         case OVS_ACTION_ATTR_SET_MASKED:
         case OVS_ACTION_ATTR_SET:

diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index e30a411f5a692cd8495433553dd51e72e1caf923..fb7589fda070cb9a2becfae5921cd82eef42fd4b 100644 (file)
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -1160,6 +1160,7 @@ dpif_sflow_read_actions(const struct flow *flow,
        case OVS_ACTION_ATTR_RECIRC:
        case OVS_ACTION_ATTR_HASH:
         case OVS_ACTION_ATTR_CT:
+    case OVS_ACTION_ATTR_CT_CLEAR:
         case OVS_ACTION_ATTR_METER:
            break;
 

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 9c484f11ad492f1e4c78f35a619d49d3ba0ba488..aee18bf33d3a42f83fa79e017cbfe2175acfc85a 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -5691,6 +5691,17 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc,
     ctx->conntracked = false;
 }
 
+static void
+compose_ct_clear_action(struct xlate_ctx *ctx)
+{
+    clear_conntrack(ctx);
+    /* This action originally existed without dpif support. So to preserve
+     * compatibility, only append it if the dpif supports it. */
+    if (ctx->xbridge->support.ct_clear) {
+        nl_msg_put_flag(ctx->odp_actions,  OVS_ACTION_ATTR_CT_CLEAR);
+    }
+}
+
 static void
 rewrite_flow_encap_ethernet(struct xlate_ctx *ctx,
                             struct flow *flow,
@@ -6447,7 +6458,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_CT_CLEAR:
-            clear_conntrack(ctx);
+            compose_ct_clear_action(ctx);
             break;
 
         case OFPACT_NAT:

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6157b70aee6036d3d0522a1247ed54fda05cb752..3365d41859265a9f8c5a6a1cb6ffad2cd9c4ded7 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1255,6 +1255,39 @@ check_ct_eventmask(struct dpif_backer *backer)
     return !error;
 }
 
+/* Tests whether 'backer''s datapath supports the OVS_ACTION_ATTR_CT_CLEAR
+ * action. */
+static bool
+check_ct_clear(struct dpif_backer *backer)
+{
+    struct odputil_keybuf keybuf;
+    uint8_t actbuf[NL_A_FLAG_SIZE];
+    struct ofpbuf actions;
+    struct ofpbuf key;
+    struct flow flow;
+    bool supported;
+
+    struct odp_flow_key_parms odp_parms = {
+        .flow = &flow,
+        .probe = true,
+    };
+
+    memset(&flow, 0, sizeof flow);
+    ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
+    odp_flow_key_from_flow(&odp_parms, &key);
+
+    ofpbuf_use_stack(&actions, &actbuf, sizeof actbuf);
+    nl_msg_put_flag(&actions, OVS_ACTION_ATTR_CT_CLEAR);
+
+    supported = dpif_probe_feature(backer->dpif, "ct_clear", &key,
+                                   &actions, NULL);
+
+    VLOG_INFO("%s: Datapath %s ct_clear action",
+              dpif_name(backer->dpif), (supported) ? "supports"
+                                                   : "does not support");
+    return supported;
+}
+
 #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE)               \
 static bool                                                                 \
 check_##NAME(struct dpif_backer *backer)                                    \
@@ -1316,6 +1349,7 @@ check_support(struct dpif_backer *backer)
     backer->rt_support.clone = check_clone(backer);
     backer->rt_support.sample_nesting = check_max_sample_nesting(backer);
     backer->rt_support.ct_eventmask = check_ct_eventmask(backer);
+    backer->rt_support.ct_clear = check_ct_clear(backer);
 
     /* Flow fields. */
     backer->rt_support.odp.ct_state = check_ct_state(backer);

diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index c96e00e6a2ab922bbc69bcf094838822ca22faee..6443cc5b2d63b1c7a02a053ba572686625b21be5 100644 (file)
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -173,7 +173,10 @@ struct group_dpif *group_dpif_lookup(struct ofproto_dpif *,
     DPIF_SUPPORT_FIELD(size_t, sample_nesting, "Sample nesting")            \
                                                                             \
     /* OVS_CT_ATTR_EVENTMASK supported by OVS_ACTION_ATTR_CT action. */     \
-    DPIF_SUPPORT_FIELD(bool, ct_eventmask, "Conntrack eventmask")
+    DPIF_SUPPORT_FIELD(bool, ct_eventmask, "Conntrack eventmask")           \
+                                                                            \
+    /* True if the datapath supports OVS_ACTION_ATTR_CT_CLEAR action. */    \
+    DPIF_SUPPORT_FIELD(bool, ct_clear, "Conntrack clear")
 
 /* Stores the various features which the corresponding backer supports. */
 struct dpif_backer_support {

diff --git a/tests/odp.at b/tests/odp.at
index 270b9ff7a82edd231852877600d0aefd23ef90fa..ea8f40ede78be376229647f4890a1f77702cedf8 100644 (file)
--- a/tests/odp.at
+++ b/tests/odp.at
@@ -354,6 +354,7 @@ ct(force_commit,nat(src=fe80::20c:29ff:fe88:a18b,random))
 ct(force_commit,nat(src=fe80::20c:29ff:fe88:1-fe80::20c:29ff:fe88:a18b,random))
 ct(force_commit,nat(src=[[fe80::20c:29ff:fe88:1]]-[[fe80::20c:29ff:fe88:a18b]]:255-4096,random))
 ct(force_commit,helper=ftp,nat(src=10.1.1.240-10.1.1.255))
+ct_clear
 trunc(100)
 clone(1)
 clone(clone(push_vlan(vid=12,pcp=0),2),1)