Re: [ovs-dev] [PATCH net-next v2 5/9] net: openvswitch: add emit_sample action

2024-06-18 Thread Ilya Maximets 
On 6/18/24 11:47, Ilya Maximets wrote:
> On 6/18/24 09:33, Adrián Moreno wrote:
>> On Mon, Jun 17, 2024 at 12:44:45PM GMT, Ilya Maximets wrote:
>>> On 6/3/24 20:56, Adrian Moreno wrote:
 Add support for a new action: emit_sample.

 This action accepts a u32 group id and a variable-length cookie and uses
 the psample multicast group to make the packet available for
 observability.

 The maximum length of the user-defined cookie is set to 16, same as
 tc_cookie, to discourage using cookies that will not be offloadable.

 Signed-off-by: Adrian Moreno 
 ---
  Documentation/netlink/specs/ovs_flow.yaml | 17 
  include/uapi/linux/openvswitch.h  | 25 
  net/openvswitch/actions.c | 50 +++
  net/openvswitch/flow_netlink.c| 33 ++-
  4 files changed, 124 insertions(+), 1 deletion(-)
>>>
>>> Some nits below, beside ones already mentioned.
>>>
>>
>> Thanks, Ilya.
>>

 diff --git a/Documentation/netlink/specs/ovs_flow.yaml 
 b/Documentation/netlink/specs/ovs_flow.yaml
 index 4fdfc6b5cae9..a7ab5593a24f 100644
 --- a/Documentation/netlink/specs/ovs_flow.yaml
 +++ b/Documentation/netlink/specs/ovs_flow.yaml
 @@ -727,6 +727,12 @@ attribute-sets:
  name: dec-ttl
  type: nest
  nested-attributes: dec-ttl-attrs
 +  -
 +name: emit-sample
 +type: nest
 +nested-attributes: emit-sample-attrs
 +doc: |
 +  Sends a packet sample to psample for external observation.
-
  name: tunnel-key-attrs
  enum-name: ovs-tunnel-key-attr
 @@ -938,6 +944,17 @@ attribute-sets:
-
  name: gbp
  type: u32
 +  -
 +name: emit-sample-attrs
 +enum-name: ovs-emit-sample-attr
 +name-prefix: ovs-emit-sample-attr-
 +attributes:
 +  -
 +name: group
 +type: u32
 +  -
 +name: cookie
 +type: binary

  operations:
name-prefix: ovs-flow-cmd-
 diff --git a/include/uapi/linux/openvswitch.h 
 b/include/uapi/linux/openvswitch.h
 index efc82c318fa2..a0e9dde0584a 100644
 --- a/include/uapi/linux/openvswitch.h
 +++ b/include/uapi/linux/openvswitch.h
 @@ -914,6 +914,30 @@ struct check_pkt_len_arg {
  };
  #endif

 +#define OVS_EMIT_SAMPLE_COOKIE_MAX_SIZE 16
 +/**
 + * enum ovs_emit_sample_attr - Attributes for %OVS_ACTION_ATTR_EMIT_SAMPLE
 + * action.
 + *
 + * @OVS_EMIT_SAMPLE_ATTR_GROUP: 32-bit number to identify the source of 
 the
 + * sample.
 + * @OVS_EMIT_SAMPLE_ATTR_COOKIE: A variable-length binary cookie that 
 contains
 + * user-defined metadata. The maximum length is 16 bytes.
>>>
>>> s/16/OVS_EMIT_SAMPLE_COOKIE_MAX_SIZE/
>>>
 + *
 + * Sends the packet to the psample multicast group with the specified 
 group and
 + * cookie. It is possible to combine this action with the
 + * %OVS_ACTION_ATTR_TRUNC action to limit the size of the packet being 
 emitted.
 + */
 +enum ovs_emit_sample_attr {
 +  OVS_EMIT_SAMPLE_ATTR_UNPSEC,
 +  OVS_EMIT_SAMPLE_ATTR_GROUP, /* u32 number. */
 +  OVS_EMIT_SAMPLE_ATTR_COOKIE,/* Optional, user specified cookie. */
 +  __OVS_EMIT_SAMPLE_ATTR_MAX
 +};
 +
 +#define OVS_EMIT_SAMPLE_ATTR_MAX (__OVS_EMIT_SAMPLE_ATTR_MAX - 1)
 +
 +
  /**
   * enum ovs_action_attr - Action types.
   *
 @@ -1004,6 +1028,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
OVS_ACTION_ATTR_DEC_TTL,  /* Nested OVS_DEC_TTL_ATTR_*. */
OVS_ACTION_ATTR_DROP, /* u32 error code. */
 +  OVS_ACTION_ATTR_EMIT_SAMPLE,  /* Nested OVS_EMIT_SAMPLE_ATTR_*. */

__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
   * from userspace. */
 diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
 index 964225580824..3b4dba0ded59 100644
 --- a/net/openvswitch/actions.c
 +++ b/net/openvswitch/actions.c
 @@ -24,6 +24,11 @@
  #include 
  #include 
  #include 
 +
 +#if IS_ENABLED(CONFIG_PSAMPLE)
 +#include 
 +#endif
 +
  #include 

  #include "datapath.h"
 @@ -1299,6 +1304,46 @@ static int execute_dec_ttl(struct sk_buff *skb, 
 struct sw_flow_key *key)
return 0;
  }

 +static int execute_emit_sample(struct datapath *dp, struct sk_buff *skb,
 + const struct sw_flow_key *key,
 + const struct nlattr *attr)
 +{
 +#if IS_ENABLED(CONFIG_PSAMPLE)
 +  struct psample_group psample_group = {};
 +  struct psample_metadata md = {};
 +  struct vport *input_vport;

Re: [ovs-dev] [PATCH net-next v2 5/9] net: openvswitch: add emit_sample action

2024-06-18 Thread Ilya Maximets 
On 6/18/24 09:33, Adrián Moreno wrote:
> On Mon, Jun 17, 2024 at 12:44:45PM GMT, Ilya Maximets wrote:
>> On 6/3/24 20:56, Adrian Moreno wrote:
>>> Add support for a new action: emit_sample.
>>>
>>> This action accepts a u32 group id and a variable-length cookie and uses
>>> the psample multicast group to make the packet available for
>>> observability.
>>>
>>> The maximum length of the user-defined cookie is set to 16, same as
>>> tc_cookie, to discourage using cookies that will not be offloadable.
>>>
>>> Signed-off-by: Adrian Moreno 
>>> ---
>>>  Documentation/netlink/specs/ovs_flow.yaml | 17 
>>>  include/uapi/linux/openvswitch.h  | 25 
>>>  net/openvswitch/actions.c | 50 +++
>>>  net/openvswitch/flow_netlink.c| 33 ++-
>>>  4 files changed, 124 insertions(+), 1 deletion(-)
>>
>> Some nits below, beside ones already mentioned.
>>
> 
> Thanks, Ilya.
> 
>>>
>>> diff --git a/Documentation/netlink/specs/ovs_flow.yaml 
>>> b/Documentation/netlink/specs/ovs_flow.yaml
>>> index 4fdfc6b5cae9..a7ab5593a24f 100644
>>> --- a/Documentation/netlink/specs/ovs_flow.yaml
>>> +++ b/Documentation/netlink/specs/ovs_flow.yaml
>>> @@ -727,6 +727,12 @@ attribute-sets:
>>>  name: dec-ttl
>>>  type: nest
>>>  nested-attributes: dec-ttl-attrs
>>> +  -
>>> +name: emit-sample
>>> +type: nest
>>> +nested-attributes: emit-sample-attrs
>>> +doc: |
>>> +  Sends a packet sample to psample for external observation.
>>>-
>>>  name: tunnel-key-attrs
>>>  enum-name: ovs-tunnel-key-attr
>>> @@ -938,6 +944,17 @@ attribute-sets:
>>>-
>>>  name: gbp
>>>  type: u32
>>> +  -
>>> +name: emit-sample-attrs
>>> +enum-name: ovs-emit-sample-attr
>>> +name-prefix: ovs-emit-sample-attr-
>>> +attributes:
>>> +  -
>>> +name: group
>>> +type: u32
>>> +  -
>>> +name: cookie
>>> +type: binary
>>>
>>>  operations:
>>>name-prefix: ovs-flow-cmd-
>>> diff --git a/include/uapi/linux/openvswitch.h 
>>> b/include/uapi/linux/openvswitch.h
>>> index efc82c318fa2..a0e9dde0584a 100644
>>> --- a/include/uapi/linux/openvswitch.h
>>> +++ b/include/uapi/linux/openvswitch.h
>>> @@ -914,6 +914,30 @@ struct check_pkt_len_arg {
>>>  };
>>>  #endif
>>>
>>> +#define OVS_EMIT_SAMPLE_COOKIE_MAX_SIZE 16
>>> +/**
>>> + * enum ovs_emit_sample_attr - Attributes for %OVS_ACTION_ATTR_EMIT_SAMPLE
>>> + * action.
>>> + *
>>> + * @OVS_EMIT_SAMPLE_ATTR_GROUP: 32-bit number to identify the source of the
>>> + * sample.
>>> + * @OVS_EMIT_SAMPLE_ATTR_COOKIE: A variable-length binary cookie that 
>>> contains
>>> + * user-defined metadata. The maximum length is 16 bytes.
>>
>> s/16/OVS_EMIT_SAMPLE_COOKIE_MAX_SIZE/
>>
>>> + *
>>> + * Sends the packet to the psample multicast group with the specified 
>>> group and
>>> + * cookie. It is possible to combine this action with the
>>> + * %OVS_ACTION_ATTR_TRUNC action to limit the size of the packet being 
>>> emitted.
>>> + */
>>> +enum ovs_emit_sample_attr {
>>> +   OVS_EMIT_SAMPLE_ATTR_UNPSEC,
>>> +   OVS_EMIT_SAMPLE_ATTR_GROUP, /* u32 number. */
>>> +   OVS_EMIT_SAMPLE_ATTR_COOKIE,/* Optional, user specified cookie. */
>>> +   __OVS_EMIT_SAMPLE_ATTR_MAX
>>> +};
>>> +
>>> +#define OVS_EMIT_SAMPLE_ATTR_MAX (__OVS_EMIT_SAMPLE_ATTR_MAX - 1)
>>> +
>>> +
>>>  /**
>>>   * enum ovs_action_attr - Action types.
>>>   *
>>> @@ -1004,6 +1028,7 @@ enum ovs_action_attr {
>>> OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
>>> OVS_ACTION_ATTR_DEC_TTL,  /* Nested OVS_DEC_TTL_ATTR_*. */
>>> OVS_ACTION_ATTR_DROP, /* u32 error code. */
>>> +   OVS_ACTION_ATTR_EMIT_SAMPLE,  /* Nested OVS_EMIT_SAMPLE_ATTR_*. */
>>>
>>> __OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
>>>* from userspace. */
>>> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
>>> index 964225580824..3b4dba0ded59 100644
>>> --- a/net/openvswitch/actions.c
>>> +++ b/net/openvswitch/actions.c
>>> @@ -24,6 +24,11 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +
>>> +#if IS_ENABLED(CONFIG_PSAMPLE)
>>> +#include 
>>> +#endif
>>> +
>>>  #include 
>>>
>>>  #include "datapath.h"
>>> @@ -1299,6 +1304,46 @@ static int execute_dec_ttl(struct sk_buff *skb, 
>>> struct sw_flow_key *key)
>>> return 0;
>>>  }
>>>
>>> +static int execute_emit_sample(struct datapath *dp, struct sk_buff *skb,
>>> +  const struct sw_flow_key *key,
>>> +  const struct nlattr *attr)
>>> +{
>>> +#if IS_ENABLED(CONFIG_PSAMPLE)
>>> +   struct psample_group psample_group = {};
>>> +   struct psample_metadata md = {};
>>> +   struct vport *input_vport;
>>> +   const struct nlattr *a;
>>> +   int rem;
>>> +
>>> +   for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
>>> +a = nla_next(a, &rem)) {
>>
>> Since

Re: [ovs-dev] [PATCH net-next v2 5/9] net: openvswitch: add emit_sample action

2024-06-18 Thread Adrián Moreno 
On Mon, Jun 17, 2024 at 12:44:45PM GMT, Ilya Maximets wrote:
> On 6/3/24 20:56, Adrian Moreno wrote:
> > Add support for a new action: emit_sample.
> >
> > This action accepts a u32 group id and a variable-length cookie and uses
> > the psample multicast group to make the packet available for
> > observability.
> >
> > The maximum length of the user-defined cookie is set to 16, same as
> > tc_cookie, to discourage using cookies that will not be offloadable.
> >
> > Signed-off-by: Adrian Moreno 
> > ---
> >  Documentation/netlink/specs/ovs_flow.yaml | 17 
> >  include/uapi/linux/openvswitch.h  | 25 
> >  net/openvswitch/actions.c | 50 +++
> >  net/openvswitch/flow_netlink.c| 33 ++-
> >  4 files changed, 124 insertions(+), 1 deletion(-)
>
> Some nits below, beside ones already mentioned.
>

Thanks, Ilya.

> >
> > diff --git a/Documentation/netlink/specs/ovs_flow.yaml 
> > b/Documentation/netlink/specs/ovs_flow.yaml
> > index 4fdfc6b5cae9..a7ab5593a24f 100644
> > --- a/Documentation/netlink/specs/ovs_flow.yaml
> > +++ b/Documentation/netlink/specs/ovs_flow.yaml
> > @@ -727,6 +727,12 @@ attribute-sets:
> >  name: dec-ttl
> >  type: nest
> >  nested-attributes: dec-ttl-attrs
> > +  -
> > +name: emit-sample
> > +type: nest
> > +nested-attributes: emit-sample-attrs
> > +doc: |
> > +  Sends a packet sample to psample for external observation.
> >-
> >  name: tunnel-key-attrs
> >  enum-name: ovs-tunnel-key-attr
> > @@ -938,6 +944,17 @@ attribute-sets:
> >-
> >  name: gbp
> >  type: u32
> > +  -
> > +name: emit-sample-attrs
> > +enum-name: ovs-emit-sample-attr
> > +name-prefix: ovs-emit-sample-attr-
> > +attributes:
> > +  -
> > +name: group
> > +type: u32
> > +  -
> > +name: cookie
> > +type: binary
> >
> >  operations:
> >name-prefix: ovs-flow-cmd-
> > diff --git a/include/uapi/linux/openvswitch.h 
> > b/include/uapi/linux/openvswitch.h
> > index efc82c318fa2..a0e9dde0584a 100644
> > --- a/include/uapi/linux/openvswitch.h
> > +++ b/include/uapi/linux/openvswitch.h
> > @@ -914,6 +914,30 @@ struct check_pkt_len_arg {
> >  };
> >  #endif
> >
> > +#define OVS_EMIT_SAMPLE_COOKIE_MAX_SIZE 16
> > +/**
> > + * enum ovs_emit_sample_attr - Attributes for %OVS_ACTION_ATTR_EMIT_SAMPLE
> > + * action.
> > + *
> > + * @OVS_EMIT_SAMPLE_ATTR_GROUP: 32-bit number to identify the source of the
> > + * sample.
> > + * @OVS_EMIT_SAMPLE_ATTR_COOKIE: A variable-length binary cookie that 
> > contains
> > + * user-defined metadata. The maximum length is 16 bytes.
>
> s/16/OVS_EMIT_SAMPLE_COOKIE_MAX_SIZE/
>
> > + *
> > + * Sends the packet to the psample multicast group with the specified 
> > group and
> > + * cookie. It is possible to combine this action with the
> > + * %OVS_ACTION_ATTR_TRUNC action to limit the size of the packet being 
> > emitted.
> > + */
> > +enum ovs_emit_sample_attr {
> > +   OVS_EMIT_SAMPLE_ATTR_UNPSEC,
> > +   OVS_EMIT_SAMPLE_ATTR_GROUP, /* u32 number. */
> > +   OVS_EMIT_SAMPLE_ATTR_COOKIE,/* Optional, user specified cookie. */
> > +   __OVS_EMIT_SAMPLE_ATTR_MAX
> > +};
> > +
> > +#define OVS_EMIT_SAMPLE_ATTR_MAX (__OVS_EMIT_SAMPLE_ATTR_MAX - 1)
> > +
> > +
> >  /**
> >   * enum ovs_action_attr - Action types.
> >   *
> > @@ -1004,6 +1028,7 @@ enum ovs_action_attr {
> > OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
> > OVS_ACTION_ATTR_DEC_TTL,  /* Nested OVS_DEC_TTL_ATTR_*. */
> > OVS_ACTION_ATTR_DROP, /* u32 error code. */
> > +   OVS_ACTION_ATTR_EMIT_SAMPLE,  /* Nested OVS_EMIT_SAMPLE_ATTR_*. */
> >
> > __OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
> >* from userspace. */
> > diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> > index 964225580824..3b4dba0ded59 100644
> > --- a/net/openvswitch/actions.c
> > +++ b/net/openvswitch/actions.c
> > @@ -24,6 +24,11 @@
> >  #include 
> >  #include 
> >  #include 
> > +
> > +#if IS_ENABLED(CONFIG_PSAMPLE)
> > +#include 
> > +#endif
> > +
> >  #include 
> >
> >  #include "datapath.h"
> > @@ -1299,6 +1304,46 @@ static int execute_dec_ttl(struct sk_buff *skb, 
> > struct sw_flow_key *key)
> > return 0;
> >  }
> >
> > +static int execute_emit_sample(struct datapath *dp, struct sk_buff *skb,
> > +  const struct sw_flow_key *key,
> > +  const struct nlattr *attr)
> > +{
> > +#if IS_ENABLED(CONFIG_PSAMPLE)
> > +   struct psample_group psample_group = {};
> > +   struct psample_metadata md = {};
> > +   struct vport *input_vport;
> > +   const struct nlattr *a;
> > +   int rem;
> > +
> > +   for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
> > +a = nla_next(a, &rem)) {
>
> Since the action is strictly validated, can use use nla_for_ea

Re: [ovs-dev] [PATCH net-next v2 5/9] net: openvswitch: add emit_sample action

2024-06-17 Thread Ilya Maximets 
On 6/3/24 20:56, Adrian Moreno wrote:
> Add support for a new action: emit_sample.
> 
> This action accepts a u32 group id and a variable-length cookie and uses
> the psample multicast group to make the packet available for
> observability.
> 
> The maximum length of the user-defined cookie is set to 16, same as
> tc_cookie, to discourage using cookies that will not be offloadable.
> 
> Signed-off-by: Adrian Moreno 
> ---
>  Documentation/netlink/specs/ovs_flow.yaml | 17 
>  include/uapi/linux/openvswitch.h  | 25 
>  net/openvswitch/actions.c | 50 +++
>  net/openvswitch/flow_netlink.c| 33 ++-
>  4 files changed, 124 insertions(+), 1 deletion(-)

Some nits below, beside ones already mentioned.

> 
> diff --git a/Documentation/netlink/specs/ovs_flow.yaml 
> b/Documentation/netlink/specs/ovs_flow.yaml
> index 4fdfc6b5cae9..a7ab5593a24f 100644
> --- a/Documentation/netlink/specs/ovs_flow.yaml
> +++ b/Documentation/netlink/specs/ovs_flow.yaml
> @@ -727,6 +727,12 @@ attribute-sets:
>  name: dec-ttl
>  type: nest
>  nested-attributes: dec-ttl-attrs
> +  -
> +name: emit-sample
> +type: nest
> +nested-attributes: emit-sample-attrs
> +doc: |
> +  Sends a packet sample to psample for external observation.
>-
>  name: tunnel-key-attrs
>  enum-name: ovs-tunnel-key-attr
> @@ -938,6 +944,17 @@ attribute-sets:
>-
>  name: gbp
>  type: u32
> +  -
> +name: emit-sample-attrs
> +enum-name: ovs-emit-sample-attr
> +name-prefix: ovs-emit-sample-attr-
> +attributes:
> +  -
> +name: group
> +type: u32
> +  -
> +name: cookie
> +type: binary
>  
>  operations:
>name-prefix: ovs-flow-cmd-
> diff --git a/include/uapi/linux/openvswitch.h 
> b/include/uapi/linux/openvswitch.h
> index efc82c318fa2..a0e9dde0584a 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -914,6 +914,30 @@ struct check_pkt_len_arg {
>  };
>  #endif
>  
> +#define OVS_EMIT_SAMPLE_COOKIE_MAX_SIZE 16
> +/**
> + * enum ovs_emit_sample_attr - Attributes for %OVS_ACTION_ATTR_EMIT_SAMPLE
> + * action.
> + *
> + * @OVS_EMIT_SAMPLE_ATTR_GROUP: 32-bit number to identify the source of the
> + * sample.
> + * @OVS_EMIT_SAMPLE_ATTR_COOKIE: A variable-length binary cookie that 
> contains
> + * user-defined metadata. The maximum length is 16 bytes.

s/16/OVS_EMIT_SAMPLE_COOKIE_MAX_SIZE/

> + *
> + * Sends the packet to the psample multicast group with the specified group 
> and
> + * cookie. It is possible to combine this action with the
> + * %OVS_ACTION_ATTR_TRUNC action to limit the size of the packet being 
> emitted.
> + */
> +enum ovs_emit_sample_attr {
> + OVS_EMIT_SAMPLE_ATTR_UNPSEC,
> + OVS_EMIT_SAMPLE_ATTR_GROUP, /* u32 number. */
> + OVS_EMIT_SAMPLE_ATTR_COOKIE,/* Optional, user specified cookie. */
> + __OVS_EMIT_SAMPLE_ATTR_MAX
> +};
> +
> +#define OVS_EMIT_SAMPLE_ATTR_MAX (__OVS_EMIT_SAMPLE_ATTR_MAX - 1)
> +
> +
>  /**
>   * enum ovs_action_attr - Action types.
>   *
> @@ -1004,6 +1028,7 @@ enum ovs_action_attr {
>   OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
>   OVS_ACTION_ATTR_DEC_TTL,  /* Nested OVS_DEC_TTL_ATTR_*. */
>   OVS_ACTION_ATTR_DROP, /* u32 error code. */
> + OVS_ACTION_ATTR_EMIT_SAMPLE,  /* Nested OVS_EMIT_SAMPLE_ATTR_*. */
>  
>   __OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
>  * from userspace. */
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index 964225580824..3b4dba0ded59 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -24,6 +24,11 @@
>  #include 
>  #include 
>  #include 
> +
> +#if IS_ENABLED(CONFIG_PSAMPLE)
> +#include 
> +#endif
> +
>  #include 
>  
>  #include "datapath.h"
> @@ -1299,6 +1304,46 @@ static int execute_dec_ttl(struct sk_buff *skb, struct 
> sw_flow_key *key)
>   return 0;
>  }
>  
> +static int execute_emit_sample(struct datapath *dp, struct sk_buff *skb,
> +const struct sw_flow_key *key,
> +const struct nlattr *attr)
> +{
> +#if IS_ENABLED(CONFIG_PSAMPLE)
> + struct psample_group psample_group = {};
> + struct psample_metadata md = {};
> + struct vport *input_vport;
> + const struct nlattr *a;
> + int rem;
> +
> + for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
> +  a = nla_next(a, &rem)) {

Since the action is strictly validated, can use use nla_for_each_attr()
or nla_for_each_nested() ?

> + switch (nla_type(a)) {
> + case OVS_EMIT_SAMPLE_ATTR_GROUP:
> + psample_group.group_num = nla_get_u32(a);
> + break;
> +
> + case OVS_EMIT_SAMPLE_ATTR_COOKIE:
> + md.

Re: [ovs-dev] [PATCH net-next v2 5/9] net: openvswitch: add emit_sample action

2024-06-14 Thread Simon Horman 
On Mon, Jun 03, 2024 at 08:56:39PM +0200, Adrian Moreno wrote:
> Add support for a new action: emit_sample.
> 
> This action accepts a u32 group id and a variable-length cookie and uses
> the psample multicast group to make the packet available for
> observability.
> 
> The maximum length of the user-defined cookie is set to 16, same as
> tc_cookie, to discourage using cookies that will not be offloadable.
> 
> Signed-off-by: Adrian Moreno 

...

> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c

...

> @@ -1299,6 +1304,46 @@ static int execute_dec_ttl(struct sk_buff *skb, struct 
> sw_flow_key *key)
>   return 0;
>  }
>  
> +static int execute_emit_sample(struct datapath *dp, struct sk_buff *skb,
> +const struct sw_flow_key *key,
> +const struct nlattr *attr)
> +{
> +#if IS_ENABLED(CONFIG_PSAMPLE)
> + struct psample_group psample_group = {};
> + struct psample_metadata md = {};
> + struct vport *input_vport;
> + const struct nlattr *a;
> + int rem;
> +
> + for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
> +  a = nla_next(a, &rem)) {
> + switch (nla_type(a)) {
> + case OVS_EMIT_SAMPLE_ATTR_GROUP:
> + psample_group.group_num = nla_get_u32(a);
> + break;
> +
> + case OVS_EMIT_SAMPLE_ATTR_COOKIE:
> + md.user_cookie = nla_data(a);
> + md.user_cookie_len = nla_len(a);
> + break;
> + }
> + }
> +
> + psample_group.net = ovs_dp_get_net(dp);
> +
> + input_vport = ovs_vport_rcu(dp, key->phy.in_port);
> + if (!input_vport)
> + input_vport = ovs_vport_rcu(dp, OVSP_LOCAL);
> +
> + md.in_ifindex = input_vport->dev->ifindex;
> + md.trunc_size = skb->len - OVS_CB(skb)->cutlen;
> +
> + psample_sample_packet(&psample_group, skb, 0, &md);
> +#endif
> +
> + return 0;
> +}
> +
>  /* Execute a list of actions against 'skb'. */
>  static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
> struct sw_flow_key *key,
> @@ -1502,6 +1547,11 @@ static int do_execute_actions(struct datapath *dp, 
> struct sk_buff *skb,
>   ovs_kfree_skb_reason(skb, reason);
>   return 0;
>   }
> +
> + case OVS_ACTION_ATTR_EMIT_SAMPLE:
> + err = execute_emit_sample(dp, skb, key, a);
> + OVS_CB(skb)->cutlen = 0;
> + break;
>   }

Hi Adrian,

execute_emit_sample always returns 0, and it seems that err will always
be 0 when the code above is executed. So perhaps the return type
of execute_emit_sample could be changed to void and the code above be
updated not to set err.

Other than that, which I don't feel particularly strongly about,
this looks good to me.

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

Re: [ovs-dev] [PATCH net-next v2 5/9] net: openvswitch: add emit_sample action

2024-06-11 Thread Adrián Moreno 
On Tue, Jun 11, 2024 at 09:54:49AM GMT, Aaron Conole wrote:
> Adrián Moreno  writes:
>
> > On Mon, Jun 10, 2024 at 11:46:14AM GMT, Aaron Conole wrote:
> >> Adrian Moreno  writes:
> >>
> >> > Add support for a new action: emit_sample.
> >> >
> >> > This action accepts a u32 group id and a variable-length cookie and uses
> >> > the psample multicast group to make the packet available for
> >> > observability.
> >> >
> >> > The maximum length of the user-defined cookie is set to 16, same as
> >> > tc_cookie, to discourage using cookies that will not be offloadable.
> >> >
> >> > Signed-off-by: Adrian Moreno 
> >> > ---
> >>
> >> I saw some of the nits Simon raised - I'll add one more below.
> >>
> >> I haven't gone through the series thoroughly enough to make a detailed
> >> review.
> >>
> >> >  Documentation/netlink/specs/ovs_flow.yaml | 17 
> >> >  include/uapi/linux/openvswitch.h  | 25 
> >> >  net/openvswitch/actions.c | 50 +++
> >> >  net/openvswitch/flow_netlink.c| 33 ++-
> >> >  4 files changed, 124 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/Documentation/netlink/specs/ovs_flow.yaml 
> >> > b/Documentation/netlink/specs/ovs_flow.yaml
> >> > index 4fdfc6b5cae9..a7ab5593a24f 100644
> >> > --- a/Documentation/netlink/specs/ovs_flow.yaml
> >> > +++ b/Documentation/netlink/specs/ovs_flow.yaml
> >> > @@ -727,6 +727,12 @@ attribute-sets:
> >> >  name: dec-ttl
> >> >  type: nest
> >> >  nested-attributes: dec-ttl-attrs
> >> > +  -
> >> > +name: emit-sample
> >> > +type: nest
> >> > +nested-attributes: emit-sample-attrs
> >> > +doc: |
> >> > +  Sends a packet sample to psample for external observation.
> >> >-
> >> >  name: tunnel-key-attrs
> >> >  enum-name: ovs-tunnel-key-attr
> >> > @@ -938,6 +944,17 @@ attribute-sets:
> >> >-
> >> >  name: gbp
> >> >  type: u32
> >> > +  -
> >> > +name: emit-sample-attrs
> >> > +enum-name: ovs-emit-sample-attr
> >> > +name-prefix: ovs-emit-sample-attr-
> >> > +attributes:
> >> > +  -
> >> > +name: group
> >> > +type: u32
> >> > +  -
> >> > +name: cookie
> >> > +type: binary
> >> >
> >> >  operations:
> >> >name-prefix: ovs-flow-cmd-
> >> > diff --git a/include/uapi/linux/openvswitch.h 
> >> > b/include/uapi/linux/openvswitch.h
> >> > index efc82c318fa2..a0e9dde0584a 100644
> >> > --- a/include/uapi/linux/openvswitch.h
> >> > +++ b/include/uapi/linux/openvswitch.h
> >> > @@ -914,6 +914,30 @@ struct check_pkt_len_arg {
> >> >  };
> >> >  #endif
> >> >
> >> > +#define OVS_EMIT_SAMPLE_COOKIE_MAX_SIZE 16
> >> > +/**
> >> > + * enum ovs_emit_sample_attr - Attributes for 
> >> > %OVS_ACTION_ATTR_EMIT_SAMPLE
> >> > + * action.
> >> > + *
> >> > + * @OVS_EMIT_SAMPLE_ATTR_GROUP: 32-bit number to identify the source of 
> >> > the
> >> > + * sample.
> >> > + * @OVS_EMIT_SAMPLE_ATTR_COOKIE: A variable-length binary cookie that 
> >> > contains
> >> > + * user-defined metadata. The maximum length is 16 bytes.
> >> > + *
> >> > + * Sends the packet to the psample multicast group with the specified 
> >> > group and
> >> > + * cookie. It is possible to combine this action with the
> >> > + * %OVS_ACTION_ATTR_TRUNC action to limit the size of the packet being 
> >> > emitted.
> >> > + */
> >> > +enum ovs_emit_sample_attr {
> >> > +OVS_EMIT_SAMPLE_ATTR_UNPSEC,
> >> > +OVS_EMIT_SAMPLE_ATTR_GROUP, /* u32 number. */
> >> > +OVS_EMIT_SAMPLE_ATTR_COOKIE,/* Optional, user specified 
> >> > cookie. */
> >> > +__OVS_EMIT_SAMPLE_ATTR_MAX
> >> > +};
> >> > +
> >> > +#define OVS_EMIT_SAMPLE_ATTR_MAX (__OVS_EMIT_SAMPLE_ATTR_MAX - 1)
> >> > +
> >> > +
> >> >  /**
> >> >   * enum ovs_action_attr - Action types.
> >> >   *
> >> > @@ -1004,6 +1028,7 @@ enum ovs_action_attr {
> >> >  OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
> >> >  OVS_ACTION_ATTR_DEC_TTL,  /* Nested OVS_DEC_TTL_ATTR_*. */
> >> >  OVS_ACTION_ATTR_DROP, /* u32 error code. */
> >> > +OVS_ACTION_ATTR_EMIT_SAMPLE,  /* Nested OVS_EMIT_SAMPLE_ATTR_*. 
> >> > */
> >> >
> >> >  __OVS_ACTION_ATTR_MAX,/* Nothing past this will be 
> >> > accepted
> >> > * from userspace. */
> >> > diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> >> > index 964225580824..3b4dba0ded59 100644
> >> > --- a/net/openvswitch/actions.c
> >> > +++ b/net/openvswitch/actions.c
> >> > @@ -24,6 +24,11 @@
> >> >  #include 
> >> >  #include 
> >> >  #include 
> >> > +
> >> > +#if IS_ENABLED(CONFIG_PSAMPLE)
> >> > +#include 
> >> > +#endif
> >> > +
> >> >  #include 
> >> >
> >> >  #include "datapath.h"
> >> > @@ -1299,6 +1304,46 @@ static int execute_dec_ttl(struct sk_buff *skb, 
> >> > struct sw_flow_key *key)
> >> >  return 0;
> >> >

Re: [ovs-dev] [PATCH net-next v2 5/9] net: openvswitch: add emit_sample action

2024-06-11 Thread Aaron Conole 
Adrián Moreno  writes:

> On Mon, Jun 10, 2024 at 11:46:14AM GMT, Aaron Conole wrote:
>> Adrian Moreno  writes:
>>
>> > Add support for a new action: emit_sample.
>> >
>> > This action accepts a u32 group id and a variable-length cookie and uses
>> > the psample multicast group to make the packet available for
>> > observability.
>> >
>> > The maximum length of the user-defined cookie is set to 16, same as
>> > tc_cookie, to discourage using cookies that will not be offloadable.
>> >
>> > Signed-off-by: Adrian Moreno 
>> > ---
>>
>> I saw some of the nits Simon raised - I'll add one more below.
>>
>> I haven't gone through the series thoroughly enough to make a detailed
>> review.
>>
>> >  Documentation/netlink/specs/ovs_flow.yaml | 17 
>> >  include/uapi/linux/openvswitch.h  | 25 
>> >  net/openvswitch/actions.c | 50 +++
>> >  net/openvswitch/flow_netlink.c| 33 ++-
>> >  4 files changed, 124 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/Documentation/netlink/specs/ovs_flow.yaml 
>> > b/Documentation/netlink/specs/ovs_flow.yaml
>> > index 4fdfc6b5cae9..a7ab5593a24f 100644
>> > --- a/Documentation/netlink/specs/ovs_flow.yaml
>> > +++ b/Documentation/netlink/specs/ovs_flow.yaml
>> > @@ -727,6 +727,12 @@ attribute-sets:
>> >  name: dec-ttl
>> >  type: nest
>> >  nested-attributes: dec-ttl-attrs
>> > +  -
>> > +name: emit-sample
>> > +type: nest
>> > +nested-attributes: emit-sample-attrs
>> > +doc: |
>> > +  Sends a packet sample to psample for external observation.
>> >-
>> >  name: tunnel-key-attrs
>> >  enum-name: ovs-tunnel-key-attr
>> > @@ -938,6 +944,17 @@ attribute-sets:
>> >-
>> >  name: gbp
>> >  type: u32
>> > +  -
>> > +name: emit-sample-attrs
>> > +enum-name: ovs-emit-sample-attr
>> > +name-prefix: ovs-emit-sample-attr-
>> > +attributes:
>> > +  -
>> > +name: group
>> > +type: u32
>> > +  -
>> > +name: cookie
>> > +type: binary
>> >
>> >  operations:
>> >name-prefix: ovs-flow-cmd-
>> > diff --git a/include/uapi/linux/openvswitch.h 
>> > b/include/uapi/linux/openvswitch.h
>> > index efc82c318fa2..a0e9dde0584a 100644
>> > --- a/include/uapi/linux/openvswitch.h
>> > +++ b/include/uapi/linux/openvswitch.h
>> > @@ -914,6 +914,30 @@ struct check_pkt_len_arg {
>> >  };
>> >  #endif
>> >
>> > +#define OVS_EMIT_SAMPLE_COOKIE_MAX_SIZE 16
>> > +/**
>> > + * enum ovs_emit_sample_attr - Attributes for %OVS_ACTION_ATTR_EMIT_SAMPLE
>> > + * action.
>> > + *
>> > + * @OVS_EMIT_SAMPLE_ATTR_GROUP: 32-bit number to identify the source of 
>> > the
>> > + * sample.
>> > + * @OVS_EMIT_SAMPLE_ATTR_COOKIE: A variable-length binary cookie that 
>> > contains
>> > + * user-defined metadata. The maximum length is 16 bytes.
>> > + *
>> > + * Sends the packet to the psample multicast group with the specified 
>> > group and
>> > + * cookie. It is possible to combine this action with the
>> > + * %OVS_ACTION_ATTR_TRUNC action to limit the size of the packet being 
>> > emitted.
>> > + */
>> > +enum ovs_emit_sample_attr {
>> > +  OVS_EMIT_SAMPLE_ATTR_UNPSEC,
>> > +  OVS_EMIT_SAMPLE_ATTR_GROUP, /* u32 number. */
>> > +  OVS_EMIT_SAMPLE_ATTR_COOKIE,/* Optional, user specified cookie. */
>> > +  __OVS_EMIT_SAMPLE_ATTR_MAX
>> > +};
>> > +
>> > +#define OVS_EMIT_SAMPLE_ATTR_MAX (__OVS_EMIT_SAMPLE_ATTR_MAX - 1)
>> > +
>> > +
>> >  /**
>> >   * enum ovs_action_attr - Action types.
>> >   *
>> > @@ -1004,6 +1028,7 @@ enum ovs_action_attr {
>> >OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
>> >OVS_ACTION_ATTR_DEC_TTL,  /* Nested OVS_DEC_TTL_ATTR_*. */
>> >OVS_ACTION_ATTR_DROP, /* u32 error code. */
>> > +  OVS_ACTION_ATTR_EMIT_SAMPLE,  /* Nested OVS_EMIT_SAMPLE_ATTR_*. */
>> >
>> >__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
>> >   * from userspace. */
>> > diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
>> > index 964225580824..3b4dba0ded59 100644
>> > --- a/net/openvswitch/actions.c
>> > +++ b/net/openvswitch/actions.c
>> > @@ -24,6 +24,11 @@
>> >  #include 
>> >  #include 
>> >  #include 
>> > +
>> > +#if IS_ENABLED(CONFIG_PSAMPLE)
>> > +#include 
>> > +#endif
>> > +
>> >  #include 
>> >
>> >  #include "datapath.h"
>> > @@ -1299,6 +1304,46 @@ static int execute_dec_ttl(struct sk_buff *skb, 
>> > struct sw_flow_key *key)
>> >return 0;
>> >  }
>> >
>> > +static int execute_emit_sample(struct datapath *dp, struct sk_buff *skb,
>> > + const struct sw_flow_key *key,
>> > + const struct nlattr *attr)
>> > +{
>> > +#if IS_ENABLED(CONFIG_PSAMPLE)
>> > +  struct psample_group psample_group = {};
>> > +  struct psample_metadata md = {};
>> > +  struct vport *input_vport;
>> > +  const struct nlattr *a;
>> > +  int

Re: [ovs-dev] [PATCH net-next v2 5/9] net: openvswitch: add emit_sample action

2024-06-11 Thread Adrián Moreno 
On Mon, Jun 10, 2024 at 11:46:14AM GMT, Aaron Conole wrote:
> Adrian Moreno  writes:
>
> > Add support for a new action: emit_sample.
> >
> > This action accepts a u32 group id and a variable-length cookie and uses
> > the psample multicast group to make the packet available for
> > observability.
> >
> > The maximum length of the user-defined cookie is set to 16, same as
> > tc_cookie, to discourage using cookies that will not be offloadable.
> >
> > Signed-off-by: Adrian Moreno 
> > ---
>
> I saw some of the nits Simon raised - I'll add one more below.
>
> I haven't gone through the series thoroughly enough to make a detailed
> review.
>
> >  Documentation/netlink/specs/ovs_flow.yaml | 17 
> >  include/uapi/linux/openvswitch.h  | 25 
> >  net/openvswitch/actions.c | 50 +++
> >  net/openvswitch/flow_netlink.c| 33 ++-
> >  4 files changed, 124 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/netlink/specs/ovs_flow.yaml 
> > b/Documentation/netlink/specs/ovs_flow.yaml
> > index 4fdfc6b5cae9..a7ab5593a24f 100644
> > --- a/Documentation/netlink/specs/ovs_flow.yaml
> > +++ b/Documentation/netlink/specs/ovs_flow.yaml
> > @@ -727,6 +727,12 @@ attribute-sets:
> >  name: dec-ttl
> >  type: nest
> >  nested-attributes: dec-ttl-attrs
> > +  -
> > +name: emit-sample
> > +type: nest
> > +nested-attributes: emit-sample-attrs
> > +doc: |
> > +  Sends a packet sample to psample for external observation.
> >-
> >  name: tunnel-key-attrs
> >  enum-name: ovs-tunnel-key-attr
> > @@ -938,6 +944,17 @@ attribute-sets:
> >-
> >  name: gbp
> >  type: u32
> > +  -
> > +name: emit-sample-attrs
> > +enum-name: ovs-emit-sample-attr
> > +name-prefix: ovs-emit-sample-attr-
> > +attributes:
> > +  -
> > +name: group
> > +type: u32
> > +  -
> > +name: cookie
> > +type: binary
> >
> >  operations:
> >name-prefix: ovs-flow-cmd-
> > diff --git a/include/uapi/linux/openvswitch.h 
> > b/include/uapi/linux/openvswitch.h
> > index efc82c318fa2..a0e9dde0584a 100644
> > --- a/include/uapi/linux/openvswitch.h
> > +++ b/include/uapi/linux/openvswitch.h
> > @@ -914,6 +914,30 @@ struct check_pkt_len_arg {
> >  };
> >  #endif
> >
> > +#define OVS_EMIT_SAMPLE_COOKIE_MAX_SIZE 16
> > +/**
> > + * enum ovs_emit_sample_attr - Attributes for %OVS_ACTION_ATTR_EMIT_SAMPLE
> > + * action.
> > + *
> > + * @OVS_EMIT_SAMPLE_ATTR_GROUP: 32-bit number to identify the source of the
> > + * sample.
> > + * @OVS_EMIT_SAMPLE_ATTR_COOKIE: A variable-length binary cookie that 
> > contains
> > + * user-defined metadata. The maximum length is 16 bytes.
> > + *
> > + * Sends the packet to the psample multicast group with the specified 
> > group and
> > + * cookie. It is possible to combine this action with the
> > + * %OVS_ACTION_ATTR_TRUNC action to limit the size of the packet being 
> > emitted.
> > + */
> > +enum ovs_emit_sample_attr {
> > +   OVS_EMIT_SAMPLE_ATTR_UNPSEC,
> > +   OVS_EMIT_SAMPLE_ATTR_GROUP, /* u32 number. */
> > +   OVS_EMIT_SAMPLE_ATTR_COOKIE,/* Optional, user specified cookie. */
> > +   __OVS_EMIT_SAMPLE_ATTR_MAX
> > +};
> > +
> > +#define OVS_EMIT_SAMPLE_ATTR_MAX (__OVS_EMIT_SAMPLE_ATTR_MAX - 1)
> > +
> > +
> >  /**
> >   * enum ovs_action_attr - Action types.
> >   *
> > @@ -1004,6 +1028,7 @@ enum ovs_action_attr {
> > OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
> > OVS_ACTION_ATTR_DEC_TTL,  /* Nested OVS_DEC_TTL_ATTR_*. */
> > OVS_ACTION_ATTR_DROP, /* u32 error code. */
> > +   OVS_ACTION_ATTR_EMIT_SAMPLE,  /* Nested OVS_EMIT_SAMPLE_ATTR_*. */
> >
> > __OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
> >* from userspace. */
> > diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> > index 964225580824..3b4dba0ded59 100644
> > --- a/net/openvswitch/actions.c
> > +++ b/net/openvswitch/actions.c
> > @@ -24,6 +24,11 @@
> >  #include 
> >  #include 
> >  #include 
> > +
> > +#if IS_ENABLED(CONFIG_PSAMPLE)
> > +#include 
> > +#endif
> > +
> >  #include 
> >
> >  #include "datapath.h"
> > @@ -1299,6 +1304,46 @@ static int execute_dec_ttl(struct sk_buff *skb, 
> > struct sw_flow_key *key)
> > return 0;
> >  }
> >
> > +static int execute_emit_sample(struct datapath *dp, struct sk_buff *skb,
> > +  const struct sw_flow_key *key,
> > +  const struct nlattr *attr)
> > +{
> > +#if IS_ENABLED(CONFIG_PSAMPLE)
> > +   struct psample_group psample_group = {};
> > +   struct psample_metadata md = {};
> > +   struct vport *input_vport;
> > +   const struct nlattr *a;
> > +   int rem;
> > +
> > +   for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
> > +a = nla_next(a, &rem)) {
> > +   switch (nla_type(a)) {
>

Re: [ovs-dev] [PATCH net-next v2 5/9] net: openvswitch: add emit_sample action

2024-06-10 Thread Aaron Conole 
Adrian Moreno  writes:

> Add support for a new action: emit_sample.
>
> This action accepts a u32 group id and a variable-length cookie and uses
> the psample multicast group to make the packet available for
> observability.
>
> The maximum length of the user-defined cookie is set to 16, same as
> tc_cookie, to discourage using cookies that will not be offloadable.
>
> Signed-off-by: Adrian Moreno 
> ---

I saw some of the nits Simon raised - I'll add one more below.

I haven't gone through the series thoroughly enough to make a detailed
review.

>  Documentation/netlink/specs/ovs_flow.yaml | 17 
>  include/uapi/linux/openvswitch.h  | 25 
>  net/openvswitch/actions.c | 50 +++
>  net/openvswitch/flow_netlink.c| 33 ++-
>  4 files changed, 124 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/netlink/specs/ovs_flow.yaml 
> b/Documentation/netlink/specs/ovs_flow.yaml
> index 4fdfc6b5cae9..a7ab5593a24f 100644
> --- a/Documentation/netlink/specs/ovs_flow.yaml
> +++ b/Documentation/netlink/specs/ovs_flow.yaml
> @@ -727,6 +727,12 @@ attribute-sets:
>  name: dec-ttl
>  type: nest
>  nested-attributes: dec-ttl-attrs
> +  -
> +name: emit-sample
> +type: nest
> +nested-attributes: emit-sample-attrs
> +doc: |
> +  Sends a packet sample to psample for external observation.
>-
>  name: tunnel-key-attrs
>  enum-name: ovs-tunnel-key-attr
> @@ -938,6 +944,17 @@ attribute-sets:
>-
>  name: gbp
>  type: u32
> +  -
> +name: emit-sample-attrs
> +enum-name: ovs-emit-sample-attr
> +name-prefix: ovs-emit-sample-attr-
> +attributes:
> +  -
> +name: group
> +type: u32
> +  -
> +name: cookie
> +type: binary
>  
>  operations:
>name-prefix: ovs-flow-cmd-
> diff --git a/include/uapi/linux/openvswitch.h 
> b/include/uapi/linux/openvswitch.h
> index efc82c318fa2..a0e9dde0584a 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -914,6 +914,30 @@ struct check_pkt_len_arg {
>  };
>  #endif
>  
> +#define OVS_EMIT_SAMPLE_COOKIE_MAX_SIZE 16
> +/**
> + * enum ovs_emit_sample_attr - Attributes for %OVS_ACTION_ATTR_EMIT_SAMPLE
> + * action.
> + *
> + * @OVS_EMIT_SAMPLE_ATTR_GROUP: 32-bit number to identify the source of the
> + * sample.
> + * @OVS_EMIT_SAMPLE_ATTR_COOKIE: A variable-length binary cookie that 
> contains
> + * user-defined metadata. The maximum length is 16 bytes.
> + *
> + * Sends the packet to the psample multicast group with the specified group 
> and
> + * cookie. It is possible to combine this action with the
> + * %OVS_ACTION_ATTR_TRUNC action to limit the size of the packet being 
> emitted.
> + */
> +enum ovs_emit_sample_attr {
> + OVS_EMIT_SAMPLE_ATTR_UNPSEC,
> + OVS_EMIT_SAMPLE_ATTR_GROUP, /* u32 number. */
> + OVS_EMIT_SAMPLE_ATTR_COOKIE,/* Optional, user specified cookie. */
> + __OVS_EMIT_SAMPLE_ATTR_MAX
> +};
> +
> +#define OVS_EMIT_SAMPLE_ATTR_MAX (__OVS_EMIT_SAMPLE_ATTR_MAX - 1)
> +
> +
>  /**
>   * enum ovs_action_attr - Action types.
>   *
> @@ -1004,6 +1028,7 @@ enum ovs_action_attr {
>   OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
>   OVS_ACTION_ATTR_DEC_TTL,  /* Nested OVS_DEC_TTL_ATTR_*. */
>   OVS_ACTION_ATTR_DROP, /* u32 error code. */
> + OVS_ACTION_ATTR_EMIT_SAMPLE,  /* Nested OVS_EMIT_SAMPLE_ATTR_*. */
>  
>   __OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
>  * from userspace. */
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index 964225580824..3b4dba0ded59 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -24,6 +24,11 @@
>  #include 
>  #include 
>  #include 
> +
> +#if IS_ENABLED(CONFIG_PSAMPLE)
> +#include 
> +#endif
> +
>  #include 
>  
>  #include "datapath.h"
> @@ -1299,6 +1304,46 @@ static int execute_dec_ttl(struct sk_buff *skb, struct 
> sw_flow_key *key)
>   return 0;
>  }
>  
> +static int execute_emit_sample(struct datapath *dp, struct sk_buff *skb,
> +const struct sw_flow_key *key,
> +const struct nlattr *attr)
> +{
> +#if IS_ENABLED(CONFIG_PSAMPLE)
> + struct psample_group psample_group = {};
> + struct psample_metadata md = {};
> + struct vport *input_vport;
> + const struct nlattr *a;
> + int rem;
> +
> + for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
> +  a = nla_next(a, &rem)) {
> + switch (nla_type(a)) {
> + case OVS_EMIT_SAMPLE_ATTR_GROUP:
> + psample_group.group_num = nla_get_u32(a);
> + break;
> +
> + case OVS_EMIT_SAMPLE_ATTR_COOKIE:
> + md.user_cookie = nla_data(a);
> + md.user_cooki

Re: [ovs-dev] [PATCH net-next v2 5/9] net: openvswitch: add emit_sample action

2024-06-06 Thread Adrián Moreno 
On Wed, Jun 05, 2024 at 08:51:17PM GMT, Simon Horman wrote:
> On Mon, Jun 03, 2024 at 08:56:39PM +0200, Adrian Moreno wrote:
> > Add support for a new action: emit_sample.
> >
> > This action accepts a u32 group id and a variable-length cookie and uses
> > the psample multicast group to make the packet available for
> > observability.
> >
> > The maximum length of the user-defined cookie is set to 16, same as
> > tc_cookie, to discourage using cookies that will not be offloadable.
> >
> > Signed-off-by: Adrian Moreno 
>
> Hi Adrian,
>
> Some minor nits from my side.
>
> ...
>
> > diff --git a/include/uapi/linux/openvswitch.h 
> > b/include/uapi/linux/openvswitch.h
> > index efc82c318fa2..a0e9dde0584a 100644
> > --- a/include/uapi/linux/openvswitch.h
> > +++ b/include/uapi/linux/openvswitch.h
> > @@ -914,6 +914,30 @@ struct check_pkt_len_arg {
> >  };
> >  #endif
> >
> > +#define OVS_EMIT_SAMPLE_COOKIE_MAX_SIZE 16
> > +/**
> > + * enum ovs_emit_sample_attr - Attributes for %OVS_ACTION_ATTR_EMIT_SAMPLE
> > + * action.
> > + *
> > + * @OVS_EMIT_SAMPLE_ATTR_GROUP: 32-bit number to identify the source of the
> > + * sample.
> > + * @OVS_EMIT_SAMPLE_ATTR_COOKIE: A variable-length binary cookie that 
> > contains
> > + * user-defined metadata. The maximum length is 16 bytes.
> > + *
> > + * Sends the packet to the psample multicast group with the specified 
> > group and
> > + * cookie. It is possible to combine this action with the
> > + * %OVS_ACTION_ATTR_TRUNC action to limit the size of the packet being 
> > emitted.
> > + */
> > +enum ovs_emit_sample_attr {
> > +   OVS_EMIT_SAMPLE_ATTR_UNPSEC,
> > +   OVS_EMIT_SAMPLE_ATTR_GROUP, /* u32 number. */
> > +   OVS_EMIT_SAMPLE_ATTR_COOKIE,/* Optional, user specified cookie. */
> > +   __OVS_EMIT_SAMPLE_ATTR_MAX
> > +};
> > +
> > +#define OVS_EMIT_SAMPLE_ATTR_MAX (__OVS_EMIT_SAMPLE_ATTR_MAX - 1)
> > +
> > +
>
> nit: One blank line is enough.
>

Ack.

>  Flagged by checkpatch.pl
>
> >  /**
> >   * enum ovs_action_attr - Action types.
> >   *
> > @@ -1004,6 +1028,7 @@ enum ovs_action_attr {
> > OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
> > OVS_ACTION_ATTR_DEC_TTL,  /* Nested OVS_DEC_TTL_ATTR_*. */
> > OVS_ACTION_ATTR_DROP, /* u32 error code. */
> > +   OVS_ACTION_ATTR_EMIT_SAMPLE,  /* Nested OVS_EMIT_SAMPLE_ATTR_*. */
>
> nit: Please add OVS_ACTION_ATTR_EMIT_SAMPLE to the Kenrel doc
>  for this structure.
>

Thanks for spotting this. Will do.


> >
> > __OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
> >* from userspace. */
>
> ...
>

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

Re: [ovs-dev] [PATCH net-next v2 5/9] net: openvswitch: add emit_sample action

2024-06-05 Thread Simon Horman 
On Wed, Jun 05, 2024 at 07:31:55PM +, Adrián Moreno wrote:
> On Wed, Jun 05, 2024 at 08:29:22AM GMT, kernel test robot wrote:
> > Hi Adrian,
> >
> > kernel test robot noticed the following build errors:
> >
> > [auto build test ERROR on net-next/main]
> >
> > url:
> > https://github.com/intel-lab-lkp/linux/commits/Adrian-Moreno/net-psample-add-user-cookie/20240604-030055
> > base:   net-next/main
> > patch link:
> > https://lore.kernel.org/r/20240603185647.2310748-6-amorenoz%40redhat.com
> > patch subject: [PATCH net-next v2 5/9] net: openvswitch: add emit_sample 
> > action
> > config: s390-randconfig-002-20240605 
> > (https://download.01.org/0day-ci/archive/20240605/202406050852.hdtfsko0-...@intel.com/config)
> > compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 
> > d7d2d4f53fc79b4b58e8d8d08151b577c3699d4a)
> > reproduce (this is a W=1 build): 
> > (https://download.01.org/0day-ci/archive/20240605/202406050852.hdtfsko0-...@intel.com/reproduce)
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new 
> > version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot 
> > | Closes: 
> > https://lore.kernel.org/oe-kbuild-all/202406050852.hdtfsko0-...@intel.com/
> >
> > All errors (new ones prefixed by >>):
> >
> >s390x-linux-ld: net/openvswitch/actions.o: in function 
> > `do_execute_actions':
> > >> actions.c:(.text+0x1d5c): undefined reference to `psample_sample_packet'
> >
> 
> Thanks robot!
> 
> OK, I think I know what's wrong. There is an optional dependency with
> PSAMPLE. Openvswitch module does compile without PSAMPLE but there is a
> link error if OPENVSWITCH=y and PSAMPLE=m.
> 
> Looking into how to express this in the Kconfig, I'm planning to add the
> following to the next version of the series.
> 
> diff --git a/net/openvswitch/Kconfig b/net/openvswitch/Kconfig
> index 29a7081858cd..2535f3f9f462 100644
> --- a/net/openvswitch/Kconfig
> +++ b/net/openvswitch/Kconfig
> @@ -10,6 +10,7 @@ config OPENVSWITCH
>  (NF_CONNTRACK && ((!NF_DEFRAG_IPV6 || NF_DEFRAG_IPV6) && \
>(!NF_NAT || NF_NAT) && \
>(!NETFILTER_CONNCOUNT || 
> NETFILTER_CONNCOUNT)))
> + depends on PSAMPLE || !PSAMPLE
>   select LIBCRC32C
>   select MPLS
>   select NET_MPLS_GSO
> 

Thanks Adrián,

I both agree that should work, and tested with the config at the link above
and found that it does work.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next v2 5/9] net: openvswitch: add emit_sample action

2024-06-05 Thread Simon Horman 
On Mon, Jun 03, 2024 at 08:56:39PM +0200, Adrian Moreno wrote:
> Add support for a new action: emit_sample.
> 
> This action accepts a u32 group id and a variable-length cookie and uses
> the psample multicast group to make the packet available for
> observability.
> 
> The maximum length of the user-defined cookie is set to 16, same as
> tc_cookie, to discourage using cookies that will not be offloadable.
> 
> Signed-off-by: Adrian Moreno 

Hi Adrian,

Some minor nits from my side.

...

> diff --git a/include/uapi/linux/openvswitch.h 
> b/include/uapi/linux/openvswitch.h
> index efc82c318fa2..a0e9dde0584a 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -914,6 +914,30 @@ struct check_pkt_len_arg {
>  };
>  #endif
>  
> +#define OVS_EMIT_SAMPLE_COOKIE_MAX_SIZE 16
> +/**
> + * enum ovs_emit_sample_attr - Attributes for %OVS_ACTION_ATTR_EMIT_SAMPLE
> + * action.
> + *
> + * @OVS_EMIT_SAMPLE_ATTR_GROUP: 32-bit number to identify the source of the
> + * sample.
> + * @OVS_EMIT_SAMPLE_ATTR_COOKIE: A variable-length binary cookie that 
> contains
> + * user-defined metadata. The maximum length is 16 bytes.
> + *
> + * Sends the packet to the psample multicast group with the specified group 
> and
> + * cookie. It is possible to combine this action with the
> + * %OVS_ACTION_ATTR_TRUNC action to limit the size of the packet being 
> emitted.
> + */
> +enum ovs_emit_sample_attr {
> + OVS_EMIT_SAMPLE_ATTR_UNPSEC,
> + OVS_EMIT_SAMPLE_ATTR_GROUP, /* u32 number. */
> + OVS_EMIT_SAMPLE_ATTR_COOKIE,/* Optional, user specified cookie. */
> + __OVS_EMIT_SAMPLE_ATTR_MAX
> +};
> +
> +#define OVS_EMIT_SAMPLE_ATTR_MAX (__OVS_EMIT_SAMPLE_ATTR_MAX - 1)
> +
> +

nit: One blank line is enough.

 Flagged by checkpatch.pl

>  /**
>   * enum ovs_action_attr - Action types.
>   *
> @@ -1004,6 +1028,7 @@ enum ovs_action_attr {
>   OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
>   OVS_ACTION_ATTR_DEC_TTL,  /* Nested OVS_DEC_TTL_ATTR_*. */
>   OVS_ACTION_ATTR_DROP, /* u32 error code. */
> + OVS_ACTION_ATTR_EMIT_SAMPLE,  /* Nested OVS_EMIT_SAMPLE_ATTR_*. */

nit: Please add OVS_ACTION_ATTR_EMIT_SAMPLE to the Kenrel doc
 for this structure.

>  
>   __OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
>  * from userspace. */

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

Re: [ovs-dev] [PATCH net-next v2 5/9] net: openvswitch: add emit_sample action

2024-06-05 Thread Adrián Moreno 
On Wed, Jun 05, 2024 at 08:29:22AM GMT, kernel test robot wrote:
> Hi Adrian,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on net-next/main]
>
> url:
> https://github.com/intel-lab-lkp/linux/commits/Adrian-Moreno/net-psample-add-user-cookie/20240604-030055
> base:   net-next/main
> patch link:
> https://lore.kernel.org/r/20240603185647.2310748-6-amorenoz%40redhat.com
> patch subject: [PATCH net-next v2 5/9] net: openvswitch: add emit_sample 
> action
> config: s390-randconfig-002-20240605 
> (https://download.01.org/0day-ci/archive/20240605/202406050852.hdtfsko0-...@intel.com/config)
> compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 
> d7d2d4f53fc79b4b58e8d8d08151b577c3699d4a)
> reproduce (this is a W=1 build): 
> (https://download.01.org/0day-ci/archive/20240605/202406050852.hdtfsko0-...@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version 
> of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot 
> | Closes: 
> https://lore.kernel.org/oe-kbuild-all/202406050852.hdtfsko0-...@intel.com/
>
> All errors (new ones prefixed by >>):
>
>s390x-linux-ld: net/openvswitch/actions.o: in function 
> `do_execute_actions':
> >> actions.c:(.text+0x1d5c): undefined reference to `psample_sample_packet'
>

Thanks robot!

OK, I think I know what's wrong. There is an optional dependency with
PSAMPLE. Openvswitch module does compile without PSAMPLE but there is a
link error if OPENVSWITCH=y and PSAMPLE=m.

Looking into how to express this in the Kconfig, I'm planning to add the
following to the next version of the series.

diff --git a/net/openvswitch/Kconfig b/net/openvswitch/Kconfig
index 29a7081858cd..2535f3f9f462 100644
--- a/net/openvswitch/Kconfig
+++ b/net/openvswitch/Kconfig
@@ -10,6 +10,7 @@ config OPENVSWITCH
   (NF_CONNTRACK && ((!NF_DEFRAG_IPV6 || NF_DEFRAG_IPV6) && \
 (!NF_NAT || NF_NAT) && \
 (!NETFILTER_CONNCOUNT || 
NETFILTER_CONNCOUNT)))
+   depends on PSAMPLE || !PSAMPLE
select LIBCRC32C
select MPLS
select NET_MPLS_GSO

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

Re: [ovs-dev] [PATCH net-next v2 5/9] net: openvswitch: add emit_sample action

2024-06-04 Thread kernel test robot 
Hi Adrian,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:
https://github.com/intel-lab-lkp/linux/commits/Adrian-Moreno/net-psample-add-user-cookie/20240604-030055
base:   net-next/main
patch link:
https://lore.kernel.org/r/20240603185647.2310748-6-amorenoz%40redhat.com
patch subject: [PATCH net-next v2 5/9] net: openvswitch: add emit_sample action
config: s390-randconfig-002-20240605 
(https://download.01.org/0day-ci/archive/20240605/202406050852.hdtfsko0-...@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 
d7d2d4f53fc79b4b58e8d8d08151b577c3699d4a)
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20240605/202406050852.hdtfsko0-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202406050852.hdtfsko0-...@intel.com/

All errors (new ones prefixed by >>):

   s390x-linux-ld: net/openvswitch/actions.o: in function `do_execute_actions':
>> actions.c:(.text+0x1d5c): undefined reference to `psample_sample_packet'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH net-next v2 5/9] net: openvswitch: add emit_sample action

2024-06-03 Thread Adrian Moreno 
Add support for a new action: emit_sample.

This action accepts a u32 group id and a variable-length cookie and uses
the psample multicast group to make the packet available for
observability.

The maximum length of the user-defined cookie is set to 16, same as
tc_cookie, to discourage using cookies that will not be offloadable.

Signed-off-by: Adrian Moreno 
---
 Documentation/netlink/specs/ovs_flow.yaml | 17 
 include/uapi/linux/openvswitch.h  | 25 
 net/openvswitch/actions.c | 50 +++
 net/openvswitch/flow_netlink.c| 33 ++-
 4 files changed, 124 insertions(+), 1 deletion(-)

diff --git a/Documentation/netlink/specs/ovs_flow.yaml 
b/Documentation/netlink/specs/ovs_flow.yaml
index 4fdfc6b5cae9..a7ab5593a24f 100644
--- a/Documentation/netlink/specs/ovs_flow.yaml
+++ b/Documentation/netlink/specs/ovs_flow.yaml
@@ -727,6 +727,12 @@ attribute-sets:
 name: dec-ttl
 type: nest
 nested-attributes: dec-ttl-attrs
+  -
+name: emit-sample
+type: nest
+nested-attributes: emit-sample-attrs
+doc: |
+  Sends a packet sample to psample for external observation.
   -
 name: tunnel-key-attrs
 enum-name: ovs-tunnel-key-attr
@@ -938,6 +944,17 @@ attribute-sets:
   -
 name: gbp
 type: u32
+  -
+name: emit-sample-attrs
+enum-name: ovs-emit-sample-attr
+name-prefix: ovs-emit-sample-attr-
+attributes:
+  -
+name: group
+type: u32
+  -
+name: cookie
+type: binary
 
 operations:
   name-prefix: ovs-flow-cmd-
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index efc82c318fa2..a0e9dde0584a 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -914,6 +914,30 @@ struct check_pkt_len_arg {
 };
 #endif
 
+#define OVS_EMIT_SAMPLE_COOKIE_MAX_SIZE 16
+/**
+ * enum ovs_emit_sample_attr - Attributes for %OVS_ACTION_ATTR_EMIT_SAMPLE
+ * action.
+ *
+ * @OVS_EMIT_SAMPLE_ATTR_GROUP: 32-bit number to identify the source of the
+ * sample.
+ * @OVS_EMIT_SAMPLE_ATTR_COOKIE: A variable-length binary cookie that contains
+ * user-defined metadata. The maximum length is 16 bytes.
+ *
+ * Sends the packet to the psample multicast group with the specified group and
+ * cookie. It is possible to combine this action with the
+ * %OVS_ACTION_ATTR_TRUNC action to limit the size of the packet being emitted.
+ */
+enum ovs_emit_sample_attr {
+   OVS_EMIT_SAMPLE_ATTR_UNPSEC,
+   OVS_EMIT_SAMPLE_ATTR_GROUP, /* u32 number. */
+   OVS_EMIT_SAMPLE_ATTR_COOKIE,/* Optional, user specified cookie. */
+   __OVS_EMIT_SAMPLE_ATTR_MAX
+};
+
+#define OVS_EMIT_SAMPLE_ATTR_MAX (__OVS_EMIT_SAMPLE_ATTR_MAX - 1)
+
+
 /**
  * enum ovs_action_attr - Action types.
  *
@@ -1004,6 +1028,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
OVS_ACTION_ATTR_DEC_TTL,  /* Nested OVS_DEC_TTL_ATTR_*. */
OVS_ACTION_ATTR_DROP, /* u32 error code. */
+   OVS_ACTION_ATTR_EMIT_SAMPLE,  /* Nested OVS_EMIT_SAMPLE_ATTR_*. */
 
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
   * from userspace. */
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 964225580824..3b4dba0ded59 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -24,6 +24,11 @@
 #include 
 #include 
 #include 
+
+#if IS_ENABLED(CONFIG_PSAMPLE)
+#include 
+#endif
+
 #include 
 
 #include "datapath.h"
@@ -1299,6 +1304,46 @@ static int execute_dec_ttl(struct sk_buff *skb, struct 
sw_flow_key *key)
return 0;
 }
 
+static int execute_emit_sample(struct datapath *dp, struct sk_buff *skb,
+  const struct sw_flow_key *key,
+  const struct nlattr *attr)
+{
+#if IS_ENABLED(CONFIG_PSAMPLE)
+   struct psample_group psample_group = {};
+   struct psample_metadata md = {};
+   struct vport *input_vport;
+   const struct nlattr *a;
+   int rem;
+
+   for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
+a = nla_next(a, &rem)) {
+   switch (nla_type(a)) {
+   case OVS_EMIT_SAMPLE_ATTR_GROUP:
+   psample_group.group_num = nla_get_u32(a);
+   break;
+
+   case OVS_EMIT_SAMPLE_ATTR_COOKIE:
+   md.user_cookie = nla_data(a);
+   md.user_cookie_len = nla_len(a);
+   break;
+   }
+   }
+
+   psample_group.net = ovs_dp_get_net(dp);
+
+   input_vport = ovs_vport_rcu(dp, key->phy.in_port);
+   if (!input_vport)
+   input_vport = ovs_vport_rcu(dp, OVSP_LOCAL);
+
+   md.in_ifindex = input_vport->dev->ifindex;
+   md.trunc_size = skb->len - OVS_CB(skb)->cutlen;
+
+   psa