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

["Jiawei(Jonny) Wang" <jiaweiw@mellanox.com>]

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.