On Wed, Jul 10, 2024 at 06:52:30PM GMT, Ilya Maximets wrote:
> On 7/10/24 15:25, Adrián Moreno wrote:
> > On Wed, Jul 10, 2024 at 02:56:44PM GMT, Ilya Maximets wrote:
> >> On 7/7/24 22:08, Adrian Moreno wrote:
> >>> Use the newly added psample action to implement OpenFlow sample() actions
> >>> with local sampling configuration if possible.
> >>>
> >>> A bit of refactoring in compose_sample_actions arguments helps make it a
> >>> bit more readable.
> >>>
> >>> Signed-off-by: Adrian Moreno <amore...@redhat.com>
> >>> ---
> >>> ofproto/ofproto-dpif-lsample.c | 16 +++
> >>> ofproto/ofproto-dpif-lsample.h | 5 +
> >>> ofproto/ofproto-dpif-xlate.c | 251 +++++++++++++++++++++++----------
> >>> ofproto/ofproto-dpif-xlate.h | 5 +-
> >>> ofproto/ofproto-dpif.c | 2 +-
> >>> tests/ofproto-dpif.at | 146 +++++++++++++++++++
> >>> 6 files changed, 345 insertions(+), 80 deletions(-)
> >>>
> >>> diff --git a/ofproto/ofproto-dpif-lsample.c
> >>> b/ofproto/ofproto-dpif-lsample.c
> >>> index d675a116f..534ad96f0 100644
> >>> --- a/ofproto/ofproto-dpif-lsample.c
> >>> +++ b/ofproto/ofproto-dpif-lsample.c
> >>> @@ -140,6 +140,22 @@ dpif_lsample_set_options(struct dpif_lsample
> >>> *lsample,
> >>> return changed;
> >>> }
> >>>
> >>> +/* Returns the group_id for a given collector_set_id, if it exists. */
> >>> +bool
> >>> +dpif_lsample_get_group_id(struct dpif_lsample *ps, uint32_t
> >>> collector_set_id,
> >>> + uint32_t *group_id)
> >>> +{
> >>> + struct lsample_exporter_node *node;
> >>> + bool found = false;
> >>> +
> >>> + node = dpif_lsample_find_exporter_node(ps, collector_set_id);
> >>> + if (node) {
> >>> + found = true;
> >>> + *group_id = node->exporter.options.group_id;
> >>> + }
> >>> + return found;
> >>> +}
> >>> +
> >>> struct dpif_lsample *
> >>> dpif_lsample_create(void)
> >>> {
> >>> diff --git a/ofproto/ofproto-dpif-lsample.h
> >>> b/ofproto/ofproto-dpif-lsample.h
> >>> index bee36c9c5..9c1026551 100644
> >>> --- a/ofproto/ofproto-dpif-lsample.h
> >>> +++ b/ofproto/ofproto-dpif-lsample.h
> >>> @@ -18,6 +18,7 @@
> >>> #define OFPROTO_DPIF_LSAMPLE_H 1
> >>>
> >>> #include <stdbool.h>
> >>> +#include <stdint.h>
> >>> #include <stdlib.h>
> >>>
> >>> struct dpif_lsample;
> >>> @@ -33,4 +34,8 @@ bool dpif_lsample_set_options(struct dpif_lsample *,
> >>> const struct ofproto_lsample_options *,
> >>> size_t n_opts);
> >>>
> >>> +bool dpif_lsample_get_group_id(struct dpif_lsample *,
> >>> + uint32_t collector_set_id,
> >>> + uint32_t *group_id);
> >>> +
> >>> #endif /* OFPROTO_DPIF_LSAMPLE_H */
> >>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> >>> index 7c4950895..5e8113d5e 100644
> >>> --- a/ofproto/ofproto-dpif-xlate.c
> >>> +++ b/ofproto/ofproto-dpif-xlate.c
> >>> @@ -47,6 +47,7 @@
> >>> #include "ofproto/ofproto-dpif-ipfix.h"
> >>> #include "ofproto/ofproto-dpif-mirror.h"
> >>> #include "ofproto/ofproto-dpif-monitor.h"
> >>> +#include "ofproto/ofproto-dpif-lsample.h"
> >>> #include "ofproto/ofproto-dpif-sflow.h"
> >>> #include "ofproto/ofproto-dpif-trace.h"
> >>> #include "ofproto/ofproto-dpif-xlate-cache.h"
> >>> @@ -117,6 +118,7 @@ struct xbridge {
> >>> struct dpif_sflow *sflow; /* SFlow handle, or null. */
> >>> struct dpif_ipfix *ipfix; /* Ipfix handle, or null. */
> >>> struct netflow *netflow; /* Netflow handle, or null. */
> >>> + struct dpif_lsample *lsample; /* Local sample handle, or null. */
> >>> struct stp *stp; /* STP or null if disabled. */
> >>> struct rstp *rstp; /* RSTP or null if disabled. */
> >>>
> >>> @@ -686,6 +688,7 @@ static void xlate_xbridge_set(struct xbridge *,
> >>> struct dpif *,
> >>> const struct mbridge *,
> >>> const struct dpif_sflow *,
> >>> const struct dpif_ipfix *,
> >>> + const struct dpif_lsample *,
> >>> const struct netflow *,
> >>> bool forward_bpdu, bool has_in_band,
> >>> const struct dpif_backer_support *,
> >>> @@ -1069,6 +1072,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
> >>> const struct mbridge *mbridge,
> >>> const struct dpif_sflow *sflow,
> >>> const struct dpif_ipfix *ipfix,
> >>> + const struct dpif_lsample *lsample,
> >>> const struct netflow *netflow,
> >>> bool forward_bpdu, bool has_in_band,
> >>> const struct dpif_backer_support *support,
> >>> @@ -1099,6 +1103,11 @@ xlate_xbridge_set(struct xbridge *xbridge,
> >>> xbridge->ipfix = dpif_ipfix_ref(ipfix);
> >>> }
> >>>
> >>> + if (xbridge->lsample != lsample) {
> >>> + dpif_lsample_unref(xbridge->lsample);
> >>> + xbridge->lsample = dpif_lsample_ref(lsample);
> >>> + }
> >>> +
> >>> if (xbridge->stp != stp) {
> >>> stp_unref(xbridge->stp);
> >>> xbridge->stp = stp_ref(stp);
> >>> @@ -1213,9 +1222,10 @@ xlate_xbridge_copy(struct xbridge *xbridge)
> >>> xlate_xbridge_set(new_xbridge,
> >>> xbridge->dpif, xbridge->ml, xbridge->stp,
> >>> xbridge->rstp, xbridge->ms, xbridge->mbridge,
> >>> - xbridge->sflow, xbridge->ipfix, xbridge->netflow,
> >>> - xbridge->forward_bpdu, xbridge->has_in_band,
> >>> - &xbridge->support, xbridge->addr);
> >>> + xbridge->sflow, xbridge->ipfix, xbridge->lsample,
> >>> + xbridge->netflow, xbridge->forward_bpdu,
> >>> + xbridge->has_in_band, &xbridge->support,
> >>> + xbridge->addr);
> >>> LIST_FOR_EACH (xbundle, list_node, &xbridge->xbundles) {
> >>> xlate_xbundle_copy(new_xbridge, xbundle);
> >>> }
> >>> @@ -1372,6 +1382,7 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto,
> >>> const char *name,
> >>> const struct mbridge *mbridge,
> >>> const struct dpif_sflow *sflow,
> >>> const struct dpif_ipfix *ipfix,
> >>> + const struct dpif_lsample *lsample,
> >>> const struct netflow *netflow,
> >>> bool forward_bpdu, bool has_in_band,
> >>> const struct dpif_backer_support *support)
> >>> @@ -1396,7 +1407,7 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto,
> >>> const char *name,
> >>> old_addr = xbridge->addr;
> >>>
> >>> xlate_xbridge_set(xbridge, dpif, ml, stp, rstp, ms, mbridge, sflow,
> >>> ipfix,
> >>> - netflow, forward_bpdu, has_in_band, support,
> >>> + lsample, netflow, forward_bpdu, has_in_band,
> >>> support,
> >>> xbridge_addr);
> >>>
> >>> if (xbridge_addr != old_addr) {
> >>> @@ -1428,6 +1439,7 @@ xlate_xbridge_remove(struct xlate_cfg *xcfg, struct
> >>> xbridge *xbridge)
> >>> mbridge_unref(xbridge->mbridge);
> >>> dpif_sflow_unref(xbridge->sflow);
> >>> dpif_ipfix_unref(xbridge->ipfix);
> >>> + dpif_lsample_unref(xbridge->lsample);
> >>> netflow_unref(xbridge->netflow);
> >>> stp_unref(xbridge->stp);
> >>> rstp_unref(xbridge->rstp);
> >>> @@ -3357,58 +3369,92 @@ xlate_normal(struct xlate_ctx *ctx)
> >>> }
> >>> }
> >>>
> >>> -/* Appends a "sample" action for sFlow or IPFIX to 'ctx->odp_actions'.
> >>> The
> >>> - * 'probability' is the number of packets out of UINT32_MAX to sample.
> >>> The
> >>> - * 'cookie' is passed back in the callback for each sampled packet.
> >>> - * 'tunnel_out_port', if not ODPP_NONE, is added as the
> >>> - * OVS_USERSPACE_ATTR_EGRESS_TUN_PORT attribute. If 'include_actions',
> >>> - * an OVS_USERSPACE_ATTR_ACTIONS attribute is added. If
> >>> - * 'emit_set_tunnel', sample(sampling_port=1) would translate into
> >>> - * datapath sample action set(tunnel(...)), sample(...) and it is used
> >>> - * for sampling egress tunnel information.
> >>> - */
> >>> +/* Psample-related arguments for compose_sample_action. */
> >>> +struct sample_psample_args {
> >>> + uint32_t group_id; /* Group to be used in psample.
> >>> */
> >>> + struct ofpbuf cookie; /* Cookie to be used in psample.
> >>> */
> >>> +};
> >>> +
> >>> +/* Userspace-related arguments for compose_sample_action. */
> >>> +struct sample_userspace_args {
> >>> + struct user_action_cookie cookie; /* Data passed back in the upcall
> >>> + * for each sampled packet. */
> >>> + odp_port_t tunnel_out_port; /* If not ODPP_NONE, it is added
> >>> in
> >>> + *
> >>> OVS_USERSPACE_ATTR_EGRESS_TUN_PORT
> >>> + * attribute. */
> >>> + bool include_actions; /* Whether
> >>> OVS_USERSPACE_ATTR_ACTIONS
> >>> + * is to be set. */
> >>> +
> >>> +};
> >>> +
> >>> +/* Arguments for compose_sample_action. */
> >>> +struct compose_sample_args {
> >>> + uint32_t probability; /* Number of packets out of
> >>> + * UINT32_MAX to sample. */
> >>> + struct sample_userspace_args *userspace; /* Optional,
> >>> + * arguments for
> >>> userspace. */
> >>> + struct sample_psample_args *psample; /* Optional,
> >>> + * arguments for psample.
> >>> */
> >>> +};
> >>> +
> >>> +/* Composes sample action according to 'args'. */
> >>> static size_t
> >>> compose_sample_action(struct xlate_ctx *ctx,
> >>> - const uint32_t probability,
> >>> - const struct user_action_cookie *cookie,
> >>> - const odp_port_t tunnel_out_port,
> >>> - bool include_actions)
> >>> + const struct compose_sample_args *args)
> >>> {
> >>> - if (probability == 0) {
> >>> + if (args->probability == 0) {
> >>> /* No need to generate sampling or the inner action. */
> >>> return 0;
> >>> }
> >>>
> >>> + /* At least one of userspace or psample config must be provided. */
> >>> + ovs_assert(args->userspace || args->psample);
> >>> +
> >>> /* If the slow path meter is configured by the controller,
> >>> * insert a meter action before the user space action. */
> >>> struct ofproto *ofproto = &ctx->xin->ofproto->up;
> >>> uint32_t meter_id = ofproto->slowpath_meter_id;
> >>> + size_t cookie_offset = 0;
> >>> +
> >>> + /* The meter action is only used to throttle userspace actions.
> >>> + * If they are not needed and the sampling rate is 100%, avoid
> >>> generating
> >>> + * a sample action. */
> >>> + bool is_sample = (args->probability < UINT32_MAX ||
> >>> + (args->userspace && meter_id != UINT32_MAX));
> >>>
> >>> - /* When meter action is not required, avoid generate sample action
> >>> - * for 100% sampling rate. */
> >>> - bool is_sample = probability < UINT32_MAX || meter_id != UINT32_MAX;
> >>> size_t sample_offset = 0, actions_offset = 0;
> >>> if (is_sample) {
> >>> sample_offset = nl_msg_start_nested(ctx->odp_actions,
> >>> OVS_ACTION_ATTR_SAMPLE);
> >>> nl_msg_put_u32(ctx->odp_actions, OVS_SAMPLE_ATTR_PROBABILITY,
> >>> - probability);
> >>> + args->probability);
> >>> actions_offset = nl_msg_start_nested(ctx->odp_actions,
> >>> OVS_SAMPLE_ATTR_ACTIONS);
> >>> }
> >>>
> >>> - if (meter_id != UINT32_MAX) {
> >>> - nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_METER,
> >>> meter_id);
> >>> + if (args->psample) {
> >>> + odp_put_psample_action(ctx->odp_actions,
> >>> + args->psample->group_id,
> >>> + args->psample->cookie.data,
> >>> + args->psample->cookie.size);
> >>> + }
> >>> +
> >>> + if (args->userspace) {
> >>> + if (meter_id != UINT32_MAX) {
> >>> + nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_METER,
> >>> meter_id);
> >>> + }
> >>> +
> >>> + odp_port_t odp_port = ofp_port_to_odp_port(
> >>> + ctx->xbridge, ctx->xin->flow.in_port.ofp_port);
> >>> + uint32_t pid = dpif_port_get_pid(ctx->xbridge->dpif, odp_port);
> >>> + int res = odp_put_userspace_action(pid, &args->userspace->cookie,
> >>> + sizeof
> >>> args->userspace->cookie,
> >>> +
> >>> args->userspace->tunnel_out_port,
> >>> +
> >>> args->userspace->include_actions,
> >>> + ctx->odp_actions,
> >>> &cookie_offset);
> >>> + ovs_assert(res == 0);
> >>> }
> >>>
> >>> - odp_port_t odp_port = ofp_port_to_odp_port(
> >>> - ctx->xbridge, ctx->xin->flow.in_port.ofp_port);
> >>> - uint32_t pid = dpif_port_get_pid(ctx->xbridge->dpif, odp_port);
> >>> - size_t cookie_offset;
> >>> - int res = odp_put_userspace_action(pid, cookie, sizeof *cookie,
> >>> - tunnel_out_port, include_actions,
> >>> - ctx->odp_actions, &cookie_offset);
> >>> - ovs_assert(res == 0);
> >>> if (is_sample) {
> >>> nl_msg_end_nested(ctx->odp_actions, actions_offset);
> >>> nl_msg_end_nested(ctx->odp_actions, sample_offset);
> >>> @@ -3428,19 +3474,23 @@ static size_t
> >>> compose_sflow_action(struct xlate_ctx *ctx)
> >>> {
> >>> struct dpif_sflow *sflow = ctx->xbridge->sflow;
> >>> + struct sample_userspace_args userspace = {};
> >>> + struct compose_sample_args args = {};
> >>> +
> >>> if (!sflow || ctx->xin->flow.in_port.ofp_port == OFPP_NONE) {
> >>> return 0;
> >>> }
> >>>
> >>> - struct user_action_cookie cookie;
> >>> + userspace.cookie.type = USER_ACTION_COOKIE_SFLOW;
> >>> + userspace.cookie.ofp_in_port = ctx->xin->flow.in_port.ofp_port;
> >>> + userspace.cookie.ofproto_uuid = ctx->xbridge->ofproto->uuid;
> >>> + userspace.tunnel_out_port = ODPP_NONE;
> >>> + userspace.include_actions = true;
> >>>
> >>> - memset(&cookie, 0, sizeof cookie);
> >>> - cookie.type = USER_ACTION_COOKIE_SFLOW;
> >>> - cookie.ofp_in_port = ctx->xin->flow.in_port.ofp_port;
> >>> - cookie.ofproto_uuid = ctx->xbridge->ofproto->uuid;
> >>> + args.probability = dpif_sflow_get_probability(sflow);
> >>> + args.userspace = &userspace;
> >>>
> >>> - return compose_sample_action(ctx, dpif_sflow_get_probability(sflow),
> >>> - &cookie, ODPP_NONE, true);
> >>> + return compose_sample_action(ctx, &args);
> >>> }
> >>>
> >>> /* If flow IPFIX is enabled, make sure IPFIX flow sample action
> >>> @@ -3451,7 +3501,10 @@ static void
> >>> compose_ipfix_action(struct xlate_ctx *ctx, odp_port_t output_odp_port)
> >>> {
> >>> struct dpif_ipfix *ipfix = ctx->xbridge->ipfix;
> >>> - odp_port_t tunnel_out_port = ODPP_NONE;
> >>> + struct sample_userspace_args userspace = {0};
> >>> + struct compose_sample_args args = {0};
> >>> +
> >>> + userspace.tunnel_out_port = ODPP_NONE;
> >>>
> >>> if (!ipfix ||
> >>> (output_odp_port == ODPP_NONE &&
> >>> @@ -3476,21 +3529,20 @@ compose_ipfix_action(struct xlate_ctx *ctx,
> >>> odp_port_t output_odp_port)
> >>> */
> >>> if (dpif_ipfix_get_bridge_exporter_tunnel_sampling(ipfix) &&
> >>> dpif_ipfix_is_tunnel_port(ipfix, output_odp_port) ) {
> >>> - tunnel_out_port = output_odp_port;
> >>> + userspace.tunnel_out_port = output_odp_port;
> >>> }
> >>> }
> >>>
> >>> - struct user_action_cookie cookie;
> >>> + userspace.cookie.type = USER_ACTION_COOKIE_IPFIX;
> >>> + userspace.cookie.ofp_in_port = ctx->xin->flow.in_port.ofp_port;
> >>> + userspace.cookie.ofproto_uuid = ctx->xbridge->ofproto->uuid;
> >>> + userspace.cookie.ipfix.output_odp_port = output_odp_port;
> >>> + userspace.include_actions = false;
> >>>
> >>> - memset(&cookie, 0, sizeof cookie);
> >>> - cookie.type = USER_ACTION_COOKIE_IPFIX;
> >>> - cookie.ofp_in_port = ctx->xin->flow.in_port.ofp_port;
> >>> - cookie.ofproto_uuid = ctx->xbridge->ofproto->uuid;
> >>> - cookie.ipfix.output_odp_port = output_odp_port;
> >>> + args.probability = dpif_ipfix_get_bridge_exporter_probability(ipfix);
> >>> + args.userspace = &userspace;
> >>>
> >>> - compose_sample_action(ctx,
> >>> -
> >>> dpif_ipfix_get_bridge_exporter_probability(ipfix),
> >>> - &cookie, tunnel_out_port, false);
> >>> + compose_sample_action(ctx, &args);
> >>> }
> >>>
> >>> /* Fix "sample" action according to data collected while composing ODP
> >>> actions,
> >>> @@ -5847,22 +5899,16 @@ xlate_fin_timeout(struct xlate_ctx *ctx,
> >>> }
> >>>
> >>> static void
> >>> -xlate_sample_action(struct xlate_ctx *ctx,
> >>> - const struct ofpact_sample *os)
> >>> +xlate_fill_ipfix_sample(struct xlate_ctx *ctx,
> >>> + const struct ofpact_sample *os,
> >>> + const struct dpif_ipfix *ipfix,
> >>> + struct sample_userspace_args *userspace)
> >>> {
> >>> odp_port_t output_odp_port = ODPP_NONE;
> >>> - odp_port_t tunnel_out_port = ODPP_NONE;
> >>> - struct dpif_ipfix *ipfix = ctx->xbridge->ipfix;
> >>> bool emit_set_tunnel = false;
> >>>
> >>> - if (!ipfix) {
> >>> - return;
> >>> - }
> >>> -
> >>> - /* Scale the probability from 16-bit to 32-bit while representing
> >>> - * the same percentage. */
> >>> - uint32_t probability =
> >>> - ((uint32_t) os->probability << 16) | os->probability;
> >>> + memset(userspace, 0, sizeof *userspace);
> >>> + userspace->tunnel_out_port = ODPP_NONE;
> >>>
> >>> /* If ofp_port in flow sample action is equel to ofp_port,
> >>> * this sample action is a input port action. */
> >>> @@ -5879,7 +5925,7 @@ xlate_sample_action(struct xlate_ctx *ctx,
> >>> if (dpif_ipfix_get_flow_exporter_tunnel_sampling(ipfix,
> >>>
> >>> os->collector_set_id)
> >>> && dpif_ipfix_is_tunnel_port(ipfix, output_odp_port)) {
> >>> - tunnel_out_port = output_odp_port;
> >>> + userspace->tunnel_out_port = output_odp_port;
> >>> emit_set_tunnel = true;
> >>> }
> >>> }
> >>> @@ -5913,20 +5959,71 @@ xlate_sample_action(struct xlate_ctx *ctx,
> >>> }
> >>> }
> >>>
> >>> - struct user_action_cookie cookie;
> >>> + userspace->cookie.type = USER_ACTION_COOKIE_FLOW_SAMPLE;
> >>> + userspace->cookie.ofp_in_port = ctx->xin->flow.in_port.ofp_port;
> >>> + 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.output_odp_port = output_odp_port;
> >>> + userspace->cookie.flow_sample.direction = os->direction;
> >>> + userspace->include_actions = false;
> >>> +}
> >>>
> >>> - memset(&cookie, 0, sizeof cookie);
> >>> - cookie.type = USER_ACTION_COOKIE_FLOW_SAMPLE;
> >>> - cookie.ofp_in_port = ctx->xin->flow.in_port.ofp_port;
> >>> - cookie.ofproto_uuid = ctx->xbridge->ofproto->uuid;
> >>> - cookie.flow_sample.probability = os->probability;
> >>> - cookie.flow_sample.collector_set_id = os->collector_set_id;
> >>> - cookie.flow_sample.obs_domain_id = os->obs_domain_id;
> >>> - cookie.flow_sample.obs_point_id = os->obs_point_id;
> >>> - cookie.flow_sample.output_odp_port = output_odp_port;
> >>> - cookie.flow_sample.direction = os->direction;
> >>> -
> >>> - compose_sample_action(ctx, probability, &cookie, tunnel_out_port,
> >>> false);
> >>> +static void
> >>> +xlate_sample_action(struct xlate_ctx *ctx,
> >>> + const struct ofpact_sample *os)
> >>> +{
> >>> + uint8_t cookie_buf[sizeof(os->obs_domain_id) +
> >>> sizeof(os->obs_point_id)];
> >>> + struct dpif_lsample *lsample = ctx->xbridge->lsample;
> >>> + struct dpif_ipfix *ipfix = ctx->xbridge->ipfix;
> >>> + struct compose_sample_args compose_args = {};
> >>> + struct sample_userspace_args userspace;
> >>> + struct sample_psample_args psample;
> >>> +
> >>> + if (!ipfix && !lsample) {
> >>> + return;
> >>> + }
> >>> +
> >>> + /* Scale the probability from 16-bit to 32-bit while representing
> >>> + * the same percentage. */
> >>> + compose_args.probability =
> >>> + ((uint32_t) os->probability << 16) | os->probability;
> >>> +
> >>> + if (ipfix) {
> >>> + xlate_fill_ipfix_sample(ctx, os, ipfix, &userspace);
> >>> + compose_args.userspace = &userspace;
> >>> + }
> >>> +
> >>> + if (lsample &&
> >>> + dpif_lsample_get_group_id(lsample,
> >>> + os->collector_set_id,
> >>> + &psample.group_id)) {
> >>
> >> This part will have to be re-worked if we'll add a different
> >> local smapling action in the userspace datapath, for example.
> >>
> >> I think, we should check which datapath action is supported here
> >> and bail. We shouldn't deligate desicion making to odp-util.
> >
> > If the datapath action is not supported, "ofproto_set_local_sample" will
> > fail and xbridge->psample will be NULL, right?
>
> True. But this code assumes that lsample is always implemented
> via psample. Maybe it's not a problem. I guess, this can be
> changed later if we'll need a different implementation.
>
That's right. When we add a different implementation we'd need to change
this function. We could check what is supported here or maybe we can
"cache" what implementation shall be used in the dpif_lsample object.
I'm not sure at this point in what will be cleaner.
For the time being, do you think we should refactor this to prepare for
the alternative implementation?
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
