Hi Stephen, > -----Original Message----- > From: Stephen Hemminger <step...@networkplumber.org> > Sent: Thursday, July 2, 2020 3:18 AM > To: Jiawei(Jonny) Wang <jiaw...@mellanox.com> > Cc: Jerin Jacob <jerinjac...@gmail.com>; Thomas Monjalon > <tho...@monjalon.net>; Ori Kam <or...@mellanox.com>; Slava Ovsiienko > <viachesl...@mellanox.com>; Matan Azrad <ma...@mellanox.com>; dpdk- > dev <dev@dpdk.org>; Raslan Darawsheh <rasl...@mellanox.com>; > ian.sto...@intel.com; f...@redhat.com; Ferruh Yigit <ferruh.yi...@intel.com>; > Andrew Rybchenko <arybche...@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" <jiaw...@mellanox.com> wrote: > > > > -----Original Message----- > > > From: Jerin Jacob <jerinjac...@gmail.com> > > > Sent: Sunday, June 28, 2020 9:38 PM > > > To: Jiawei(Jonny) Wang <jiaw...@mellanox.com> > > > Cc: Thomas Monjalon <tho...@monjalon.net>; Ori Kam > > > <or...@mellanox.com>; Slava Ovsiienko <viachesl...@mellanox.com>; > > > Matan Azrad <ma...@mellanox.com>; dpdk-dev <dev@dpdk.org>; Raslan > > > Darawsheh <rasl...@mellanox.com>; ian.sto...@intel.com; > > > f...@redhat.com; Ferruh Yigit <ferruh.yi...@intel.com>; Andrew Rybchenko > > > <arybche...@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 > > > <jiaw...@mellanox.com> wrote: > > > > > > > > > > > > On Friday, June 26, 2020 7:10 PM Jerin Jacob <jerinjac...@gmail.com> > > > Wrote: > > > > > > > > > > On Fri, Jun 26, 2020 at 4:16 PM Thomas Monjalon > > > > > <tho...@monjalon.net> > > > > > wrote: > > > > > > > > > > > > 26/06/2020 12:35, Jerin Jacob: > > > > > > > On Fri, Jun 26, 2020 at 12:59 AM Thomas Monjalon > > > > > <tho...@monjalon.net> wrote: > > > > > > > > > > > > > > > > 25/06/2020 19:55, Jerin Jacob: > > > > > > > > > On Thu, Jun 25, 2020 at 10:20 PM Jiawei Wang > > > > > <jiaw...@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