Hi all,
> -----Original Message-----
> From: Andrew Rybchenko <arybche...@solarflare.com>
> Sent: Sunday, June 28, 2020 7:19 PM
> To: Jiawei(Jonny) Wang <jiaw...@mellanox.com>; Ori Kam
> <or...@mellanox.com>; Slava Ovsiienko <viachesl...@mellanox.com>; Matan
> Azrad <ma...@mellanox.com>
> Cc: dev@dpdk.org; Thomas Monjalon <tho...@monjalon.net>; Raslan
> Darawsheh <rasl...@mellanox.com>; ian.sto...@intel.com; f...@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
> <arybche...@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 <jiaw...@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
