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

[Jiawei(Jonny) Wang]

> -----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.
> > > >
> > > >
> > > >