[1/8] ethdev: introduce sample action for rte flow

Message ID 1593102379-400132-2-git-send-email-jiaweiw@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series support the flow-based traffic sampling |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-testing fail Testing issues

Commit Message

Jiawei Wang June 25, 2020, 4:26 p.m. UTC
  When using full offload, all traffic will be handled by the HW, and
directed to the requested vf or wire, the control application loses
visibility on the traffic.
So there's a need for an action that will enable the control application
some visibility.

The solution is introduced a new action that will sample the incoming
traffic and send a duplicated traffic in some predefined ratio to the
application, while the original packet will continue to the target
destination.

The packets sampled equals is '1/ratio', if the ratio value be set to 1
, means that the packets would be completely mirrored. The sample packet
can be assigned with different set of actions from the original packet.

In order to support the sample packet in rte_flow, new rte_flow action
definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure rte_flow_action_sample
will be introduced.

Signed-off-by: Jiawei Wang <jiaweiw@mellanox.com>
---
 lib/librte_ethdev/rte_flow.c |  1 +
 lib/librte_ethdev/rte_flow.h | 29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+)
  

Comments

Jerin Jacob June 25, 2020, 5:55 p.m. UTC | #1
On Thu, Jun 25, 2020 at 10:20 PM Jiawei Wang <jiaweiw@mellanox.com> wrote:
>
> When using full offload, all traffic will be handled by the HW, and
> directed to the requested vf or wire, the control application loses
> visibility on the traffic.
> So there's a need for an action that will enable the control application
> some visibility.
>
> The solution is introduced a new action that will sample the incoming
> traffic and send a duplicated traffic in some predefined ratio to the
> application, while the original packet will continue to the target
> destination.
>
> The packets sampled equals is '1/ratio', if the ratio value be set to 1
> , means that the packets would be completely mirrored. The sample packet
> can be assigned with different set of actions from the original packet.
>
> In order to support the sample packet in rte_flow, new rte_flow action
> definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure rte_flow_action_sample

Isn't mirroring the packet? How about, RTE_FLOW_ACTION_TYPE_MIRROR
I am not able to understand, Why it is called sample.


> will be introduced.
>
> Signed-off-by: Jiawei Wang <jiaweiw@mellanox.com>
> ---
>  lib/librte_ethdev/rte_flow.c |  1 +
>  lib/librte_ethdev/rte_flow.h | 29 +++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+)
>

> + * Adds a sample action to a matched flow.
> + *
> + * The matching packets will be duplicated to a special queue or vport
> + * in the predefined probabiilty, All the packets continues processing
> + * on the default flow path.
> + *
> + * When the sample ratio is set to 1 then the packets will be 100% mirrored.
> + * Additional action list be supported to add for sampled or mirrored packets.
> + */
> +struct rte_flow_action_sample {
> +       /* packets sampled equals to '1/ratio' */
> +       const uint32_t ratio;
> +       /* sub-action list specific for the sampling hit cases */

Why not use, RTE_FLOW_ACTION_TYPE_PASSTHRU action to append further
action for this ACTION.

> +       const struct rte_flow_action *actions;
> +};
> +
> +/**
>   * Verbose error types.
>   *
>   * Most of them provide the type of the object referenced by struct
> --
> 1.8.3.1
>
  
Thomas Monjalon June 25, 2020, 7:29 p.m. UTC | #2
25/06/2020 19:55, Jerin Jacob:
> On Thu, Jun 25, 2020 at 10:20 PM Jiawei Wang <jiaweiw@mellanox.com> wrote:
> >
> > When using full offload, all traffic will be handled by the HW, and
> > directed to the requested vf or wire, the control application loses
> > visibility on the traffic.
> > So there's a need for an action that will enable the control application
> > some visibility.
> >
> > The solution is introduced a new action that will sample the incoming
> > traffic and send a duplicated traffic in some predefined ratio to the
> > application, while the original packet will continue to the target
> > destination.
> >
> > The packets sampled equals is '1/ratio', if the ratio value be set to 1
> > , means that the packets would be completely mirrored. The sample packet
> > can be assigned with different set of actions from the original packet.
> >
> > In order to support the sample packet in rte_flow, new rte_flow action
> > definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure rte_flow_action_sample
> 
> Isn't mirroring the packet? How about, RTE_FLOW_ACTION_TYPE_MIRROR
> I am not able to understand, Why it is called sample.

Sampling is a partial mirroring.
Full mirroring is sampling 100% packets (ratio = 1).
That's why only one action is enough.
  
Jerin Jacob June 26, 2020, 10:35 a.m. UTC | #3
On Fri, Jun 26, 2020 at 12:59 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 25/06/2020 19:55, Jerin Jacob:
> > On Thu, Jun 25, 2020 at 10:20 PM Jiawei Wang <jiaweiw@mellanox.com> wrote:
> > >
> > > When using full offload, all traffic will be handled by the HW, and
> > > directed to the requested vf or wire, the control application loses
> > > visibility on the traffic.
> > > So there's a need for an action that will enable the control application
> > > some visibility.
> > >
> > > The solution is introduced a new action that will sample the incoming
> > > traffic and send a duplicated traffic in some predefined ratio to the
> > > application, while the original packet will continue to the target
> > > destination.
> > >
> > > The packets sampled equals is '1/ratio', if the ratio value be set to 1
> > > , means that the packets would be completely mirrored. The sample packet
> > > can be assigned with different set of actions from the original packet.
> > >
> > > In order to support the sample packet in rte_flow, new rte_flow action
> > > definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure rte_flow_action_sample
> >
> > Isn't mirroring the packet? How about, RTE_FLOW_ACTION_TYPE_MIRROR
> > I am not able to understand, Why it is called sample.
>
> Sampling is a partial mirroring.

I think, By definition, _sampling_ is the _selection_ of items from a
specific group.
I think, _sampling_ is not dictating, what is the real action for the
"selected"  items.
One can get confused with the selected ones can be for forward, drop
any other action.

So IMO, explicit mirror keyword usage makes it is clear.

Some more related questions:
1) What is the real use case for ratio? I am not against adding a
ratio attribute if the MLX hardware supports it. It will be good to
know the use case from the application perspective? And what basics
application set ratio != 1?
2) If it is for "rate-limiting" or "policing", why not use rte_mtr
object (rte_mtr.h) via rte_flow action.
3) One of the issue for driver developers and application writers are
overlapping APIs. This would overlap with rte_eth_mirror_rule_set()
API.

Can we deprecate rte_eth_mirror_rule_set() API? It will be a pain for
all to have overlapping APIs. We have not fixed the VLAN filter API
overlap with rte_flow in ethdev. Its being TODO for multiple releases
now.


> Full mirroring is sampling 100% packets (ratio = 1).
> That's why only one action is enough.
>
>
  
Thomas Monjalon June 26, 2020, 10:45 a.m. UTC | #4
26/06/2020 12:35, Jerin Jacob:
> On Fri, Jun 26, 2020 at 12:59 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > 25/06/2020 19:55, Jerin Jacob:
> > > On Thu, Jun 25, 2020 at 10:20 PM Jiawei Wang <jiaweiw@mellanox.com> wrote:
> > > >
> > > > When using full offload, all traffic will be handled by the HW, and
> > > > directed to the requested vf or wire, the control application loses
> > > > visibility on the traffic.
> > > > So there's a need for an action that will enable the control application
> > > > some visibility.
> > > >
> > > > The solution is introduced a new action that will sample the incoming
> > > > traffic and send a duplicated traffic in some predefined ratio to the
> > > > application, while the original packet will continue to the target
> > > > destination.
> > > >
> > > > The packets sampled equals is '1/ratio', if the ratio value be set to 1
> > > > , means that the packets would be completely mirrored. The sample packet
> > > > can be assigned with different set of actions from the original packet.
> > > >
> > > > In order to support the sample packet in rte_flow, new rte_flow action
> > > > definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure rte_flow_action_sample
> > >
> > > Isn't mirroring the packet? How about, RTE_FLOW_ACTION_TYPE_MIRROR
> > > I am not able to understand, Why it is called sample.
> >
> > Sampling is a partial mirroring.
> 
> I think, By definition, _sampling_ is the _selection_ of items from a
> specific group.
> I think, _sampling_ is not dictating, what is the real action for the
> "selected"  items.
> One can get confused with the selected ones can be for forward, drop
> any other action.

I see. Good design question (I will let others reply).

> So IMO, explicit mirror keyword usage makes it is clear.
> 
> Some more related questions:
> 1) What is the real use case for ratio? I am not against adding a
> ratio attribute if the MLX hardware supports it. It will be good to
> know the use case from the application perspective? And what basics
> application set ratio != 1?

If I understand well, some applications want to check,
by picking random packets, that the processing is not failing.

> 2) If it is for "rate-limiting" or "policing", why not use rte_mtr
> object (rte_mtr.h) via rte_flow action.
> 3) One of the issue for driver developers and application writers are
> overlapping APIs. This would overlap with rte_eth_mirror_rule_set()
> API.
> 
> Can we deprecate rte_eth_mirror_rule_set() API? It will be a pain for
> all to have overlapping APIs. We have not fixed the VLAN filter API
> overlap with rte_flow in ethdev. Its being TODO for multiple releases
> now.

Ooooooooh yes!
I think flow-based API is more powerful, and should deprecate
old port-based API.
I want to help deprecating such API in 20.11 if possible.

> > Full mirroring is sampling 100% packets (ratio = 1).
> > That's why only one action is enough.
  
Jerin Jacob June 26, 2020, 11:10 a.m. UTC | #5
On Fri, Jun 26, 2020 at 4:16 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 26/06/2020 12:35, Jerin Jacob:
> > On Fri, Jun 26, 2020 at 12:59 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> > >
> > > 25/06/2020 19:55, Jerin Jacob:
> > > > On Thu, Jun 25, 2020 at 10:20 PM Jiawei Wang <jiaweiw@mellanox.com> wrote:
> > > > >
> > > > > When using full offload, all traffic will be handled by the HW, and
> > > > > directed to the requested vf or wire, the control application loses
> > > > > visibility on the traffic.
> > > > > So there's a need for an action that will enable the control application
> > > > > some visibility.
> > > > >
> > > > > The solution is introduced a new action that will sample the incoming
> > > > > traffic and send a duplicated traffic in some predefined ratio to the
> > > > > application, while the original packet will continue to the target
> > > > > destination.
> > > > >
> > > > > The packets sampled equals is '1/ratio', if the ratio value be set to 1
> > > > > , means that the packets would be completely mirrored. The sample packet
> > > > > can be assigned with different set of actions from the original packet.
> > > > >
> > > > > In order to support the sample packet in rte_flow, new rte_flow action
> > > > > definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure rte_flow_action_sample
> > > >
> > > > Isn't mirroring the packet? How about, RTE_FLOW_ACTION_TYPE_MIRROR
> > > > I am not able to understand, Why it is called sample.
> > >
> > > Sampling is a partial mirroring.
> >
> > I think, By definition, _sampling_ is the _selection_ of items from a
> > specific group.
> > I think, _sampling_ is not dictating, what is the real action for the
> > "selected"  items.
> > One can get confused with the selected ones can be for forward, drop
> > any other action.
>
> I see. Good design question (I will let others reply).
>
> > So IMO, explicit mirror keyword usage makes it is clear.
> >
> > Some more related questions:
> > 1) What is the real use case for ratio? I am not against adding a
> > ratio attribute if the MLX hardware supports it. It will be good to
> > know the use case from the application perspective? And what basics
> > application set ratio != 1?
>
> If I understand well, some applications want to check,
> by picking random packets, that the processing is not failing.

Not clear to me. I will wait for another explanation if any.
In what basics application set .1 vs .8?

>
> > 2) If it is for "rate-limiting" or "policing", why not use rte_mtr
> > object (rte_mtr.h) via rte_flow action.
> > 3) One of the issue for driver developers and application writers are
> > overlapping APIs. This would overlap with rte_eth_mirror_rule_set()
> > API.
> >
> > Can we deprecate rte_eth_mirror_rule_set() API? It will be a pain for
> > all to have overlapping APIs. We have not fixed the VLAN filter API
> > overlap with rte_flow in ethdev. Its being TODO for multiple releases
> > now.
>
> Ooooooooh yes!
> I think flow-based API is more powerful, and should deprecate
> old port-based API.

+1 from me.

it is taking too much effort and time to make support duplicate APIs.

> I want to help deprecating such API in 20.11 if possible.

Please start that discussion. In this case, it is clear API overlap
with rte_eth_mirror_rule_set().
We should not have two separate paths for the same function in the
same ethdev library.



>
> > > Full mirroring is sampling 100% packets (ratio = 1).
> > > That's why only one action is enough.
>
>
>
  
Andrew Rybchenko June 28, 2020, 8:14 a.m. UTC | #6
On 6/26/20 2:10 PM, Jerin Jacob wrote:
> On Fri, Jun 26, 2020 at 4:16 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>>
>> 26/06/2020 12:35, Jerin Jacob:
>>> On Fri, Jun 26, 2020 at 12:59 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>>>>
>>>> 25/06/2020 19:55, Jerin Jacob:
>>>>> On Thu, Jun 25, 2020 at 10:20 PM Jiawei Wang <jiaweiw@mellanox.com> wrote:
>>>>>>
>>>>>> When using full offload, all traffic will be handled by the HW, and
>>>>>> directed to the requested vf or wire, the control application loses
>>>>>> visibility on the traffic.
>>>>>> So there's a need for an action that will enable the control application
>>>>>> some visibility.
>>>>>>
>>>>>> The solution is introduced a new action that will sample the incoming
>>>>>> traffic and send a duplicated traffic in some predefined ratio to the
>>>>>> application, while the original packet will continue to the target
>>>>>> destination.
>>>>>>
>>>>>> The packets sampled equals is '1/ratio', if the ratio value be set to 1
>>>>>> , means that the packets would be completely mirrored. The sample packet
>>>>>> can be assigned with different set of actions from the original packet.
>>>>>>
>>>>>> In order to support the sample packet in rte_flow, new rte_flow action
>>>>>> definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure rte_flow_action_sample
>>>>>
>>>>> Isn't mirroring the packet? How about, RTE_FLOW_ACTION_TYPE_MIRROR
>>>>> I am not able to understand, Why it is called sample.
>>>>
>>>> Sampling is a partial mirroring.
>>>
>>> I think, By definition, _sampling_ is the _selection_ of items from a
>>> specific group.
>>> I think, _sampling_ is not dictating, what is the real action for the
>>> "selected"  items.
>>> One can get confused with the selected ones can be for forward, drop
>>> any other action.
>>
>> I see. Good design question (I will let others reply).
>>
>>> So IMO, explicit mirror keyword usage makes it is clear.
>>>
>>> Some more related questions:
>>> 1) What is the real use case for ratio? I am not against adding a
>>> ratio attribute if the MLX hardware supports it. It will be good to
>>> know the use case from the application perspective? And what basics
>>> application set ratio != 1?
>>
>> If I understand well, some applications want to check,
>> by picking random packets, that the processing is not failing.
> 
> Not clear to me. I will wait for another explanation if any.
> In what basics application set .1 vs .8?
> 
>>
>>> 2) If it is for "rate-limiting" or "policing", why not use rte_mtr
>>> object (rte_mtr.h) via rte_flow action.
>>> 3) One of the issue for driver developers and application writers are
>>> overlapping APIs. This would overlap with rte_eth_mirror_rule_set()
>>> API.
>>>
>>> Can we deprecate rte_eth_mirror_rule_set() API? It will be a pain for
>>> all to have overlapping APIs. We have not fixed the VLAN filter API
>>> overlap with rte_flow in ethdev. Its being TODO for multiple releases
>>> now.
>>
>> Ooooooooh yes!
>> I think flow-based API is more powerful, and should deprecate
>> old port-based API.
> 
> +1 from me.

+1

> it is taking too much effort and time to make support duplicate APIs.
> 
>> I want to help deprecating such API in 20.11 if possible.
> 
> Please start that discussion. In this case, it is clear API overlap
> with rte_eth_mirror_rule_set().
> We should not have two separate paths for the same function in the
> same ethdev library.
> 
> 
> 
>>
>>>> Full mirroring is sampling 100% packets (ratio = 1).
>>>> That's why only one action is enough.
>>
>>
>>
  
Andrew Rybchenko June 28, 2020, 8:27 a.m. UTC | #7
On 6/25/20 7:26 PM, Jiawei Wang wrote:
> When using full offload, all traffic will be handled by the HW, and
> directed to the requested vf or wire, the control application loses
> visibility on the traffic.
> So there's a need for an action that will enable the control application
> some visibility.
> 
> The solution is introduced a new action that will sample the incoming
> traffic and send a duplicated traffic in some predefined ratio to the
> application, while the original packet will continue to the target
> destination.
> 
> The packets sampled equals is '1/ratio', if the ratio value be set to 1
> , means that the packets would be completely mirrored. The sample packet
> can be assigned with different set of actions from the original packet.
> 
> In order to support the sample packet in rte_flow, new rte_flow action
> definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure rte_flow_action_sample
> will be introduced.
> 
> Signed-off-by: Jiawei Wang <jiaweiw@mellanox.com>

[snip]

> @@ -2709,6 +2716,28 @@ struct rte_flow_action {
>  struct rte_flow;
>  
>  /**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * RTE_FLOW_ACTION_TYPE_SAMPLE
> + *
> + * Adds a sample action to a matched flow.
> + *
> + * The matching packets will be duplicated to a special queue or vport
> + * in the predefined probabiilty, All the packets continues processing
> + * on the default flow path.
> + *
> + * When the sample ratio is set to 1 then the packets will be 100% mirrored.
> + * Additional action list be supported to add for sampled or mirrored packets.
> + */
> +struct rte_flow_action_sample {
> +	/* packets sampled equals to '1/ratio' */
> +	const uint32_t ratio;
> +	/* sub-action list specific for the sampling hit cases */
> +	const struct rte_flow_action *actions;

This design idea does not look good to me from the very
beginning. IMHO it does not fit flow API overall design.
I mean sub-action list.

As I understand Linux iptables solves it on match level
(i.e. in pattern). E.g. "limit" extension which is basically
sampling. Sampling using meta pattern item in combination
with PASSTHRU action (to make sampling actions non-terminating
if required) is a better solution from design point of view.
  
Jiawei Wang June 28, 2020, 1:16 p.m. UTC | #8
On Friday, June 26, 2020 7:10 PM Jerin Jacob <jerinjacobk@gmail.com> Wrote:
>
> On Fri, Jun 26, 2020 at 4:16 PM Thomas Monjalon <thomas@monjalon.net>
> wrote:
> >
> > 26/06/2020 12:35, Jerin Jacob:
> > > On Fri, Jun 26, 2020 at 12:59 AM Thomas Monjalon
> <thomas@monjalon.net> wrote:
> > > >
> > > > 25/06/2020 19:55, Jerin Jacob:
> > > > > On Thu, Jun 25, 2020 at 10:20 PM Jiawei Wang
> <jiaweiw@mellanox.com> wrote:
> > > > > >
> > > > > > When using full offload, all traffic will be handled by the
> > > > > > HW, and directed to the requested vf or wire, the control
> > > > > > application loses visibility on the traffic.
> > > > > > So there's a need for an action that will enable the control
> > > > > > application some visibility.
> > > > > >
> > > > > > The solution is introduced a new action that will sample the
> > > > > > incoming traffic and send a duplicated traffic in some
> > > > > > predefined ratio to the application, while the original packet
> > > > > > will continue to the target destination.
> > > > > >
> > > > > > The packets sampled equals is '1/ratio', if the ratio value be
> > > > > > set to 1 , means that the packets would be completely
> > > > > > mirrored. The sample packet can be assigned with different set of
> actions from the original packet.
> > > > > >
> > > > > > In order to support the sample packet in rte_flow, new
> > > > > > rte_flow action definition RTE_FLOW_ACTION_TYPE_SAMPLE and
> > > > > > structure rte_flow_action_sample
> > > > >
> > > > > Isn't mirroring the packet? How about,
> > > > > RTE_FLOW_ACTION_TYPE_MIRROR I am not able to understand, Why
> it is called sample.
> > > >
> > > > Sampling is a partial mirroring.
> > >
> > > I think, By definition, _sampling_ is the _selection_ of items from
> > > a specific group.
> > > I think, _sampling_ is not dictating, what is the real action for
> > > the "selected"  items.
> > > One can get confused with the selected ones can be for forward, drop
> > > any other action.
> >
> > I see. Good design question (I will let others reply).
> >
> > > So IMO, explicit mirror keyword usage makes it is clear.

Sampled packet is duplicated from incoming traffic at specific ratio and will go to different sample actions;
ratio=1 is 100% duplication or mirroring.
All packets will continue to go to default flow actions.

> > >
> > > Some more related questions:
> > > 1) What is the real use case for ratio? I am not against adding a
> > > ratio attribute if the MLX hardware supports it. It will be good to
> > > know the use case from the application perspective? And what basics
> > > application set ratio != 1?
> >
> > If I understand well, some applications want to check, by picking
> > random packets, that the processing is not failing.
> 
> Not clear to me. I will wait for another explanation if any.
> In what basics application set .1 vs .8?

The real case is like monitor the traffic with full-offload. 
While packet hit the sample flow, the matching packets will be sampled and sent to specific Queue,
align with OVS sflow probability, user application can set it different value.

> 
> >
> > > 2) If it is for "rate-limiting" or "policing", why not use rte_mtr
> > > object (rte_mtr.h) via rte_flow action.

The sample ratio isn’t the same as “meter’, the ratio of sampling will be calculated with incoming packets mask (every some packets sampled 1). Then the packets will be duplicated and go to do the other sample actions.


> > > 3) One of the issue for driver developers and application writers
> > > are overlapping APIs. This would overlap with
> > > rte_eth_mirror_rule_set() API.
> > >
> > > Can we deprecate rte_eth_mirror_rule_set() API? It will be a pain
> > > for all to have overlapping APIs. We have not fixed the VLAN filter
> > > API overlap with rte_flow in ethdev. Its being TODO for multiple
> > > releases now.
> >
> > Ooooooooh yes!
> > I think flow-based API is more powerful, and should deprecate old
> > port-based API.
> 
> +1 from me.
> 
> it is taking too much effort and time to make support duplicate APIs.
> 
> > I want to help deprecating such API in 20.11 if possible.
> 
> Please start that discussion. In this case, it is clear API overlap with
> rte_eth_mirror_rule_set().
> We should not have two separate paths for the same function in the same
> ethdev library.
> 
> 
> 
> >
> > > > Full mirroring is sampling 100% packets (ratio = 1).
> > > > That's why only one action is enough.
> >
> >
> >
  
Jerin Jacob June 28, 2020, 1:37 p.m. UTC | #9
On Sun, Jun 28, 2020 at 6:46 PM Jiawei(Jonny) Wang <jiaweiw@mellanox.com> wrote:
>
>
> On Friday, June 26, 2020 7:10 PM Jerin Jacob <jerinjacobk@gmail.com> Wrote:
> >
> > On Fri, Jun 26, 2020 at 4:16 PM Thomas Monjalon <thomas@monjalon.net>
> > wrote:
> > >
> > > 26/06/2020 12:35, Jerin Jacob:
> > > > On Fri, Jun 26, 2020 at 12:59 AM Thomas Monjalon
> > <thomas@monjalon.net> wrote:
> > > > >
> > > > > 25/06/2020 19:55, Jerin Jacob:
> > > > > > On Thu, Jun 25, 2020 at 10:20 PM Jiawei Wang
> > <jiaweiw@mellanox.com> wrote:
> > > > > > >
> > > > > > > When using full offload, all traffic will be handled by the
> > > > > > > HW, and directed to the requested vf or wire, the control
> > > > > > > application loses visibility on the traffic.
> > > > > > > So there's a need for an action that will enable the control
> > > > > > > application some visibility.
> > > > > > >
> > > > > > > The solution is introduced a new action that will sample the
> > > > > > > incoming traffic and send a duplicated traffic in some
> > > > > > > predefined ratio to the application, while the original packet
> > > > > > > will continue to the target destination.
> > > > > > >
> > > > > > > The packets sampled equals is '1/ratio', if the ratio value be
> > > > > > > set to 1 , means that the packets would be completely
> > > > > > > mirrored. The sample packet can be assigned with different set of
> > actions from the original packet.
> > > > > > >
> > > > > > > In order to support the sample packet in rte_flow, new
> > > > > > > rte_flow action definition RTE_FLOW_ACTION_TYPE_SAMPLE and
> > > > > > > structure rte_flow_action_sample
> > > > > >
> > > > > > Isn't mirroring the packet? How about,
> > > > > > RTE_FLOW_ACTION_TYPE_MIRROR I am not able to understand, Why
> > it is called sample.
> > > > >
> > > > > Sampling is a partial mirroring.
> > > >
> > > > I think, By definition, _sampling_ is the _selection_ of items from
> > > > a specific group.
> > > > I think, _sampling_ is not dictating, what is the real action for
> > > > the "selected"  items.
> > > > One can get confused with the selected ones can be for forward, drop
> > > > any other action.
> > >
> > > I see. Good design question (I will let others reply).
> > >
> > > > So IMO, explicit mirror keyword usage makes it is clear.
>
> Sampled packet is duplicated from incoming traffic at specific ratio and will go to different sample actions;
> ratio=1 is 100% duplication or mirroring.
> All packets will continue to go to default flow actions.

Functionality is clear from the git commit log(Not from action name).
The only question is what would be the appropriate name
for this action. RTE_FLOW_ACTION_TYPE_SAMPLE vs RTE_FLOW_ACTION_TYPE_MIRROR

>
> > > >
> > > > Some more related questions:
> > > > 1) What is the real use case for ratio? I am not against adding a
> > > > ratio attribute if the MLX hardware supports it. It will be good to
> > > > know the use case from the application perspective? And what basics
> > > > application set ratio != 1?
> > >
> > > If I understand well, some applications want to check, by picking
> > > random packets, that the processing is not failing.
> >
> > Not clear to me. I will wait for another explanation if any.
> > In what basics application set .1 vs .8?
>
> The real case is like monitor the traffic with full-offload.
> While packet hit the sample flow, the matching packets will be sampled and sent to specific Queue,
> align with OVS sflow probability, user application can set it different value.

I understand the use case for mirror and supported in a lot of HW.
What I would like to understand is the use case for "ratio"?
Is the "ratio" part of OpenFlow spec? Or Is it an MLX hardware feature?



>
> >
> > >
> > > > 2) If it is for "rate-limiting" or "policing", why not use rte_mtr
> > > > object (rte_mtr.h) via rte_flow action.
>
> The sample ratio isn’t the same as “meter’, the ratio of sampling will be calculated with incoming packets mask (every some packets sampled 1). Then the packets will be duplicated and go to do the other sample actions.

What I meant here is , If the ratio is used for rate-limiting then
having a cascade rule like RTE_FLOW_ACTION_TYPE_MIRROR,
RTE_FLOW_ACTION_TYPE_MTR will do the job.

>
>
> > > > 3) One of the issue for driver developers and application writers
> > > > are overlapping APIs. This would overlap with
> > > > rte_eth_mirror_rule_set() API.
> > > >
> > > > Can we deprecate rte_eth_mirror_rule_set() API? It will be a pain
> > > > for all to have overlapping APIs. We have not fixed the VLAN filter
> > > > API overlap with rte_flow in ethdev. Its being TODO for multiple
> > > > releases now.
> > >
> > > Ooooooooh yes!
> > > I think flow-based API is more powerful, and should deprecate old
> > > port-based API.
> >
> > +1 from me.
> >
> > it is taking too much effort and time to make support duplicate APIs.
> >
> > > I want to help deprecating such API in 20.11 if possible.
> >
> > Please start that discussion. In this case, it is clear API overlap with
> > rte_eth_mirror_rule_set().
> > We should not have two separate paths for the same function in the same
> > ethdev library.
> >
> >
> >
> > >
> > > > > Full mirroring is sampling 100% packets (ratio = 1).
> > > > > That's why only one action is enough.
> > >
> > >
> > >
  
Jiawei Wang June 28, 2020, 3:52 p.m. UTC | #10
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Sunday, June 28, 2020 9:38 PM
> To: Jiawei(Jonny) Wang <jiaweiw@mellanox.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>; Ori Kam
> <orika@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>;
> Matan Azrad <matan@mellanox.com>; dpdk-dev <dev@dpdk.org>; Raslan
> Darawsheh <rasland@mellanox.com>; ian.stokes@intel.com;
> fbl@redhat.com; Ferruh Yigit <ferruh.yigit@intel.com>; Andrew Rybchenko
> <arybchenko@solarflare.com>
> Subject: Re: [dpdk-dev] [PATCH 1/8] ethdev: introduce sample action for rte
> flow
> 
> On Sun, Jun 28, 2020 at 6:46 PM Jiawei(Jonny) Wang
> <jiaweiw@mellanox.com> wrote:
> >
> >
> > On Friday, June 26, 2020 7:10 PM Jerin Jacob <jerinjacobk@gmail.com>
> Wrote:
> > >
> > > On Fri, Jun 26, 2020 at 4:16 PM Thomas Monjalon
> > > <thomas@monjalon.net>
> > > wrote:
> > > >
> > > > 26/06/2020 12:35, Jerin Jacob:
> > > > > On Fri, Jun 26, 2020 at 12:59 AM Thomas Monjalon
> > > <thomas@monjalon.net> wrote:
> > > > > >
> > > > > > 25/06/2020 19:55, Jerin Jacob:
> > > > > > > On Thu, Jun 25, 2020 at 10:20 PM Jiawei Wang
> > > <jiaweiw@mellanox.com> wrote:
> > > > > > > >
> > > > > > > > When using full offload, all traffic will be handled by
> > > > > > > > the HW, and directed to the requested vf or wire, the
> > > > > > > > control application loses visibility on the traffic.
> > > > > > > > So there's a need for an action that will enable the
> > > > > > > > control application some visibility.
> > > > > > > >
> > > > > > > > The solution is introduced a new action that will sample
> > > > > > > > the incoming traffic and send a duplicated traffic in some
> > > > > > > > predefined ratio to the application, while the original
> > > > > > > > packet will continue to the target destination.
> > > > > > > >
> > > > > > > > The packets sampled equals is '1/ratio', if the ratio
> > > > > > > > value be set to 1 , means that the packets would be
> > > > > > > > completely mirrored. The sample packet can be assigned
> > > > > > > > with different set of
> > > actions from the original packet.
> > > > > > > >
> > > > > > > > In order to support the sample packet in rte_flow, new
> > > > > > > > rte_flow action definition RTE_FLOW_ACTION_TYPE_SAMPLE
> and
> > > > > > > > structure rte_flow_action_sample
> > > > > > >
> > > > > > > Isn't mirroring the packet? How about,
> > > > > > > RTE_FLOW_ACTION_TYPE_MIRROR I am not able to understand,
> Why
> > > it is called sample.
> > > > > >
> > > > > > Sampling is a partial mirroring.
> > > > >
> > > > > I think, By definition, _sampling_ is the _selection_ of items
> > > > > from a specific group.
> > > > > I think, _sampling_ is not dictating, what is the real action
> > > > > for the "selected"  items.
> > > > > One can get confused with the selected ones can be for forward,
> > > > > drop any other action.
> > > >
> > > > I see. Good design question (I will let others reply).
> > > >
> > > > > So IMO, explicit mirror keyword usage makes it is clear.
> >
> > Sampled packet is duplicated from incoming traffic at specific ratio
> > and will go to different sample actions;
> > ratio=1 is 100% duplication or mirroring.
> > All packets will continue to go to default flow actions.
> 
> Functionality is clear from the git commit log(Not from action name).
> The only question is what would be the appropriate name for this action.
> RTE_FLOW_ACTION_TYPE_SAMPLE vs RTE_FLOW_ACTION_TYPE_MIRROR
> 
> >
> > > > >
> > > > > Some more related questions:
> > > > > 1) What is the real use case for ratio? I am not against adding
> > > > > a ratio attribute if the MLX hardware supports it. It will be
> > > > > good to know the use case from the application perspective? And
> > > > > what basics application set ratio != 1?
> > > >
> > > > If I understand well, some applications want to check, by picking
> > > > random packets, that the processing is not failing.
> > >
> > > Not clear to me. I will wait for another explanation if any.
> > > In what basics application set .1 vs .8?
> >
> > The real case is like monitor the traffic with full-offload.
> > While packet hit the sample flow, the matching packets will be sampled
> > and sent to specific Queue, align with OVS sflow probability, user
> application can set it different value.
> 
> I understand the use case for mirror and supported in a lot of HW.
> What I would like to understand is the use case for "ratio"?
> Is the "ratio" part of OpenFlow spec? Or Is it an MLX hardware feature?
> 
The same usage of the 'probability' variable of ovs sample action;
MLX HW implemented it.
> 
> 
> >
> > >
> > > >
> > > > > 2) If it is for "rate-limiting" or "policing", why not use
> > > > > rte_mtr object (rte_mtr.h) via rte_flow action.
> >
> > The sample ratio isn’t the same as “meter’, the ratio of sampling will be
> calculated with incoming packets mask (every some packets sampled 1).
> Then the packets will be duplicated and go to do the other sample actions.
> 
> What I meant here is , If the ratio is used for rate-limiting then having a
> cascade rule like RTE_FLOW_ACTION_TYPE_MIRROR,
> RTE_FLOW_ACTION_TYPE_MTR will do the job.
> 
The ratio means the probability with packet replication, we don't need add METER action here.
> >
> >
> > > > > 3) One of the issue for driver developers and application
> > > > > writers are overlapping APIs. This would overlap with
> > > > > rte_eth_mirror_rule_set() API.
> > > > >
> > > > > Can we deprecate rte_eth_mirror_rule_set() API? It will be a
> > > > > pain for all to have overlapping APIs. We have not fixed the
> > > > > VLAN filter API overlap with rte_flow in ethdev. Its being TODO
> > > > > for multiple releases now.
> > > >
> > > > Ooooooooh yes!
> > > > I think flow-based API is more powerful, and should deprecate old
> > > > port-based API.
> > >
> > > +1 from me.
> > >
> > > it is taking too much effort and time to make support duplicate APIs.
> > >
> > > > I want to help deprecating such API in 20.11 if possible.
> > >
> > > Please start that discussion. In this case, it is clear API overlap
> > > with rte_eth_mirror_rule_set().
> > > We should not have two separate paths for the same function in the
> > > same ethdev library.
> > >
> > >
> > >
> > > >
> > > > > > Full mirroring is sampling 100% packets (ratio = 1).
> > > > > > That's why only one action is enough.
> > > >
> > > >
> > > >
  
Jiawei Wang June 28, 2020, 4:16 p.m. UTC | #11
On Sunday, June 28, 2020 4:27 PM, Andrew Rybchenko <arybchenko@solarflare.com> wrote:
> 
> On 6/25/20 7:26 PM, Jiawei Wang wrote:
> > When using full offload, all traffic will be handled by the HW, and
> > directed to the requested vf or wire, the control application loses
> > visibility on the traffic.
> > So there's a need for an action that will enable the control
> > application some visibility.
> >
> > The solution is introduced a new action that will sample the incoming
> > traffic and send a duplicated traffic in some predefined ratio to the
> > application, while the original packet will continue to the target
> > destination.
> >
> > The packets sampled equals is '1/ratio', if the ratio value be set to
> > 1 , means that the packets would be completely mirrored. The sample
> > packet can be assigned with different set of actions from the original
> packet.
> >
> > In order to support the sample packet in rte_flow, new rte_flow action
> > definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure
> > rte_flow_action_sample will be introduced.
> >
> > Signed-off-by: Jiawei Wang <jiaweiw@mellanox.com>
> 
> [snip]
> 
> > @@ -2709,6 +2716,28 @@ struct rte_flow_action {  struct rte_flow;
> >
> >  /**
> > + * @warning
> > + * @b EXPERIMENTAL: this structure may change without prior notice
> > + *
> > + * RTE_FLOW_ACTION_TYPE_SAMPLE
> > + *
> > + * Adds a sample action to a matched flow.
> > + *
> > + * The matching packets will be duplicated to a special queue or
> > +vport
> > + * in the predefined probabiilty, All the packets continues
> > +processing
> > + * on the default flow path.
> > + *
> > + * When the sample ratio is set to 1 then the packets will be 100%
> mirrored.
> > + * Additional action list be supported to add for sampled or mirrored
> packets.
> > + */
> > +struct rte_flow_action_sample {
> > +	/* packets sampled equals to '1/ratio' */
> > +	const uint32_t ratio;
> > +	/* sub-action list specific for the sampling hit cases */
> > +	const struct rte_flow_action *actions;
> 
> This design idea does not look good to me from the very beginning. IMHO it
> does not fit flow API overall design.
> I mean sub-action list.
> 
> As I understand Linux iptables solves it on match level (i.e. in pattern). E.g.
> "limit" extension which is basically sampling. Sampling using meta pattern
> item in combination with PASSTHRU action (to make sampling actions non-
> terminating if required) is a better solution from design point of view.

On our design, there're sample flow path and normal flow path, each path can have different actions.
The defined sub-actions list only applied for sampled packets in the sample flow path;
For normal path, all packets will continue to go with the original actions.
  
Andrew Rybchenko June 28, 2020, 4:18 p.m. UTC | #12
On 6/28/20 7:16 PM, Jiawei(Jonny) Wang wrote:
> 
> On Sunday, June 28, 2020 4:27 PM, Andrew Rybchenko <arybchenko@solarflare.com> wrote:
>>
>> On 6/25/20 7:26 PM, Jiawei Wang wrote:
>>> When using full offload, all traffic will be handled by the HW, and
>>> directed to the requested vf or wire, the control application loses
>>> visibility on the traffic.
>>> So there's a need for an action that will enable the control
>>> application some visibility.
>>>
>>> The solution is introduced a new action that will sample the incoming
>>> traffic and send a duplicated traffic in some predefined ratio to the
>>> application, while the original packet will continue to the target
>>> destination.
>>>
>>> The packets sampled equals is '1/ratio', if the ratio value be set to
>>> 1 , means that the packets would be completely mirrored. The sample
>>> packet can be assigned with different set of actions from the original
>> packet.
>>>
>>> In order to support the sample packet in rte_flow, new rte_flow action
>>> definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure
>>> rte_flow_action_sample will be introduced.
>>>
>>> Signed-off-by: Jiawei Wang <jiaweiw@mellanox.com>
>>
>> [snip]
>>
>>> @@ -2709,6 +2716,28 @@ struct rte_flow_action {  struct rte_flow;
>>>
>>>  /**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this structure may change without prior notice
>>> + *
>>> + * RTE_FLOW_ACTION_TYPE_SAMPLE
>>> + *
>>> + * Adds a sample action to a matched flow.
>>> + *
>>> + * The matching packets will be duplicated to a special queue or
>>> +vport
>>> + * in the predefined probabiilty, All the packets continues
>>> +processing
>>> + * on the default flow path.
>>> + *
>>> + * When the sample ratio is set to 1 then the packets will be 100%
>> mirrored.
>>> + * Additional action list be supported to add for sampled or mirrored
>> packets.
>>> + */
>>> +struct rte_flow_action_sample {
>>> +	/* packets sampled equals to '1/ratio' */
>>> +	const uint32_t ratio;
>>> +	/* sub-action list specific for the sampling hit cases */
>>> +	const struct rte_flow_action *actions;
>>
>> This design idea does not look good to me from the very beginning. IMHO it
>> does not fit flow API overall design.
>> I mean sub-action list.
>>
>> As I understand Linux iptables solves it on match level (i.e. in pattern). E.g.
>> "limit" extension which is basically sampling. Sampling using meta pattern
>> item in combination with PASSTHRU action (to make sampling actions non-
>> terminating if required) is a better solution from design point of view.
> 
> On our design, there're sample flow path and normal flow path, each path can have different actions.
> The defined sub-actions list only applied for sampled packets in the sample flow path;
> For normal path, all packets will continue to go with the original actions.
> 

In my too.
  
Ori Kam June 29, 2020, 11:40 a.m. UTC | #13
Hi all,

> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Sunday, June 28, 2020 7:19 PM
> To: Jiawei(Jonny) Wang <jiaweiw@mellanox.com>; Ori Kam
> <orika@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>; Matan
> Azrad <matan@mellanox.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Raslan
> Darawsheh <rasland@mellanox.com>; ian.stokes@intel.com; fbl@redhat.com
> Subject: Re: [dpdk-dev] [PATCH 1/8] ethdev: introduce sample action for rte
> flow
> 
> On 6/28/20 7:16 PM, Jiawei(Jonny) Wang wrote:
> >
> > On Sunday, June 28, 2020 4:27 PM, Andrew Rybchenko
> <arybchenko@solarflare.com> wrote:
> >>
> >> On 6/25/20 7:26 PM, Jiawei Wang wrote:
> >>> When using full offload, all traffic will be handled by the HW, and
> >>> directed to the requested vf or wire, the control application loses
> >>> visibility on the traffic.
> >>> So there's a need for an action that will enable the control
> >>> application some visibility.
> >>>
> >>> The solution is introduced a new action that will sample the incoming
> >>> traffic and send a duplicated traffic in some predefined ratio to the
> >>> application, while the original packet will continue to the target
> >>> destination.
> >>>
> >>> The packets sampled equals is '1/ratio', if the ratio value be set to
> >>> 1 , means that the packets would be completely mirrored. The sample
> >>> packet can be assigned with different set of actions from the original
> >> packet.
> >>>
> >>> In order to support the sample packet in rte_flow, new rte_flow action
> >>> definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure
> >>> rte_flow_action_sample will be introduced.
> >>>
> >>> Signed-off-by: Jiawei Wang <jiaweiw@mellanox.com>
> >>
> >> [snip]
> >>
> >>> @@ -2709,6 +2716,28 @@ struct rte_flow_action {  struct rte_flow;
> >>>
> >>>  /**
> >>> + * @warning
> >>> + * @b EXPERIMENTAL: this structure may change without prior notice
> >>> + *
> >>> + * RTE_FLOW_ACTION_TYPE_SAMPLE
> >>> + *
> >>> + * Adds a sample action to a matched flow.
> >>> + *
> >>> + * The matching packets will be duplicated to a special queue or
> >>> +vport
> >>> + * in the predefined probabiilty, All the packets continues
> >>> +processing
> >>> + * on the default flow path.
> >>> + *
> >>> + * When the sample ratio is set to 1 then the packets will be 100%
> >> mirrored.
> >>> + * Additional action list be supported to add for sampled or mirrored
> >> packets.
> >>> + */
> >>> +struct rte_flow_action_sample {
> >>> +	/* packets sampled equals to '1/ratio' */
> >>> +	const uint32_t ratio;
> >>> +	/* sub-action list specific for the sampling hit cases */
> >>> +	const struct rte_flow_action *actions;
> >>
> >> This design idea does not look good to me from the very beginning. IMHO it
> >> does not fit flow API overall design.
> >> I mean sub-action list.
> >>
> >> As I understand Linux iptables solves it on match level (i.e. in pattern). E.g.
> >> "limit" extension which is basically sampling. Sampling using meta pattern
> >> item in combination with PASSTHRU action (to make sampling actions non-
> >> terminating if required) is a better solution from design point of view.
> >
> > On our design, there're sample flow path and normal flow path, each path
> can have different actions.
> > The defined sub-actions list only applied for sampled packets in the sample
> flow path;
> > For normal path, all packets will continue to go with the original actions.
> >
> 
> In my too.

First as far as I know TC works close to the suggest approach (that by itself doesn’t mean anything)
The concept of a PASSTHRU is a good one but it has some issue to consider:
1. When using PASSTHRU it will mean that the matching part will be needed to be checked 
more times this will have performance penalty , also number of HW have limited number of flow that can be offload
this will approach will waste resources.
2. Using PASSTHRU will force the order of flows (sure it can be done using priorities but it is more complex to 
the application to implement) 
3. PASSTHRU will mean that there will be 2 terminal action for each flow (for example queue index 2 / passthru)
this also is not native to RTE flow. 
4. since we want to select only part of the packets, and we want to have some of the actions done on both 
packets (the sampled and the standard one) and then we want   on the sampled packet do some specific actions
while on the standard packet do different actions.
Lest check the following use case:
Application is using full offload traffic from the wire to a VM, which should decaped 
So the basic flow is:
Flow create 0  transfer ingress pattern eth / outer.ip =x / end  actions decap / port id 3 
Since after the offload the application loses visibility of the traffic. it still wants to sample some of the traffic
in order to verify that the traffic is valid. So the application request to receive some of the original traffic and
mark it with id.

If we use the original approach (the one in the patch) we will need something like this:
Flow 1: flow create 0 transfer ingress pattern eth / outer.ip=x / end actions sample(ratio 2,  actions mark id 3 / port pf)) / decap / port 3

In the PASSTHRU concept (I'm not sure I can even create such flows)
Flow 1: flow create 0 transfer ingress pattern eth / outer.ip =x / end  actions decap / port 2  /passtthru // original request
Flow 2: flow create 0 transfer ingress pattern eth/ outer.ip=x / should sample (new item that selects if the packet is selected based on the ratio)end act / mark / port pf

The main issue with this case the decap is before the sample so the sample will get decap packet.

So when looking at everything I think the original API is the best approach.
For the record I think that passthru action is very important and should be supported but not the best one for this feature.

Thanks,
Ori
  
Andrew Rybchenko June 29, 2020, 1:11 p.m. UTC | #14
Hi all,

CC Adrien

(I apologize for pulling you to the rte_flow API discussions
once again, but may be you can find spare time and share your
thoughts. Your opinion as an author and architect of the
rte_flow API would be very useful and highly appreciated.)

On 6/29/20 2:40 PM, Ori Kam wrote:
> Hi all,
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <arybchenko@solarflare.com>
>> Sent: Sunday, June 28, 2020 7:19 PM
>> To: Jiawei(Jonny) Wang <jiaweiw@mellanox.com>; Ori Kam
>> <orika@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>; Matan
>> Azrad <matan@mellanox.com>
>> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Raslan
>> Darawsheh <rasland@mellanox.com>; ian.stokes@intel.com; fbl@redhat.com
>> Subject: Re: [dpdk-dev] [PATCH 1/8] ethdev: introduce sample action for rte
>> flow
>>
>> On 6/28/20 7:16 PM, Jiawei(Jonny) Wang wrote:
>>>
>>> On Sunday, June 28, 2020 4:27 PM, Andrew Rybchenko
>> <arybchenko@solarflare.com> wrote:
>>>>
>>>> On 6/25/20 7:26 PM, Jiawei Wang wrote:
>>>>> When using full offload, all traffic will be handled by the HW, and
>>>>> directed to the requested vf or wire, the control application loses
>>>>> visibility on the traffic.
>>>>> So there's a need for an action that will enable the control
>>>>> application some visibility.
>>>>>
>>>>> The solution is introduced a new action that will sample the incoming
>>>>> traffic and send a duplicated traffic in some predefined ratio to the
>>>>> application, while the original packet will continue to the target
>>>>> destination.
>>>>>
>>>>> The packets sampled equals is '1/ratio', if the ratio value be set to
>>>>> 1 , means that the packets would be completely mirrored. The sample
>>>>> packet can be assigned with different set of actions from the original
>>>> packet.
>>>>>
>>>>> In order to support the sample packet in rte_flow, new rte_flow action
>>>>> definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure
>>>>> rte_flow_action_sample will be introduced.
>>>>>
>>>>> Signed-off-by: Jiawei Wang <jiaweiw@mellanox.com>
>>>>
>>>> [snip]
>>>>
>>>>> @@ -2709,6 +2716,28 @@ struct rte_flow_action {  struct rte_flow;
>>>>>
>>>>>  /**
>>>>> + * @warning
>>>>> + * @b EXPERIMENTAL: this structure may change without prior notice
>>>>> + *
>>>>> + * RTE_FLOW_ACTION_TYPE_SAMPLE
>>>>> + *
>>>>> + * Adds a sample action to a matched flow.
>>>>> + *
>>>>> + * The matching packets will be duplicated to a special queue or
>>>>> +vport
>>>>> + * in the predefined probabiilty, All the packets continues
>>>>> +processing
>>>>> + * on the default flow path.
>>>>> + *
>>>>> + * When the sample ratio is set to 1 then the packets will be 100%
>>>> mirrored.
>>>>> + * Additional action list be supported to add for sampled or mirrored
>>>> packets.
>>>>> + */
>>>>> +struct rte_flow_action_sample {
>>>>> +	/* packets sampled equals to '1/ratio' */
>>>>> +	const uint32_t ratio;
>>>>> +	/* sub-action list specific for the sampling hit cases */
>>>>> +	const struct rte_flow_action *actions;
>>>>
>>>> This design idea does not look good to me from the very beginning. IMHO it
>>>> does not fit flow API overall design.
>>>> I mean sub-action list.
>>>>
>>>> As I understand Linux iptables solves it on match level (i.e. in pattern). E.g.
>>>> "limit" extension which is basically sampling. Sampling using meta pattern
>>>> item in combination with PASSTHRU action (to make sampling actions non-
>>>> terminating if required) is a better solution from design point of view.
>>>
>>> On our design, there're sample flow path and normal flow path, each path
>> can have different actions.
>>> The defined sub-actions list only applied for sampled packets in the sample
>> flow path;
>>> For normal path, all packets will continue to go with the original actions.
>>>
>>
>> In my too.
> 
> First as far as I know TC works close to the suggest approach (that by itself doesn’t mean anything)
> The concept of a PASSTHRU is a good one but it has some issue to consider:
> 1. When using PASSTHRU it will mean that the matching part will be needed to be checked 
> more times this will have performance penalty , also number of HW have limited number of flow that can be offload
> this will approach will waste resources.

Marking or tagging could be used to address it. E.g. target
traffic could be tagged first, then matching by tag should be
used to sample and to do HW offloads.

Moreover, mapping of rte_flow API rules into HW rule is not
required to be 1-to-1. Yes, 1-to-1 is simple, but it could be
more complicated 1-to-N (when one rte_flow API rule is
represented by many HW rules) or N-to-1 (when few flow API
rules are represented as one HW rule) or even N-to-M.
For example, tagging which is not visible outside, could be
purely SW and used to build such constructions in SW.
It is an implementation detail is out of scope of the generic
API definition.

Yes, it sounds like over-complicating, but I really dislike
above sub-action list from design point of view and that's
why I"m trying to think in different directions.

> 2. Using PASSTHRU will force the order of flows (sure it can be done using priorities but it is more complex to 
> the application to implement) 

See above.

> 3. PASSTHRU will mean that there will be 2 terminal action for each flow (for example queue index 2 / passthru)
> this also is not native to RTE flow. 

Sorry, but there are two branches for terminating actions in
the sampling action design anyway (yes, internal/hidden).
You need two copies of the packet, so whatever you do it
will be two terminating actions.

> 4. since we want to select only part of the packets, and we want to have some of the actions done on both 
> packets (the sampled and the standard one) and then we want   on the sampled packet do some specific actions
> while on the standard packet do different actions.

Yes, it is not a problem with PASSTHRU.

> Lest check the following use case:
> Application is using full offload traffic from the wire to a VM, which should decaped 
> So the basic flow is:
> Flow create 0  transfer ingress pattern eth / outer.ip =x / end  actions decap / port id 3 
> Since after the offload the application loses visibility of the traffic. it still wants to sample some of the traffic
> in order to verify that the traffic is valid. So the application request to receive some of the original traffic and
> mark it with id.
> 
> If we use the original approach (the one in the patch) we will need something like this:
> Flow 1: flow create 0 transfer ingress pattern eth / outer.ip=x / end actions sample(ratio 2,  actions mark id 3 / port pf)) / decap / port 3
> 
> In the PASSTHRU concept (I'm not sure I can even create such flows)
> Flow 1: flow create 0 transfer ingress pattern eth / outer.ip =x / end  actions decap / port 2  /passtthru // original request
> Flow 2: flow create 0 transfer ingress pattern eth/ outer.ip=x / should sample (new item that selects if the packet is selected based on the ratio)end act / mark / port pf
> 
> The main issue with this case the decap is before the sample so the sample will get decap packet.

Order should be simply different: first sampling with pass-
through, second decap and deliver to VM.

Yes, I realize that two actions with basically the same match
(modulo sampling match) is not ideal for mapping to HW (even
if it collapsed into trivial tag match which pre-rule to make
tagging). I'm not 100% happy with it, but I'm even less happy
with sub-action list design and just trying to find better
alternative solution.

> So when looking at everything I think the original API is the best approach.
> For the record I think that passthru action is very important and should be supported but not the best one for this feature.
> 
> Thanks,
> Ori
>
  
Ori Kam June 29, 2020, 2:29 p.m. UTC | #15
Hi All,

> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Monday, June 29, 2020 4:12 PM
> To: Ori Kam <orika@mellanox.com>; Jiawei(Jonny) Wang
> <jiaweiw@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>;
> Matan Azrad <matan@mellanox.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Raslan
> Darawsheh <rasland@mellanox.com>; ian.stokes@intel.com; fbl@redhat.com;
> Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Subject: Re: [dpdk-dev] [PATCH 1/8] ethdev: introduce sample action for rte
> flow
> 
> Hi all,
> 
> CC Adrien
> 
> (I apologize for pulling you to the rte_flow API discussions
> once again, but may be you can find spare time and share your
> thoughts. Your opinion as an author and architect of the
> rte_flow API would be very useful and highly appreciated.)
> 
> On 6/29/20 2:40 PM, Ori Kam wrote:
> > Hi all,
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <arybchenko@solarflare.com>
> >> Sent: Sunday, June 28, 2020 7:19 PM
> >> To: Jiawei(Jonny) Wang <jiaweiw@mellanox.com>; Ori Kam
> >> <orika@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>;
> Matan
> >> Azrad <matan@mellanox.com>
> >> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Raslan
> >> Darawsheh <rasland@mellanox.com>; ian.stokes@intel.com;
> fbl@redhat.com
> >> Subject: Re: [dpdk-dev] [PATCH 1/8] ethdev: introduce sample action for rte
> >> flow
> >>
> >> On 6/28/20 7:16 PM, Jiawei(Jonny) Wang wrote:
> >>>
> >>> On Sunday, June 28, 2020 4:27 PM, Andrew Rybchenko
> >> <arybchenko@solarflare.com> wrote:
> >>>>
> >>>> On 6/25/20 7:26 PM, Jiawei Wang wrote:
> >>>>> When using full offload, all traffic will be handled by the HW, and
> >>>>> directed to the requested vf or wire, the control application loses
> >>>>> visibility on the traffic.
> >>>>> So there's a need for an action that will enable the control
> >>>>> application some visibility.
> >>>>>
> >>>>> The solution is introduced a new action that will sample the incoming
> >>>>> traffic and send a duplicated traffic in some predefined ratio to the
> >>>>> application, while the original packet will continue to the target
> >>>>> destination.
> >>>>>
> >>>>> The packets sampled equals is '1/ratio', if the ratio value be set to
> >>>>> 1 , means that the packets would be completely mirrored. The sample
> >>>>> packet can be assigned with different set of actions from the original
> >>>> packet.
> >>>>>
> >>>>> In order to support the sample packet in rte_flow, new rte_flow action
> >>>>> definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure
> >>>>> rte_flow_action_sample will be introduced.
> >>>>>
> >>>>> Signed-off-by: Jiawei Wang <jiaweiw@mellanox.com>
> >>>>
> >>>> [snip]
> >>>>
> >>>>> @@ -2709,6 +2716,28 @@ struct rte_flow_action {  struct rte_flow;
> >>>>>
> >>>>>  /**
> >>>>> + * @warning
> >>>>> + * @b EXPERIMENTAL: this structure may change without prior notice
> >>>>> + *
> >>>>> + * RTE_FLOW_ACTION_TYPE_SAMPLE
> >>>>> + *
> >>>>> + * Adds a sample action to a matched flow.
> >>>>> + *
> >>>>> + * The matching packets will be duplicated to a special queue or
> >>>>> +vport
> >>>>> + * in the predefined probabiilty, All the packets continues
> >>>>> +processing
> >>>>> + * on the default flow path.
> >>>>> + *
> >>>>> + * When the sample ratio is set to 1 then the packets will be 100%
> >>>> mirrored.
> >>>>> + * Additional action list be supported to add for sampled or mirrored
> >>>> packets.
> >>>>> + */
> >>>>> +struct rte_flow_action_sample {
> >>>>> +	/* packets sampled equals to '1/ratio' */
> >>>>> +	const uint32_t ratio;
> >>>>> +	/* sub-action list specific for the sampling hit cases */
> >>>>> +	const struct rte_flow_action *actions;
> >>>>
> >>>> This design idea does not look good to me from the very beginning. IMHO
> it
> >>>> does not fit flow API overall design.
> >>>> I mean sub-action list.
> >>>>
> >>>> As I understand Linux iptables solves it on match level (i.e. in pattern). E.g.
> >>>> "limit" extension which is basically sampling. Sampling using meta pattern
> >>>> item in combination with PASSTHRU action (to make sampling actions
> non-
> >>>> terminating if required) is a better solution from design point of view.
> >>>
> >>> On our design, there're sample flow path and normal flow path, each path
> >> can have different actions.
> >>> The defined sub-actions list only applied for sampled packets in the sample
> >> flow path;
> >>> For normal path, all packets will continue to go with the original actions.
> >>>
> >>
> >> In my too.
> >
> > First as far as I know TC works close to the suggest approach (that by itself
> doesn’t mean anything)
> > The concept of a PASSTHRU is a good one but it has some issue to consider:
> > 1. When using PASSTHRU it will mean that the matching part will be needed
> to be checked
> > more times this will have performance penalty , also number of HW have
> limited number of flow that can be offload
> > this will approach will waste resources.
> 
> Marking or tagging could be used to address it. E.g. target
> traffic could be tagged first, then matching by tag should be
> used to sample and to do HW offloads.
> 
In this case you are forcing at least two steps, this will hurt performance.
Matching on mark is the same as matching on other items. While it may have extra
penalty to add the extra mark action. (this is general HW issue I assume number of 
manufactures have the same limitation)

> Moreover, mapping of rte_flow API rules into HW rule is not
> required to be 1-to-1. Yes, 1-to-1 is simple, but it could be
> more complicated 1-to-N (when one rte_flow API rule is
> represented by many HW rules) or N-to-1 (when few flow API
> rules are represented as one HW rule) or even N-to-M.
> For example, tagging which is not visible outside, could be
> purely SW and used to build such constructions in SW.
> It is an implementation detail is out of scope of the generic
> API definition.
> 
I don’t understand this part. I never said that 1 to 1 is needed
but if you try to combine flows in SW it means that you must keep all flows
in the PMD in order to combine them. I now some HW must do that, but not 
all of them and if they don’t it is just a huge memory waste.

> Yes, it sounds like over-complicating, but I really dislike
> above sub-action list from design point of view and that's
> why I"m trying to think in different directions.
> 
Thinking in different direction is always good. 
I just think that between the two approaches I like the original better.
May be you can explain your reason for your opinion and we can find the best
solution together?



> > 2. Using PASSTHRU will force the order of flows (sure it can be done using
> priorities but it is more complex to
> > the application to implement)
> 
> See above.
> 

See above 😊

> > 3. PASSTHRU will mean that there will be 2 terminal action for each flow (for
> example queue index 2 / passthru)
> > this also is not native to RTE flow.
> 
> Sorry, but there are two branches for terminating actions in
> the sampling action design anyway (yes, internal/hidden).
> You need two copies of the packet, so whatever you do it
> will be two terminating actions.
> 
Yes but you can look at it as 1 flow with 2 sets of actions and not two flows. 

> > 4. since we want to select only part of the packets, and we want to have some
> of the actions done on both
> > packets (the sampled and the standard one) and then we want   on the
> sampled packet do some specific actions
> > while on the standard packet do different actions.
> 
> Yes, it is not a problem with PASSTHRU.
> 
> > Lest check the following use case:
> > Application is using full offload traffic from the wire to a VM, which should
> decaped
> > So the basic flow is:
> > Flow create 0  transfer ingress pattern eth / outer.ip =x / end  actions decap /
> port id 3
> > Since after the offload the application loses visibility of the traffic. it still
> wants to sample some of the traffic
> > in order to verify that the traffic is valid. So the application request to receive
> some of the original traffic and
> > mark it with id.
> >
> > If we use the original approach (the one in the patch) we will need something
> like this:
> > Flow 1: flow create 0 transfer ingress pattern eth / outer.ip=x / end actions
> sample(ratio 2,  actions mark id 3 / port pf)) / decap / port 3
> >
> > In the PASSTHRU concept (I'm not sure I can even create such flows)
> > Flow 1: flow create 0 transfer ingress pattern eth / outer.ip =x / end  actions
> decap / port 2  /passtthru // original request
> > Flow 2: flow create 0 transfer ingress pattern eth/ outer.ip=x / should sample
> (new item that selects if the packet is selected based on the ratio)end act /
> mark / port pf
> >
> > The main issue with this case the decap is before the sample so the sample
> will get decap packet.
> 
> Order should be simply different: first sampling with pass-
> through, second decap and deliver to VM.
>
Then in your case the packet will be marked. Which is not what the 
application requested.
 
> Yes, I realize that two actions with basically the same match
> (modulo sampling match) is not ideal for mapping to HW (even
> if it collapsed into trivial tag match which pre-rule to make
> tagging). I'm not 100% happy with it, but I'm even less happy
> with sub-action list design and just trying to find better
> alternative solution.
> 
Please see above, maybe we can find the best solution together. 

> > So when looking at everything I think the original API is the best approach.
> > For the record I think that passthru action is very important and should be
> supported but not the best one for this feature.
> >
> > Thanks,
> > Ori
> >
Best,
Ori
  
Ori Kam June 30, 2020, 4:42 p.m. UTC | #16
Hi All,

After considering both approaches, I think the original is the better approach.

Acked-by: Ori Kam <orika@mellanox.com>
Thanks,
Ori

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ori Kam
> Sent: Monday, June 29, 2020 5:30 PM
> To: Andrew Rybchenko <arybchenko@solarflare.com>; Jiawei(Jonny) Wang
> <jiaweiw@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>;
> Matan Azrad <matan@mellanox.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Raslan
> Darawsheh <rasland@mellanox.com>; ian.stokes@intel.com; fbl@redhat.com;
> Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Subject: Re: [dpdk-dev] [PATCH 1/8] ethdev: introduce sample action for rte
> flow
> 
> Hi All,
> 
> > -----Original Message-----
> > From: Andrew Rybchenko <arybchenko@solarflare.com>
> > Sent: Monday, June 29, 2020 4:12 PM
> > To: Ori Kam <orika@mellanox.com>; Jiawei(Jonny) Wang
> > <jiaweiw@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>;
> > Matan Azrad <matan@mellanox.com>
> > Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Raslan
> > Darawsheh <rasland@mellanox.com>; ian.stokes@intel.com;
> fbl@redhat.com;
> > Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > Subject: Re: [dpdk-dev] [PATCH 1/8] ethdev: introduce sample action for rte
> > flow
> >
> > Hi all,
> >
> > CC Adrien
> >
> > (I apologize for pulling you to the rte_flow API discussions
> > once again, but may be you can find spare time and share your
> > thoughts. Your opinion as an author and architect of the
> > rte_flow API would be very useful and highly appreciated.)
> >
> > On 6/29/20 2:40 PM, Ori Kam wrote:
> > > Hi all,
> > >
> > >> -----Original Message-----
> > >> From: Andrew Rybchenko <arybchenko@solarflare.com>
> > >> Sent: Sunday, June 28, 2020 7:19 PM
> > >> To: Jiawei(Jonny) Wang <jiaweiw@mellanox.com>; Ori Kam
> > >> <orika@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>;
> > Matan
> > >> Azrad <matan@mellanox.com>
> > >> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Raslan
> > >> Darawsheh <rasland@mellanox.com>; ian.stokes@intel.com;
> > fbl@redhat.com
> > >> Subject: Re: [dpdk-dev] [PATCH 1/8] ethdev: introduce sample action for
> rte
> > >> flow
> > >>
> > >> On 6/28/20 7:16 PM, Jiawei(Jonny) Wang wrote:
> > >>>
> > >>> On Sunday, June 28, 2020 4:27 PM, Andrew Rybchenko
> > >> <arybchenko@solarflare.com> wrote:
> > >>>>
> > >>>> On 6/25/20 7:26 PM, Jiawei Wang wrote:
> > >>>>> When using full offload, all traffic will be handled by the HW, and
> > >>>>> directed to the requested vf or wire, the control application loses
> > >>>>> visibility on the traffic.
> > >>>>> So there's a need for an action that will enable the control
> > >>>>> application some visibility.
> > >>>>>
> > >>>>> The solution is introduced a new action that will sample the incoming
> > >>>>> traffic and send a duplicated traffic in some predefined ratio to the
> > >>>>> application, while the original packet will continue to the target
> > >>>>> destination.
> > >>>>>
> > >>>>> The packets sampled equals is '1/ratio', if the ratio value be set to
> > >>>>> 1 , means that the packets would be completely mirrored. The sample
> > >>>>> packet can be assigned with different set of actions from the original
> > >>>> packet.
> > >>>>>
> > >>>>> In order to support the sample packet in rte_flow, new rte_flow action
> > >>>>> definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure
> > >>>>> rte_flow_action_sample will be introduced.
> > >>>>>
> > >>>>> Signed-off-by: Jiawei Wang <jiaweiw@mellanox.com>
> > >>>>
> > >>>> [snip]
> > >>>>
> > >>>>> @@ -2709,6 +2716,28 @@ struct rte_flow_action {  struct rte_flow;
> > >>>>>
> > >>>>>  /**
> > >>>>> + * @warning
> > >>>>> + * @b EXPERIMENTAL: this structure may change without prior notice
> > >>>>> + *
> > >>>>> + * RTE_FLOW_ACTION_TYPE_SAMPLE
> > >>>>> + *
> > >>>>> + * Adds a sample action to a matched flow.
> > >>>>> + *
> > >>>>> + * The matching packets will be duplicated to a special queue or
> > >>>>> +vport
> > >>>>> + * in the predefined probabiilty, All the packets continues
> > >>>>> +processing
> > >>>>> + * on the default flow path.
> > >>>>> + *
> > >>>>> + * When the sample ratio is set to 1 then the packets will be 100%
> > >>>> mirrored.
> > >>>>> + * Additional action list be supported to add for sampled or mirrored
> > >>>> packets.
> > >>>>> + */
> > >>>>> +struct rte_flow_action_sample {
> > >>>>> +	/* packets sampled equals to '1/ratio' */
> > >>>>> +	const uint32_t ratio;
> > >>>>> +	/* sub-action list specific for the sampling hit cases */
> > >>>>> +	const struct rte_flow_action *actions;
> > >>>>
> > >>>> This design idea does not look good to me from the very beginning.
> IMHO
> > it
> > >>>> does not fit flow API overall design.
> > >>>> I mean sub-action list.
> > >>>>
> > >>>> As I understand Linux iptables solves it on match level (i.e. in pattern).
> E.g.
> > >>>> "limit" extension which is basically sampling. Sampling using meta
> pattern
> > >>>> item in combination with PASSTHRU action (to make sampling actions
> > non-
> > >>>> terminating if required) is a better solution from design point of view.
> > >>>
> > >>> On our design, there're sample flow path and normal flow path, each
> path
> > >> can have different actions.
> > >>> The defined sub-actions list only applied for sampled packets in the
> sample
> > >> flow path;
> > >>> For normal path, all packets will continue to go with the original actions.
> > >>>
> > >>
> > >> In my too.
> > >
> > > First as far as I know TC works close to the suggest approach (that by itself
> > doesn’t mean anything)
> > > The concept of a PASSTHRU is a good one but it has some issue to consider:
> > > 1. When using PASSTHRU it will mean that the matching part will be needed
> > to be checked
> > > more times this will have performance penalty , also number of HW have
> > limited number of flow that can be offload
> > > this will approach will waste resources.
> >
> > Marking or tagging could be used to address it. E.g. target
> > traffic could be tagged first, then matching by tag should be
> > used to sample and to do HW offloads.
> >
> In this case you are forcing at least two steps, this will hurt performance.
> Matching on mark is the same as matching on other items. While it may have
> extra
> penalty to add the extra mark action. (this is general HW issue I assume
> number of
> manufactures have the same limitation)
> 
> > Moreover, mapping of rte_flow API rules into HW rule is not
> > required to be 1-to-1. Yes, 1-to-1 is simple, but it could be
> > more complicated 1-to-N (when one rte_flow API rule is
> > represented by many HW rules) or N-to-1 (when few flow API
> > rules are represented as one HW rule) or even N-to-M.
> > For example, tagging which is not visible outside, could be
> > purely SW and used to build such constructions in SW.
> > It is an implementation detail is out of scope of the generic
> > API definition.
> >
> I don’t understand this part. I never said that 1 to 1 is needed
> but if you try to combine flows in SW it means that you must keep all flows
> in the PMD in order to combine them. I now some HW must do that, but not
> all of them and if they don’t it is just a huge memory waste.
> 
> > Yes, it sounds like over-complicating, but I really dislike
> > above sub-action list from design point of view and that's
> > why I"m trying to think in different directions.
> >
> Thinking in different direction is always good.
> I just think that between the two approaches I like the original better.
> May be you can explain your reason for your opinion and we can find the best
> solution together?
> 
> 
> 
> > > 2. Using PASSTHRU will force the order of flows (sure it can be done using
> > priorities but it is more complex to
> > > the application to implement)
> >
> > See above.
> >
> 
> See above 😊
> 
> > > 3. PASSTHRU will mean that there will be 2 terminal action for each flow
> (for
> > example queue index 2 / passthru)
> > > this also is not native to RTE flow.
> >
> > Sorry, but there are two branches for terminating actions in
> > the sampling action design anyway (yes, internal/hidden).
> > You need two copies of the packet, so whatever you do it
> > will be two terminating actions.
> >
> Yes but you can look at it as 1 flow with 2 sets of actions and not two flows.
> 
> > > 4. since we want to select only part of the packets, and we want to have
> some
> > of the actions done on both
> > > packets (the sampled and the standard one) and then we want   on the
> > sampled packet do some specific actions
> > > while on the standard packet do different actions.
> >
> > Yes, it is not a problem with PASSTHRU.
> >
> > > Lest check the following use case:
> > > Application is using full offload traffic from the wire to a VM, which should
> > decaped
> > > So the basic flow is:
> > > Flow create 0  transfer ingress pattern eth / outer.ip =x / end  actions decap
> /
> > port id 3
> > > Since after the offload the application loses visibility of the traffic. it still
> > wants to sample some of the traffic
> > > in order to verify that the traffic is valid. So the application request to
> receive
> > some of the original traffic and
> > > mark it with id.
> > >
> > > If we use the original approach (the one in the patch) we will need
> something
> > like this:
> > > Flow 1: flow create 0 transfer ingress pattern eth / outer.ip=x / end actions
> > sample(ratio 2,  actions mark id 3 / port pf)) / decap / port 3
> > >
> > > In the PASSTHRU concept (I'm not sure I can even create such flows)
> > > Flow 1: flow create 0 transfer ingress pattern eth / outer.ip =x / end  actions
> > decap / port 2  /passtthru // original request
> > > Flow 2: flow create 0 transfer ingress pattern eth/ outer.ip=x / should
> sample
> > (new item that selects if the packet is selected based on the ratio)end act /
> > mark / port pf
> > >
> > > The main issue with this case the decap is before the sample so the sample
> > will get decap packet.
> >
> > Order should be simply different: first sampling with pass-
> > through, second decap and deliver to VM.
> >
> Then in your case the packet will be marked. Which is not what the
> application requested.
> 
> > Yes, I realize that two actions with basically the same match
> > (modulo sampling match) is not ideal for mapping to HW (even
> > if it collapsed into trivial tag match which pre-rule to make
> > tagging). I'm not 100% happy with it, but I'm even less happy
> > with sub-action list design and just trying to find better
> > alternative solution.
> >
> Please see above, maybe we can find the best solution together.
> 
> > > So when looking at everything I think the original API is the best approach.
> > > For the record I think that passthru action is very important and should be
> > supported but not the best one for this feature.
> > >
> > > Thanks,
> > > Ori
> > >
> Best,
> Ori
  
Ori Kam July 1, 2020, 9:37 a.m. UTC | #17
Hi Jiawei,

Please note that you are missing the doc changes.

Please update them.

Best,
Ori

> -----Original Message-----
> From: Jiawei Wang <jiaweiw@mellanox.com>
> Sent: Thursday, June 25, 2020 7:26 PM
> To: Ori Kam <orika@mellanox.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>; Matan Azrad <matan@mellanox.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Raslan
> Darawsheh <rasland@mellanox.com>; ian.stokes@intel.com; fbl@redhat.com;
> Jiawei(Jonny) Wang <jiaweiw@mellanox.com>
> Subject: [PATCH 1/8] ethdev: introduce sample action for rte flow
> 
> When using full offload, all traffic will be handled by the HW, and
> directed to the requested vf or wire, the control application loses
> visibility on the traffic.
> So there's a need for an action that will enable the control application
> some visibility.
> 
> The solution is introduced a new action that will sample the incoming
> traffic and send a duplicated traffic in some predefined ratio to the
> application, while the original packet will continue to the target
> destination.
> 
> The packets sampled equals is '1/ratio', if the ratio value be set to 1
> , means that the packets would be completely mirrored. The sample packet
> can be assigned with different set of actions from the original packet.
> 
> In order to support the sample packet in rte_flow, new rte_flow action
> definition RTE_FLOW_ACTION_TYPE_SAMPLE and structure
> rte_flow_action_sample
> will be introduced.
> 
> Signed-off-by: Jiawei Wang <jiaweiw@mellanox.com>
> ---
>  lib/librte_ethdev/rte_flow.c |  1 +
>  lib/librte_ethdev/rte_flow.h | 29 +++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index 1685be5..733871d 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -173,6 +173,7 @@ struct rte_flow_desc_data {
>  	MK_FLOW_ACTION(SET_IPV4_DSCP, sizeof(struct
> rte_flow_action_set_dscp)),
>  	MK_FLOW_ACTION(SET_IPV6_DSCP, sizeof(struct
> rte_flow_action_set_dscp)),
>  	MK_FLOW_ACTION(AGE, sizeof(struct rte_flow_action_age)),
> +	MK_FLOW_ACTION(SAMPLE, sizeof(struct rte_flow_action_sample)),
>  };
> 
>  int
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index b0e4199..71dd82c 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -2099,6 +2099,13 @@ enum rte_flow_action_type {
>  	 * see enum RTE_ETH_EVENT_FLOW_AGED
>  	 */
>  	RTE_FLOW_ACTION_TYPE_AGE,
> +
> +	/**
> +	 * Redirects specific ratio of packets to vport or queue.
> +	 *
> +	 * See struct rte_flow_action_sample.
> +	 */
> +	RTE_FLOW_ACTION_TYPE_SAMPLE,
>  };
> 
>  /**
> @@ -2709,6 +2716,28 @@ struct rte_flow_action {
>  struct rte_flow;
> 
>  /**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * RTE_FLOW_ACTION_TYPE_SAMPLE
> + *
> + * Adds a sample action to a matched flow.
> + *
> + * The matching packets will be duplicated to a special queue or vport
> + * in the predefined probabiilty, All the packets continues processing
> + * on the default flow path.
> + *
> + * When the sample ratio is set to 1 then the packets will be 100% mirrored.
> + * Additional action list be supported to add for sampled or mirrored packets.
> + */
> +struct rte_flow_action_sample {
> +	/* packets sampled equals to '1/ratio' */
> +	const uint32_t ratio;
> +	/* sub-action list specific for the sampling hit cases */
> +	const struct rte_flow_action *actions;
> +};
> +
> +/**
>   * Verbose error types.
>   *
>   * Most of them provide the type of the object referenced by struct
> --
> 1.8.3.1
  
Stephen Hemminger July 2, 2020, 12:18 a.m. UTC | #18
On Sun, 28 Jun 2020 15:52:27 +0000
"Jiawei(Jonny) Wang" <jiaweiw@mellanox.com> wrote:

> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Sunday, June 28, 2020 9:38 PM
> > To: Jiawei(Jonny) Wang <jiaweiw@mellanox.com>
> > Cc: Thomas Monjalon <thomas@monjalon.net>; Ori Kam
> > <orika@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>;
> > Matan Azrad <matan@mellanox.com>; dpdk-dev <dev@dpdk.org>; Raslan
> > Darawsheh <rasland@mellanox.com>; ian.stokes@intel.com;
> > fbl@redhat.com; Ferruh Yigit <ferruh.yigit@intel.com>; Andrew Rybchenko
> > <arybchenko@solarflare.com>
> > Subject: Re: [dpdk-dev] [PATCH 1/8] ethdev: introduce sample action for rte
> > flow
> > 
> > On Sun, Jun 28, 2020 at 6:46 PM Jiawei(Jonny) Wang
> > <jiaweiw@mellanox.com> wrote:  
> > >
> > >
> > > On Friday, June 26, 2020 7:10 PM Jerin Jacob <jerinjacobk@gmail.com>  
> > Wrote:  
> > > >
> > > > On Fri, Jun 26, 2020 at 4:16 PM Thomas Monjalon
> > > > <thomas@monjalon.net>
> > > > wrote:  
> > > > >
> > > > > 26/06/2020 12:35, Jerin Jacob:  
> > > > > > On Fri, Jun 26, 2020 at 12:59 AM Thomas Monjalon  
> > > > <thomas@monjalon.net> wrote:  
> > > > > > >
> > > > > > > 25/06/2020 19:55, Jerin Jacob:  
> > > > > > > > On Thu, Jun 25, 2020 at 10:20 PM Jiawei Wang  
> > > > <jiaweiw@mellanox.com> wrote:  
> > > > > > > > >
> > > > > > > > > When using full offload, all traffic will be handled by
> > > > > > > > > the HW, and directed to the requested vf or wire, the
> > > > > > > > > control application loses visibility on the traffic.
> > > > > > > > > So there's a need for an action that will enable the
> > > > > > > > > control application some visibility.
> > > > > > > > >
> > > > > > > > > The solution is introduced a new action that will sample
> > > > > > > > > the incoming traffic and send a duplicated traffic in some
> > > > > > > > > predefined ratio to the application, while the original
> > > > > > > > > packet will continue to the target destination.
> > > > > > > > >
> > > > > > > > > The packets sampled equals is '1/ratio', if the ratio
> > > > > > > > > value be set to 1 , means that the packets would be
> > > > > > > > > completely mirrored. The sample packet can be assigned
> > > > > > > > > with different set of  
> > > > actions from the original packet.  
> > > > > > > > >
> > > > > > > > > In order to support the sample packet in rte_flow, new
> > > > > > > > > rte_flow action definition RTE_FLOW_ACTION_TYPE_SAMPLE  
> > and  
> > > > > > > > > structure rte_flow_action_sample  
> > > > > > > >
> > > > > > > > Isn't mirroring the packet? How about,
> > > > > > > > RTE_FLOW_ACTION_TYPE_MIRROR I am not able to understand,  
> > Why  
> > > > it is called sample.  
> > > > > > >
> > > > > > > Sampling is a partial mirroring.  
> > > > > >
> > > > > > I think, By definition, _sampling_ is the _selection_ of items
> > > > > > from a specific group.
> > > > > > I think, _sampling_ is not dictating, what is the real action
> > > > > > for the "selected"  items.
> > > > > > One can get confused with the selected ones can be for forward,
> > > > > > drop any other action.  
> > > > >
> > > > > I see. Good design question (I will let others reply).
> > > > >  
> > > > > > So IMO, explicit mirror keyword usage makes it is clear.  
> > >
> > > Sampled packet is duplicated from incoming traffic at specific ratio
> > > and will go to different sample actions;
> > > ratio=1 is 100% duplication or mirroring.
> > > All packets will continue to go to default flow actions.  
> > 
> > Functionality is clear from the git commit log(Not from action name).
> > The only question is what would be the appropriate name for this action.
> > RTE_FLOW_ACTION_TYPE_SAMPLE vs RTE_FLOW_ACTION_TYPE_MIRROR
> >   
> > >  
> > > > > >
> > > > > > Some more related questions:
> > > > > > 1) What is the real use case for ratio? I am not against adding
> > > > > > a ratio attribute if the MLX hardware supports it. It will be
> > > > > > good to know the use case from the application perspective? And
> > > > > > what basics application set ratio != 1?  
> > > > >
> > > > > If I understand well, some applications want to check, by picking
> > > > > random packets, that the processing is not failing.  
> > > >
> > > > Not clear to me. I will wait for another explanation if any.
> > > > In what basics application set .1 vs .8?  
> > >
> > > The real case is like monitor the traffic with full-offload.
> > > While packet hit the sample flow, the matching packets will be sampled
> > > and sent to specific Queue, align with OVS sflow probability, user  
> > application can set it different value.
> > 
> > I understand the use case for mirror and supported in a lot of HW.
> > What I would like to understand is the use case for "ratio"?
> > Is the "ratio" part of OpenFlow spec? Or Is it an MLX hardware feature?
> >   
> The same usage of the 'probability' variable of ovs sample action;
> MLX HW implemented it.
> > 
> >   
> > >  
> > > >  
> > > > >  
> > > > > > 2) If it is for "rate-limiting" or "policing", why not use
> > > > > > rte_mtr object (rte_mtr.h) via rte_flow action.  
> > >
> > > The sample ratio isn’t the same as “meter’, the ratio of sampling will be  
> > calculated with incoming packets mask (every some packets sampled 1).
> > Then the packets will be duplicated and go to do the other sample actions.
> > 
> > What I meant here is , If the ratio is used for rate-limiting then having a
> > cascade rule like RTE_FLOW_ACTION_TYPE_MIRROR,
> > RTE_FLOW_ACTION_TYPE_MTR will do the job.
> >   
> The ratio means the probability with packet replication, we don't need add METER action here.
> > >
> > >  
> > > > > > 3) One of the issue for driver developers and application
> > > > > > writers are overlapping APIs. This would overlap with
> > > > > > rte_eth_mirror_rule_set() API.
> > > > > >
> > > > > > Can we deprecate rte_eth_mirror_rule_set() API? It will be a
> > > > > > pain for all to have overlapping APIs. We have not fixed the
> > > > > > VLAN filter API overlap with rte_flow in ethdev. Its being TODO
> > > > > > for multiple releases now.  
> > > > >
> > > > > Ooooooooh yes!
> > > > > I think flow-based API is more powerful, and should deprecate old
> > > > > port-based API.  
> > > >
> > > > +1 from me.
> > > >
> > > > it is taking too much effort and time to make support duplicate APIs.
> > > >  
> > > > > I want to help deprecating such API in 20.11 if possible.  
> > > >
> > > > Please start that discussion. In this case, it is clear API overlap
> > > > with rte_eth_mirror_rule_set().
> > > > We should not have two separate paths for the same function in the
> > > > same ethdev library.
> > > >
> > > >
> > > >  
> > > > >  
> > > > > > > Full mirroring is sampling 100% packets (ratio = 1).
> > > > > > > That's why only one action is enough.  
> > > > >
> > > > >
> > > > >  

One use case would be simulating packet loss or duplication. (like netem does).
In that use case, the sample action would have to not have an implicit copy.

Could sample be defined as "if random number hits the ratio" then execute
this alternative rule, otherwise go to next rule.

The the alternative rule could mirror if it wanted, or drop, or count, or ...

It could even be used as a form of transmit load balancing.
  
Ori Kam July 2, 2020, 7:16 a.m. UTC | #19
Hi Stephen,

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Thursday, July 2, 2020 3:18 AM
> To: Jiawei(Jonny) Wang <jiaweiw@mellanox.com>
> Cc: Jerin Jacob <jerinjacobk@gmail.com>; Thomas Monjalon
> <thomas@monjalon.net>; Ori Kam <orika@mellanox.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>; Matan Azrad <matan@mellanox.com>; dpdk-
> dev <dev@dpdk.org>; Raslan Darawsheh <rasland@mellanox.com>;
> ian.stokes@intel.com; fbl@redhat.com; Ferruh Yigit <ferruh.yigit@intel.com>;
> Andrew Rybchenko <arybchenko@solarflare.com>
> Subject: Re: [dpdk-dev] [PATCH 1/8] ethdev: introduce sample action for rte
> flow
> 
> On Sun, 28 Jun 2020 15:52:27 +0000
> "Jiawei(Jonny) Wang" <jiaweiw@mellanox.com> wrote:
> 
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Sunday, June 28, 2020 9:38 PM
> > > To: Jiawei(Jonny) Wang <jiaweiw@mellanox.com>
> > > Cc: Thomas Monjalon <thomas@monjalon.net>; Ori Kam
> > > <orika@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>;
> > > Matan Azrad <matan@mellanox.com>; dpdk-dev <dev@dpdk.org>; Raslan
> > > Darawsheh <rasland@mellanox.com>; ian.stokes@intel.com;
> > > fbl@redhat.com; Ferruh Yigit <ferruh.yigit@intel.com>; Andrew Rybchenko
> > > <arybchenko@solarflare.com>
> > > Subject: Re: [dpdk-dev] [PATCH 1/8] ethdev: introduce sample action for rte
> > > flow
> > >
> > > On Sun, Jun 28, 2020 at 6:46 PM Jiawei(Jonny) Wang
> > > <jiaweiw@mellanox.com> wrote:
> > > >
> > > >
> > > > On Friday, June 26, 2020 7:10 PM Jerin Jacob <jerinjacobk@gmail.com>
> > > Wrote:
> > > > >
> > > > > On Fri, Jun 26, 2020 at 4:16 PM Thomas Monjalon
> > > > > <thomas@monjalon.net>
> > > > > wrote:
> > > > > >
> > > > > > 26/06/2020 12:35, Jerin Jacob:
> > > > > > > On Fri, Jun 26, 2020 at 12:59 AM Thomas Monjalon
> > > > > <thomas@monjalon.net> wrote:
> > > > > > > >
> > > > > > > > 25/06/2020 19:55, Jerin Jacob:
> > > > > > > > > On Thu, Jun 25, 2020 at 10:20 PM Jiawei Wang
> > > > > <jiaweiw@mellanox.com> wrote:
> > > > > > > > > >
> > > > > > > > > > When using full offload, all traffic will be handled by
> > > > > > > > > > the HW, and directed to the requested vf or wire, the
> > > > > > > > > > control application loses visibility on the traffic.
> > > > > > > > > > So there's a need for an action that will enable the
> > > > > > > > > > control application some visibility.
> > > > > > > > > >
> > > > > > > > > > The solution is introduced a new action that will sample
> > > > > > > > > > the incoming traffic and send a duplicated traffic in some
> > > > > > > > > > predefined ratio to the application, while the original
> > > > > > > > > > packet will continue to the target destination.
> > > > > > > > > >
> > > > > > > > > > The packets sampled equals is '1/ratio', if the ratio
> > > > > > > > > > value be set to 1 , means that the packets would be
> > > > > > > > > > completely mirrored. The sample packet can be assigned
> > > > > > > > > > with different set of
> > > > > actions from the original packet.
> > > > > > > > > >
> > > > > > > > > > In order to support the sample packet in rte_flow, new
> > > > > > > > > > rte_flow action definition RTE_FLOW_ACTION_TYPE_SAMPLE
> > > and
> > > > > > > > > > structure rte_flow_action_sample
> > > > > > > > >
> > > > > > > > > Isn't mirroring the packet? How about,
> > > > > > > > > RTE_FLOW_ACTION_TYPE_MIRROR I am not able to understand,
> > > Why
> > > > > it is called sample.
> > > > > > > >
> > > > > > > > Sampling is a partial mirroring.
> > > > > > >
> > > > > > > I think, By definition, _sampling_ is the _selection_ of items
> > > > > > > from a specific group.
> > > > > > > I think, _sampling_ is not dictating, what is the real action
> > > > > > > for the "selected"  items.
> > > > > > > One can get confused with the selected ones can be for forward,
> > > > > > > drop any other action.
> > > > > >
> > > > > > I see. Good design question (I will let others reply).
> > > > > >
> > > > > > > So IMO, explicit mirror keyword usage makes it is clear.
> > > >
> > > > Sampled packet is duplicated from incoming traffic at specific ratio
> > > > and will go to different sample actions;
> > > > ratio=1 is 100% duplication or mirroring.
> > > > All packets will continue to go to default flow actions.
> > >
> > > Functionality is clear from the git commit log(Not from action name).
> > > The only question is what would be the appropriate name for this action.
> > > RTE_FLOW_ACTION_TYPE_SAMPLE vs RTE_FLOW_ACTION_TYPE_MIRROR
> > >
> > > >
> > > > > > >
> > > > > > > Some more related questions:
> > > > > > > 1) What is the real use case for ratio? I am not against adding
> > > > > > > a ratio attribute if the MLX hardware supports it. It will be
> > > > > > > good to know the use case from the application perspective? And
> > > > > > > what basics application set ratio != 1?
> > > > > >
> > > > > > If I understand well, some applications want to check, by picking
> > > > > > random packets, that the processing is not failing.
> > > > >
> > > > > Not clear to me. I will wait for another explanation if any.
> > > > > In what basics application set .1 vs .8?
> > > >
> > > > The real case is like monitor the traffic with full-offload.
> > > > While packet hit the sample flow, the matching packets will be sampled
> > > > and sent to specific Queue, align with OVS sflow probability, user
> > > application can set it different value.
> > >
> > > I understand the use case for mirror and supported in a lot of HW.
> > > What I would like to understand is the use case for "ratio"?
> > > Is the "ratio" part of OpenFlow spec? Or Is it an MLX hardware feature?
> > >
> > The same usage of the 'probability' variable of ovs sample action;
> > MLX HW implemented it.
> > >
> > >
> > > >
> > > > >
> > > > > >
> > > > > > > 2) If it is for "rate-limiting" or "policing", why not use
> > > > > > > rte_mtr object (rte_mtr.h) via rte_flow action.
> > > >
> > > > The sample ratio isn’t the same as “meter’, the ratio of sampling will be
> > > calculated with incoming packets mask (every some packets sampled 1).
> > > Then the packets will be duplicated and go to do the other sample actions.
> > >
> > > What I meant here is , If the ratio is used for rate-limiting then having a
> > > cascade rule like RTE_FLOW_ACTION_TYPE_MIRROR,
> > > RTE_FLOW_ACTION_TYPE_MTR will do the job.
> > >
> > The ratio means the probability with packet replication, we don't need add
> METER action here.
> > > >
> > > >
> > > > > > > 3) One of the issue for driver developers and application
> > > > > > > writers are overlapping APIs. This would overlap with
> > > > > > > rte_eth_mirror_rule_set() API.
> > > > > > >
> > > > > > > Can we deprecate rte_eth_mirror_rule_set() API? It will be a
> > > > > > > pain for all to have overlapping APIs. We have not fixed the
> > > > > > > VLAN filter API overlap with rte_flow in ethdev. Its being TODO
> > > > > > > for multiple releases now.
> > > > > >
> > > > > > Ooooooooh yes!
> > > > > > I think flow-based API is more powerful, and should deprecate old
> > > > > > port-based API.
> > > > >
> > > > > +1 from me.
> > > > >
> > > > > it is taking too much effort and time to make support duplicate APIs.
> > > > >
> > > > > > I want to help deprecating such API in 20.11 if possible.
> > > > >
> > > > > Please start that discussion. In this case, it is clear API overlap
> > > > > with rte_eth_mirror_rule_set().
> > > > > We should not have two separate paths for the same function in the
> > > > > same ethdev library.
> > > > >
> > > > >
> > > > >
> > > > > >
> > > > > > > > Full mirroring is sampling 100% packets (ratio = 1).
> > > > > > > > That's why only one action is enough.
> > > > > >
> > > > > >
> > > > > >
> 
> One use case would be simulating packet loss or duplication. (like netem does).
> In that use case, the sample action would have to not have an implicit copy.
> 
> Could sample be defined as "if random number hits the ratio" then execute
> this alternative rule, otherwise go to next rule.
> 
> The the alternative rule could mirror if it wanted, or drop, or count, or ...
> 
> It could even be used as a form of transmit load balancing.

I think what you are suggesting is a different action, by definition (at least in kernel)
The sample is a duplication of a packet, it comes to solve the lack of visibility when doing full offload.
What you are suggesting is to add 2 flows, like discussed in a different thread will have major impact on performance
(duplicate the matching, waste two flows since number of HW have limited rules capabilities )
Also it seems a bug that application can't relay to where the packet will get to. It will break applications that are
counting on traffic to get to a specific queue.

Best,
Ori
  

Patch

diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 1685be5..733871d 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -173,6 +173,7 @@  struct rte_flow_desc_data {
 	MK_FLOW_ACTION(SET_IPV4_DSCP, sizeof(struct rte_flow_action_set_dscp)),
 	MK_FLOW_ACTION(SET_IPV6_DSCP, sizeof(struct rte_flow_action_set_dscp)),
 	MK_FLOW_ACTION(AGE, sizeof(struct rte_flow_action_age)),
+	MK_FLOW_ACTION(SAMPLE, sizeof(struct rte_flow_action_sample)),
 };
 
 int
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index b0e4199..71dd82c 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -2099,6 +2099,13 @@  enum rte_flow_action_type {
 	 * see enum RTE_ETH_EVENT_FLOW_AGED
 	 */
 	RTE_FLOW_ACTION_TYPE_AGE,
+
+	/**
+	 * Redirects specific ratio of packets to vport or queue.
+	 *
+	 * See struct rte_flow_action_sample.
+	 */
+	RTE_FLOW_ACTION_TYPE_SAMPLE,
 };
 
 /**
@@ -2709,6 +2716,28 @@  struct rte_flow_action {
 struct rte_flow;
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ACTION_TYPE_SAMPLE
+ *
+ * Adds a sample action to a matched flow.
+ *
+ * The matching packets will be duplicated to a special queue or vport
+ * in the predefined probabiilty, All the packets continues processing
+ * on the default flow path.
+ *
+ * When the sample ratio is set to 1 then the packets will be 100% mirrored.
+ * Additional action list be supported to add for sampled or mirrored packets.
+ */
+struct rte_flow_action_sample {
+	/* packets sampled equals to '1/ratio' */
+	const uint32_t ratio;
+	/* sub-action list specific for the sampling hit cases */
+	const struct rte_flow_action *actions;
+};
+
+/**
  * Verbose error types.
  *
  * Most of them provide the type of the object referenced by struct