ofproto-dpif: Enable NXAST_SAMPLE only if the datapath supports it.

This prevents using an older datapath from breaking forwarding.

CC: Romain Lenglet <rlenglet@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 848c778..558598c 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -87,6 +87,11 @@ struct xbridge {
     enum ofp_config_flags frag;   /* Fragmentation handling. */
     bool has_in_band;             /* Bridge has in band control? */
     bool forward_bpdu;            /* Bridge forwards STP BPDUs? */
+
+    /* True if the datapath supports variable-length
+     * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions.
+     * False if the datapath supports only 8-byte (or shorter) userdata. */
+    bool variable_length_userdata;
 };
 
 struct xbundle {
@@ -249,7 +254,8 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,
                   const struct dpif_sflow *sflow,
                   const struct dpif_ipfix *ipfix,
                   const struct netflow *netflow, enum ofp_config_flags frag,
-                  bool forward_bpdu, bool has_in_band)
+                  bool forward_bpdu, bool has_in_band,
+                  bool variable_length_userdata)
 {
     struct xbridge *xbridge = xbridge_lookup(ofproto);
 
@@ -301,6 +307,7 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,
     xbridge->frag = frag;
     xbridge->miss_rule = miss_rule;
     xbridge->no_packet_in_rule = no_packet_in_rule;
+    xbridge->variable_length_userdata = variable_length_userdata;
 }
 
 void
@@ -2521,6 +2528,15 @@ xlate_sample_action(struct xlate_ctx *ctx,
    * the same percentage. */
   uint32_t probability = (os->probability << 16) | os->probability;
 
+  if (!ctx->xbridge->variable_length_userdata) {
+      static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+
+      VLOG_ERR_RL(&rl, "ignoring NXAST_SAMPLE action because datapath "
+                  "lacks support (needs Linux 3.10+ or kernel module from "
+                  "OVS 1.11+)");
+      return;
+  }
+
   ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
                                         &ctx->xout->odp_actions,
                                         &ctx->xout->wc,

diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 68076ca..982f1a4 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -129,7 +129,7 @@ void xlate_ofproto_set(struct ofproto_dpif *, const char *name,
                        const struct mbridge *, const struct dpif_sflow *,
                        const struct dpif_ipfix *, const struct netflow *,
                        enum ofp_config_flags, bool forward_bpdu,
-                       bool has_in_band)
+                       bool has_in_band, bool variable_length_userdata)
     OVS_REQ_WRLOCK(xlate_rwlock);
 void xlate_remove_ofproto(struct ofproto_dpif *) OVS_REQ_WRLOCK(xlate_rwlock);
 

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 52759b5..e48cc00 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -42,6 +42,7 @@
 #include "netdev-vport.h"
 #include "netdev.h"
 #include "netlink.h"
+#include "netlink-socket.h"
 #include "nx-match.h"
 #include "odp-util.h"
 #include "odp-execute.h"
@@ -251,6 +252,11 @@ struct dpif_backer {
     enum revalidate_reason need_revalidate; /* Revalidate all flows. */
 
     bool recv_set_enable; /* Enables or disables receiving packets. */
+
+    /* True if the datapath supports variable-length
+     * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions.
+     * False if the datapath supports only 8-byte (or shorter) userdata. */
+    bool variable_length_userdata;
 };
 
 /* All existing ofproto_backer instances, indexed by ofproto->up.type. */
@@ -552,7 +558,8 @@ type_run(const char *type)
                               ofproto->sflow, ofproto->ipfix,
                               ofproto->netflow, ofproto->up.frag_handling,
                               ofproto->up.forward_bpdu,
-                              connmgr_has_in_band(ofproto->up.connmgr));
+                              connmgr_has_in_band(ofproto->up.connmgr),
+                              ofproto->backer->variable_length_userdata);
 
             HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) {
                 xlate_bundle_set(ofproto, bundle, bundle->name,
@@ -774,6 +781,8 @@ struct odp_garbage {
     odp_port_t odp_port;
 };
 
+static bool check_variable_length_userdata(struct dpif_backer *backer);
+
 static int
 open_dpif_backer(const char *type, struct dpif_backer **backerp)
 {
@@ -872,6 +881,7 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
         close_dpif_backer(backer);
         return error;
     }
+    backer->variable_length_userdata = check_variable_length_userdata(backer);
 
     if (backer->recv_set_enable) {
         udpif_set_threads(backer->udpif, n_handlers, n_revalidators);
@@ -880,6 +890,86 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
     return error;
 }
 
+/* Tests whether 'backer''s datapath supports variable-length
+ * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions.  We need
+ * to disable some features on older datapaths that don't support this
+ * feature.
+ *
+ * Returns false if 'backer' definitely does not support variable-length
+ * userdata, true if it seems to support them or if at least the error we get
+ * is ambiguous. */
+static bool
+check_variable_length_userdata(struct dpif_backer *backer)
+{
+    struct eth_header *eth;
+    struct ofpbuf actions;
+    struct ofpbuf key;
+    struct ofpbuf packet;
+    size_t start;
+    int error;
+
+    /* Compose a userspace action that will cause an ERANGE error on older
+     * datapaths that don't support variable-length userdata.
+     *
+     * We really test for using userdata longer than 8 bytes, but older
+     * datapaths accepted these, silently truncating the userdata to 8 bytes.
+     * The same older datapaths rejected userdata shorter than 8 bytes, so we
+     * test for that instead as a proxy for longer userdata support. */
+    ofpbuf_init(&actions, 64);
+    start = nl_msg_start_nested(&actions, OVS_ACTION_ATTR_USERSPACE);
+    nl_msg_put_u32(&actions, OVS_USERSPACE_ATTR_PID,
+                   dpif_port_get_pid(backer->dpif, ODPP_NONE));
+    nl_msg_put_unspec_zero(&actions, OVS_USERSPACE_ATTR_USERDATA, 4);
+    nl_msg_end_nested(&actions, start);
+
+    /* Compose an ODP flow key.  The key is arbitrary but it must match the
+     * packet that we compose later. */
+    ofpbuf_init(&key, 64);
+    nl_msg_put_u32(&key, OVS_KEY_ATTR_IN_PORT, 0);
+    nl_msg_put_unspec_zero(&key, OVS_KEY_ATTR_ETHERNET,
+                           sizeof(struct ovs_key_ethernet));
+    nl_msg_put_be16(&key, OVS_KEY_ATTR_ETHERTYPE, htons(0x1234));
+
+    /* Compose a packet that matches the key. */
+    ofpbuf_init(&packet, ETH_HEADER_LEN);
+    eth = ofpbuf_put_zeros(&packet, ETH_HEADER_LEN);
+    eth->eth_type = htons(0x1234);
+
+    /* Execute the actions.  On older datapaths this fails with -ERANGE, on
+     * newer datapaths it succeeds. */
+    error = dpif_execute(backer->dpif, key.data, key.size,
+                         actions.data, actions.size, &packet, false);
+
+    ofpbuf_uninit(&packet);
+    ofpbuf_uninit(&key);
+    ofpbuf_uninit(&actions);
+
+    switch (error) {
+    case 0:
+        /* Variable-length userdata is supported.
+         *
+         * Purge received packets to avoid processing the nonsense packet we
+         * sent to userspace, then report success. */
+        dpif_recv_purge(backer->dpif);
+        return true;
+
+    case ERANGE:
+        /* Variable-length userdata is not supported. */
+        VLOG_WARN("%s: datapath does not support variable-length userdata "
+                  "feature (needs Linux 3.10+ or kernel module from OVS "
+                  "1..11+).  The NXAST_SAMPLE action will be ignored.",
+                  dpif_name(backer->dpif));
+        return false;
+
+    default:
+        /* Something odd happened.  We're not sure whether variable-length
+         * userdata is supported.  Default to "yes". */
+        VLOG_WARN("%s: variable-length userdata feature probe failed (%s)",
+                  dpif_name(backer->dpif), ovs_strerror(error));
+        return true;
+    }
+}
+
 static int
 construct(struct ofproto *ofproto_)
 {