[ovs-dev,v6,1/3] actions: add sample action

sample ovn action encodes into the OFPACT_SAMPLE ovs action.

OVN action allows the following parameters:

- obs_domain_id: 8-bit integer that identifies the sampling application.
  This value will be combined with the datapath's tunnel_id to form the
  final observation_domain_id that will be used in the OVS action as:
    ObservationDomainID = obs_domain_id << 24 | (dp_key & 0xFFFFFF)

- obs_point_id: a 32-bit integer or the $cookie macro that will be
  expanded into the first 32 bits of the lflow's UUID.

- probability: a 16-bit integer that specifies the sampling probability.
  Specifying 0 has no effect and 65535 means sampling all packets.

- collector_set: the 32-bit id that has to be configured in OVS's
  Flow_Sample_Collector_Set table in order to configure IPFIX sampling.

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 controller/lflow.c    |   1 +
 include/ovn/actions.h |  16 ++++++
 lib/actions.c         | 120 ++++++++++++++++++++++++++++++++++++++++++
 ovn-sb.xml            |  52 ++++++++++++++++++
 tests/ovn.at          |  28 ++++++++++
 tests/test-ovn.c      |   3 ++
 utilities/ovn-trace.c |   3 ++
 7 files changed, 223 insertions(+)

On Mon, Nov 21, 2022 at 11:13 AM Adrian Moreno <amorenoz@redhat.com> wrote:
>
> sample ovn action encodes into the OFPACT_SAMPLE ovs action.
>
> OVN action allows the following parameters:
>
> - obs_domain_id: 8-bit integer that identifies the sampling application.
>   This value will be combined with the datapath's tunnel_id to form the
>   final observation_domain_id that will be used in the OVS action as:
>     ObservationDomainID = obs_domain_id << 24 | (dp_key & 0xFFFFFF)
>
> - obs_point_id: a 32-bit integer or the $cookie macro that will be
>   expanded into the first 32 bits of the lflow's UUID.
>
> - probability: a 16-bit integer that specifies the sampling probability.
>   Specifying 0 has no effect and 65535 means sampling all packets.
>
> - collector_set: the 32-bit id that has to be configured in OVS's
>   Flow_Sample_Collector_Set table in order to configure IPFIX sampling.
>
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>

Acked-by: Numan Siddique <numans@ovn.org>

This patch series needs a rebase.  Can you please spin up  v7 ?

Thanks
Numan

> ---
>  controller/lflow.c    |   1 +
>  include/ovn/actions.h |  16 ++++++
>  lib/actions.c         | 120 ++++++++++++++++++++++++++++++++++++++++++
>  ovn-sb.xml            |  52 ++++++++++++++++++
>  tests/ovn.at          |  28 ++++++++++
>  tests/test-ovn.c      |   3 ++
>  utilities/ovn-trace.c |   3 ++
>  7 files changed, 223 insertions(+)
>
> diff --git a/controller/lflow.c b/controller/lflow.c
> index cc0f31db0..ad316c17f 100644
> --- a/controller/lflow.c
> +++ b/controller/lflow.c
> @@ -1007,6 +1007,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow,
>          .group_table = l_ctx_out->group_table,
>          .meter_table = l_ctx_out->meter_table,
>          .lflow_uuid = lflow->header_.uuid,
> +        .dp_key = ldp->datapath->tunnel_key,
>
>          .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS,
>          .ingress_ptable = OFTABLE_LOG_INGRESS_PIPELINE,
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index d7ee84dac..009487cfc 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -121,6 +121,7 @@ struct ovn_extend_table;
>      OVNACT(COMMIT_ECMP_NH,    ovnact_commit_ecmp_nh)  \
>      OVNACT(CHK_ECMP_NH_MAC,   ovnact_result)          \
>      OVNACT(CHK_ECMP_NH,       ovnact_result)          \
> +    OVNACT(SAMPLE,            ovnact_sample)          \
>
>  /* enum ovnact_type, with a member OVNACT_<ENUM> for each action. */
>  enum OVS_PACKED_ENUM ovnact_type {
> @@ -456,6 +457,18 @@ struct ovnact_lookup_fdb {
>      struct expr_field dst;     /* 1-bit destination field. */
>  };
>
> +/* OVNACT_SAMPLE */
> +struct ovnact_sample {
> +    struct ovnact ovnact;
> +    uint16_t probability;       /* probability over UINT16_MAX. */
> +    uint8_t obs_domain_id;      /* most significant byte of the
> +                                   observation domain id. The other 24 bits
> +                                   will come from the datapath's tunnel key. */
> +    uint32_t collector_set_id;  /* colector_set_id. */
> +    uint32_t obs_point_id;      /* observation point id. */
> +    bool use_cookie;            /* use cookie as obs_point_id */
> +};
> +
>  /* OVNACT_COMMIT_ECMP_NH. */
>  struct ovnact_commit_ecmp_nh {
>      struct ovnact ovnact;
> @@ -785,6 +798,9 @@ struct ovnact_encode_params {
>      /* The logical flow uuid that drove this action. */
>      struct uuid lflow_uuid;
>
> +    /* The datapath key. */
> +    uint32_t dp_key;
> +
>      /* OVN maps each logical flow table (ltable), one-to-one, onto a physical
>       * OpenFlow flow table (ptable).  A number of parameters describe this
>       * mapping and data related to flow tables:
> diff --git a/lib/actions.c b/lib/actions.c
> index adbb42db4..65205aaba 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -4279,6 +4279,124 @@ encode_CHECK_OUT_PORT_SEC(const struct ovnact_result *dl,
>                             MLF_CHECK_PORT_SEC_BIT, ofpacts);
>  }
>
> +static void
> +format_SAMPLE(const struct ovnact_sample *sample, struct ds *s)
> +{
> +    ds_put_format(s, "sample(probability=%"PRIu16, sample->probability);
> +
> +    ds_put_format(s, ",collector_set=%"PRIu32, sample->collector_set_id);
> +    ds_put_format(s, ",obs_domain=%"PRIu8, sample->obs_domain_id);
> +    if (sample->use_cookie) {
> +        ds_put_cstr(s, ",obs_point=$cookie");
> +    } else {
> +        ds_put_format(s, ",obs_point=%"PRIu32, sample->obs_point_id);
> +    }
> +    ds_put_format(s, ");");
> +}
> +
> +static void
> +encode_SAMPLE(const struct ovnact_sample *sample,
> +              const struct ovnact_encode_params *ep,
> +              struct ofpbuf *ofpacts)
> +{
> +    struct ofpact_sample *os = ofpact_put_SAMPLE(ofpacts);
> +    os->probability = sample->probability;
> +    os->collector_set_id = sample->collector_set_id;
> +    os->obs_domain_id =
> +        (sample->obs_domain_id << 24) | (ep->dp_key & 0xFFFFFF);
> +
> +    if (sample->use_cookie) {
> +        os->obs_point_id = ep->lflow_uuid.parts[0];
> +    } else {
> +        os->obs_point_id = sample->obs_point_id;
> +    }
> +    os->sampling_port = OFPP_NONE;
> +}
> +
> +static void
> +parse_sample_arg(struct action_context *ctx, struct ovnact_sample *sample)
> +{
> +    if (lexer_match_id(ctx->lexer, "probability")) {
> +        if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
> +            return;
> +        }
> +        if (ctx->lexer->token.type == LEX_T_INTEGER
> +            && ctx->lexer->token.format == LEX_F_DECIMAL) {
> +            if (!action_parse_uint16(ctx, &sample->probability,
> +                                     "probability")) {
> +                return;
> +            }
> +        }
> +    } else if (lexer_match_id(ctx->lexer, "obs_point")) {
> +        if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
> +            return;
> +        }
> +        if (ctx->lexer->token.type == LEX_T_MACRO &&
> +            !strcmp(ctx->lexer->token.s, "cookie")) {
> +            sample->use_cookie = true;
> +            lexer_get(ctx->lexer);
> +        } else if (ctx->lexer->token.type == LEX_T_INTEGER
> +                && ctx->lexer->token.format == LEX_F_DECIMAL) {
> +            sample->obs_point_id = ntohll(ctx->lexer->token.value.integer);
> +            lexer_get(ctx->lexer);
> +        } else {
> +            lexer_syntax_error(ctx->lexer,
> +                               "malformed sample observation_point_id");
> +        }
> +    } else if (lexer_match_id(ctx->lexer, "obs_domain")) {
> +        if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
> +            return;
> +        }
> +        if (ctx->lexer->token.type == LEX_T_INTEGER
> +            && ctx->lexer->token.format == LEX_F_DECIMAL) {
> +            uint32_t obs_domain = ntohll(ctx->lexer->token.value.integer);
> +            if (obs_domain > UINT8_MAX) {
> +                lexer_syntax_error(ctx->lexer,
> +                     "obs_domain must be 8-bit long");
> +                return;
> +            }
> +            sample->obs_domain_id = obs_domain;
> +        }
> +        lexer_get(ctx->lexer);
> +    } else if (lexer_match_id(ctx->lexer, "collector_set")) {
> +        if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
> +            return;
> +        }
> +        if (ctx->lexer->token.type == LEX_T_INTEGER
> +            && ctx->lexer->token.format == LEX_F_DECIMAL) {
> +            sample->collector_set_id = ntohll(ctx->lexer->token.value.integer);
> +        }
> +        lexer_get(ctx->lexer);
> +    } else {
> +        lexer_syntax_error(ctx->lexer, "unknown argument");
> +    }
> +}
> +
> +static void
> +parse_sample(struct action_context *ctx)
> +{
> +    struct ovnact_sample * sample = ovnact_put_SAMPLE(ctx->ovnacts);
> +
> +    if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
> +        while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) {
> +            parse_sample_arg(ctx, sample);
> +            if (ctx->lexer->error) {
> +                return;
> +            }
> +            lexer_match(ctx->lexer, LEX_T_COMMA);
> +        }
> +    }
> +    if (!sample->probability) {
> +        lexer_error(ctx->lexer, "probability must be greater than zero");
> +        return;
> +    }
> +}
> +
> +static void
> +ovnact_sample_free(struct ovnact_sample *sample OVS_UNUSED)
> +{
> +}
> +
>  static void
>  parse_commit_ecmp_nh(struct action_context *ctx,
>                       struct ovnact_commit_ecmp_nh *ecmp_nh)
> @@ -4790,6 +4908,8 @@ parse_action(struct action_context *ctx)
>          parse_put_fdb(ctx, ovnact_put_PUT_FDB(ctx->ovnacts));
>      } else if (lexer_match_id(ctx->lexer, "commit_ecmp_nh")) {
>          parse_commit_ecmp_nh(ctx, ovnact_put_COMMIT_ECMP_NH(ctx->ovnacts));
> +    } else if (lexer_match_id(ctx->lexer, "sample")) {
> +        parse_sample(ctx);
>      } else {
>          lexer_syntax_error(ctx->lexer, "expecting action");
>      }
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index 42e6fa3ee..75ead78fa 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -2626,6 +2626,58 @@ tcp.flags = RST;
>              register <var>R</var> is set to 1.
>            </p>
>          </dd>
> +
> +        <dt><code>sample(probability=<var>packets</var>, ...)</code></dt>
> +        <dd>
> +          <p>
> +            This action causes the matched traffic to be sampled using
> +            IPFIX protocol. More information about how per-flow IPFIX sampling
> +            works in OVS can be found in <code>ovs-actions</code>(7) and
> +            <code>ovs-vswitchd.conf.db</code>(5).
> +          </p>
> +
> +          <p>
> +            In order to reliably identify each sampled packet when it is
> +            received by the IPFIX collector, this action sets the content of
> +            the <code>ObservationDomainID</code> and
> +            <code>ObservationPointID</code> IPFIX fields (see argument
> +            description below).
> +          </p>
> +
> +          <p>
> +            The following key-value arguments are supported:
> +          </p>
> +
> +          <dl>
> +            <dt><code>probability=</code><var>packets</var></dt>
> +            <dd>
> +              The number of sampled packets out of 65535. It must be greater or
> +              equal to 1.
> +            </dd>
> +            <dt><code>collector_set=</code><var>id</var></dt>
> +            <dd>
> +              The unsigned 32-bit integer identifier of the sample collector to
> +              send sampled packets to. It must match the value configured in
> +              the <code>Flow_Sample_Collector_Set</code> Table in OVS.
> +              Defaults to 0.
> +            </dd>
> +            <dt><code>obs_domain=</code><var>id</var></dt>
> +            <dd>
> +              An unsigned 8-bit integer that identifies the sampling
> +              application. It will be placed in the 8 most significant bits of
> +              the <code>ObservationDomainID</code> field of IPFIX samples.
> +              The 24 less significant bits will be automatically filled in with
> +              the datapath key. Defaults to 0.
> +            </dd>
> +            <dt><code>obs_point=</code><var>id</var></dt>
> +            <dd>
> +              An unsigned 32-bit integer to be used as
> +              <code>ObsservationPointID</code> or the string
> +              <code>@cookie</code> to indicate that the first 32 bits of the
> +              <code>Logical_Flow</code>'s UUID shall be used instead.
> +            </dd>
> +          </dl>
> +        </dd>
>        </dl>
>      </column>
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 6552681bd..08d91e2e8 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -2136,6 +2136,34 @@ pop(eth.type);
>  push(abc);
>      Syntax error at `abc' expecting field name.
>
> +# sample
> +sample(probability=100,collector_set=200,obs_domain=0,obs_point=1000);
> +    encodes as sample(probability=100,collector_set_id=200,obs_domain_id=11259375,obs_point_id=1000)
> +
> +# sample with obs_domain = 10. Final obs_domain is 0xA << 24 | 0xABCDEF.
> +sample(probability=100,collector_set=200,obs_domain=10,obs_point=$cookie);
> +    encodes as sample(probability=100,collector_set_id=200,obs_domain_id=179031535,obs_point_id=2863311530)
> +
> +sample(probability=10);
> +    formats as sample(probability=10,collector_set=0,obs_domain=0,obs_point=0);
> +    encodes as sample(probability=10,collector_set_id=0,obs_domain_id=11259375,obs_point_id=0)
> +
> +sample(probability=10);
> +    formats as sample(probability=10,collector_set=0,obs_domain=0,obs_point=0);
> +    encodes as sample(probability=10,collector_set_id=0,obs_domain_id=11259375,obs_point_id=0)
> +
> +sample(probability=0,collector_set=200,obs_domain=0,obs_point=1000);
> +    probability must be greater than zero
> +
> +sample(probability=0,collector_set=200,obs_domain=0,obs_point=foo);
> +    Syntax error at `foo' malformed sample observation_point_id.
> +
> +sample(probability=0,collector_set=200,obs_domain=300,obs_point=foo);
> +    Syntax error at `300' obs_domain must be 8-bit long.
> +
> +sample(probability=10,foo=bar,obs_domain=0,obs_point=1000);
> +    Syntax error at `foo' unknown argument.
> +
>  # Miscellaneous negative tests.
>  ;
>      Syntax error at `;'.
> diff --git a/tests/test-ovn.c b/tests/test-ovn.c
> index a241f150d..fd580b5df 100644
> --- a/tests/test-ovn.c
> +++ b/tests/test-ovn.c
> @@ -1355,6 +1355,9 @@ test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED)
>                  .common_nat_ct_zone = MFF_LOG_DNAT_ZONE,
>                  .in_port_sec_ptable = OFTABLE_CHK_IN_PORT_SEC,
>                  .out_port_sec_ptable = OFTABLE_CHK_OUT_PORT_SEC,
> +                .lflow_uuid.parts =
> +                    { 0xaaaaaaaa, 0xbbbbbbbb, 0xcccccccc, 0xdddddddd},
> +                .dp_key = 0xabcdef,
>              };
>              struct ofpbuf ofpacts;
>              ofpbuf_init(&ofpacts, 0);
> diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
> index 6fa5137d9..854385bc1 100644
> --- a/utilities/ovn-trace.c
> +++ b/utilities/ovn-trace.c
> @@ -1466,6 +1466,7 @@ execute_load(const struct ovnact_load *load,
>      const struct ovnact_encode_params ep = {
>          .lookup_port = ovntrace_lookup_port,
>          .aux = dp,
> +        .dp_key = dp->tunnel_key,
>      };
>      uint64_t stub[512 / 8];
>      struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub);
> @@ -3290,6 +3291,8 @@ trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len,
>              break;
>          case OVNACT_CHK_ECMP_NH:
>              break;
> +        case OVNACT_SAMPLE:
> +            break;
>          }
>      }
>      ofpbuf_uninit(&stack);
> --
> 2.38.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>