[dpdk-dev] [PATCH 1/8] ethdev: introduce sample action for rte flow

[Ori Kam]

Hi Stephen,

> -----Original Message-----
> From: Stephen Hemminger <stephen at networkplumber.org>
> Sent: Thursday, July 2, 2020 3:18 AM
> To: Jiawei(Jonny) Wang <jiaweiw at mellanox.com>
> Cc: Jerin Jacob <jerinjacobk at gmail.com>; Thomas Monjalon
> <thomas at monjalon.net>; Ori Kam <orika at mellanox.com>; Slava Ovsiienko
> <viacheslavo at mellanox.com>; Matan Azrad <matan at mellanox.com>; dpdk-
> dev <dev at dpdk.org>; Raslan Darawsheh <rasland at mellanox.com>;
> ian.stokes at intel.com; fbl at redhat.com; Ferruh Yigit <ferruh.yigit at intel.com>;
> Andrew Rybchenko <arybchenko at 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 at mellanox.com> wrote:
> 
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk at gmail.com>
> > > Sent: Sunday, June 28, 2020 9:38 PM
> > > To: Jiawei(Jonny) Wang <jiaweiw at mellanox.com>
> > > Cc: Thomas Monjalon <thomas at monjalon.net>; Ori Kam
> > > <orika at mellanox.com>; Slava Ovsiienko <viacheslavo at mellanox.com>;
> > > Matan Azrad <matan at mellanox.com>; dpdk-dev <dev at dpdk.org>; Raslan
> > > Darawsheh <rasland at mellanox.com>; ian.stokes at intel.com;
> > > fbl at redhat.com; Ferruh Yigit <ferruh.yigit at intel.com>; Andrew Rybchenko
> > > <arybchenko at 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 at mellanox.com> wrote:
> > > >
> > > >
> > > > On Friday, June 26, 2020 7:10 PM Jerin Jacob <jerinjacobk at gmail.com>
> > > Wrote:
> > > > >
> > > > > On Fri, Jun 26, 2020 at 4:16 PM Thomas Monjalon
> > > > > <thomas at monjalon.net>
> > > > > wrote:
> > > > > >
> > > > > > 26/06/2020 12:35, Jerin Jacob:
> > > > > > > On Fri, Jun 26, 2020 at 12:59 AM Thomas Monjalon
> > > > > <thomas at monjalon.net> wrote:
> > > > > > > >
> > > > > > > > 25/06/2020 19:55, Jerin Jacob:
> > > > > > > > > On Thu, Jun 25, 2020 at 10:20 PM Jiawei Wang
> > > > > <jiaweiw at 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