Hi all,
CC Adrien
(I apologize for pulling you to the rte_flow API discussions
once again, but may be you can find spare time and share your
thoughts. Your opinion as an author and architect of the
rte_flow API would be very useful and highly appreciated.)
On 6/29/20 2:40 PM, Ori Kam wrote:
> 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.
Marking or tagging could be used to address it. E.g. target
traffic could be tagged first, then matching by tag should be
used to sample and to do HW offloads.
Moreover, mapping of rte_flow API rules into HW rule is not
required to be 1-to-1. Yes, 1-to-1 is simple, but it could be
more complicated 1-to-N (when one rte_flow API rule is
represented by many HW rules) or N-to-1 (when few flow API
rules are represented as one HW rule) or even N-to-M.
For example, tagging which is not visible outside, could be
purely SW and used to build such constructions in SW.
It is an implementation detail is out of scope of the generic
API definition.
Yes, it sounds like over-complicating, but I really dislike
above sub-action list from design point of view and that's
why I"m trying to think in different directions.
> 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)
See above.
> 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.
Sorry, but there are two branches for terminating actions in
the sampling action design anyway (yes, internal/hidden).
You need two copies of the packet, so whatever you do it
will be two terminating actions.
> 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.
Yes, it is not a problem with PASSTHRU.
> 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.
Order should be simply different: first sampling with pass-
through, second decap and deliver to VM.
Yes, I realize that two actions with basically the same match
(modulo sampling match) is not ideal for mapping to HW (even
if it collapsed into trivial tag match which pre-rule to make
tagging). I'm not 100% happy with it, but I'm even less happy
with sub-action list design and just trying to find better
alternative solution.
> 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
>
