This prevents using an older datapath from breaking forwarding.
CC: Romain Lenglet <rleng...@vmware.com>
Signed-off-by: Ben Pfaff <b...@nicira.com>
---
ofproto/ofproto-dpif-ipfix.c | 28 ++++++++++---
ofproto/ofproto-dpif-ipfix.h | 5 ++-
ofproto/ofproto-dpif.c | 91 +++++++++++++++++++++++++++++++++++++++++-
3 files changed, 116 insertions(+), 8 deletions(-)
diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
index a9cc73e..1a0ebae 100644
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012 Nicira, Inc.
+ * Copyright (c) 2012, 2013 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -71,6 +71,7 @@ struct dpif_ipfix {
struct dpif_ipfix_bridge_exporter bridge_exporter;
struct hmap flow_exporter_map; /* dpif_ipfix_flow_exporter_map_node. */
atomic_int ref_cnt;
+ bool variable_length_userdata;
};
#define IPFIX_VERSION 0x000a
@@ -632,14 +633,21 @@ dpif_ipfix_set_options(
ovs_mutex_unlock(&mutex);
}
+/* Creates and returns a new dpif_ipfix structure.
+ *
+ * 'variable_length_userdata' specifies whether the datapath supports
+ * variable-length OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE
+ * actions. A datapath that doesn't support such actions cannot support flow
+ * sampling, so the module will suppress its use. */
struct dpif_ipfix *
-dpif_ipfix_create(void)
+dpif_ipfix_create(bool variable_length_userdata)
{
struct dpif_ipfix *di;
di = xzalloc(sizeof *di);
dpif_ipfix_bridge_exporter_init(&di->bridge_exporter);
hmap_init(&di->flow_exporter_map);
atomic_init(&di->ref_cnt, 1);
+ di->variable_length_userdata = variable_length_userdata;
return di;
}
@@ -1239,13 +1247,23 @@ ipfix_send_data_msg(struct dpif_ipfix_exporter
*exporter,
}
static void
-dpif_ipfix_sample(struct dpif_ipfix_exporter *exporter,
+dpif_ipfix_sample(struct dpif_ipfix *di,
+ struct dpif_ipfix_exporter *exporter,
struct ofpbuf *packet, const struct flow *flow,
uint64_t packet_delta_count, uint32_t obs_domain_id,
uint32_t obs_point_id)
{
struct ipfix_flow_cache_entry *entry;
+ if (!di->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;
+ }
+
/* Create a flow cache entry from the sample. */
entry = xmalloc(sizeof *entry);
ipfix_cache_entry_init(entry, packet, flow, packet_delta_count,
@@ -1263,7 +1281,7 @@ dpif_ipfix_bridge_sample(struct dpif_ipfix *di, struct
ofpbuf *packet,
/* Use the sampling probability as an approximation of the number
* of matched packets. */
packet_delta_count = UINT32_MAX / di->bridge_exporter.probability;
- dpif_ipfix_sample(&di->bridge_exporter.exporter, packet, flow,
+ dpif_ipfix_sample(di, &di->bridge_exporter.exporter, packet, flow,
packet_delta_count,
di->bridge_exporter.options->obs_domain_id,
di->bridge_exporter.options->obs_point_id);
@@ -1284,7 +1302,7 @@ dpif_ipfix_flow_sample(struct dpif_ipfix *di, struct
ofpbuf *packet,
ovs_mutex_lock(&mutex);
node = dpif_ipfix_find_flow_exporter_map_node(di, collector_set_id);
if (node) {
- dpif_ipfix_sample(&node->exporter.exporter, packet, flow,
+ dpif_ipfix_sample(di, &node->exporter.exporter, packet, flow,
packet_delta_count, obs_domain_id, obs_point_id);
}
ovs_mutex_unlock(&mutex);
diff --git a/ofproto/ofproto-dpif-ipfix.h b/ofproto/ofproto-dpif-ipfix.h
index 6ebf8b0..a81976c 100644
--- a/ofproto/ofproto-dpif-ipfix.h
+++ b/ofproto/ofproto-dpif-ipfix.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012 Nicira, Inc.
+ * Copyright (c) 2012, 2013 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@
#ifndef OFPROTO_DPIF_IPFIX_H
#define OFPROTO_DPIF_IPFIX_H 1
+#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>
@@ -25,7 +26,7 @@ struct ofpbuf;
struct ofproto_ipfix_bridge_exporter_options;
struct ofproto_ipfix_flow_exporter_options;
-struct dpif_ipfix *dpif_ipfix_create(void);
+struct dpif_ipfix *dpif_ipfix_create(bool variable_length_userdata);
struct dpif_ipfix *dpif_ipfix_ref(const struct dpif_ipfix *);
void dpif_ipfix_unref(struct dpif_ipfix *);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 80e97e0..e1a690b 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -41,6 +41,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"
@@ -440,6 +441,10 @@ struct dpif_backer {
/* Number of upcall handling threads. */
unsigned int n_handler_threads;
+
+ /* True if the datapath supports variable-length
+ * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions. */
+ bool variable_length_userdata;
};
/* All existing ofproto_backer instances, indexed by ofproto->up.type. */
@@ -1095,6 +1100,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)
{
@@ -1197,6 +1204,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);
udpif_recv_set(backer->udpif, n_handler_threads,
backer->recv_set_enable);
backer->n_handler_threads = n_handler_threads;
@@ -1209,6 +1217,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);
+
+ 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. Definitely 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_)
{
@@ -1881,7 +1969,8 @@ set_ipfix(
bool has_options = bridge_exporter_options || flow_exporters_options;
if (has_options && !di) {
- di = ofproto->ipfix = dpif_ipfix_create();
+ di = ofproto->ipfix = dpif_ipfix_create(
+ ofproto->backer->variable_length_userdata);
}
if (di) {
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
