Re: [ovs-dev] [PATCH v1 03/13] ofproto_dpif: Check for psample support.
On Wed, Jul 10, 2024 at 01:28:29PM GMT, Ilya Maximets wrote:
> On 7/7/24 22:08, 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
> > ---
> > lib/dpif.c | 7 +++
> > lib/dpif.h | 1 +
> > ofproto/ofproto-dpif.c | 43 ++
> > ofproto/ofproto-dpif.h | 7 ++-
> > 4 files changed, 57 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/dpif.c b/lib/dpif.c
> > index 71728badc..0a964ba89 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_psample(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..6bef7d5b3 100644
> > --- a/lib/dpif.h
> > +++ b/lib/dpif.h
> > @@ -941,6 +941,7 @@ int dpif_get_pmds_for_port(const struct dpif * dpif,
> > odp_port_t port_no,
> > 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_may_support_psample(const struct dpif *);
> > bool dpif_synced_dp_layers(struct dpif *);
> >
> > /* Log functions. */
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index 4712d10a8..94c84d697 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_psample_supported(struct ofproto_dpif *ofproto)
> > +{
> > +return ofproto->backer->rt_support.psample;
> > +}
> > +
> > /* 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,42 @@ check_add_mpls(struct dpif_backer *backer)
> > return supported;
> > }
> >
> > +/* Tests whether 'backer''s datapath supports the OVS_ACTION_ATTR_PSAMPLE
> > + * action. */
> > +static bool
> > +check_psample(struct dpif_backer *backer)
> > +{
> > +uint8_t cookie[OVS_PSAMPLE_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));
>
> Don't parenthesize the argument of sizeof.
>
Ack.
> > +
> > +odp_put_psample_action(&actions, 10, cookie, sizeof cookie);
> > +
> > +supported = dpif_may_support_psample(backer->dpif) &&
> > +dpif_probe_feature(backer->dpif, "psample", &key, &actions, NULL);
>
> This function actually installs the flow into the kernel.
> You need to make sure that no packet can match it, e.g.
> by setting a bogus eth type as a match. Some existing
> probe functions require specific matches, some are just
> missing this part as well, but we should not blindly copy
> bad patterns.
>
I guess I was always thinking on a freshly started system but you're
right! If OVS is restarted, the installed flow can affect actual
traffic. I'll add a bogus key to make sure it's not problematic.
> > +
> > +ofpbuf_uninit(&actions);
> > +VLOG_INFO("%s: Datapath %s psample", dpif_name(backer->dpif),
>
> '%s: Datapath %s psample action' will be more clear.
>
Ack.
> > + supported ? "supports" : "does not support");
> > +return supported;
> > +}
> > +
> > #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE)
> > \
> > static bool
> > \
> > check_##NAME(struct dpif_backer *backer)
> > \
> > @@ -1698,6 +1740,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.psample = check_psample(backer);
> >
> > /* Flow fields. */
> > backer->rt_support.odp.ct_state = check_ct_state(backer);
> > diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofprot
Re: [ovs-dev] [PATCH v1 03/13] ofproto_dpif: Check for psample support.
On 7/7/24 22:08, 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
> ---
> lib/dpif.c | 7 +++
> lib/dpif.h | 1 +
> ofproto/ofproto-dpif.c | 43 ++
> ofproto/ofproto-dpif.h | 7 ++-
> 4 files changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 71728badc..0a964ba89 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_psample(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..6bef7d5b3 100644
> --- a/lib/dpif.h
> +++ b/lib/dpif.h
> @@ -941,6 +941,7 @@ int dpif_get_pmds_for_port(const struct dpif * dpif,
> odp_port_t port_no,
> 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_may_support_psample(const struct dpif *);
> bool dpif_synced_dp_layers(struct dpif *);
>
> /* Log functions. */
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 4712d10a8..94c84d697 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_psample_supported(struct ofproto_dpif *ofproto)
> +{
> +return ofproto->backer->rt_support.psample;
> +}
> +
> /* 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,42 @@ check_add_mpls(struct dpif_backer *backer)
> return supported;
> }
>
> +/* Tests whether 'backer''s datapath supports the OVS_ACTION_ATTR_PSAMPLE
> + * action. */
> +static bool
> +check_psample(struct dpif_backer *backer)
> +{
> +uint8_t cookie[OVS_PSAMPLE_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));
Don't parenthesize the argument of sizeof.
> +
> +odp_put_psample_action(&actions, 10, cookie, sizeof cookie);
> +
> +supported = dpif_may_support_psample(backer->dpif) &&
> +dpif_probe_feature(backer->dpif, "psample", &key, &actions, NULL);
This function actually installs the flow into the kernel.
You need to make sure that no packet can match it, e.g.
by setting a bogus eth type as a match. Some existing
probe functions require specific matches, some are just
missing this part as well, but we should not blindly copy
bad patterns.
> +
> +ofpbuf_uninit(&actions);
> +VLOG_INFO("%s: Datapath %s psample", dpif_name(backer->dpif),
'%s: Datapath %s psample action' will be more clear.
> + supported ? "supports" : "does not support");
> +return supported;
> +}
> +
> #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE) \
> static bool \
> check_##NAME(struct dpif_backer *backer)\
> @@ -1698,6 +1740,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.psample = check_psample(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..bc7a19dab 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,
Re: [ovs-dev] [PATCH v1 03/13] ofproto_dpif: Check for psample support.
On Tue, Jul 09, 2024 at 11:45:17AM GMT, Eelco Chaudron wrote:
> On 4 Jul 2024, at 9:52, Adrian Moreno wrote:
>
> > Only kernel datapath supports this action so add a function in dpif.c
> > that checks for that.
>
> One small nit below, rest looks good. If this is the only change in the
> next series you can add my ACK.
>
> //Eelco
>
> > Signed-off-by: Adrian Moreno
> > ---
> > lib/dpif.c | 7 +++
> > lib/dpif.h | 1 +
> > ofproto/ofproto-dpif.c | 43 ++
> > ofproto/ofproto-dpif.h | 7 ++-
> > 4 files changed, 57 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/dpif.c b/lib/dpif.c
> > index 71728badc..0a964ba89 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_psample(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..6bef7d5b3 100644
> > --- a/lib/dpif.h
> > +++ b/lib/dpif.h
> > @@ -941,6 +941,7 @@ int dpif_get_pmds_for_port(const struct dpif * dpif,
> > odp_port_t port_no,
> > 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_may_support_psample(const struct dpif *);
> > bool dpif_synced_dp_layers(struct dpif *);
> >
> > /* Log functions. */
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index 4712d10a8..94c84d697 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_psample_supported(struct ofproto_dpif *ofproto)
> > +{
> > +return ofproto->backer->rt_support.psample;
> > +}
> > +
> > /* 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,42 @@ check_add_mpls(struct dpif_backer *backer)
> > return supported;
> > }
> >
> > +/* Tests whether 'backer''s datapath supports the OVS_ACTION_ATTR_PSAMPLE
> > + * action. */
> > +static bool
> > +check_psample(struct dpif_backer *backer)
> > +{
> > +uint8_t cookie[OVS_PSAMPLE_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_psample_action(&actions, 10, cookie, sizeof cookie);
> > +
> > +supported = dpif_may_support_psample(backer->dpif) &&
> > +dpif_probe_feature(backer->dpif, "psample", &key, &actions, NULL);
> > +
> > +ofpbuf_uninit(&actions);
> > +VLOG_INFO("%s: Datapath %s psample", dpif_name(backer->dpif),
> > + supported ? "supports" : "does not support");
> > +return supported;
> > +}
> > +
> > #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE)
> > \
> > static bool
> > \
> > check_##NAME(struct dpif_backer *backer)
> > \
> > @@ -1698,6 +1740,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.psample = check_psample(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..bc7a19dab 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_mp
Re: [ovs-dev] [PATCH v1 03/13] ofproto_dpif: Check for psample support.
On 4 Jul 2024, at 9:52, Adrian Moreno wrote:
> Only kernel datapath supports this action so add a function in dpif.c
> that checks for that.
One small nit below, rest looks good. If this is the only change in the
next series you can add my ACK.
//Eelco
> Signed-off-by: Adrian Moreno
> ---
> lib/dpif.c | 7 +++
> lib/dpif.h | 1 +
> ofproto/ofproto-dpif.c | 43 ++
> ofproto/ofproto-dpif.h | 7 ++-
> 4 files changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 71728badc..0a964ba89 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_psample(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..6bef7d5b3 100644
> --- a/lib/dpif.h
> +++ b/lib/dpif.h
> @@ -941,6 +941,7 @@ int dpif_get_pmds_for_port(const struct dpif * dpif,
> odp_port_t port_no,
> 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_may_support_psample(const struct dpif *);
> bool dpif_synced_dp_layers(struct dpif *);
>
> /* Log functions. */
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 4712d10a8..94c84d697 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_psample_supported(struct ofproto_dpif *ofproto)
> +{
> +return ofproto->backer->rt_support.psample;
> +}
> +
> /* 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,42 @@ check_add_mpls(struct dpif_backer *backer)
> return supported;
> }
>
> +/* Tests whether 'backer''s datapath supports the OVS_ACTION_ATTR_PSAMPLE
> + * action. */
> +static bool
> +check_psample(struct dpif_backer *backer)
> +{
> +uint8_t cookie[OVS_PSAMPLE_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_psample_action(&actions, 10, cookie, sizeof cookie);
> +
> +supported = dpif_may_support_psample(backer->dpif) &&
> +dpif_probe_feature(backer->dpif, "psample", &key, &actions, NULL);
> +
> +ofpbuf_uninit(&actions);
> +VLOG_INFO("%s: Datapath %s psample", dpif_name(backer->dpif),
> + supported ? "supports" : "does not support");
> +return supported;
> +}
> +
> #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE) \
> static bool \
> check_##NAME(struct dpif_backer *backer)\
> @@ -1698,6 +1740,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.psample = check_psample(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..bc7a19dab 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 psample action. */ \
> +DPIF_SUPPORT_FIELD(bool, psample, "psample")
>
>
> /* Stores the various features
Re: [ovs-dev] [PATCH v1 03/13] ofproto_dpif: Check for psample support.
On Mon, Jul 08, 2024 at 12:10:15PM GMT, Eelco Chaudron wrote:
>
>
> On 8 Jul 2024, at 10:37, Adrián Moreno wrote:
>
> > On Sun, Jul 07, 2024 at 10:08:55PM GMT, 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
> >> ---
> >> lib/dpif.c | 7 +++
> >> lib/dpif.h | 1 +
> >> ofproto/ofproto-dpif.c | 43 ++
> >> ofproto/ofproto-dpif.h | 7 ++-
> >> 4 files changed, 57 insertions(+), 1 deletion(-)
> >>
> >
> > After Dumitru's comment on the last patch, I think we also need a
> > capability for this exposed in the OVSDB. I'll add it in the next
> > version.
>
> ACK, can you wait with the v2 until I review the RFC/v1?
Sure, thanks.
>
> >> diff --git a/lib/dpif.c b/lib/dpif.c
> >> index 71728badc..0a964ba89 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_psample(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..6bef7d5b3 100644
> >> --- a/lib/dpif.h
> >> +++ b/lib/dpif.h
> >> @@ -941,6 +941,7 @@ int dpif_get_pmds_for_port(const struct dpif * dpif,
> >> odp_port_t port_no,
> >> 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_may_support_psample(const struct dpif *);
> >> bool dpif_synced_dp_layers(struct dpif *);
> >>
> >> /* Log functions. */
> >> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> >> index 4712d10a8..94c84d697 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_psample_supported(struct ofproto_dpif *ofproto)
> >> +{
> >> +return ofproto->backer->rt_support.psample;
> >> +}
> >> +
> >> /* 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,42 @@ check_add_mpls(struct dpif_backer *backer)
> >> return supported;
> >> }
> >>
> >> +/* Tests whether 'backer''s datapath supports the OVS_ACTION_ATTR_PSAMPLE
> >> + * action. */
> >> +static bool
> >> +check_psample(struct dpif_backer *backer)
> >> +{
> >> +uint8_t cookie[OVS_PSAMPLE_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_psample_action(&actions, 10, cookie, sizeof cookie);
> >> +
> >> +supported = dpif_may_support_psample(backer->dpif) &&
> >> +dpif_probe_feature(backer->dpif, "psample", &key, &actions, NULL);
> >> +
> >> +ofpbuf_uninit(&actions);
> >> +VLOG_INFO("%s: Datapath %s psample", dpif_name(backer->dpif),
> >> + supported ? "supports" : "does not support");
> >> +return supported;
> >> +}
> >> +
> >> #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE)
> >> \
> >> static bool
> >> \
> >> check_##NAME(struct dpif_backer *backer)
> >> \
> >> @@ -1698,6 +1740,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.psample = check_psample(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..bc7a19dab 100644
> >> --- a/ofproto/ofproto-dpif.h
> >> +++ b/ofproto/ofproto-dpif.h
> >> @@ -213,7 +213,10 @@ struct group_dpif *group_dpif_lookup(struct
> >> ofproto_dpif
Re: [ovs-dev] [PATCH v1 03/13] ofproto_dpif: Check for psample support.
On 8 Jul 2024, at 10:37, Adrián Moreno wrote:
> On Sun, Jul 07, 2024 at 10:08:55PM GMT, 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
>> ---
>> lib/dpif.c | 7 +++
>> lib/dpif.h | 1 +
>> ofproto/ofproto-dpif.c | 43 ++
>> ofproto/ofproto-dpif.h | 7 ++-
>> 4 files changed, 57 insertions(+), 1 deletion(-)
>>
>
> After Dumitru's comment on the last patch, I think we also need a
> capability for this exposed in the OVSDB. I'll add it in the next
> version.
ACK, can you wait with the v2 until I review the RFC/v1?
>> diff --git a/lib/dpif.c b/lib/dpif.c
>> index 71728badc..0a964ba89 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_psample(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..6bef7d5b3 100644
>> --- a/lib/dpif.h
>> +++ b/lib/dpif.h
>> @@ -941,6 +941,7 @@ int dpif_get_pmds_for_port(const struct dpif * dpif,
>> odp_port_t port_no,
>> 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_may_support_psample(const struct dpif *);
>> bool dpif_synced_dp_layers(struct dpif *);
>>
>> /* Log functions. */
>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> index 4712d10a8..94c84d697 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_psample_supported(struct ofproto_dpif *ofproto)
>> +{
>> +return ofproto->backer->rt_support.psample;
>> +}
>> +
>> /* 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,42 @@ check_add_mpls(struct dpif_backer *backer)
>> return supported;
>> }
>>
>> +/* Tests whether 'backer''s datapath supports the OVS_ACTION_ATTR_PSAMPLE
>> + * action. */
>> +static bool
>> +check_psample(struct dpif_backer *backer)
>> +{
>> +uint8_t cookie[OVS_PSAMPLE_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_psample_action(&actions, 10, cookie, sizeof cookie);
>> +
>> +supported = dpif_may_support_psample(backer->dpif) &&
>> +dpif_probe_feature(backer->dpif, "psample", &key, &actions, NULL);
>> +
>> +ofpbuf_uninit(&actions);
>> +VLOG_INFO("%s: Datapath %s psample", dpif_name(backer->dpif),
>> + supported ? "supports" : "does not support");
>> +return supported;
>> +}
>> +
>> #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE)
>> \
>> static bool
>> \
>> check_##NAME(struct dpif_backer *backer)
>> \
>> @@ -1698,6 +1740,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.psample = check_psample(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..bc7a19dab 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_SUPPO
Re: [ovs-dev] [PATCH v1 03/13] ofproto_dpif: Check for psample support.
On Sun, Jul 07, 2024 at 10:08:55PM GMT, 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
> ---
> lib/dpif.c | 7 +++
> lib/dpif.h | 1 +
> ofproto/ofproto-dpif.c | 43 ++
> ofproto/ofproto-dpif.h | 7 ++-
> 4 files changed, 57 insertions(+), 1 deletion(-)
>
After Dumitru's comment on the last patch, I think we also need a
capability for this exposed in the OVSDB. I'll add it in the next
version.
Thanks.
Adrián
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 71728badc..0a964ba89 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_psample(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..6bef7d5b3 100644
> --- a/lib/dpif.h
> +++ b/lib/dpif.h
> @@ -941,6 +941,7 @@ int dpif_get_pmds_for_port(const struct dpif * dpif,
> odp_port_t port_no,
> 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_may_support_psample(const struct dpif *);
> bool dpif_synced_dp_layers(struct dpif *);
>
> /* Log functions. */
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 4712d10a8..94c84d697 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_psample_supported(struct ofproto_dpif *ofproto)
> +{
> +return ofproto->backer->rt_support.psample;
> +}
> +
> /* 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,42 @@ check_add_mpls(struct dpif_backer *backer)
> return supported;
> }
>
> +/* Tests whether 'backer''s datapath supports the OVS_ACTION_ATTR_PSAMPLE
> + * action. */
> +static bool
> +check_psample(struct dpif_backer *backer)
> +{
> +uint8_t cookie[OVS_PSAMPLE_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_psample_action(&actions, 10, cookie, sizeof cookie);
> +
> +supported = dpif_may_support_psample(backer->dpif) &&
> +dpif_probe_feature(backer->dpif, "psample", &key, &actions, NULL);
> +
> +ofpbuf_uninit(&actions);
> +VLOG_INFO("%s: Datapath %s psample", dpif_name(backer->dpif),
> + supported ? "supports" : "does not support");
> +return supported;
> +}
> +
> #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE) \
> static bool \
> check_##NAME(struct dpif_backer *backer)\
> @@ -1698,6 +1740,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.psample = check_psample(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..bc7a19dab 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 psample action. */ \
> +DPIF_SUPPORT_FIELD(b
[ovs-dev] [PATCH v1 03/13] ofproto_dpif: Check for psample support.
Only kernel datapath supports this action so add a function in dpif.c
that checks for that.
Signed-off-by: Adrian Moreno
---
lib/dpif.c | 7 +++
lib/dpif.h | 1 +
ofproto/ofproto-dpif.c | 43 ++
ofproto/ofproto-dpif.h | 7 ++-
4 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/lib/dpif.c b/lib/dpif.c
index 71728badc..0a964ba89 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_psample(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..6bef7d5b3 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -941,6 +941,7 @@ int dpif_get_pmds_for_port(const struct dpif * dpif,
odp_port_t port_no,
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_may_support_psample(const struct dpif *);
bool dpif_synced_dp_layers(struct dpif *);
/* Log functions. */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 4712d10a8..94c84d697 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_psample_supported(struct ofproto_dpif *ofproto)
+{
+return ofproto->backer->rt_support.psample;
+}
+
/* 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,42 @@ check_add_mpls(struct dpif_backer *backer)
return supported;
}
+/* Tests whether 'backer''s datapath supports the OVS_ACTION_ATTR_PSAMPLE
+ * action. */
+static bool
+check_psample(struct dpif_backer *backer)
+{
+uint8_t cookie[OVS_PSAMPLE_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_psample_action(&actions, 10, cookie, sizeof cookie);
+
+supported = dpif_may_support_psample(backer->dpif) &&
+dpif_probe_feature(backer->dpif, "psample", &key, &actions, NULL);
+
+ofpbuf_uninit(&actions);
+VLOG_INFO("%s: Datapath %s psample", dpif_name(backer->dpif),
+ supported ? "supports" : "does not support");
+return supported;
+}
+
#define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE) \
static bool \
check_##NAME(struct dpif_backer *backer)\
@@ -1698,6 +1740,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.psample = check_psample(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..bc7a19dab 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 psample action. */ \
+DPIF_SUPPORT_FIELD(bool, psample, "psample")
/* Stores the various features which the corresponding backer supports. */
@@ -412,4 +415,6 @@ bool ofproto_dpif_ct_zone_timeout_policy_get_name(
bool ovs_explicit_drop_action_supported(struct ofproto_dpif *);
+bool ovs_psample_supported(struct ofproto_dpif *);
+
#endif /* ofproto-dpif.h */
--
2.45.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailm
