diff mbox

[ovs-dev] ofp-actions: Add "ingress" and "egress" options to "sample" action.

Message ID BN6PR05MB3106081EBE94FAC5E065CD82DDB60@BN6PR05MB3106.namprd05.prod.outlook.com
State Not Applicable
Headers show

Commit Message

Daniel Benli Ye Nov. 24, 2016, 7:09 a.m. UTC
Hi Ben,

Thanks for this fix. I applied your patch and verified from end to end. The patch looks good to me. One comment inline.

Acked-by: Benli Ye <daniely@vmware.com>


Bests,
Daniel

-----Original Message-----
From: Ben Pfaff [mailto:blp@ovn.org] 
Sent: Thursday, November 24, 2016 6:56 AM
To: dev@openvswitch.org
Cc: Ben Pfaff <blp@ovn.org>; Daniel Ye <daniely@vmware.com>
Subject: [PATCH] ofp-actions: Add "ingress" and "egress" options to "sample" action.

Before Open vSwitch 2.5.90, IPFIX reports from Open vSwitch didn't include whether the packet was ingressing or egressing the switch.  Starting in OVS 2.5.90, this information was available but only accurate if the action included a port number that indicated a tunnel.  Conflating these two does not always make sense (not every packet involves a tunnel!), so this patch makes it possible for the sample action to simply say whether it's for ingress or egress.

This is difficult to test, since the "tests" directory of OVS does not have a proper IPFIX listener.  This passes those tests, plus a couple that just verify that the actions are properly parsed and formatted, but it has not been tested end-to-end.

Requested-by: Daniel Ye <daniely@vmware.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 NEWS                              |  1 +
 include/openvswitch/ofp-actions.h | 15 +++++++
 lib/odp-util.c                    | 25 ++++++++++-
 lib/odp-util.h                    |  6 ++-
 lib/ofp-actions.c                 | 92 ++++++++++++++++++++++++++++++---------
 ofproto/ofproto-dpif-ipfix.c      | 16 ++++---
 ofproto/ofproto-dpif-xlate.c      |  1 +
 tests/odp.at                      |  2 +
 tests/ofp-actions.at              |  3 ++
 tests/ovs-ofctl.at                |  4 ++
 utilities/ovs-ofctl.8.in          | 18 +++++---
 11 files changed, 149 insertions(+), 34 deletions(-)

dev@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Comments

Ben Pfaff Nov. 24, 2016, 7:19 a.m. UTC | #1
Thanks, I posted a v2 and added a Tested-by from you, which in this case
is more valuable than an Acked-by since it's harder to get.

I'm going to give until at least Monday to get other reviews since
Thursday and Friday are holidays here.

On Thu, Nov 24, 2016 at 07:09:22AM +0000, Daniel Ye wrote:
> Hi Ben,
> 
> Thanks for this fix. I applied your patch and verified from end to end. The patch looks good to me. One comment inline.
> 
> Acked-by: Benli Ye <daniely@vmware.com>
> 
> 
> Bests,
> Daniel
> 
> -----Original Message-----
> From: Ben Pfaff [mailto:blp@ovn.org] 
> Sent: Thursday, November 24, 2016 6:56 AM
> To: dev@openvswitch.org
> Cc: Ben Pfaff <blp@ovn.org>; Daniel Ye <daniely@vmware.com>
> Subject: [PATCH] ofp-actions: Add "ingress" and "egress" options to "sample" action.
> 
> Before Open vSwitch 2.5.90, IPFIX reports from Open vSwitch didn't include whether the packet was ingressing or egressing the switch.  Starting in OVS 2.5.90, this information was available but only accurate if the action included a port number that indicated a tunnel.  Conflating these two does not always make sense (not every packet involves a tunnel!), so this patch makes it possible for the sample action to simply say whether it's for ingress or egress.
> 
> This is difficult to test, since the "tests" directory of OVS does not have a proper IPFIX listener.  This passes those tests, plus a couple that just verify that the actions are properly parsed and formatted, but it has not been tested end-to-end.
> 
> Requested-by: Daniel Ye <daniely@vmware.com>
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> ---
>  NEWS                              |  1 +
>  include/openvswitch/ofp-actions.h | 15 +++++++
>  lib/odp-util.c                    | 25 ++++++++++-
>  lib/odp-util.h                    |  6 ++-
>  lib/ofp-actions.c                 | 92 ++++++++++++++++++++++++++++++---------
>  ofproto/ofproto-dpif-ipfix.c      | 16 ++++---
>  ofproto/ofproto-dpif-xlate.c      |  1 +
>  tests/odp.at                      |  2 +
>  tests/ofp-actions.at              |  3 ++
>  tests/ovs-ofctl.at                |  4 ++
>  utilities/ovs-ofctl.8.in          | 18 +++++---
>  11 files changed, 149 insertions(+), 34 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index a4806d3..ab70a45 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -20,6 +20,7 @@ Post-v2.6.0
>         control over the OpenFlow select groups selection method.  See
>         "selection_method" and related options in ovs-ofctl(8) for
>         details.
> +     * The "sample" action now supports "ingress" and "egress" options.
>     - ovs-ofctl:
>       * 'bundle' command now supports packet-out messages.
>       * New syntax for 'ovs-ofctl packet-out' command, which uses the diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-actions.h
> index 74e9dcc..af55f19 100644
> --- a/include/openvswitch/ofp-actions.h
> +++ b/include/openvswitch/ofp-actions.h
> @@ -841,6 +841,20 @@ struct ofpact_note {
>      uint8_t data[];
>  };
>  
> +/* Direction of sampled packets. */
> +enum nx_action_sample_direction {
> +    /* OVS will attempt to infer the sample's direction based on whether
> +     * 'sampling_port' is the packet's output port.  This is generally
> +     * effective except when sampling happens as part of an output to a patch
> +     * port, which doesn't involve a datapath output action. */
> +    NX_ACTION_SAMPLE_DEFAULT,
> +
> +    /* Explicit direction.  This is useful for sampling packets coming in from
> +     * or going out of a patch port, where the direction cannot be inferred. */
> +    NX_ACTION_SAMPLE_INGRESS,
> +    NX_ACTION_SAMPLE_EGRESS
> +};
> +
>  /* OFPACT_SAMPLE.
>   *
> Daniel:
>   -  Used for NXAST_SAMPLE and NXAST_SAMPLE2. */ @@ -851,6 +865,7 @@ struct ofpact_sample {
>   + Used for NXAST_SAMPLE, NXAST_SAMPLE2 and  NXAST_SAMPLE3. */ @@ -851,6 +865,7 @@ struct ofpact_sample {
>      uint32_t obs_domain_id;
>      uint32_t obs_point_id;
>      ofp_port_t sampling_port;
> +    enum nx_action_sample_direction direction;
>  };
>  
>  /* OFPACT_DEC_TTL.
> diff --git a/lib/odp-util.c b/lib/odp-util.c index 626a82c..f09aabe 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -313,12 +313,18 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr)
>                                ",collector_set_id=%"PRIu32
>                                ",obs_domain_id=%"PRIu32
>                                ",obs_point_id=%"PRIu32
> -                              ",output_port=%"PRIu32")",
> +                              ",output_port=%"PRIu32,
>                                cookie.flow_sample.probability,
>                                cookie.flow_sample.collector_set_id,
>                                cookie.flow_sample.obs_domain_id,
>                                cookie.flow_sample.obs_point_id,
>                                cookie.flow_sample.output_odp_port);
> +                if (cookie.flow_sample.direction == NX_ACTION_SAMPLE_INGRESS) {
> +                    ds_put_cstr(ds, ",ingress");
> +                } else if (cookie.flow_sample.direction == NX_ACTION_SAMPLE_EGRESS) {
> +                    ds_put_cstr(ds, ",egress");
> +                }
> +                ds_put_char(ds, ')');
>              } else if (userdata_len >= sizeof cookie.ipfix
>                         && cookie.type == USER_ACTION_COOKIE_IPFIX) {
>                  ds_put_format(ds, ",ipfix(output_port=%"PRIu32")", @@ -963,7 +969,7 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
>                              "collector_set_id=%"SCNi32","
>                              "obs_domain_id=%"SCNi32","
>                              "obs_point_id=%"SCNi32","
> -                            "output_port=%"SCNi32")%n",
> +                            "output_port=%"SCNi32"%n",
>                              &probability, &collector_set_id,
>                              &obs_domain_id, &obs_point_id,
>                              &output, &n1)) { @@ -977,6 +983,21 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
>              cookie.flow_sample.output_odp_port = u32_to_odp(output);
>              user_data = &cookie;
>              user_data_size = sizeof cookie.flow_sample;
> +
> +            if (ovs_scan(&s[n], ",ingress%n", &n1)) {
> +                cookie.flow_sample.direction = NX_ACTION_SAMPLE_INGRESS;
> +                n += n1;
> +            } else if (ovs_scan(&s[n], ",egress%n", &n1)) {
> +                cookie.flow_sample.direction = NX_ACTION_SAMPLE_EGRESS;
> +                n += n1;
> +            } else {
> +                cookie.flow_sample.direction = NX_ACTION_SAMPLE_DEFAULT;
> +            }
> +            if (s[n] != ')') {
> +                res = -EINVAL;
> +                goto out;
> +            }
> +            n++;
>          } else if (ovs_scan(&s[n], ",ipfix(output_port=%"SCNi32")%n",
>                              &output, &n1) ) {
>              n += n1;
> diff --git a/lib/odp-util.h b/lib/odp-util.h index 9c8b05f..42011bc 100644
> --- a/lib/odp-util.h
> +++ b/lib/odp-util.h
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
> + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -24,6 +24,7 @@
>  #include "flow.h"
>  #include "hash.h"
>  #include "openvswitch/hmap.h"
> +#include "openvswitch/ofp-actions.h"
>  #include "odp-netlink.h"
>  #include "openflow/openflow.h"
>  #include "util.h"
> @@ -287,6 +288,7 @@ union user_action_cookie {
>          uint32_t obs_domain_id; /* Observation Domain ID. */
>          uint32_t obs_point_id;  /* Observation Point ID. */
>          odp_port_t output_odp_port; /* The output odp port. */
> +        enum nx_action_sample_direction direction;
>      } flow_sample;
>  
>      struct {
> @@ -294,7 +296,7 @@ union user_action_cookie {
>          odp_port_t output_odp_port; /* The output odp port. */
>      } ipfix;
>  };
> -BUILD_ASSERT_DECL(sizeof(union user_action_cookie) == 20);
> +BUILD_ASSERT_DECL(sizeof(union user_action_cookie) == 24);
>  
>  size_t odp_put_userspace_action(uint32_t pid,
>                                  const void *userdata, size_t userdata_size, diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index 16f0f7c..7507558 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -291,6 +291,8 @@ enum ofp_raw_action_type {
>      NXAST_RAW_SAMPLE,
>      /* NX1.0+(38): struct nx_action_sample2. */
>      NXAST_RAW_SAMPLE2,
> +    /* NX1.0+(41): struct nx_action_sample2. */
> +    NXAST_RAW_SAMPLE3,
>  
>      /* NX1.0+(34): struct nx_action_conjunction. */
>      NXAST_RAW_CONJUNCTION,
> @@ -4824,10 +4826,13 @@ struct nx_action_sample {  };  OFP_ASSERT(sizeof(struct nx_action_sample) == 24);
>  
> -/* Action structure for NXAST_SAMPLE2.
> +/* Action structure for NXAST_SAMPLE2 and NXAST_SAMPLE3.
>   *
> - * This replacement for NXAST_SAMPLE makes it support exporting
> - * egress tunnel information. */
> + * NXAST_SAMPLE2 was added in Open vSwitch 2.5.90.  Compared to 
> + NXAST_SAMPLE,
> + * it adds support for exporting egress tunnel information.
> + *
> + * NXAST_SAMPLE3 was added in Open vSwitch 2.6.90.  Compared to 
> + NXAST_SAMPLE2,
> + * it adds support for the 'direction' field. */
>  struct nx_action_sample2 {
>      ovs_be16 type;                  /* OFPAT_VENDOR. */
>      ovs_be16 len;                   /* Length is 32. */
> @@ -4838,7 +4843,8 @@ struct nx_action_sample2 {
>      ovs_be32 obs_domain_id;         /* ID of sampling observation domain. */
>      ovs_be32 obs_point_id;          /* ID of sampling observation point. */
>      ovs_be16 sampling_port;         /* Sampling port. */
> -    uint8_t  pad[6];                /* Pad to a multiple of 8 bytes */
> +    uint8_t  direction;             /* NXAST_SAMPLE3 only. */
> +    uint8_t  zeros[5];              /* Pad to a multiple of 8 bytes */
>   };
>   OFP_ASSERT(sizeof(struct nx_action_sample2) == 32);
>  
> @@ -4855,8 +4861,8 @@ decode_NXAST_RAW_SAMPLE(const struct nx_action_sample *nas,
>      sample->collector_set_id = ntohl(nas->collector_set_id);
>      sample->obs_domain_id = ntohl(nas->obs_domain_id);
>      sample->obs_point_id = ntohl(nas->obs_point_id);
> -    /* Default value for sampling port is OFPP_NONE */
>      sample->sampling_port = OFPP_NONE;
> +    sample->direction = NX_ACTION_SAMPLE_DEFAULT;
>  
>      if (sample->probability == 0) {
>          return OFPERR_OFPBAC_BAD_ARGUMENT; @@ -4866,19 +4872,18 @@ decode_NXAST_RAW_SAMPLE(const struct nx_action_sample *nas,  }
>  
>  static enum ofperr
> -decode_NXAST_RAW_SAMPLE2(const struct nx_action_sample2 *nas,
> -                         enum ofp_version ofp_version OVS_UNUSED,
> -                         struct ofpbuf *out)
> +decode_SAMPLE2(const struct nx_action_sample2 *nas,
> +               enum ofp_raw_action_type raw,
> +               enum nx_action_sample_direction direction,
> +               struct ofpact_sample *sample)
>  {
> -    struct ofpact_sample *sample;
> -
> -    sample = ofpact_put_SAMPLE(out);
> -    sample->ofpact.raw = NXAST_RAW_SAMPLE2;
> +    sample->ofpact.raw = raw;
>      sample->probability = ntohs(nas->probability);
>      sample->collector_set_id = ntohl(nas->collector_set_id);
>      sample->obs_domain_id = ntohl(nas->obs_domain_id);
>      sample->obs_point_id = ntohl(nas->obs_point_id);
>      sample->sampling_port = u16_to_ofp(ntohs(nas->sampling_port));
> +    sample->direction = direction;
>  
>      if (sample->probability == 0) {
>          return OFPERR_OFPBAC_BAD_ARGUMENT; @@ -4887,18 +4892,55 @@ decode_NXAST_RAW_SAMPLE2(const struct nx_action_sample2 *nas,
>      return 0;
>  }
>  
> +static enum ofperr
> +decode_NXAST_RAW_SAMPLE2(const struct nx_action_sample2 *nas,
> +                         enum ofp_version ofp_version OVS_UNUSED,
> +                         struct ofpbuf *out) {
> +    return decode_SAMPLE2(nas, NXAST_RAW_SAMPLE2, NX_ACTION_SAMPLE_DEFAULT,
> +                          ofpact_put_SAMPLE(out)); }
> +
> +static enum ofperr
> +decode_NXAST_RAW_SAMPLE3(const struct nx_action_sample2 *nas,
> +                         enum ofp_version ofp_version OVS_UNUSED,
> +                         struct ofpbuf *out) {
> +    struct ofpact_sample *sample = ofpact_put_SAMPLE(out);
> +    if (!is_all_zeros(nas->zeros, sizeof nas->zeros)) {
> +        return OFPERR_NXBRC_MUST_BE_ZERO;
> +    }
> +    if (nas->direction != NX_ACTION_SAMPLE_DEFAULT &&
> +        nas->direction != NX_ACTION_SAMPLE_INGRESS &&
> +        nas->direction != NX_ACTION_SAMPLE_EGRESS) {
> +        VLOG_WARN_RL(&rl, "invalid sample direction %"PRIu8, nas->direction);
> +        return OFPERR_OFPBAC_BAD_ARGUMENT;
> +    }
> +    return decode_SAMPLE2(nas, NXAST_RAW_SAMPLE3, nas->direction, 
> +sample); }
> +
> +static void
> +encode_SAMPLE2(const struct ofpact_sample *sample,
> +               struct nx_action_sample2 *nas) {
> +    nas->probability = htons(sample->probability);
> +    nas->collector_set_id = htonl(sample->collector_set_id);
> +    nas->obs_domain_id = htonl(sample->obs_domain_id);
> +    nas->obs_point_id = htonl(sample->obs_point_id);
> +    nas->sampling_port = htons(ofp_to_u16(sample->sampling_port));
> +    nas->direction = sample->direction; }
> +
>  static void
>  encode_SAMPLE(const struct ofpact_sample *sample,
>                enum ofp_version ofp_version OVS_UNUSED, struct ofpbuf *out)  {
> -    if (sample->ofpact.raw == NXAST_RAW_SAMPLE2
> -        || sample->sampling_port != OFPP_NONE) {
> -        struct nx_action_sample2 *nas = put_NXAST_SAMPLE2(out);
> -        nas->probability = htons(sample->probability);
> -        nas->collector_set_id = htonl(sample->collector_set_id);
> -        nas->obs_domain_id = htonl(sample->obs_domain_id);
> -        nas->obs_point_id = htonl(sample->obs_point_id);
> -        nas->sampling_port = htons(ofp_to_u16(sample->sampling_port));
> +    if (sample->ofpact.raw == NXAST_RAW_SAMPLE3
> +        || sample->direction != NX_ACTION_SAMPLE_DEFAULT) {
> +        encode_SAMPLE2(sample, put_NXAST_SAMPLE3(out));
> +    } else if (sample->ofpact.raw == NXAST_RAW_SAMPLE2
> +               || sample->sampling_port != OFPP_NONE) {
> +        encode_SAMPLE2(sample, put_NXAST_SAMPLE2(out));
>      } else {
>          struct nx_action_sample *nas = put_NXAST_SAMPLE(out);
>          nas->probability = htons(sample->probability); @@ -4919,6 +4961,7 @@ parse_SAMPLE(char *arg, struct ofpbuf *ofpacts,  {
>      struct ofpact_sample *os = ofpact_put_SAMPLE(ofpacts);
>      os->sampling_port = OFPP_NONE;
> +    os->direction = NX_ACTION_SAMPLE_DEFAULT;
>  
>      char *key, *value;
>      while (ofputil_parse_key_value(&arg, &key, &value)) { @@ -4939,6 +4982,10 @@ parse_SAMPLE(char *arg, struct ofpbuf *ofpacts,
>              if (!ofputil_port_from_string(value, &os->sampling_port)) {
>                  error = xasprintf("%s: unknown port", value);
>              }
> +        } else if (!strcmp(key, "ingress")) {
> +            os->direction = NX_ACTION_SAMPLE_INGRESS;
> +        } else if (!strcmp(key, "egress")) {
> +            os->direction = NX_ACTION_SAMPLE_EGRESS;
>          } else {
>              error = xasprintf("invalid key \"%s\" in \"sample\" argument",
>                                key);
> @@ -4970,6 +5017,11 @@ format_SAMPLE(const struct ofpact_sample *a, struct ds *s)
>          ds_put_format(s, ",%ssampling_port=%s%"PRIu16,
>                        colors.param, colors.end, a->sampling_port);
>      }
> +    if (a->direction == NX_ACTION_SAMPLE_INGRESS) {
> +        ds_put_format(s, ",%singress%s", colors.param, colors.end);
> +    } else if (a->direction == NX_ACTION_SAMPLE_EGRESS) {
> +        ds_put_format(s, ",%segress%s", colors.param, colors.end);
> +    }
>      ds_put_format(s, "%s)%s", colors.paren, colors.end);  }
>  
> 
> diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c index 6b00b77..86a6646 100644
> --- a/ofproto/ofproto-dpif-ipfix.c
> +++ b/ofproto/ofproto-dpif-ipfix.c
> @@ -1562,6 +1562,7 @@ ipfix_cache_entry_init(struct ipfix_flow_cache_entry *entry,
>                         const struct dp_packet *packet, const struct flow *flow,
>                         uint64_t packet_delta_count, uint32_t obs_domain_id,
>                         uint32_t obs_point_id, odp_port_t output_odp_port,
> +                       enum nx_action_sample_direction direction,
>                         const struct dpif_ipfix_port *tunnel_port,
>                         const struct flow_tnl *tunnel_key)  { @@ -1648,7 +1649,9 @@ ipfix_cache_entry_init(struct ipfix_flow_cache_entry *entry,
>          data_common = dp_packet_put_zeros(&msg, sizeof *data_common);
>          data_common->observation_point_id = htonl(obs_point_id);
>          data_common->flow_direction =
> -            (output_odp_port == ODPP_NONE) ? INGRESS_FLOW : EGRESS_FLOW;
> +            (direction == NX_ACTION_SAMPLE_INGRESS ? INGRESS_FLOW
> +             : direction == NX_ACTION_SAMPLE_EGRESS ? EGRESS_FLOW
> +             : output_odp_port == ODPP_NONE ? INGRESS_FLOW : 
> + EGRESS_FLOW);
>          data_common->source_mac_address = flow->dl_src;
>          data_common->destination_mac_address = flow->dl_dst;
>          data_common->ethernet_type = flow->dl_type; @@ -1884,6 +1887,7 @@ dpif_ipfix_sample(struct dpif_ipfix_exporter *exporter,
>                    const struct dp_packet *packet, const struct flow *flow,
>                    uint64_t packet_delta_count, uint32_t obs_domain_id,
>                    uint32_t obs_point_id, odp_port_t output_odp_port,
> +                  enum nx_action_sample_direction direction,
>                    const struct dpif_ipfix_port *tunnel_port,
>                    const struct flow_tnl *tunnel_key)  { @@ -1895,8 +1899,8 @@ dpif_ipfix_sample(struct dpif_ipfix_exporter *exporter,
>      sampled_packet_type = ipfix_cache_entry_init(entry, packet,
>                                                   flow, packet_delta_count,
>                                                   obs_domain_id, obs_point_id,
> -                                                 output_odp_port, tunnel_port,
> -                                                 tunnel_key);
> +                                                 output_odp_port, direction,
> +                                                 tunnel_port, 
> + tunnel_key);
>      ipfix_cache_update(exporter, entry, sampled_packet_type);  }
>  
> @@ -1959,7 +1963,8 @@ dpif_ipfix_bridge_sample(struct dpif_ipfix *di, const struct dp_packet *packet,
>                        packet_delta_count,
>                        di->bridge_exporter.options->obs_domain_id,
>                        di->bridge_exporter.options->obs_point_id,
> -                      output_odp_port, tunnel_port, tunnel_key);
> +                      output_odp_port, NX_ACTION_SAMPLE_DEFAULT,
> +                      tunnel_port, tunnel_key);
>      ovs_mutex_unlock(&mutex);
>  }
>  
> @@ -2002,7 +2007,8 @@ dpif_ipfix_flow_sample(struct dpif_ipfix *di, const struct dp_packet *packet,
>                            packet_delta_count,
>                            cookie->flow_sample.obs_domain_id,
>                            cookie->flow_sample.obs_point_id,
> -                          output_odp_port, tunnel_port, tunnel_key);
> +                          output_odp_port, cookie->flow_sample.direction,
> +                          tunnel_port, tunnel_key);
>      }
>      ovs_mutex_unlock(&mutex);
>  }
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 8fee375..55ac371 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4282,6 +4282,7 @@ xlate_sample_action(struct xlate_ctx *ctx,
>              .obs_domain_id = os->obs_domain_id,
>              .obs_point_id = os->obs_point_id,
>              .output_odp_port = output_odp_port,
> +            .direction = os->direction,
>          }
>      };
>      compose_sample_action(ctx, probability, &cookie, sizeof cookie.flow_sample, diff --git a/tests/odp.at b/tests/odp.at index e630855..64aabe1 100644
> --- a/tests/odp.at
> +++ b/tests/odp.at
> @@ -256,7 +256,9 @@ userspace(pid=9765,slow_path(cfm),tunnel_out_port=10)
>  userspace(pid=1234567,userdata(0102030405060708090a0b0c0d0e0f),actions)
>  userspace(pid=1234567,userdata(0102030405060708090a0b0c0d0e0f),tunnel_out_port=10)
>  userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,obs_domain_id=2345,obs_point_id=3456,output_port=10))
> +userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,ob
> +s_domain_id=2345,obs_point_id=3456,output_port=10,ingress))
>  userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,obs_domain_id=2345,obs_point_id=3456,output_port=10),tunnel_out_port=10)
> +userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,ob
> +s_domain_id=2345,obs_point_id=3456,output_port=10,egress),tunnel_out_po
> +rt=10)
>  userspace(pid=6633,ipfix(output_port=10))
>  userspace(pid=6633,ipfix(output_port=10),tunnel_out_port=10)
>  set(in_port(2))
> diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at index 5e2474f..25ee06c 100644
> --- a/tests/ofp-actions.at
> +++ b/tests/ofp-actions.at
> @@ -130,6 +130,9 @@ ffff 0018 00002320 001d 3039 00005BA0 00008707 0000B26E  # actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789)
>  ffff 0020 00002320 0026 3039 00005BA0 00008707 0000B26E DDD50000 00000000
>  
> +# 
> +actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=3
> +4567,obs_point_id=45678,sampling_port=56789,egress)
> +ffff 0020 00002320 0029 3039 00005BA0 00008707 0000B26E DDD50200 
> +00000000
> +
>  # bad OpenFlow10 actions: OFPBAC_BAD_LEN  & ofp_actions|WARN|OpenFlow action OFPAT_OUTPUT length 240 exceeds action buffer length 8  & ofp_actions|WARN|bad action at offset 0 (OFPBAC_BAD_LEN):
> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index cbb818e..f2ee970 100644
> --- a/tests/ovs-ofctl.at
> +++ b/tests/ovs-ofctl.at
> @@ -189,7 +189,9 @@ sctp actions=drop
>  sctp actions=drop
>  in_port=0 actions=resubmit:0
>  actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678)
> +actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=3
> +4567,obs_point_id=45678,ingress)
>  actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789)
> +actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=3
> +4567,obs_point_id=45678,sampling_port=56789,egress)
>  ip,actions=ct(nat)
>  ip,actions=ct(commit,nat(dst))
>  ip,actions=ct(commit,nat(src))
> @@ -220,7 +222,9 @@ OFPT_FLOW_MOD: ADD sctp actions=drop
>  OFPT_FLOW_MOD: ADD sctp actions=drop
>  OFPT_FLOW_MOD: ADD in_port=0 actions=resubmit:0
>  OFPT_FLOW_MOD: ADD actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678)
> +OFPT_FLOW_MOD: ADD 
> +actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=3
> +4567,obs_point_id=45678,ingress)
>  OFPT_FLOW_MOD: ADD actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789)
> +OFPT_FLOW_MOD: ADD 
> +actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=3
> +4567,obs_point_id=45678,sampling_port=56789,egress)
>  OFPT_FLOW_MOD: ADD ip actions=ct(nat)
>  OFPT_FLOW_MOD: ADD ip actions=ct(commit,nat(dst))
>  OFPT_FLOW_MOD: ADD ip actions=ct(commit,nat(src)) diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index c112e6a..96135ea 100644
> --- a/utilities/ovs-ofctl.8.in
> +++ b/utilities/ovs-ofctl.8.in
> @@ -2380,11 +2380,19 @@ Observation Domain ID sent in every IPFIX flow record.  Defaults to 0.
>  When sending samples to IPFIX collectors, the unsigned 32-bit integer  Observation Point ID sent in every IPFIX flow record.  Defaults to 0.
>  .IP "\fBsampling_port=\fIport\fR"
> -Sample packets on the port.  It can be set as input port or output -port.  When this option is omitted, or specified as \fBNONE\fR, IPFIX -does not differentiate between ingress packets and egress packets and -does not export egress tunnel information.  This option was added in -Open vSwitch 2.5.90.
> +Sample packets on \fIport\fR, which should be the ingress or egress 
> +port.  This option, which was added in Open vSwitch 2.5.90, allows the 
> +IPFIX implementation to export egress tunnel information.
> +.IP "\fBingress\fR"
> +.IQ "\fBegress\fR"
> +Specifies explicitly that the packet is being sampled on ingress to or 
> +egress from the switch.  IPFIX reports sent by Open vSwitch before 
> +version 2.5.90 did not include a direction.  From 2.5.90 until 2.6.90, 
> +IPFIX reports inferred a direction from \fBsampling_port\fR: if it was 
> +the packet's output port, then the direction was reported as egress, 
> +otherwise as ingress.  Open vSwitch 2.6.90 introduced these options, 
> +which allow the inferred direction to be overridden.  This is 
> +particularly useful when the ingress (or egress) port is not a tunnel.
>  .RE
>  .IP
>  Refer to \fBovs\-vswitchd.conf.db\fR(5) for more details on
> --
> 2.1.3
>
diff mbox

Patch

diff --git a/NEWS b/NEWS
index a4806d3..ab70a45 100644
--- a/NEWS
+++ b/NEWS
@@ -20,6 +20,7 @@  Post-v2.6.0
        control over the OpenFlow select groups selection method.  See
        "selection_method" and related options in ovs-ofctl(8) for
        details.
+     * The "sample" action now supports "ingress" and "egress" options.
    - ovs-ofctl:
      * 'bundle' command now supports packet-out messages.
      * New syntax for 'ovs-ofctl packet-out' command, which uses the diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-actions.h
index 74e9dcc..af55f19 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -841,6 +841,20 @@  struct ofpact_note {
     uint8_t data[];
 };
 
+/* Direction of sampled packets. */
+enum nx_action_sample_direction {
+    /* OVS will attempt to infer the sample's direction based on whether
+     * 'sampling_port' is the packet's output port.  This is generally
+     * effective except when sampling happens as part of an output to a patch
+     * port, which doesn't involve a datapath output action. */
+    NX_ACTION_SAMPLE_DEFAULT,
+
+    /* Explicit direction.  This is useful for sampling packets coming in from
+     * or going out of a patch port, where the direction cannot be inferred. */
+    NX_ACTION_SAMPLE_INGRESS,
+    NX_ACTION_SAMPLE_EGRESS
+};
+
 /* OFPACT_SAMPLE.
  *
Daniel:
  -  Used for NXAST_SAMPLE and NXAST_SAMPLE2. */ @@ -851,6 +865,7 @@ struct ofpact_sample {
  + Used for NXAST_SAMPLE, NXAST_SAMPLE2 and  NXAST_SAMPLE3. */ @@ -851,6 +865,7 @@ struct ofpact_sample {
     uint32_t obs_domain_id;
     uint32_t obs_point_id;
     ofp_port_t sampling_port;
+    enum nx_action_sample_direction direction;
 };
 
 /* OFPACT_DEC_TTL.
diff --git a/lib/odp-util.c b/lib/odp-util.c index 626a82c..f09aabe 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -313,12 +313,18 @@  format_odp_userspace_action(struct ds *ds, const struct nlattr *attr)
                               ",collector_set_id=%"PRIu32
                               ",obs_domain_id=%"PRIu32
                               ",obs_point_id=%"PRIu32
-                              ",output_port=%"PRIu32")",
+                              ",output_port=%"PRIu32,
                               cookie.flow_sample.probability,
                               cookie.flow_sample.collector_set_id,
                               cookie.flow_sample.obs_domain_id,
                               cookie.flow_sample.obs_point_id,
                               cookie.flow_sample.output_odp_port);
+                if (cookie.flow_sample.direction == NX_ACTION_SAMPLE_INGRESS) {
+                    ds_put_cstr(ds, ",ingress");
+                } else if (cookie.flow_sample.direction == NX_ACTION_SAMPLE_EGRESS) {
+                    ds_put_cstr(ds, ",egress");
+                }
+                ds_put_char(ds, ')');
             } else if (userdata_len >= sizeof cookie.ipfix
                        && cookie.type == USER_ACTION_COOKIE_IPFIX) {
                 ds_put_format(ds, ",ipfix(output_port=%"PRIu32")", @@ -963,7 +969,7 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
                             "collector_set_id=%"SCNi32","
                             "obs_domain_id=%"SCNi32","
                             "obs_point_id=%"SCNi32","
-                            "output_port=%"SCNi32")%n",
+                            "output_port=%"SCNi32"%n",
                             &probability, &collector_set_id,
                             &obs_domain_id, &obs_point_id,
                             &output, &n1)) { @@ -977,6 +983,21 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
             cookie.flow_sample.output_odp_port = u32_to_odp(output);
             user_data = &cookie;
             user_data_size = sizeof cookie.flow_sample;
+
+            if (ovs_scan(&s[n], ",ingress%n", &n1)) {
+                cookie.flow_sample.direction = NX_ACTION_SAMPLE_INGRESS;
+                n += n1;
+            } else if (ovs_scan(&s[n], ",egress%n", &n1)) {
+                cookie.flow_sample.direction = NX_ACTION_SAMPLE_EGRESS;
+                n += n1;
+            } else {
+                cookie.flow_sample.direction = NX_ACTION_SAMPLE_DEFAULT;
+            }
+            if (s[n] != ')') {
+                res = -EINVAL;
+                goto out;
+            }
+            n++;
         } else if (ovs_scan(&s[n], ",ipfix(output_port=%"SCNi32")%n",
                             &output, &n1) ) {
             n += n1;
diff --git a/lib/odp-util.h b/lib/odp-util.h index 9c8b05f..42011bc 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@ 
 #include "flow.h"
 #include "hash.h"
 #include "openvswitch/hmap.h"
+#include "openvswitch/ofp-actions.h"
 #include "odp-netlink.h"
 #include "openflow/openflow.h"
 #include "util.h"
@@ -287,6 +288,7 @@  union user_action_cookie {
         uint32_t obs_domain_id; /* Observation Domain ID. */
         uint32_t obs_point_id;  /* Observation Point ID. */
         odp_port_t output_odp_port; /* The output odp port. */
+        enum nx_action_sample_direction direction;
     } flow_sample;
 
     struct {
@@ -294,7 +296,7 @@  union user_action_cookie {
         odp_port_t output_odp_port; /* The output odp port. */
     } ipfix;
 };
-BUILD_ASSERT_DECL(sizeof(union user_action_cookie) == 20);
+BUILD_ASSERT_DECL(sizeof(union user_action_cookie) == 24);
 
 size_t odp_put_userspace_action(uint32_t pid,
                                 const void *userdata, size_t userdata_size, diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index 16f0f7c..7507558 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -291,6 +291,8 @@  enum ofp_raw_action_type {
     NXAST_RAW_SAMPLE,
     /* NX1.0+(38): struct nx_action_sample2. */
     NXAST_RAW_SAMPLE2,
+    /* NX1.0+(41): struct nx_action_sample2. */
+    NXAST_RAW_SAMPLE3,
 
     /* NX1.0+(34): struct nx_action_conjunction. */
     NXAST_RAW_CONJUNCTION,
@@ -4824,10 +4826,13 @@  struct nx_action_sample {  };  OFP_ASSERT(sizeof(struct nx_action_sample) == 24);
 
-/* Action structure for NXAST_SAMPLE2.
+/* Action structure for NXAST_SAMPLE2 and NXAST_SAMPLE3.
  *
- * This replacement for NXAST_SAMPLE makes it support exporting
- * egress tunnel information. */
+ * NXAST_SAMPLE2 was added in Open vSwitch 2.5.90.  Compared to 
+ NXAST_SAMPLE,
+ * it adds support for exporting egress tunnel information.
+ *
+ * NXAST_SAMPLE3 was added in Open vSwitch 2.6.90.  Compared to 
+ NXAST_SAMPLE2,
+ * it adds support for the 'direction' field. */
 struct nx_action_sample2 {
     ovs_be16 type;                  /* OFPAT_VENDOR. */
     ovs_be16 len;                   /* Length is 32. */
@@ -4838,7 +4843,8 @@  struct nx_action_sample2 {
     ovs_be32 obs_domain_id;         /* ID of sampling observation domain. */
     ovs_be32 obs_point_id;          /* ID of sampling observation point. */
     ovs_be16 sampling_port;         /* Sampling port. */
-    uint8_t  pad[6];                /* Pad to a multiple of 8 bytes */
+    uint8_t  direction;             /* NXAST_SAMPLE3 only. */
+    uint8_t  zeros[5];              /* Pad to a multiple of 8 bytes */
  };
  OFP_ASSERT(sizeof(struct nx_action_sample2) == 32);
 
@@ -4855,8 +4861,8 @@  decode_NXAST_RAW_SAMPLE(const struct nx_action_sample *nas,
     sample->collector_set_id = ntohl(nas->collector_set_id);
     sample->obs_domain_id = ntohl(nas->obs_domain_id);
     sample->obs_point_id = ntohl(nas->obs_point_id);
-    /* Default value for sampling port is OFPP_NONE */
     sample->sampling_port = OFPP_NONE;
+    sample->direction = NX_ACTION_SAMPLE_DEFAULT;
 
     if (sample->probability == 0) {
         return OFPERR_OFPBAC_BAD_ARGUMENT; @@ -4866,19 +4872,18 @@ decode_NXAST_RAW_SAMPLE(const struct nx_action_sample *nas,  }
 
 static enum ofperr
-decode_NXAST_RAW_SAMPLE2(const struct nx_action_sample2 *nas,
-                         enum ofp_version ofp_version OVS_UNUSED,
-                         struct ofpbuf *out)
+decode_SAMPLE2(const struct nx_action_sample2 *nas,
+               enum ofp_raw_action_type raw,
+               enum nx_action_sample_direction direction,
+               struct ofpact_sample *sample)
 {
-    struct ofpact_sample *sample;
-
-    sample = ofpact_put_SAMPLE(out);
-    sample->ofpact.raw = NXAST_RAW_SAMPLE2;
+    sample->ofpact.raw = raw;
     sample->probability = ntohs(nas->probability);
     sample->collector_set_id = ntohl(nas->collector_set_id);
     sample->obs_domain_id = ntohl(nas->obs_domain_id);
     sample->obs_point_id = ntohl(nas->obs_point_id);
     sample->sampling_port = u16_to_ofp(ntohs(nas->sampling_port));
+    sample->direction = direction;
 
     if (sample->probability == 0) {
         return OFPERR_OFPBAC_BAD_ARGUMENT; @@ -4887,18 +4892,55 @@ decode_NXAST_RAW_SAMPLE2(const struct nx_action_sample2 *nas,
     return 0;
 }
 
+static enum ofperr
+decode_NXAST_RAW_SAMPLE2(const struct nx_action_sample2 *nas,
+                         enum ofp_version ofp_version OVS_UNUSED,
+                         struct ofpbuf *out) {
+    return decode_SAMPLE2(nas, NXAST_RAW_SAMPLE2, NX_ACTION_SAMPLE_DEFAULT,
+                          ofpact_put_SAMPLE(out)); }
+
+static enum ofperr
+decode_NXAST_RAW_SAMPLE3(const struct nx_action_sample2 *nas,
+                         enum ofp_version ofp_version OVS_UNUSED,
+                         struct ofpbuf *out) {
+    struct ofpact_sample *sample = ofpact_put_SAMPLE(out);
+    if (!is_all_zeros(nas->zeros, sizeof nas->zeros)) {
+        return OFPERR_NXBRC_MUST_BE_ZERO;
+    }
+    if (nas->direction != NX_ACTION_SAMPLE_DEFAULT &&
+        nas->direction != NX_ACTION_SAMPLE_INGRESS &&
+        nas->direction != NX_ACTION_SAMPLE_EGRESS) {
+        VLOG_WARN_RL(&rl, "invalid sample direction %"PRIu8, nas->direction);
+        return OFPERR_OFPBAC_BAD_ARGUMENT;
+    }
+    return decode_SAMPLE2(nas, NXAST_RAW_SAMPLE3, nas->direction, 
+sample); }
+
+static void
+encode_SAMPLE2(const struct ofpact_sample *sample,
+               struct nx_action_sample2 *nas) {
+    nas->probability = htons(sample->probability);
+    nas->collector_set_id = htonl(sample->collector_set_id);
+    nas->obs_domain_id = htonl(sample->obs_domain_id);
+    nas->obs_point_id = htonl(sample->obs_point_id);
+    nas->sampling_port = htons(ofp_to_u16(sample->sampling_port));
+    nas->direction = sample->direction; }
+
 static void
 encode_SAMPLE(const struct ofpact_sample *sample,
               enum ofp_version ofp_version OVS_UNUSED, struct ofpbuf *out)  {
-    if (sample->ofpact.raw == NXAST_RAW_SAMPLE2
-        || sample->sampling_port != OFPP_NONE) {
-        struct nx_action_sample2 *nas = put_NXAST_SAMPLE2(out);
-        nas->probability = htons(sample->probability);
-        nas->collector_set_id = htonl(sample->collector_set_id);
-        nas->obs_domain_id = htonl(sample->obs_domain_id);
-        nas->obs_point_id = htonl(sample->obs_point_id);
-        nas->sampling_port = htons(ofp_to_u16(sample->sampling_port));
+    if (sample->ofpact.raw == NXAST_RAW_SAMPLE3
+        || sample->direction != NX_ACTION_SAMPLE_DEFAULT) {
+        encode_SAMPLE2(sample, put_NXAST_SAMPLE3(out));
+    } else if (sample->ofpact.raw == NXAST_RAW_SAMPLE2
+               || sample->sampling_port != OFPP_NONE) {
+        encode_SAMPLE2(sample, put_NXAST_SAMPLE2(out));
     } else {
         struct nx_action_sample *nas = put_NXAST_SAMPLE(out);
         nas->probability = htons(sample->probability); @@ -4919,6 +4961,7 @@ parse_SAMPLE(char *arg, struct ofpbuf *ofpacts,  {
     struct ofpact_sample *os = ofpact_put_SAMPLE(ofpacts);
     os->sampling_port = OFPP_NONE;
+    os->direction = NX_ACTION_SAMPLE_DEFAULT;
 
     char *key, *value;
     while (ofputil_parse_key_value(&arg, &key, &value)) { @@ -4939,6 +4982,10 @@ parse_SAMPLE(char *arg, struct ofpbuf *ofpacts,
             if (!ofputil_port_from_string(value, &os->sampling_port)) {
                 error = xasprintf("%s: unknown port", value);
             }
+        } else if (!strcmp(key, "ingress")) {
+            os->direction = NX_ACTION_SAMPLE_INGRESS;
+        } else if (!strcmp(key, "egress")) {
+            os->direction = NX_ACTION_SAMPLE_EGRESS;
         } else {
             error = xasprintf("invalid key \"%s\" in \"sample\" argument",
                               key);
@@ -4970,6 +5017,11 @@  format_SAMPLE(const struct ofpact_sample *a, struct ds *s)
         ds_put_format(s, ",%ssampling_port=%s%"PRIu16,
                       colors.param, colors.end, a->sampling_port);
     }
+    if (a->direction == NX_ACTION_SAMPLE_INGRESS) {
+        ds_put_format(s, ",%singress%s", colors.param, colors.end);
+    } else if (a->direction == NX_ACTION_SAMPLE_EGRESS) {
+        ds_put_format(s, ",%segress%s", colors.param, colors.end);
+    }
     ds_put_format(s, "%s)%s", colors.paren, colors.end);  }
 

diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c index 6b00b77..86a6646 100644
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -1562,6 +1562,7 @@  ipfix_cache_entry_init(struct ipfix_flow_cache_entry *entry,
                        const struct dp_packet *packet, const struct flow *flow,
                        uint64_t packet_delta_count, uint32_t obs_domain_id,
                        uint32_t obs_point_id, odp_port_t output_odp_port,
+                       enum nx_action_sample_direction direction,
                        const struct dpif_ipfix_port *tunnel_port,
                        const struct flow_tnl *tunnel_key)  { @@ -1648,7 +1649,9 @@ ipfix_cache_entry_init(struct ipfix_flow_cache_entry *entry,
         data_common = dp_packet_put_zeros(&msg, sizeof *data_common);
         data_common->observation_point_id = htonl(obs_point_id);
         data_common->flow_direction =
-            (output_odp_port == ODPP_NONE) ? INGRESS_FLOW : EGRESS_FLOW;
+            (direction == NX_ACTION_SAMPLE_INGRESS ? INGRESS_FLOW
+             : direction == NX_ACTION_SAMPLE_EGRESS ? EGRESS_FLOW
+             : output_odp_port == ODPP_NONE ? INGRESS_FLOW : 
+ EGRESS_FLOW);
         data_common->source_mac_address = flow->dl_src;
         data_common->destination_mac_address = flow->dl_dst;
         data_common->ethernet_type = flow->dl_type; @@ -1884,6 +1887,7 @@ dpif_ipfix_sample(struct dpif_ipfix_exporter *exporter,
                   const struct dp_packet *packet, const struct flow *flow,
                   uint64_t packet_delta_count, uint32_t obs_domain_id,
                   uint32_t obs_point_id, odp_port_t output_odp_port,
+                  enum nx_action_sample_direction direction,
                   const struct dpif_ipfix_port *tunnel_port,
                   const struct flow_tnl *tunnel_key)  { @@ -1895,8 +1899,8 @@ dpif_ipfix_sample(struct dpif_ipfix_exporter *exporter,
     sampled_packet_type = ipfix_cache_entry_init(entry, packet,
                                                  flow, packet_delta_count,
                                                  obs_domain_id, obs_point_id,
-                                                 output_odp_port, tunnel_port,
-                                                 tunnel_key);
+                                                 output_odp_port, direction,
+                                                 tunnel_port, 
+ tunnel_key);
     ipfix_cache_update(exporter, entry, sampled_packet_type);  }
 
@@ -1959,7 +1963,8 @@  dpif_ipfix_bridge_sample(struct dpif_ipfix *di, const struct dp_packet *packet,
                       packet_delta_count,
                       di->bridge_exporter.options->obs_domain_id,
                       di->bridge_exporter.options->obs_point_id,
-                      output_odp_port, tunnel_port, tunnel_key);
+                      output_odp_port, NX_ACTION_SAMPLE_DEFAULT,
+                      tunnel_port, tunnel_key);
     ovs_mutex_unlock(&mutex);
 }
 
@@ -2002,7 +2007,8 @@  dpif_ipfix_flow_sample(struct dpif_ipfix *di, const struct dp_packet *packet,
                           packet_delta_count,
                           cookie->flow_sample.obs_domain_id,
                           cookie->flow_sample.obs_point_id,
-                          output_odp_port, tunnel_port, tunnel_key);
+                          output_odp_port, cookie->flow_sample.direction,
+                          tunnel_port, tunnel_key);
     }
     ovs_mutex_unlock(&mutex);
 }
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 8fee375..55ac371 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4282,6 +4282,7 @@  xlate_sample_action(struct xlate_ctx *ctx,
             .obs_domain_id = os->obs_domain_id,
             .obs_point_id = os->obs_point_id,
             .output_odp_port = output_odp_port,
+            .direction = os->direction,
         }
     };
     compose_sample_action(ctx, probability, &cookie, sizeof cookie.flow_sample, diff --git a/tests/odp.at b/tests/odp.at index e630855..64aabe1 100644
--- a/tests/odp.at
+++ b/tests/odp.at
@@ -256,7 +256,9 @@  userspace(pid=9765,slow_path(cfm),tunnel_out_port=10)
 userspace(pid=1234567,userdata(0102030405060708090a0b0c0d0e0f),actions)
 userspace(pid=1234567,userdata(0102030405060708090a0b0c0d0e0f),tunnel_out_port=10)
 userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,obs_domain_id=2345,obs_point_id=3456,output_port=10))
+userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,ob
+s_domain_id=2345,obs_point_id=3456,output_port=10,ingress))
 userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,obs_domain_id=2345,obs_point_id=3456,output_port=10),tunnel_out_port=10)
+userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,ob
+s_domain_id=2345,obs_point_id=3456,output_port=10,egress),tunnel_out_po
+rt=10)
 userspace(pid=6633,ipfix(output_port=10))
 userspace(pid=6633,ipfix(output_port=10),tunnel_out_port=10)
 set(in_port(2))
diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at index 5e2474f..25ee06c 100644
--- a/tests/ofp-actions.at
+++ b/tests/ofp-actions.at
@@ -130,6 +130,9 @@  ffff 0018 00002320 001d 3039 00005BA0 00008707 0000B26E  # actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789)
 ffff 0020 00002320 0026 3039 00005BA0 00008707 0000B26E DDD50000 00000000
 
+# 
+actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=3
+4567,obs_point_id=45678,sampling_port=56789,egress)
+ffff 0020 00002320 0029 3039 00005BA0 00008707 0000B26E DDD50200 
+00000000
+
 # bad OpenFlow10 actions: OFPBAC_BAD_LEN  & ofp_actions|WARN|OpenFlow action OFPAT_OUTPUT length 240 exceeds action buffer length 8  & ofp_actions|WARN|bad action at offset 0 (OFPBAC_BAD_LEN):
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index cbb818e..f2ee970 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -189,7 +189,9 @@  sctp actions=drop
 sctp actions=drop
 in_port=0 actions=resubmit:0
 actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678)
+actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=3
+4567,obs_point_id=45678,ingress)
 actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789)
+actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=3
+4567,obs_point_id=45678,sampling_port=56789,egress)
 ip,actions=ct(nat)
 ip,actions=ct(commit,nat(dst))
 ip,actions=ct(commit,nat(src))
@@ -220,7 +222,9 @@  OFPT_FLOW_MOD: ADD sctp actions=drop
 OFPT_FLOW_MOD: ADD sctp actions=drop
 OFPT_FLOW_MOD: ADD in_port=0 actions=resubmit:0
 OFPT_FLOW_MOD: ADD actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678)
+OFPT_FLOW_MOD: ADD 
+actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=3
+4567,obs_point_id=45678,ingress)
 OFPT_FLOW_MOD: ADD actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789)
+OFPT_FLOW_MOD: ADD 
+actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=3
+4567,obs_point_id=45678,sampling_port=56789,egress)
 OFPT_FLOW_MOD: ADD ip actions=ct(nat)
 OFPT_FLOW_MOD: ADD ip actions=ct(commit,nat(dst))
 OFPT_FLOW_MOD: ADD ip actions=ct(commit,nat(src)) diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index c112e6a..96135ea 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -2380,11 +2380,19 @@  Observation Domain ID sent in every IPFIX flow record.  Defaults to 0.
 When sending samples to IPFIX collectors, the unsigned 32-bit integer  Observation Point ID sent in every IPFIX flow record.  Defaults to 0.
 .IP "\fBsampling_port=\fIport\fR"
-Sample packets on the port.  It can be set as input port or output -port.  When this option is omitted, or specified as \fBNONE\fR, IPFIX -does not differentiate between ingress packets and egress packets and -does not export egress tunnel information.  This option was added in -Open vSwitch 2.5.90.
+Sample packets on \fIport\fR, which should be the ingress or egress 
+port.  This option, which was added in Open vSwitch 2.5.90, allows the 
+IPFIX implementation to export egress tunnel information.
+.IP "\fBingress\fR"
+.IQ "\fBegress\fR"
+Specifies explicitly that the packet is being sampled on ingress to or 
+egress from the switch.  IPFIX reports sent by Open vSwitch before 
+version 2.5.90 did not include a direction.  From 2.5.90 until 2.6.90, 
+IPFIX reports inferred a direction from \fBsampling_port\fR: if it was 
+the packet's output port, then the direction was reported as egress, 
+otherwise as ingress.  Open vSwitch 2.6.90 introduced these options, 
+which allow the inferred direction to be overridden.  This is 
+particularly useful when the ingress (or egress) port is not a tunnel.
 .RE
 .IP
 Refer to \fBovs\-vswitchd.conf.db\fR(5) for more details on
--
2.1.3

_______________________________________________
dev mailing list