On Fri, Jul 12, 2024 at 05:14:30PM GMT, Adrián Moreno wrote:
> On Fri, Jul 12, 2024 at 04:35:48PM GMT, Eelco Chaudron wrote:
> > On 12 Jul 2024, at 1:32, Adrian Moreno wrote:
> >
> > > When sample action gets used as a way of sampling traffic with
> > > controller-generated metadata (i.e: obs_domain_id and obs_point_id),
> > > the controller will have to increase the number of flows to ensure each
> > > part of the pipeline contains the right metadata.
> > >
> > > As an example, if the controller decides to sample stateful traffic, it
> > > could store the computed metadata for each connection in the conntrack
> > > label. However, for established connections, a flow must be created for
> > > each different ct_label value with a sample action that contains a
> > > different hardcoded obs_domain and obs_point id.
> > >
> > > This patch adds a new version of the NXAST_RAW_SAMPLE* action (number 4)
> > > that supports specifying the observation point and domain using an
> > > OpenFlow field reference, so now the controller can express:
> > >
> > >  sample(...
> > >         obs_domain_id=NXM_NX_CT_LABEL[0..31],
> > >         obs_point_id=NXM_NX_CT_LABEL[32..63]
> > >         ...
> > >        )
> > >
> >
> > Some comments below on the test case, the rest looks good.
> >
> > Cheers,
> >
> > Eelco
> >
> > > Signed-off-by: Adrian Moreno <amore...@redhat.com>
> > > ---
> > >  Documentation/ref/ovs-actions.7.rst |  12 +-
> > >  NEWS                                |   2 +
> > >  include/openvswitch/ofp-actions.h   |   8 +-
> > >  lib/ofp-actions.c                   | 245 +++++++++++++++++++++++++---
> > >  ofproto/ofproto-dpif-xlate.c        |  44 ++++-
> > >  python/ovs/flow/ofp.py              |   8 +-
> > >  python/ovs/flow/ofp_act.py          |   6 +-
> > >  tests/ofp-actions.at                |   8 +
> > >  tests/ofproto-dpif.at               |  44 +++++
> > >  tests/ovs-ofctl.at                  |  14 ++
> > >  tests/system-traffic.at             |  79 +++++++++
> > >  11 files changed, 429 insertions(+), 41 deletions(-)
> > >
> > > diff --git a/Documentation/ref/ovs-actions.7.rst 
> > > b/Documentation/ref/ovs-actions.7.rst
> > > index 80acd9070..01021dc0a 100644
> > > --- a/Documentation/ref/ovs-actions.7.rst
> > > +++ b/Documentation/ref/ovs-actions.7.rst
> > > @@ -2201,13 +2201,17 @@ The following *argument* forms are accepted:
> > >      The unsigned 32-bit integer identifier of the set of sample 
> > > collectors to
> > >      send sampled packets to.  Defaults to 0.
> > >
> > > -  ``obs_domain_id=``\ *id*
> > > +  ``obs_domain_id=``\ *value*
> > >      When sending samples to IPFIX collectors, the unsigned 32-bit integer
> > > -    Observation Domain ID sent in every IPFIX flow record.  Defaults to 
> > > 0.
> > > +    Observation Domain ID sent in every IPFIX flow record. The *value* 
> > > may
> > > +    be specified as a 32-bit integer or a field or subfield in the syntax
> > > +    described under `Field Specifications`_ above. Defaults to 0.
> > >
> > > -  ``obs_point_id=``\ *id*
> > > +  ``obs_point_id=``\ *value*
> > >      When sending samples to IPFIX collectors, the unsigned 32-bit integer
> > > -    Observation Point ID sent in every IPFIX flow record.  Defaults to 0.
> > > +    Observation Point ID sent in every IPFIX flow record. The *value* may
> > > +    be specified as a 32-bit integer or a field or subfield in the syntax
> > > +    described under `Field Specifications`_ above. Defaults to 0.
> > >
> > >    ``sampling_port=``\ *port*
> > >      Sample packets on *port*, which should be the ingress or egress 
> > > port.  This
> > > diff --git a/NEWS b/NEWS
> > > index 096ff4d7d..7d7461c57 100644
> > > --- a/NEWS
> > > +++ b/NEWS
> > > @@ -22,6 +22,8 @@ Post-v3.3.0
> > >       support this feature by using the new datapath "psample" action.
> > >     - A new unixctl command 'lsample/show' shows packet and bytes 
> > > statistics
> > >       per local sample exporter.
> > > +   - OpenFlow: a new version of the "sample" action is introduced that 
> > > allows
> > > +     the use of subfields in obs_point_id and obs_domain_id.
> > >
> > >
> > >  v3.3.0 - 16 Feb 2024
> > > diff --git a/include/openvswitch/ofp-actions.h 
> > > b/include/openvswitch/ofp-actions.h
> > > index 7b57e49ad..56dc2c147 100644
> > > --- a/include/openvswitch/ofp-actions.h
> > > +++ b/include/openvswitch/ofp-actions.h
> > > @@ -1015,14 +1015,16 @@ enum nx_action_sample_direction {
> > >
> > >  /* OFPACT_SAMPLE.
> > >   *
> > > - * Used for NXAST_SAMPLE, NXAST_SAMPLE2, and NXAST_SAMPLE3. */
> > > + * Used for NXAST_SAMPLE, NXAST_SAMPLE2, NXAST_SAMPLE3 and 
> > > NXAST_SAMPLE4. */
> > >  struct ofpact_sample {
> > >      OFPACT_PADDED_MEMBERS(
> > >          struct ofpact ofpact;
> > >          uint16_t probability;   /* Always positive. */
> > >          uint32_t collector_set_id;
> > > -        uint32_t obs_domain_id;
> > > -        uint32_t obs_point_id;
> > > +        uint32_t obs_domain_imm;
> > > +        struct mf_subfield obs_domain_src;
> > > +        uint32_t obs_point_imm;
> > > +        struct mf_subfield obs_point_src;
> > >          ofp_port_t sampling_port;
> > >          enum nx_action_sample_direction direction;
> > >      );
> > > diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> > > index da7b1dd31..6b030bf29 100644
> > > --- a/lib/ofp-actions.c
> > > +++ b/lib/ofp-actions.c
> > > @@ -330,6 +330,8 @@ enum ofp_raw_action_type {
> > >      NXAST_RAW_SAMPLE2,
> > >      /* NX1.0+(41): struct nx_action_sample2. */
> > >      NXAST_RAW_SAMPLE3,
> > > +    /* NX1.0+(51): struct nx_action_sample4. VLMFF */
> > > +    NXAST_RAW_SAMPLE4,
> > >
> > >      /* NX1.0+(34): struct nx_action_conjunction. */
> > >      NXAST_RAW_CONJUNCTION,
> > > @@ -6188,6 +6190,34 @@ struct nx_action_sample2 {
> > >   };
> > >   OFP_ASSERT(sizeof(struct nx_action_sample2) == 32);
> > >
> > > +/* Action structure for NXAST_SAMPLE4
> > > + *
> > > + * NXAST_SAMPLE4 was added in Open vSwitch 3.4.0.  Compared to 
> > > NXAST_SAMPLE3,
> > > + * it adds support for using field specifiers for observation_domain_id 
> > > and
> > > + * observation_point_id. */
> > > +struct nx_action_sample4 {
> > > +    ovs_be16 type;                      /* OFPAT_VENDOR. */
> > > +    ovs_be16 len;                       /* Length is 40. */
> > > +    ovs_be32 vendor;                    /* NX_VENDOR_ID. */
> > > +    ovs_be16 subtype;                   /* NXAST_SAMPLE4. */
> > > +    ovs_be16 probability;               /* Fraction of packets to 
> > > sample. */
> > > +    ovs_be32 collector_set_id;          /* ID of collector set in OVSDB. 
> > > */
> > > +    ovs_be32 obs_domain_src;            /* The observation_domain_id 
> > > source. */
> > > +    union {
> > > +        ovs_be16 obs_domain_ofs_nbits;  /* Range to use from source 
> > > field. */
> > > +        ovs_be32 obs_domain_imm;        /* Immediate value for domain 
> > > id. */
> > > +    };
> > > +    ovs_be32 obs_point_src;             /* The observation_point_id 
> > > source. */
> > > +    union {
> > > +        ovs_be16 obs_point_ofs_nbits;   /* Range to use from source 
> > > field. */
> > > +        ovs_be32 obs_point_imm;         /* Immediate value for point id. 
> > > */
> > > +    };
> > > +    ovs_be16 sampling_port;             /* Sampling port. */
> > > +    uint8_t direction;                  /* Sampling direction. */
> > > +    uint8_t zeros[5];                   /* Pad to a multiple of 8 bytes 
> > > */
> > > + };
> > > + OFP_ASSERT(sizeof(struct nx_action_sample4) == 40);
> > > +
> > >  static enum ofperr
> > >  decode_NXAST_RAW_SAMPLE(const struct nx_action_sample *nas,
> > >                          enum ofp_version ofp_version OVS_UNUSED,
> > > @@ -6199,11 +6229,14 @@ decode_NXAST_RAW_SAMPLE(const struct 
> > > nx_action_sample *nas,
> > >      sample->ofpact.raw = NXAST_RAW_SAMPLE;
> > >      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->obs_domain_imm = ntohl(nas->obs_domain_id);
> > > +    sample->obs_domain_src.field = NULL;
> > > +    sample->obs_point_imm = ntohl(nas->obs_point_id);
> > > +    sample->obs_point_src.field = NULL;
> > >      sample->sampling_port = OFPP_NONE;
> > >      sample->direction = NX_ACTION_SAMPLE_DEFAULT;
> > > -
> > > +    sample->obs_domain_src.field = NULL;
> > > +    sample->obs_point_src.field = NULL;
> > >      if (sample->probability == 0) {
> > >          return OFPERR_OFPBAC_BAD_ARGUMENT;
> > >      }
> > > @@ -6220,8 +6253,10 @@ decode_SAMPLE2(const struct nx_action_sample2 *nas,
> > >      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->obs_domain_imm = ntohl(nas->obs_domain_id);
> > > +    sample->obs_domain_src.field = NULL;
> > > +    sample->obs_point_imm = ntohl(nas->obs_point_id);
> > > +    sample->obs_point_src.field = NULL;
> > >      sample->sampling_port = u16_to_ofp(ntohs(nas->sampling_port));
> > >      sample->direction = direction;
> > >
> > > @@ -6241,41 +6276,170 @@ decode_NXAST_RAW_SAMPLE2(const struct 
> > > nx_action_sample2 *nas,
> > >                            ofpact_put_SAMPLE(out));
> > >  }
> > >
> > > +static int
> > > +check_sample_direction(enum nx_action_sample_direction direction)
> > > +{
> > > +    if (direction != NX_ACTION_SAMPLE_DEFAULT &&
> > > +        direction != NX_ACTION_SAMPLE_INGRESS &&
> > > +        direction != NX_ACTION_SAMPLE_EGRESS) {
> > > +        VLOG_WARN_RL(&rl, "invalid sample direction %"PRIu8, direction);
> > > +        return OFPERR_OFPBAC_BAD_ARGUMENT;
> > > +    }
> > > +    return 0;
> > > +}
> > > +
> > >  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);
> > > +    int err;
> > > +
> > >      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;
> > > +    err = check_sample_direction(nas->direction);
> > > +    if (err) {
> > > +        return err;
> > >      }
> > >      return decode_SAMPLE2(nas, NXAST_RAW_SAMPLE3, nas->direction, 
> > > sample);
> > >  }
> > >
> > > +static int
> > > +decode_sample_obs_id(ovs_be32 src, ovs_be16 ofs_nbits, ovs_be32 imm,
> > > +                     const struct vl_mff_map *vl_mff_map, uint64_t 
> > > *tlv_bitmap,
> > > +                     struct mf_subfield *src_out, uint32_t *imm_out)
> > > +{
> > > +    if (src) {
> > > +        enum ofperr error;
> > > +
> > > +        src_out->ofs = nxm_decode_ofs(ofs_nbits);
> > > +        src_out->n_bits = nxm_decode_n_bits(ofs_nbits);
> > > +        error = mf_vl_mff_mf_from_nxm_header(ntohl(src),
> > > +                                             vl_mff_map, &src_out->field,
> > > +                                             tlv_bitmap);
> > > +        if (error) {
> > > +            return error;
> > > +        }
> > > +
> > > +        error = mf_check_src(src_out, NULL);
> > > +        if (error) {
> > > +            return error;
> > > +        }
> > > +
> > > +        if (src_out->n_bits > 32) {
> > > +            VLOG_WARN_RL(&rl, "size of field used in observation_id (%d) 
> > > "
> > > +                               "exceeds maximum (32)", src_out->n_bits);
> > > +            return OFPERR_OFPBAC_BAD_ARGUMENT;
> > > +        }
> > > +    } else {
> > > +        src_out->field = NULL;
> > > +        *imm_out = ntohl(imm);
> > > +    }
> > > +
> > > +    return 0;
> > > +}
> > > +
> > > +static enum ofperr
> > > +decode_NXAST_RAW_SAMPLE4(const struct nx_action_sample4 *nas,
> > > +                         enum ofp_version ofp_version OVS_UNUSED,
> > > +                         const struct vl_mff_map *vl_mff_map,
> > > +                         uint64_t *tlv_bitmap,
> > > +                         struct ofpbuf *out)
> > > +{
> > > +    struct ofpact_sample *sample = ofpact_put_SAMPLE(out);
> > > +    int err;
> > > +
> > > +    if (!is_all_zeros(nas->zeros, sizeof nas->zeros)) {
> > > +        return OFPERR_NXBRC_MUST_BE_ZERO;
> > > +    }
> > > +
> > > +    err = check_sample_direction(nas->direction);
> > > +    if (err) {
> > > +        return err;
> > > +    }
> > > +
> > > +    sample->ofpact.raw = NXAST_RAW_SAMPLE4;
> > > +    sample->probability = ntohs(nas->probability);
> > > +    sample->collector_set_id = ntohl(nas->collector_set_id);
> > > +    sample->sampling_port = u16_to_ofp(ntohs(nas->sampling_port));
> > > +    sample->direction = nas->direction;
> > > +
> > > +    if (sample->probability == 0) {
> > > +        return OFPERR_OFPBAC_BAD_ARGUMENT;
> > > +    }
> > > +
> > > +    err = decode_sample_obs_id(nas->obs_domain_src,
> > > +                               nas->obs_domain_ofs_nbits,
> > > +                               nas->obs_domain_imm,
> > > +                               vl_mff_map, tlv_bitmap,
> > > +                               &sample->obs_domain_src,
> > > +                               &sample->obs_domain_imm);
> > > +    if (err) {
> > > +        return err;
> > > +    }
> > > +
> > > +    return decode_sample_obs_id(nas->obs_point_src,
> > > +                                nas->obs_point_ofs_nbits,
> > > +                                nas->obs_point_imm,
> > > +                                vl_mff_map, tlv_bitmap,
> > > +                                &sample->obs_point_src,
> > > +                                &sample->obs_point_imm);
> > > +}
> > > +
> > >  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->obs_domain_id = htonl(sample->obs_domain_imm);
> > > +    nas->obs_point_id = htonl(sample->obs_point_imm);
> > > +    nas->sampling_port = htons(ofp_to_u16(sample->sampling_port));
> > > +    nas->direction = sample->direction;
> > > +}
> > > +
> > > +static void
> > > +encode_SAMPLE4(const struct ofpact_sample *sample,
> > > +               struct nx_action_sample4 *nas)
> > > +{
> > > +    nas->probability = htons(sample->probability);
> > > +    nas->collector_set_id = htonl(sample->collector_set_id);
> > >      nas->sampling_port = htons(ofp_to_u16(sample->sampling_port));
> > >      nas->direction = sample->direction;
> > > +
> > > +    if (sample->obs_domain_src.field) {
> > > +        nas->obs_domain_src =
> > > +            htonl(nxm_header_from_mff(sample->obs_domain_src.field));
> > > +        nas->obs_domain_ofs_nbits =
> > > +            nxm_encode_ofs_nbits(sample->obs_domain_src.ofs,
> > > +                                 sample->obs_domain_src.n_bits);
> > > +    } else {
> > > +        nas->obs_domain_src = htonl(0);
> > > +        nas->obs_domain_imm = htonl(sample->obs_domain_imm);
> > > +    }
> > > +    if (sample->obs_point_src.field) {
> > > +        nas->obs_point_src =
> > > +            htonl(nxm_header_from_mff(sample->obs_point_src.field));
> > > +        nas->obs_point_ofs_nbits =
> > > +            nxm_encode_ofs_nbits(sample->obs_point_src.ofs,
> > > +                                 sample->obs_point_src.n_bits);
> > > +    } else {
> > > +        nas->obs_point_src = htonl(0);
> > > +        nas->obs_point_imm = htonl(sample->obs_point_imm);
> > > +    }
> > >  }
> > >
> > >  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_SAMPLE3
> > > +    if (sample->ofpact.raw == NXAST_RAW_SAMPLE4 ||
> > > +        sample->obs_domain_src.field ||
> > > +        sample->obs_point_src.field) {
> > > +        encode_SAMPLE4(sample, put_NXAST_SAMPLE4(out));
> > > +    } else 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
> > > @@ -6285,8 +6449,8 @@ encode_SAMPLE(const struct ofpact_sample *sample,
> > >          struct nx_action_sample *nas = put_NXAST_SAMPLE(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->obs_domain_id = htonl(sample->obs_domain_imm);
> > > +        nas->obs_point_id = htonl(sample->obs_point_imm);
> > >      }
> > >  }
> > >
> > > @@ -6314,9 +6478,35 @@ parse_SAMPLE(char *arg, const struct 
> > > ofpact_parse_params *pp)
> > >          } else if (!strcmp(key, "collector_set_id")) {
> > >              error = str_to_u32(value, &os->collector_set_id);
> > >          } else if (!strcmp(key, "obs_domain_id")) {
> > > -            error = str_to_u32(value, &os->obs_domain_id);
> > > +            error = str_to_u32(value, &os->obs_domain_imm);
> > > +
> > > +            if (error) {
> > > +                free(error);
> > > +                error = mf_parse_subfield(&os->obs_domain_src, value);
> > > +                if (error) {
> > > +                    return error;
> > > +                }
> > > +                if (os->obs_domain_src.n_bits > 32) {
> > > +                    return xasprintf("size of obs_domain_id field (%d) "
> > > +                                     "exceeds maximum (32)",
> > > +                                     os->obs_point_src.n_bits);
> > > +                }
> > > +            }
> > >          } else if (!strcmp(key, "obs_point_id")) {
> > > -            error = str_to_u32(value, &os->obs_point_id);
> > > +            error = str_to_u32(value, &os->obs_point_imm);
> > > +
> > > +            if (error) {
> > > +                free(error);
> > > +                error = mf_parse_subfield(&os->obs_point_src, value);
> > > +                if (error) {
> > > +                    return error;
> > > +                }
> > > +                if (os->obs_point_src.n_bits != 32) {
> > > +                    return xasprintf("size of obs_point_id field (%d) "
> > > +                                     "exceeds maximum (32)",
> > > +                                     os->obs_point_src.n_bits);
> > > +                }
> > > +            }
> > >          } else if (!strcmp(key, "sampling_port")) {
> > >              if (!ofputil_port_from_string(value, pp->port_map,
> > >                                            &os->sampling_port)) {
> > > @@ -6346,14 +6536,23 @@ format_SAMPLE(const struct ofpact_sample *a,
> > >                const struct ofpact_format_params *fp)
> > >  {
> > >      ds_put_format(fp->s, "%ssample(%s%sprobability=%s%"PRIu16
> > > -                  ",%scollector_set_id=%s%"PRIu32
> > > -                  ",%sobs_domain_id=%s%"PRIu32
> > > -                  ",%sobs_point_id=%s%"PRIu32,
> > > +                  ",%scollector_set_id=%s%"PRIu32,
> > >                    colors.paren, colors.end,
> > >                    colors.param, colors.end, a->probability,
> > > -                  colors.param, colors.end, a->collector_set_id,
> > > -                  colors.param, colors.end, a->obs_domain_id,
> > > -                  colors.param, colors.end, a->obs_point_id);
> > > +                  colors.param, colors.end, a->collector_set_id);
> > > +
> > > +    ds_put_format(fp->s, ",%sobs_domain_id=%s", colors.param, 
> > > colors.end);
> > > +    if (a->obs_domain_src.field) {
> > > +        mf_format_subfield(&a->obs_domain_src, fp->s);
> > > +    } else {
> > > +        ds_put_format(fp->s, "%"PRIu32, a->obs_domain_imm);
> > > +    }
> > > +    ds_put_format(fp->s, ",%sobs_point_id=%s", colors.param, colors.end);
> > > +    if (a->obs_point_src.field) {
> > > +        mf_format_subfield(&a->obs_point_src, fp->s);
> > > +    } else {
> > > +        ds_put_format(fp->s, "%"PRIu32, a->obs_point_imm);
> > > +    }
> > >      if (a->sampling_port != OFPP_NONE) {
> > >          ds_put_format(fp->s, ",%ssampling_port=%s", colors.param, 
> > > colors.end);
> > >          ofputil_format_port(a->sampling_port, fp->port_map, fp->s);
> > > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > > index 843ea2f79..7aa4a05a6 100644
> > > --- a/ofproto/ofproto-dpif-xlate.c
> > > +++ b/ofproto/ofproto-dpif-xlate.c
> > > @@ -5902,6 +5902,40 @@ xlate_fin_timeout(struct xlate_ctx *ctx,
> > >      }
> > >  }
> > >
> > > +static uint32_t
> > > +ofpact_sample_get_domain(struct xlate_ctx *ctx,
> > > +                         const struct ofpact_sample *os)
> > > +{
> > > +    if (os->obs_domain_src.field) {
> > > +        uint32_t obs_domain_id;
> > > +
> > > +        obs_domain_id = mf_get_subfield(&os->obs_domain_src, 
> > > &ctx->xin->flow);
> > > +        mf_write_subfield_flow(&os->obs_domain_src, 
> > > &exact_sub_match_mask,
> > > +                               &ctx->wc->masks);
> > > +
> > > +        return obs_domain_id;
> > > +    } else {
> > > +        return os->obs_domain_imm;
> > > +    }
> > > +}
> > > +
> > > +static uint32_t
> > > +ofpact_sample_get_point(struct xlate_ctx *ctx,
> > > +                        const struct ofpact_sample *os)
> > > +{
> > > +    if (os->obs_point_src.field) {
> > > +        uint32_t obs_point_id;
> > > +
> > > +        obs_point_id = mf_get_subfield(&os->obs_point_src, 
> > > &ctx->xin->flow);
> > > +        mf_write_subfield_flow(&os->obs_point_src, &exact_sub_match_mask,
> > > +                               &ctx->wc->masks);
> > > +
> > > +        return obs_point_id;
> > > +    } else {
> > > +        return os->obs_point_imm;
> > > +    }
> > > +}
> > > +
> > >  static void
> > >  xlate_fill_ipfix_sample(struct xlate_ctx *ctx,
> > >                          const struct ofpact_sample *os,
> > > @@ -5968,8 +6002,10 @@ xlate_fill_ipfix_sample(struct xlate_ctx *ctx,
> > >      userspace->cookie.ofproto_uuid = ctx->xbridge->ofproto->uuid;
> > >      userspace->cookie.flow_sample.probability = os->probability;
> > >      userspace->cookie.flow_sample.collector_set_id = 
> > > os->collector_set_id;
> > > -    userspace->cookie.flow_sample.obs_domain_id = os->obs_domain_id;
> > > -    userspace->cookie.flow_sample.obs_point_id = os->obs_point_id;
> > > +    userspace->cookie.flow_sample.obs_domain_id =
> > > +        ofpact_sample_get_domain(ctx, os);
> > > +    userspace->cookie.flow_sample.obs_point_id =
> > > +        ofpact_sample_get_point(ctx, os);
> > >      userspace->cookie.flow_sample.output_odp_port = output_odp_port;
> > >      userspace->cookie.flow_sample.direction = os->direction;
> > >      userspace->include_actions = false;
> > > @@ -6003,8 +6039,8 @@ xlate_sample_action(struct xlate_ctx *ctx,
> > >          dpif_lsample_get_group_id(lsample,
> > >                                    os->collector_set_id,
> > >                                    &psample.group_id)) {
> > > -        psample.cookie.hi = htonl(os->obs_domain_id);
> > > -        psample.cookie.lo = htonl(os->obs_point_id);
> > > +        psample.cookie.hi = htonl(ofpact_sample_get_domain(ctx, os));
> > > +        psample.cookie.lo = htonl(ofpact_sample_get_point(ctx,os));
> > >
> > >          compose_args.psample = &psample;
> > >
> > > diff --git a/python/ovs/flow/ofp.py b/python/ovs/flow/ofp.py
> > > index 3d3226c91..f011b0460 100644
> > > --- a/python/ovs/flow/ofp.py
> > > +++ b/python/ovs/flow/ofp.py
> > > @@ -30,7 +30,7 @@ from ovs.flow.ofp_act import (
> > >      decode_move_field,
> > >      decode_dec_ttl,
> > >      decode_chk_pkt_larger,
> > > -    decode_zone,
> > > +    decode_field_or_int,
> > >      decode_learn,
> > >  )
> > >
> > > @@ -330,7 +330,7 @@ class OFPFlow(Flow):
> > >                  KVDecoders(
> > >                      {
> > >                          "commit": decode_flag,
> > > -                        "zone": decode_zone,
> > > +                        "zone": decode_field_or_int,
> > >                          "table": decode_int,
> > >                          "nat": decode_nat,
> > >                          "force": decode_flag,
> > > @@ -426,8 +426,8 @@ class OFPFlow(Flow):
> > >                      {
> > >                          "probability": decode_int,
> > >                          "collector_set_id": decode_int,
> > > -                        "obs_domain_id": decode_int,
> > > -                        "obs_point_id": decode_int,
> > > +                        "obs_domain_id": decode_field_or_int,
> > > +                        "obs_point_id": decode_field_or_int,
> > >                          "sampling_port": decode_default,
> > >                          "ingress": decode_flag,
> > >                          "egress": decode_flag,
> > > diff --git a/python/ovs/flow/ofp_act.py b/python/ovs/flow/ofp_act.py
> > > index 2c85076a3..73727428a 100644
> > > --- a/python/ovs/flow/ofp_act.py
> > > +++ b/python/ovs/flow/ofp_act.py
> > > @@ -246,9 +246,9 @@ def decode_chk_pkt_larger(value):
> > >      return {"pkt_len": pkt_len, "dst": dst}
> > >
> > >
> > > -# CT decoders
> > > -def decode_zone(value):
> > > -    """Decodes the value of the 'zone' keyword (part of the ct 
> > > action)."""
> > > +def decode_field_or_int(value):
> > > +    """Decodes a value that can be either a subfield specification or an
> > > +    integer."""
> > >      try:
> > >          return int(value, 0)
> > >      except ValueError:
> > > diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at
> > > index 40a23bb15..86aec12e8 100644
> > > --- a/tests/ofp-actions.at
> > > +++ b/tests/ofp-actions.at
> > > @@ -136,6 +136,9 @@ ffff 0020 00002320 0026 3039 00005BA0 00008707 
> > > 0000B26E DDD50000 00000000
> > >  # 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789,egress)
> > >  ffff 0020 00002320 0029 3039 00005BA0 00008707 0000B26E DDD50200 00000000
> > >
> > > +# 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=NXM_OF_IN_PORT[],obs_point_id=NXM_NX_CT_LABEL[32..63],sampling_port=0)
> > > +ffff 0028 00002320 0033 3039 00005ba0 00000002 000f0000 0001d810 
> > > 081f0000 0000 000000000000
> > > +
> > >  # 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):
> > > @@ -489,6 +492,9 @@ ffff 0020 00002320 0015 000500000000 80003039005A02fd 
> > > 0400000000000000
> > >  # 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678)
> > >  ffff 0018 00002320 001d 3039 00005BA0 00008707 0000B26E
> > >
> > > +# 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=NXM_OF_IN_PORT[],obs_point_id=NXM_NX_CT_LABEL[32..63],sampling_port=0)
> > > +ffff 0028 00002320 0033 3039 00005ba0 00000002 000f0000 0001d810 
> > > 081f0000 0000 000000000000
> > > +
> > >  # bad OpenFlow11 actions: OFPBAC_BAD_OUT_PORT
> > >  & ofp_actions|WARN|bad action at offset 0 (OFPBAC_BAD_OUT_PORT):
> > >  & 00000000  00 00 00 10 ff ff ff ff-00 00 00 00 00 00 00 00
> > > @@ -1121,6 +1127,8 @@ bad_action 'unroll_xlate' "UNROLL is an internal 
> > > action that shouldn't be used v
> > >  # sample
> > >  bad_action 'sample(probability=0)' 'invalid probability value "0"'
> > >  bad_action 'sample(sampling_port=asdf)' 'asdf: unknown port'
> > > +bad_action 
> > > 'sample(probability=12345,obs_point_id=NXM_NX_CT_LABEL[[0..32]])' \
> > > +    'size of obs_point_id field (33) exceeds maximum (32)'
> > >  bad_action 'sample(foo=bar)' 'invalid key "foo" in "sample" argument'
> > >  bad_action 'sample' 'non-zero "probability" must be specified on sample'
> > >
> > > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> > > index c94dca9b1..a77ebcdd4 100644
> > > --- a/tests/ofproto-dpif.at
> > > +++ b/tests/ofproto-dpif.at
> > > @@ -8304,6 +8304,50 @@ AT_CHECK([ovs-vsctl destroy 
> > > Flow_Sample_Collector_Set 1], [0], [ignore])
> > >  OVS_VSWITCHD_STOP
> > >  AT_CLEANUP
> > >
> > > +AT_SETUP([ofproto-dpif - Flow IPFIX sanity check - from field])
> > > +OVS_VSWITCHD_START
> > > +add_of_ports br0 1 2
> > > +
> > > +AT_CHECK([ovs-vsctl -- --id=@br0 get Bridge br0 \
> > > +                    -- --id=@ipfix create IPFIX 
> > > targets=\"127.0.0.1:5500\" \
> > > +                    -- --id=@cs create Flow_Sample_Collector_Set id=0 \
> > > +                         bridge=@br0 ipfix=@ipfix],
> > > +         [0], [ignore])
> > > +
> > > +m4_define([SAMPLE_ACTION],
> > > +    
> > > [sample(probability=65535,collector_set_id=1,obs_domain_id=NXM_OF_IN_PORT,obs_point_id=$1)]dnl
> > > +)
> > > +
> > > +dnl Store in_port in obs_domain_id and dp_hash in the obs_point_id.
> > > +AT_DATA([flows.txt], [dnl
> > > +priority=100,arp,action=normal
> > > +priority=10,in_port=1,ip actions=SAMPLE_ACTION(NXM_NX_DP_HASH),2
> > > +priority=10,in_port=2,ip 
> > > actions=SAMPLE_ACTION(NXM_NX_CT_LABEL[[[0..31]]]),1
> > > +])
> > > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt], [0], [ignore])
> > > +
> > > +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy \
> > > +    
> > > "in_port(1),dp_hash(45),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),\
> > > +     
> > > ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)"],
> > >  [0], [stdout])
> > > +
> > > +AT_CHECK([tail -2 stdout], [0], [dnl
> > > +Megaflow: recirc_id=0,dp_hash=0x2d,eth,ip,in_port=1,nw_frag=no
> > > +Datapath actions: 
> > > userspace(pid=0,flow_sample(probability=65535,collector_set_id=1,obs_domain_id=1,obs_point_id=45,output_port=4294967295)),2
> > > +])
> > > +
> > > +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy \
> > > +    "in_port(2),ct_label(0x1234567890abcdef1234567890abcdef),\
> > > +     eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),\
> > > +     
> > > ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)"],
> > >  [0], [stdout])
> > > +
> > > +AT_CHECK([tail -2 stdout], [0], [dnl
> > > +Megaflow: 
> > > recirc_id=0,ct_label=0x90abcdef/0xffffffff,eth,ip,in_port=2,nw_frag=no
> > > +Datapath actions: 
> > > userspace(pid=0,flow_sample(probability=65535,collector_set_id=1,obs_domain_id=2,obs_point_id=2427178479,output_port=4294967295)),1
> > > +])
> > > +
> > > +OVS_VSWITCHD_STOP
> > > +AT_CLEANUP
> > > +
> > >  AT_SETUP([ofproto-dpif - clone action])
> > >  OVS_VSWITCHD_START
> > >  add_of_ports br0 1 2 3 4
> > > diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> > > index d03d36500..e2f4429ae 100644
> > > --- a/tests/ovs-ofctl.at
> > > +++ b/tests/ovs-ofctl.at
> > > @@ -198,6 +198,8 @@ 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_
> > >  
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,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=34567,obs_point_id=45678,sampling_port=56789,egress)
> > > +actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=NXM_OF_IN_PORT[],obs_point_id=NXM_NX_CT_LABEL[32..63],sampling_port=56789,egress)
> > > +actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=NXM_OF_IN_PORT[],obs_point_id=NXM_NX_CT_LABEL[32..63])
> > >  ip,actions=ct(nat)
> > >  ip,actions=ct(commit,nat(dst))
> > >  ip,actions=ct(commit,nat(src))
> > > @@ -233,6 +235,8 @@ OFPT_FLOW_MOD: ADD 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_d
> > >  OFPT_FLOW_MOD: ADD 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,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=34567,obs_point_id=45678,sampling_port=56789,egress)
> > > +OFPT_FLOW_MOD: ADD 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=NXM_OF_IN_PORT[],obs_point_id=NXM_NX_CT_LABEL[32..63],sampling_port=56789,egress)
> > > +OFPT_FLOW_MOD: ADD 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=NXM_OF_IN_PORT[],obs_point_id=NXM_NX_CT_LABEL[32..63])
> > >  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))
> > > @@ -265,6 +269,7 @@ 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=34567,obs_point_id=45678,sampling_port=56789)
> > > +actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=NXM_OF_IN_PORT[],obs_point_id=NXM_NX_CT_LABEL[32..63],sampling_port=0)
> > >  ]])
> > >
> > >  AT_CHECK([ovs-ofctl --protocols OpenFlow11 parse-flows flows.txt
> > > @@ -286,6 +291,7 @@ OFPT_FLOW_MOD (OF1.1): ADD sctp actions=drop
> > >  OFPT_FLOW_MOD (OF1.1): ADD in_port=0 actions=resubmit:0
> > >  OFPT_FLOW_MOD (OF1.1): ADD 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678)
> > >  OFPT_FLOW_MOD (OF1.1): ADD 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789)
> > > +OFPT_FLOW_MOD (OF1.1): ADD 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=NXM_OF_IN_PORT[],obs_point_id=NXM_NX_CT_LABEL[32..63],sampling_port=0)
> > >  ]])
> > >  AT_CLEANUP
> > >
> > > @@ -312,6 +318,7 @@ in_port=0 
> > > actions=mod_dl_src:11:22:33:44:55:66,mod_dl_dst:10:20:30:40:50:60
> > >  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=34567,obs_point_id=45678,sampling_port=56789)
> > > +actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=NXM_OF_IN_PORT[],obs_point_id=NXM_NX_CT_LABEL[32..63],sampling_port=0)
> > >  ]])
> > >
> > >  AT_CHECK([ovs-ofctl --protocols OpenFlow12 parse-flows flows.txt
> > > @@ -339,6 +346,7 @@ OFPT_FLOW_MOD (OF1.2): ADD in_port=0 
> > > actions=set_field:11:22:33:44:55:66->eth_sr
> > >  OFPT_FLOW_MOD (OF1.2): ADD in_port=0 actions=resubmit:0
> > >  OFPT_FLOW_MOD (OF1.2): ADD 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678)
> > >  OFPT_FLOW_MOD (OF1.2): ADD 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789)
> > > +OFPT_FLOW_MOD (OF1.2): ADD 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=NXM_OF_IN_PORT[],obs_point_id=NXM_NX_CT_LABEL[32..63],sampling_port=0)
> > >  ]])
> > >  AT_CLEANUP
> > >
> > > @@ -441,6 +449,7 @@ 
> > > tcp,actions=fin_timeout(idle_timeout=5,hard_timeout=15)
> > >  actions=controller(max_len=123,reason=invalid_ttl,id=555)
> > >  
> > > 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=34567,obs_point_id=45678,sampling_port=56789)
> > > +actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=NXM_OF_IN_PORT[],obs_point_id=NXM_NX_CT_LABEL[32..63],sampling_port=56789)
> > >  mpls,mpls_label=5,mpls_tc=1,mpls_ttl=1,mpls_bos=0,actions=drop
> > >  ip,actions=ct(commit,zone=5)
> > >  ip,actions=ct(commit,exec(load(1->NXM_NX_CT_MARK[])))
> > > @@ -508,6 +517,7 @@ NXT_FLOW_MOD: ADD table:255 tcp 
> > > actions=fin_timeout(idle_timeout=5,hard_timeout=
> > >  NXT_FLOW_MOD: ADD table:255 
> > > actions=controller(reason=invalid_ttl,max_len=123,id=555)
> > >  NXT_FLOW_MOD: ADD table:255 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678)
> > >  NXT_FLOW_MOD: ADD table:255 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789)
> > > +NXT_FLOW_MOD: ADD table:255 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=NXM_OF_IN_PORT[],obs_point_id=NXM_NX_CT_LABEL[32..63],sampling_port=56789)
> > >  NXT_FLOW_MOD: ADD table:255 
> > > mpls,mpls_label=5,mpls_tc=1,mpls_ttl=1,mpls_bos=0 actions=drop
> > >  NXT_FLOW_MOD: ADD table:255 ip actions=ct(commit,zone=5)
> > >  NXT_FLOW_MOD: ADD table:255 ip 
> > > actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[]))
> > > @@ -567,6 +577,7 @@ 
> > > dl_dst=aa:bb:cc:dd:ee:ff/fe:ff:ff:ff:ff:ff,actions=drop
> > >  dl_dst=aa:bb:cc:dd:ee:ff/00:00:00:00:00:00,actions=drop
> > >  
> > > 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=34567,obs_point_id=45678,sampling_port=56789)
> > > +actions=sample(probability=12341,collector_set_id=23456,obs_domain_id=NXM_OF_IN_PORT[[]],obs_point_id=NXM_NX_CT_LABEL[[32..63]],sampling_port=56789,egress)
> > >  ip,actions=ct(commit,zone=5)
> > >  ip,actions=ct(commit,exec(load(1->NXM_NX_CT_MARK[[]])))
> > >  ip,actions=ct(commit,exec(load(0x1->NXM_NX_CT_LABEL[[]])))
> > > @@ -608,6 +619,7 @@ NXT_FLOW_MOD: ADD 
> > > dl_dst=aa:bb:cc:dd:ee:ff/fe:ff:ff:ff:ff:ff actions=drop
> > >  NXT_FLOW_MOD: ADD actions=drop
> > >  NXT_FLOW_MOD: ADD 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678)
> > >  NXT_FLOW_MOD: ADD 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789)
> > > +NXT_FLOW_MOD: ADD 
> > > actions=sample(probability=12341,collector_set_id=23456,obs_domain_id=NXM_OF_IN_PORT[[]],obs_point_id=NXM_NX_CT_LABEL[[32..63]],sampling_port=56789,egress)
> > >  NXT_FLOW_MOD: ADD ip actions=ct(commit,zone=5)
> > >  NXT_FLOW_MOD: ADD ip 
> > > actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]]))
> > >  NXT_FLOW_MOD: ADD ip 
> > > actions=ct(commit,exec(load:0x1->NXM_NX_CT_LABEL[[0..63]],load:0->NXM_NX_CT_LABEL[[64..127]]))
> > > @@ -648,6 +660,7 @@ actions=push:reg0[0..31],pop:reg0
> > >  vlan_tci=0x1123/0x1fff,actions=drop
> > >  
> > > 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=34567,obs_point_id=45678,sampling_port=56789)
> > > +actions=sample(probability=12341,collector_set_id=23456,obs_domain_id=NXM_OF_IN_PORT[],obs_point_id=NXM_NX_CT_LABEL[32..63],sampling_port=56789,egress)
> > >  ip,actions=ct(commit,zone=5)
> > >  ip,actions=ct(commit,exec(load(1->NXM_NX_CT_MARK[])))
> > >  ip,actions=ct(commit,exec(load(1->NXM_NX_CT_LABEL[])))
> > > @@ -688,6 +701,7 @@ NXT_FLOW_MOD: ADD <any> 
> > > actions=push:NXM_NX_REG0[],pop:NXM_NX_REG0[]
> > >  NXT_FLOW_MOD: ADD NXM_OF_VLAN_TCI_W(1123/1fff) actions=drop
> > >  NXT_FLOW_MOD: ADD <any> 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678)
> > >  NXT_FLOW_MOD: ADD <any> 
> > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789)
> > > +NXT_FLOW_MOD: ADD <any> 
> > > actions=sample(probability=12341,collector_set_id=23456,obs_domain_id=NXM_OF_IN_PORT[],obs_point_id=NXM_NX_CT_LABEL[32..63],sampling_port=56789,egress)
> > >  NXT_FLOW_MOD: ADD NXM_OF_ETH_TYPE(0800) actions=ct(commit,zone=5)
> > >  NXT_FLOW_MOD: ADD NXM_OF_ETH_TYPE(0800) 
> > > actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[]))
> > >  NXT_FLOW_MOD: ADD NXM_OF_ETH_TYPE(0800) 
> > > actions=ct(commit,exec(load:0x1->NXM_NX_CT_LABEL[0..63],load:0->NXM_NX_CT_LABEL[64..127]))
> > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> > > index a83099304..14e43261b 100644
> > > --- a/tests/system-traffic.at
> > > +++ b/tests/system-traffic.at
> > > @@ -9462,3 +9462,82 @@ dnl OVS will fail to send IPFIX packets because 
> > > the target is localhost
> > >  dnl and the port is closed. Ignore the message it generates.
> > >  OVS_TRAFFIC_VSWITCHD_STOP(["/sending to collector failed/d"])
> > >  AT_CLEANUP
> > > +
> > > +AT_SETUP([psample - from ct label])
> > > +CHECK_CONNTRACK()
> > > +OVS_TRAFFIC_VSWITCHD_START()
> > > +OVS_CHECK_PSAMPLE()
> > > +
> > > +ADD_NAMESPACES(at_ns0, at_ns1)
> > > +NS_CHECK_EXEC([at_ns0], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], 
> > > [0], [ignore])
> > > +NS_CHECK_EXEC([at_ns1], [sysctl -w net.ipv6.conf.all.disable_ipv6=1], 
> > > [0], [ignore])
> > > +
> > > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "e4:11:22:33:44:55")
> > > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "e4:11:22:33:44:66")
> > > +
> > > +AT_CHECK([ovs-vsctl -- --id=@br0 get Bridge br0 \
> > > +                    -- --id=@ipfix create IPFIX 
> > > targets=\"127.0.0.1:4739\" \
> > > +                    -- create Flow_Sample_Collector_Set id=1 bridge=@br0 
> > > \
> > > +                       ipfix=@ipfix, local_group_id=10 \
> > > +                    -- create Flow_Sample_Collector_Set id=2 bridge=@br0 
> > > \
> > > +                       ipfix=@ipfix, local_group_id=12],
> >
> > You are using IPFix as the reference, but not verifying IPFix, so maybe 
> > just use the sanity one as a reference?
> >
>
> So, the issue is I can't really verify the cookie has the right value in
> IFPIX. I can see if the sample is emitted but nothing more. I added  it
> to make sure this OFP action also works with IPFIX but I verify its
> content using psample.
>
> I'll add a check for the number of emitted IPFIX frames just in case.
>

Just realized this was being checked. As I said, what I wanted to verify
is that IPFIX samples still get emitted when created with the new OFP
action (although I cannot verify the content of them).

> > > +         [0], [ignore])
> > > +
> > > +
> >
> > Need a single new line.
>
> Ack.
>
> >
> > > +m4_define([CT_STORE_ACT],
> > > +    
> > > [ct(zone=5,commit,exec(load:0x0bb102030->NXM_NX_CT_LABEL[[0..31]],load:0xbb405060->NXM_NX_CT_LABEL[[32..63]]))])
> > > +
> > > +AT_DATA([flows.txt], [dnl
> > > +priority=100,ip actions=ct(zone=5, table=10)
> > > +priority=0 actions=NORMAL
> > > +table=10,priority=100,ip,ct_state=+trk+new action=SAMPLE_ACTION(1, 
> > > 2853183536, 2856341600),CT_STORE_ACT,NORMAL
> > > +table=10,priority=100,ip,ct_state=+trk-new action=SAMPLE_ACTION(2, 
> > > NXM_NX_CT_LABEL[[[0..31]]], NXM_NX_CT_LABEL[[[32..63]]]),NORMAL
> > > +table=10, priority=50, ip, actions=DROP
> > > +])
> > > +
> > > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> > > +
> > > +OVS_DAEMONIZE([ovstest test-psample > psample.out], [psample1.pid])
> >
> > Missing 'OVS_WAIT_UNTIL([grep -q "Listening for psample events" 
> > psample.out])'
> >
> > > +
> > > +NS_CHECK_EXEC([at_ns0], [ping -q -c 1 10.1.1.2 | FORMAT_PING], [0], [dnl
> > > +1 packets transmitted, 1 received, 0% packet loss, time 0ms
> > > +])
> > > +
> > > +m4_define([SAMPLE1], [m4_join([ ],
> > > +    [group_id=0xa,prob=4294967295],
> > > +    [obs_domain=0xaa102030,obs_point=0xaa405060],
> > > +    [.*icmp.*nw_src=10.1.1.1,nw_dst=10.1.1.2])])
> > > +
> > > +m4_define([SAMPLE2], [m4_join([ ],
> > > +    [group_id=0xc,prob=4294967295],
> > > +    [obs_domain=0xbb102030,obs_point=0xbb405060],
> > > +    [.*icmp.*nw_src=10.1.1.2,nw_dst=10.1.1.1])])
> >
> > New line
> >
> > > +AT_CHECK([grep -qE 'SAMPLE1' psample.out])
> > > +AT_CHECK([grep -qE 'SAMPLE2' psample.out])
> > > +
> > > +m4_define([FLOW_MATCH], [m4_join([],
> > > +    [ct_label(0xbb405060bb102030/0xffffffffffffffff).*actions:],
> > > +    [actions:psample(group=12,cookie=0xbb102030bb405060),],
> > > +    
> > > [userspace(pid=[[0-9]]+,flow_sample(.*obs_domain_id=3138396208,obs_point_id=3141554272.*))]
> > > +)])
> > > +
> > > +AT_CHECK([ovs-appctl dpctl/dump-flows --names filter=in_port=ovs-p1 \
> > > +          | grep -qE 'FLOW_MATCH' ], [0], [])
> > > +
> > > +AT_CHECK([ovs-appctl lsample/show br0], [0], [dnl
> > > +Local sample statistics for bridge "br0":
> > > +Collector Set ID: 1:
> > > +  Group ID     : 10
> > > +  Total packets: 1
> > > +  Total bytes  : 98
> > > +
> > > +Collector Set ID: 2:
> > > +  Group ID     : 12
> > > +  Total packets: 1
> > > +  Total bytes  : 98
> > > +])
> > > +
> > > +dnl OVS will fail to send IPFIX packets because the target is localhost
> > > +dnl and the port is closed. Ignore the message it generates.
> > > +OVS_TRAFFIC_VSWITCHD_STOP(["/sending to collector failed/d"])
> > > +AT_CLEANUP
> > > --
> > > 2.45.2
> >

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to