This patch makes sampling support more generic/simple by adding
Datapath SAMPLING action. When sampling is turned on, a SAMPLING
action is added to all flows for given bridge.
This patch cleanup sampling upcall by striping it down to minimum
essential that userspace can not figure out for itself. Now sample
packet is sent along with actions checksum. So that sflow module can
make sure it has consistent actions.
Signed-off-by: Pravin Shelar <pshe...@nicira.com>
---
datapath/actions.c | 63 ++++++++++++++-----------------
datapath/datapath.c | 25 ++++--------
datapath/datapath.h | 8 +---
include/openvswitch/datapath-protocol.h | 11 +++--
lib/dpif-linux.c | 14 ++-----
lib/dpif-netdev.c | 1 +
lib/dpif.h | 6 +--
lib/odp-util.c | 4 ++
ofproto/ofproto-dpif-sflow.c | 29 +++++++++++++-
ofproto/ofproto-dpif-sflow.h | 4 +-
ofproto/ofproto-dpif.c | 43 ++++++++++++++++++--
ofproto/ofproto.h | 4 ++
12 files changed, 126 insertions(+), 86 deletions(-)
diff --git a/datapath/actions.c b/datapath/actions.c
index 78712c6..45e327f 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -218,7 +218,8 @@ error:
kfree_skb(skb);
}
-static int output_userspace(struct datapath *dp, struct sk_buff *skb, u64 arg)
+static int output_userspace(struct datapath *dp, struct sk_buff *skb, u8 cmd,
+ u64 arg, u16 csum)
{
struct dp_upcall_info upcall;
@@ -226,15 +227,31 @@ static int output_userspace(struct datapath *dp, struct
sk_buff *skb, u64 arg)
if (!skb)
return -ENOMEM;
- upcall.cmd = ODP_PACKET_CMD_ACTION;
+ upcall.cmd = cmd;
upcall.key = &OVS_CB(skb)->flow->key;
upcall.userdata = arg;
- upcall.sample_pool = 0;
- upcall.actions = NULL;
- upcall.actions_len = 0;
+ upcall.actions_csum = csum;
return dp_upcall(dp, skb, &upcall);
}
+static int sflow_sample(struct datapath *dp, struct sk_buff *skb,
+ struct sw_flow_actions *acts)
+{
+ struct vport *p = OVS_CB(skb)->vport;
+ u16 actions_csum;
+ if (unlikely(!p))
+ return 0;
+
+ atomic_inc(&p->sflow_pool);
+ if (net_random() >= dp->sflow_probability)
+ return 0;
+
+ actions_csum = ip_compute_csum(acts->actions, acts->actions_len);
+
+ return output_userspace(dp, skb, ODP_PACKET_CMD_SAMPLE,
+ atomic_read(&p->sflow_pool), actions_csum);
+}
+
/* Execute a list of actions against 'skb'. */
static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
struct sw_flow_actions *acts)
@@ -258,12 +275,17 @@ static int do_execute_actions(struct datapath *dp, struct
sk_buff *skb,
}
switch (nla_type(a)) {
+ case ODP_ACTION_ATTR_SAMPLING:
+ err = sflow_sample(dp, skb, acts);
+ break;
+
case ODP_ACTION_ATTR_OUTPUT:
prev_port = nla_get_u32(a);
break;
case ODP_ACTION_ATTR_USERSPACE:
- err = output_userspace(dp, skb, nla_get_u64(a));
+ err = output_userspace(dp, skb, ODP_PACKET_CMD_ACTION,
+ nla_get_u64(a), 0);
break;
case ODP_ACTION_ATTR_SET_TUNNEL:
@@ -327,32 +349,6 @@ static int do_execute_actions(struct datapath *dp, struct
sk_buff *skb,
return 0;
}
-static void sflow_sample(struct datapath *dp, struct sk_buff *skb,
- struct sw_flow_actions *acts)
-{
- struct sk_buff *nskb;
- struct vport *p = OVS_CB(skb)->vport;
- struct dp_upcall_info upcall;
-
- if (unlikely(!p))
- return;
-
- atomic_inc(&p->sflow_pool);
- if (net_random() >= dp->sflow_probability)
- return;
-
- nskb = skb_clone(skb, GFP_ATOMIC);
- if (unlikely(!nskb))
- return;
-
- upcall.cmd = ODP_PACKET_CMD_SAMPLE;
- upcall.key = &OVS_CB(skb)->flow->key;
- upcall.userdata = 0;
- upcall.sample_pool = atomic_read(&p->sflow_pool);
- upcall.actions = acts->actions;
- upcall.actions_len = acts->actions_len;
- dp_upcall(dp, nskb, &upcall);
-}
/* Execute a list of actions against 'skb'. */
int execute_actions(struct datapath *dp, struct sk_buff *skb)
@@ -371,9 +367,6 @@ int execute_actions(struct datapath *dp, struct sk_buff
*skb)
goto out_loop;
}
- /* Really execute actions. */
- if (dp->sflow_probability)
- sflow_sample(dp, skb, acts);
OVS_CB(skb)->tun_id = 0;
error = do_execute_actions(dp, skb, acts);
diff --git a/datapath/datapath.c b/datapath/datapath.c
index b191239..235018d 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -303,9 +303,7 @@ void dp_process_received_packet(struct vport *p, struct
sk_buff *skb)
upcall.cmd = ODP_PACKET_CMD_MISS;
upcall.key = &key;
upcall.userdata = 0;
- upcall.sample_pool = 0;
- upcall.actions = NULL;
- upcall.actions_len = 0;
+ upcall.actions_csum = 0;
dp_upcall(dp, skb, &upcall);
stats_counter_off = offsetof(struct dp_stats_percpu,
n_missed);
goto out;
@@ -466,10 +464,8 @@ static int queue_userspace_packets(struct datapath *dp,
struct sk_buff *skb,
len += nla_total_size(FLOW_BUFSIZE);
if (upcall_info->userdata)
len += nla_total_size(8);
- if (upcall_info->sample_pool)
- len += nla_total_size(4);
- if (upcall_info->actions_len)
- len += nla_total_size(upcall_info->actions_len);
+ if (upcall_info->actions_csum)
+ len += nla_total_size(2);
user_skb = genlmsg_new(len, GFP_ATOMIC);
if (!user_skb) {
@@ -486,16 +482,9 @@ static int queue_userspace_packets(struct datapath *dp,
struct sk_buff *skb,
if (upcall_info->userdata)
nla_put_u64(user_skb, ODP_PACKET_ATTR_USERDATA,
upcall_info->userdata);
- if (upcall_info->sample_pool)
- nla_put_u32(user_skb, ODP_PACKET_ATTR_SAMPLE_POOL,
upcall_info->sample_pool);
- if (upcall_info->actions_len) {
- const struct nlattr *actions = upcall_info->actions;
- u32 actions_len = upcall_info->actions_len;
-
- nla = nla_nest_start(user_skb, ODP_PACKET_ATTR_ACTIONS);
- memcpy(__skb_put(user_skb, actions_len), actions,
actions_len);
- nla_nest_end(user_skb, nla);
- }
+ if (upcall_info->actions_csum)
+ nla_put_u16(user_skb, ODP_PACKET_ATTR_ACTS_CSUM,
+ upcall_info->actions_csum);
nla = __nla_reserve(user_skb, ODP_PACKET_ATTR_PACKET, skb->len);
if (skb->ip_summed == CHECKSUM_PARTIAL)
@@ -565,6 +554,7 @@ static int validate_actions(const struct nlattr *attr)
[ODP_ACTION_ATTR_SET_TUNNEL] = 8,
[ODP_ACTION_ATTR_SET_PRIORITY] = 4,
[ODP_ACTION_ATTR_POP_PRIORITY] = 0,
+ [ODP_ACTION_ATTR_SAMPLING] = 0,
};
int type = nla_type(a);
@@ -586,6 +576,7 @@ static int validate_actions(const struct nlattr *attr)
case ODP_ACTION_ATTR_SET_TUNNEL:
case ODP_ACTION_ATTR_SET_PRIORITY:
case ODP_ACTION_ATTR_POP_PRIORITY:
+ case ODP_ACTION_ATTR_SAMPLING:
/* No validation needed. */
break;
diff --git a/datapath/datapath.h b/datapath/datapath.h
index 15a9898..8901118 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -130,17 +130,13 @@ struct ovs_skb_cb {
* @cmd: One of %ODP_PACKET_CMD_*.
* @key: Becomes %ODP_PACKET_ATTR_KEY. Must be nonnull.
* @userdata: Becomes %ODP_PACKET_ATTR_USERDATA if nonzero.
- * @sample_pool: Becomes %ODP_PACKET_ATTR_SAMPLE_POOL if nonzero.
- * @actions: Becomes %ODP_PACKET_ATTR_ACTIONS if nonnull.
- * @actions_len: Number of bytes in @actions.
+ * @actions_csum: Becomes %ODP_PACKET_ATTR_ACTS_CSUM if nonzero.
*/
struct dp_upcall_info {
u8 cmd;
+ u16 actions_csum;
const struct sw_flow_key *key;
u64 userdata;
- u32 sample_pool;
- const struct nlattr *actions;
- u32 actions_len;
};
extern struct notifier_block dp_device_notifier;
diff --git a/include/openvswitch/datapath-protocol.h
b/include/openvswitch/datapath-protocol.h
index 426236c..4796a4f 100644
--- a/include/openvswitch/datapath-protocol.h
+++ b/include/openvswitch/datapath-protocol.h
@@ -166,9 +166,9 @@ enum odp_packet_cmd {
* flow key against the kernel's.
* @ODP_PACKET_ATTR_USERDATA: Present for an %ODP_PACKET_CMD_ACTION
* notification if the %ODP_ACTION_ATTR_USERSPACE, action's argument was
- * nonzero.
- * @ODP_PACKET_ATTR_SAMPLE_POOL: Present for %ODP_PACKET_CMD_SAMPLE. Contains
- * the number of packets processed so far that were candidates for sampling.
+ * nonzero. for action ODP_ACTION_ATTR_SAMPLING it passes sample pool
+ * i.e. the number of packets processed so far that were candidates
+ * for sampling.
* @ODP_PACKET_ATTR_ACTIONS: Present for %ODP_PACKET_CMD_SAMPLE. Contains a
* copy of the actions applied to the packet, as nested %ODP_ACTION_ATTR_*
* attributes.
@@ -180,9 +180,9 @@ enum odp_packet_attr {
ODP_PACKET_ATTR_UNSPEC,
ODP_PACKET_ATTR_PACKET, /* Packet data. */
ODP_PACKET_ATTR_KEY, /* Nested ODP_KEY_ATTR_* attributes. */
- ODP_PACKET_ATTR_USERDATA, /* u64 ODP_ACTION_ATTR_USERSPACE arg. */
- ODP_PACKET_ATTR_SAMPLE_POOL, /* # sampling candidate packets so far. */
+ ODP_PACKET_ATTR_USERDATA, /* u64 arg. */
ODP_PACKET_ATTR_ACTIONS, /* Nested ODP_ACTION_ATTR_* attributes. */
+ ODP_PACKET_ATTR_ACTS_CSUM, /* actions csum, used for validation */
__ODP_PACKET_ATTR_MAX
};
@@ -423,6 +423,7 @@ enum odp_action_type {
ODP_ACTION_ATTR_SET_TUNNEL, /* Set the encapsulating tunnel ID. */
ODP_ACTION_ATTR_SET_PRIORITY, /* Set skb->priority. */
ODP_ACTION_ATTR_POP_PRIORITY, /* Restore original skb->priority. */
+ ODP_ACTION_ATTR_SAMPLING, /* Restore original skb->priority. */
__ODP_ACTION_ATTR_MAX
};
diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 0f6a140..a5fc2ed 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -931,10 +931,8 @@ parse_odp_packet(struct ofpbuf *buf, struct dpif_upcall
*upcall,
/* ODP_PACKET_CMD_ACTION only. */
[ODP_PACKET_ATTR_USERDATA] = { .type = NL_A_U64, .optional = true },
-
- /* ODP_PACKET_CMD_SAMPLE only. */
- [ODP_PACKET_ATTR_SAMPLE_POOL] = { .type = NL_A_U32, .optional = true },
- [ODP_PACKET_ATTR_ACTIONS] = { .type = NL_A_NESTED, .optional = true },
+ /* ODP_ACTION_ATTR_SAMPLING only */
+ [ODP_PACKET_ATTR_ACTS_CSUM] = { .type = NL_A_U16, .optional = true },
};
struct odp_header *odp_header;
@@ -974,13 +972,9 @@ parse_odp_packet(struct ofpbuf *buf, struct dpif_upcall
*upcall,
upcall->userdata = (a[ODP_PACKET_ATTR_USERDATA]
? nl_attr_get_u64(a[ODP_PACKET_ATTR_USERDATA])
: 0);
- upcall->sample_pool = (a[ODP_PACKET_ATTR_SAMPLE_POOL]
- ? nl_attr_get_u32(a[ODP_PACKET_ATTR_SAMPLE_POOL])
+ upcall->actions_csum = (a[ODP_PACKET_ATTR_ACTS_CSUM]
+ ? nl_attr_get_u16(a[ODP_PACKET_ATTR_ACTS_CSUM])
: 0);
- if (a[ODP_PACKET_ATTR_ACTIONS]) {
- upcall->actions = (void *) nl_attr_get(a[ODP_PACKET_ATTR_ACTIONS]);
- upcall->actions_len = nl_attr_get_size(a[ODP_PACKET_ATTR_ACTIONS]);
- }
*dp_ifindex = odp_header->dp_ifindex;
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index ffcc28a..0ed2554 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -739,6 +739,7 @@ dpif_netdev_validate_actions(const struct nlattr *actions,
case ODP_ACTION_ATTR_SET_TUNNEL:
case ODP_ACTION_ATTR_SET_PRIORITY:
case ODP_ACTION_ATTR_POP_PRIORITY:
+ case ODP_ACTION_ATTR_SAMPLING:
default:
return EOPNOTSUPP;
}
diff --git a/lib/dpif.h b/lib/dpif.h
index 4df2318..f051a8c 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -179,10 +179,8 @@ struct dpif_upcall {
/* DPIF_UC_ACTION only. */
uint64_t userdata; /* Argument to ODP_ACTION_ATTR_USERSPACE. */
- /* DPIF_UC_SAMPLE only. */
- uint32_t sample_pool; /* # of sampling candidate packets so far. */
- struct nlattr *actions; /* Associated flow actions. */
- size_t actions_len;
+ /* ODP_ACTION_ATTR_SAMPLING only */
+ uint16_t actions_csum;
};
int dpif_recv_get_mask(const struct dpif *, int *listen_mask);
diff --git a/lib/odp-util.c b/lib/odp-util.c
index b416380..46d9b6d 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -55,6 +55,7 @@ odp_action_len(uint16_t type)
case ODP_ACTION_ATTR_SET_TUNNEL: return 8;
case ODP_ACTION_ATTR_SET_PRIORITY: return 4;
case ODP_ACTION_ATTR_POP_PRIORITY: return 0;
+ case ODP_ACTION_ATTR_SAMPLING: return 0;
case ODP_ACTION_ATTR_UNSPEC:
case __ODP_ACTION_ATTR_MAX:
@@ -146,6 +147,9 @@ format_odp_action(struct ds *ds, const struct nlattr *a)
case ODP_ACTION_ATTR_POP_PRIORITY:
ds_put_cstr(ds, "pop_priority");
break;
+ case ODP_ACTION_ATTR_SAMPLING:
+ ds_put_cstr(ds, "sampling");
+ break;
default:
format_generic_odp_action(ds, a);
break;
diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index 303623c..c2b89e0 100644
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -19,6 +19,7 @@
#include "ofproto-dpif-sflow.h"
#include <inttypes.h>
#include <stdlib.h>
+#include "csum.h"
#include "collectors.h"
#include "compiler.h"
#include "dpif.h"
@@ -53,6 +54,7 @@ struct dpif_sflow {
time_t next_tick;
size_t n_flood, n_all;
struct hmap ports; /* Contains "struct dpif_sflow_port"s. */
+ uint32_t actions_miss_count;
};
static void dpif_sflow_del_port__(struct dpif_sflow *,
@@ -279,13 +281,15 @@ dpif_sflow_is_enabled(const struct dpif_sflow *ds)
}
struct dpif_sflow *
-dpif_sflow_create(struct dpif *dpif)
+dpif_sflow_create(struct ofproto *ofproto, struct dpif *dpif)
{
struct dpif_sflow *ds;
ds = xcalloc(1, sizeof *ds);
+ ds->ofproto = ofproto;
ds->dpif = dpif;
ds->next_tick = time_now() + 1;
+ ds->actions_miss_count = 0;
hmap_init(&ds->ports);
return ds;
}
@@ -486,12 +490,15 @@ dpif_sflow_received(struct dpif_sflow *ds, const struct
dpif_upcall *upcall,
unsigned int left;
struct nlattr *a;
size_t n_outputs;
+ struct nlattr *actions;
+ size_t actions_len;
+ uint16_t actions_csum;
/* Build a flow sample */
memset(&fs, 0, sizeof fs);
fs.input = dpif_sflow_odp_port_to_ifindex(ds, flow->in_port);
fs.output = 0; /* Filled in correctly below. */
- fs.sample_pool = upcall->sample_pool;
+ fs.sample_pool = (uint32_t) upcall->userdata;
/* We are going to give it to the sampler that represents this input port.
* By implementing "ingress-only" sampling like this we ensure that we
@@ -528,8 +535,24 @@ dpif_sflow_received(struct dpif_sflow *ds, const struct
dpif_upcall *upcall,
switchElem.flowType.sw.dst_priority = switchElem.flowType.sw.src_priority;
/* Figure out the output ports. */
+ actions = ofproto_get_actions(ds->ofproto, flow, &actions_len);
+ if (!actions) {
+ ds->actions_miss_count++;
+ VLOG_DBG_RL(&rl, "Could not lookup action. Count %u",
+ ds->actions_miss_count);
+ return;
+ }
+ actions_csum = csum(actions, actions_len);
+
+ if (actions_csum != upcall->actions_csum) {
+ ds->actions_miss_count++;
+ VLOG_DBG_RL(&rl, "Action has changed. Count %u",
+ ds->actions_miss_count);
+ return;
+ }
+
n_outputs = 0;
- NL_ATTR_FOR_EACH_UNSAFE (a, left, upcall->actions, upcall->actions_len) {
+ NL_ATTR_FOR_EACH_UNSAFE (a, left, actions, actions_len) {
ovs_be16 tci;
switch (nl_attr_type(a)) {
diff --git a/ofproto/ofproto-dpif-sflow.h b/ofproto/ofproto-dpif-sflow.h
index acafa32..4735b3a 100644
--- a/ofproto/ofproto-dpif-sflow.h
+++ b/ofproto/ofproto-dpif-sflow.h
@@ -25,8 +25,10 @@ struct dpif;
struct dpif_upcall;
struct flow;
struct ofproto_sflow_options;
+struct ofproto;
-struct dpif_sflow *dpif_sflow_create(struct dpif *);
+struct dpif_sflow *dpif_sflow_create(struct ofproto *ofproto,
+ struct dpif *dpif);
void dpif_sflow_destroy(struct dpif_sflow *);
void dpif_sflow_set_options(struct dpif_sflow *,
const struct ofproto_sflow_options *);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index a6e1782..b53a6a6 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -772,16 +772,20 @@ set_sflow(struct ofproto *ofproto_,
if (!ds) {
struct ofport_dpif *ofport;
- ds = ofproto->sflow = dpif_sflow_create(ofproto->dpif);
+ ds = ofproto->sflow = dpif_sflow_create(ofproto_, ofproto->dpif);
HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
dpif_sflow_add_port(ds, ofport->odp_port,
netdev_get_name(ofport->up.netdev));
}
+ flush(ofproto_);
}
dpif_sflow_set_options(ds, sflow_options);
} else {
dpif_sflow_destroy(ds);
- ofproto->sflow = NULL;
+ if (ds) {
+ flush(ofproto_);
+ ofproto->sflow = NULL;
+ }
}
return 0;
}
@@ -2074,9 +2078,6 @@ execute_odp_actions(struct ofproto_dpif *ofproto, const
struct flow *flow,
upcall.key = NULL;
upcall.key_len = 0;
upcall.userdata = nl_attr_get_u64(odp_actions);
- upcall.sample_pool = 0;
- upcall.actions = NULL;
- upcall.actions_len = 0;
send_packet_in(ofproto, &upcall, flow, false);
@@ -2559,6 +2560,25 @@ facet_push_stats(struct facet *facet)
}
}
+/*
+ * Searches ofproto's table of facets for exact flow to get actions.
+ * actions returned might not be valid action as facet is not revalidated.
+ * returns action for given flow */
+struct nlattr *
+ofproto_get_actions(struct ofproto *ofproto_, const struct flow *flow,
+ size_t *len)
+{
+ struct facet *facet;
+ struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
+
+ facet = facet_find(ofproto, flow);
+ if (facet) {
+ *len = facet->actions_len;
+ return facet->actions;
+ }
+ return NULL;
+}
+
struct ofproto_push {
struct action_xlate_ctx ctx;
uint64_t packets;
@@ -2805,12 +2825,24 @@ static void do_xlate_actions(const union ofp_action
*in, size_t n_in,
static void xlate_normal(struct action_xlate_ctx *);
static void
+add_sample_action(struct action_xlate_ctx *ctx)
+{
+ if (ctx->ofproto->sflow) {
+ struct ofpbuf *odp_actions = ctx->odp_actions;
+
+ nl_msg_put_flag(odp_actions, ODP_ACTION_ATTR_SAMPLING);
+ }
+}
+
+static void
commit_odp_actions(struct action_xlate_ctx *ctx)
{
const struct flow *flow = &ctx->flow;
struct flow *base = &ctx->base_flow;
struct ofpbuf *odp_actions = ctx->odp_actions;
+ add_sample_action(ctx);
+
if (base->tun_id != flow->tun_id) {
nl_msg_put_be64(odp_actions, ODP_ACTION_ATTR_SET_TUNNEL, flow->tun_id);
base->tun_id = flow->tun_id;
@@ -3612,6 +3644,7 @@ compose_actions(struct action_xlate_ctx *ctx, uint16_t
vlan,
compose_mirror_dsts(ctx, vlan, in_bundle, &set);
/* Output all the packets we can without having to change the VLAN. */
+ add_sample_action(ctx);
initial_vlan = vlan_tci_to_vid(ctx->flow.vlan_tci);
if (initial_vlan == 0) {
initial_vlan = OFP_VLAN_NONE;
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index e0c99ea..5dca0e1 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -239,6 +239,10 @@ int ofproto_port_get_cfm_fault(const struct ofproto *,
uint16_t ofp_port);
void ofproto_get_ofproto_controller_info(const struct ofproto *, struct shash
*);
void ofproto_free_ofproto_controller_info(struct shash *);
+struct nlattr *
+ofproto_get_actions(struct ofproto *ofproto, const struct flow *flow,
+ size_t *len);
+
#ifdef __cplusplus
}
#endif
--
1.7.6.134.gcf13f
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
