I noticed this patch doesn't correctly handle the case of a zero sampling
probability.
I'll fix that by not translating the action in that case, and re-send a patch.
--
Romain Lenglet
----- Original Message -----
> From: "Romain Lenglet" <rleng...@vmware.com>
> To: dev@openvswitch.org
> Cc: "Romain Lenglet" <rleng...@vmware.com>
> Sent: Monday, December 10, 2012 3:24:06 PM
> Subject: [PATCH 2/2] ofproto: Translate NXAST_SAMPLE actions into SAMPLE
> datapath actions
>
> ---
> lib/odp-util.c | 14 ++++++++
> lib/odp-util.h | 9 +++++-
> ofproto/ofproto-dpif.c | 86
> ++++++++++++++++++++++++++++++++++++++++----------
> tests/odp.at | 1 +
> 4 files changed, 93 insertions(+), 17 deletions(-)
>
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index de97fd2..50feeb5 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -279,6 +279,11 @@ format_odp_userspace_action(struct ds *ds, const
> struct nlattr *attr)
> ds_put_format(ds, ")");
> break;
>
> + case USER_ACTION_COOKIE_IPFIX:
> + ds_put_format(ds, ",IPFIX(obs_point_id=%"PRIu32")",
> + cookie.ipfix.obs_point_id);
> + break;
> +
> case USER_ACTION_COOKIE_UNSPEC:
> default:
> ds_put_format(ds, ",userdata=0x%"PRIx64, userdata);
> @@ -419,6 +424,7 @@ parse_odp_action(const char *s, const struct
> simap *port_names,
> {
> unsigned long long int pid;
> unsigned long long int output;
> + unsigned long long int obs_point_id;
> char userdata_s[32];
> int vid, pcp;
> int n = -1;
> @@ -464,6 +470,14 @@ parse_odp_action(const char *s, const struct
> simap *port_names,
>
> odp_put_userspace_action(pid, &cookie, actions);
> return n;
> + } else if (sscanf(s,
> "userspace(pid=%lli,IPFIX(obs_point_id=%lli))%n",
> + &pid, &obs_point_id, &n) > 0 && n > 0) {
> + union user_action_cookie cookie;
> +
> + cookie.type = USER_ACTION_COOKIE_IPFIX;
> + cookie.ipfix.obs_point_id = obs_point_id;
> + odp_put_userspace_action(pid, &cookie, actions);
> + return n;
> } else if (sscanf(s, "userspace(pid=%lli,userdata="
> "%31[x0123456789abcdefABCDEF])%n", &pid,
> userdata_s,
> &n) > 0 && n > 0) {
> diff --git a/lib/odp-util.h b/lib/odp-util.h
> index 9d38f33..44cde1d 100644
> --- a/lib/odp-util.h
> +++ b/lib/odp-util.h
> @@ -119,7 +119,8 @@ void commit_odp_actions(const struct flow *,
> struct flow *base,
> enum user_action_cookie_type {
> USER_ACTION_COOKIE_UNSPEC,
> USER_ACTION_COOKIE_SFLOW, /* Packet for sFlow sampling.
> */
> - USER_ACTION_COOKIE_SLOW_PATH /* Userspace must process this
> flow. */
> + USER_ACTION_COOKIE_SLOW_PATH, /* Userspace must process this
> flow. */
> + USER_ACTION_COOKIE_IPFIX /* Packet for IPFIX sampling.
> */
> };
>
> /* user_action_cookie is passed as argument to
> OVS_ACTION_ATTR_USERSPACE.
> @@ -138,6 +139,12 @@ union user_action_cookie {
> uint16_t unused;
> uint32_t reason; /* enum slow_path_reason. */
> } slow_path;
> +
> + struct {
> + uint16_t type; /* USER_ACTION_COOKIE_IPFIX. */
> + uint16_t unused;
> + uint32_t obs_point_id; /* Observation Point ID. */
> + } ipfix;
> };
> BUILD_ASSERT_DECL(sizeof(union user_action_cookie) == 8);
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 854ad29..c585038 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -3552,7 +3552,7 @@ handle_miss_upcalls(struct dpif_backer *backer,
> struct dpif_upcall *upcalls,
> hmap_destroy(&todo);
> }
>
> -static enum { SFLOW_UPCALL, MISS_UPCALL, BAD_UPCALL }
> +static enum { SFLOW_UPCALL, MISS_UPCALL, BAD_UPCALL, IPFIX_UPCALL }
> classify_upcall(const struct dpif_upcall *upcall)
> {
> union user_action_cookie cookie;
> @@ -3580,6 +3580,9 @@ classify_upcall(const struct dpif_upcall
> *upcall)
> case USER_ACTION_COOKIE_SLOW_PATH:
> return MISS_UPCALL;
>
> + case USER_ACTION_COOKIE_IPFIX:
> + return IPFIX_UPCALL;
> +
> case USER_ACTION_COOKIE_UNSPEC:
> default:
> VLOG_WARN_RL(&rl, "invalid user cookie : 0x%"PRIx64,
> upcall->userdata);
> @@ -3624,6 +3627,13 @@ handle_sflow_upcall(struct dpif_backer
> *backer,
> odp_in_port, &cookie);
> }
>
> +static void
> +handle_ipfix_upcall(struct dpif_backer *backer OVS_UNUSED,
> + const struct dpif_upcall *upcall OVS_UNUSED)
> +{
> + /* TODO: Send an IPFIX sample. */
> +}
> +
> static int
> handle_upcalls(struct dpif_backer *backer, unsigned int max_batch)
> {
> @@ -3661,6 +3671,11 @@ handle_upcalls(struct dpif_backer *backer,
> unsigned int max_batch)
> ofpbuf_uninit(buf);
> break;
>
> + case IPFIX_UPCALL:
> + handle_ipfix_upcall(backer, upcall);
> + ofpbuf_uninit(buf);
> + break;
> +
> case BAD_UPCALL:
> ofpbuf_uninit(buf);
> break;
> @@ -5268,6 +5283,30 @@ put_userspace_action(const struct ofproto_dpif
> *ofproto,
> return odp_put_userspace_action(pid, cookie, odp_actions);
> }
>
> +/* Compose SAMPLE action for sFlow or IPFIX. */
> +static size_t
> +compose_sample_action(const struct ofproto_dpif *ofproto,
> + struct ofpbuf *odp_actions,
> + const struct flow *flow,
> + const uint32_t probability,
> + const union user_action_cookie *cookie)
> +{
> + size_t sample_offset, actions_offset;
> + int cookie_offset;
> +
> + sample_offset = nl_msg_start_nested(odp_actions,
> OVS_ACTION_ATTR_SAMPLE);
> +
> + /* Number of packets out of UINT_MAX to sample. */
> + nl_msg_put_u32(odp_actions, OVS_SAMPLE_ATTR_PROBABILITY,
> probability);
> +
> + actions_offset = nl_msg_start_nested(odp_actions,
> OVS_SAMPLE_ATTR_ACTIONS);
> + cookie_offset = put_userspace_action(ofproto, odp_actions, flow,
> cookie);
> +
> + nl_msg_end_nested(odp_actions, actions_offset);
> + nl_msg_end_nested(odp_actions, sample_offset);
> + return cookie_offset;
> +}
> +
> static void
> compose_sflow_cookie(const struct ofproto_dpif *ofproto,
> ovs_be16 vlan_tci, uint32_t odp_port,
> @@ -5309,27 +5348,24 @@ compose_sflow_action(const struct
> ofproto_dpif *ofproto,
> {
> uint32_t probability;
> union user_action_cookie cookie;
> - size_t sample_offset, actions_offset;
> - int cookie_offset;
>
> if (!ofproto->sflow || flow->in_port == OFPP_NONE) {
> return 0;
> }
>
> - sample_offset = nl_msg_start_nested(odp_actions,
> OVS_ACTION_ATTR_SAMPLE);
> -
> - /* Number of packets out of UINT_MAX to sample. */
> probability = dpif_sflow_get_probability(ofproto->sflow);
> - nl_msg_put_u32(odp_actions, OVS_SAMPLE_ATTR_PROBABILITY,
> probability);
> -
> - actions_offset = nl_msg_start_nested(odp_actions,
> OVS_SAMPLE_ATTR_ACTIONS);
> compose_sflow_cookie(ofproto, htons(0), odp_port,
> odp_port == OVSP_NONE ? 0 : 1, &cookie);
> - cookie_offset = put_userspace_action(ofproto, odp_actions, flow,
> &cookie);
>
> - nl_msg_end_nested(odp_actions, actions_offset);
> - nl_msg_end_nested(odp_actions, sample_offset);
> - return cookie_offset;
> + return compose_sample_action(ofproto, odp_actions, flow,
> probability,
> + &cookie);
> +}
> +
> +static void
> +compose_ipfix_cookie(uint32_t obs_point_id, union user_action_cookie
> *cookie)
> +{
> + cookie->type = USER_ACTION_COOKIE_IPFIX;
> + cookie->ipfix.obs_point_id = obs_point_id;
> }
>
> /* SAMPLE action must be first action in any given list of actions.
> @@ -5841,6 +5877,16 @@ xlate_fin_timeout(struct action_xlate_ctx
> *ctx,
> }
> }
>
> +static void
> +xlate_sample_action(struct action_xlate_ctx *ctx,
> + const struct ofpact_sample *os)
> +{
> + union user_action_cookie cookie;
> + compose_ipfix_cookie(os->obs_point_id, &cookie);
> + compose_sample_action(ctx->ofproto, ctx->odp_actions,
> &ctx->flow,
> + os->probability, &cookie);
> +}
> +
> static bool
> may_receive(const struct ofport_dpif *port, struct action_xlate_ctx
> *ctx)
> {
> @@ -5869,6 +5915,7 @@ do_xlate_actions(const struct ofpact *ofpacts,
> size_t ofpacts_len,
> const struct ofport_dpif *port;
> bool was_evictable = true;
> const struct ofpact *a;
> + int non_sample_actions_count = 0;
>
> port = get_ofp_port(ctx->ofproto, ctx->flow.in_port);
> if (port && !may_receive(port, ctx)) {
> @@ -6053,11 +6100,18 @@ do_xlate_actions(const struct ofpact
> *ofpacts, size_t ofpacts_len,
> }
>
> case OFPACT_SAMPLE:
> - /* TODO: Check that SAMPLE actions are always the first
> - * actions in a flow. */
> - /* TODO: Actually implement the translation here. */
> + /* SAMPLE actions should be the first in a flow's
> actions. */
> + if (non_sample_actions_count > 0) {
> + VLOG_WARN_RL(&rl,
> + "sample action not at beginning of
> action list");
> + }
> + xlate_sample_action(ctx, ofpact_get_SAMPLE(a));
> break;
> }
> +
> + if (a->type != OFPACT_SAMPLE) {
> + non_sample_actions_count++;
> + }
> }
>
> out:
> diff --git a/tests/odp.at b/tests/odp.at
> index a5f6dbe..bcc32bf 100644
> --- a/tests/odp.at
> +++ b/tests/odp.at
> @@ -76,6 +76,7 @@ userspace(pid=9765,slow_path())
> userspace(pid=9765,slow_path(cfm))
> userspace(pid=9765,slow_path(cfm,match))
> userspace(pid=9123,userdata=0x815309)
> +userspace(pid=6633,IPFIX(obs_point_id=12345))
> set(tun_id(0x7f10354))
> set(in_port(2))
> set(eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15))
> --
> 1.8.0.1
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
