On 5 Jun 2024, at 22:23, Adrian Moreno wrote:
> Only kernel datapath supports this action so add a function in dpif.c
> that checks for that.
>
> Signed-off-by: Adrian Moreno <amore...@redhat.com>
Small nits below, other than that the patches looks good to me.
//Eelco
Acked-by: Eelco Chaudron <echau...@redhat.com>
> ---
> lib/dpif.c | 7 +++++++
> lib/dpif.h | 1 +
> ofproto/ofproto-dpif.c | 45 ++++++++++++++++++++++++++++++++++++++++++
> ofproto/ofproto-dpif.h | 6 +++++-
> 4 files changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 489d6a095..ecba967c3 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -1953,6 +1953,13 @@ dpif_supports_lb_output_action(const struct dpif *dpif)
> return dpif_is_netdev(dpif);
> }
>
> +bool
> +dpif_may_support_emit_sample(const struct dpif *dpif)
> +{
> + /* Userspace datapath does not support this action. */
> + return !dpif_is_netdev(dpif);
> +}
> +
> /* Meters */
> void
> dpif_meter_get_features(const struct dpif *dpif,
> diff --git a/lib/dpif.h b/lib/dpif.h
> index a764e8a59..08473ea6f 100644
> --- a/lib/dpif.h
> +++ b/lib/dpif.h
> @@ -942,6 +942,7 @@ char *dpif_get_dp_version(const struct dpif *);
> bool dpif_supports_tnl_push_pop(const struct dpif *);
> bool dpif_may_support_explicit_drop_action(const struct dpif *);
> bool dpif_synced_dp_layers(struct dpif *);
> +bool dpif_may_support_emit_sample(const struct dpif *);
Maybe add this above dpif_may_support_explicit_drop_action()?
> /* Log functions. */
> struct vlog_module;
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 32d037be6..035479285 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -873,6 +873,12 @@ ovs_lb_output_action_supported(struct ofproto_dpif
> *ofproto)
> return ofproto->backer->rt_support.lb_output_action;
> }
>
> +bool
> +ovs_emit_sample_supported(struct ofproto_dpif *ofproto)
> +{
> + return ofproto->backer->rt_support.emit_sample;
> +}
> +
> /* Tests whether 'backer''s datapath supports recirculation. Only newer
> * datapaths support OVS_KEY_ATTR_RECIRC_ID in keys. We need to disable some
> * features on older datapaths that don't support this feature.
> @@ -1609,6 +1615,44 @@ check_add_mpls(struct dpif_backer *backer)
> return supported;
> }
>
> +/* Tests whether 'backer''s datapath supports the OVS_ACTION_ATTR_EMIT_SAMPLE
> + * action. */
> +static bool
> +check_emit_sample(struct dpif_backer *backer)
> +{
> + uint8_t cookie[OVS_EMIT_SAMPLE_COOKIE_MAX_SIZE];
> + struct odputil_keybuf keybuf;
> + struct ofpbuf actions;
> + struct ofpbuf key;
> + struct flow flow;
> + bool supported;
> +
> + struct odp_flow_key_parms odp_parms = {
> + .flow = &flow,
> + .probe = true,
> + };
> +
> + memset(&flow, 0, sizeof flow);
> + ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
> + odp_flow_key_from_flow(&odp_parms, &key);
> + ofpbuf_init(&actions, 64);
> +
> + /* Generate a random max-size cookie. */
> + random_bytes(cookie, sizeof(cookie));
> +
> + odp_put_emit_sample_action(&actions, 10, cookie, sizeof cookie);
> +
> + supported = dpif_may_support_emit_sample(backer->dpif) &&
> + dpif_probe_feature(backer->dpif, "emit_sample", &key, &actions,
> NULL);
> +
> + ofpbuf_uninit(&actions);
> + VLOG_INFO("%s: Datapath %s emit_sample",
> + dpif_name(backer->dpif),
> + supported ? "supports" : "does not support");
> + return supported;
> +}
> +
> +
Guess a single new line would be enough.
> #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE) \
> static bool \
> check_##NAME(struct dpif_backer *backer) \
> @@ -1698,6 +1742,7 @@ check_support(struct dpif_backer *backer)
> dpif_supports_lb_output_action(backer->dpif);
> backer->rt_support.ct_zero_snat = dpif_supports_ct_zero_snat(backer);
> backer->rt_support.add_mpls = check_add_mpls(backer);
> + backer->rt_support.emit_sample = check_emit_sample(backer);
>
> /* Flow fields. */
> backer->rt_support.odp.ct_state = check_ct_state(backer);
> diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
> index d33f73df8..ae6568463 100644
> --- a/ofproto/ofproto-dpif.h
> +++ b/ofproto/ofproto-dpif.h
> @@ -213,7 +213,10 @@ struct group_dpif *group_dpif_lookup(struct ofproto_dpif
> *,
> DPIF_SUPPORT_FIELD(bool, ct_zero_snat, "Conntrack all-zero IP SNAT") \
> \
> /* True if the datapath supports add_mpls action. */ \
> - DPIF_SUPPORT_FIELD(bool, add_mpls, "MPLS Label add")
> + DPIF_SUPPORT_FIELD(bool, add_mpls, "MPLS Label add") \
> + \
> + /* True if the datapath supports emit_sample action. */ \
> + DPIF_SUPPORT_FIELD(bool, emit_sample, "emit_sample")
>
>
> /* Stores the various features which the corresponding backer supports. */
> @@ -411,5 +414,6 @@ bool ofproto_dpif_ct_zone_timeout_policy_get_name(
> uint8_t nw_proto, char **tp_name, bool *unwildcard);
>
> bool ovs_explicit_drop_action_supported(struct ofproto_dpif *);
> +bool ovs_emit_sample_supported(struct ofproto_dpif *);
nit: Move this above ovs_explicit_drop_action_supported()
> #endif /* ofproto-dpif.h */
> --
> 2.45.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
