[dpdk-dev] [PATCH v3 2/2] examples/ipsec-secgw: add target queues in flow actions

[Anoob Joseph]

Hi Nelio,


On 01/09/2018 06:18 PM, Nelio Laranjeiro wrote:
> Hi Anoob,
>
> On Fri, Jan 05, 2018 at 11:48:50AM +0530, Anoob Joseph wrote:
>> Hi Adrien,
>>
>>
>> On 12/21/2017 07:52 PM, Adrien Mazarguil wrote:
>>> On Thu, Dec 21, 2017 at 12:12:29PM +0200, Boris Pismenny wrote:
>>> <snip>
>>>> On 12/21/2017 10:06 AM, Anoob Joseph wrote:
>>> <snip>
>>>>> I can see the benefits of using rte_flow in both rx & tx, but this
>>>>> unnecessarily introduces hardware requirements for supporting inline.
>>>>> Rte_flow would do two operations:
>>>>> 1) Pattern matching
>>>>> 2) Perform some operation with the above matched packets
>>>>>
>>>>> Issue is with pattern matching, not about performing the operations
>>>>> specified. If we need rte_flow in the tx, the PMD should support pattern
>>>>> matching in software or hardware. Since the application will have to do
>>>>> a lookup in it's space to determine the SA, the secondary lookup in the
>>>>> PMD may not be necessary. But making rte_flow mandatory would make tx
>>>>> side pattern matching mandatory, which may not be supported on all
>>>>> hardware (with inline crypto/protocol). Also the pattern matching
>>>>> hardware module should be able to submit to inline performing module for
>>>>> this to work.
>>>>>
>>>>> May be the right approach is to decouple pattern matching from actions
>>>>> to be performed for the flow. In other words, add a new API to allow
>>>>> application to submit a packet to a flow. In such case, application
>>>>> could do the lookup and submit packet to a flow. The packet submitted
>>>>> could be validated to see if it is matching the flow properties. If it
>>>>> is matching, then the actions specified for the flow would be performed.
>>>>> Adding such an API will allow rte_flow to be used with hardware which
>>>>> doesn't have packet filtering features.
>>>>>
>>>>> The flow could have a "pattern item" which would say whether application
>>>>> can submit packets to the flow. Submit would be allowed only for those
>>>>> flows. flow_validate would give PMD the option to accept or reject such
>>>>> a model. This may need some thought before we can start implementing,
>>>>> like, whether we should support "submit" for flows which doesn't have
>>>>> terminating action.
>>>>>
>>>>> Any thoughts?
>>>> I think that your suggested API is more or less the intended use of rte_flow
>>>> egress actions with rte_security.
>>>>
>>>> Would it be wrong to say that you could use rte_flow without doing pattern
>>>> matching in HW or in the PMD in the data-path?
>>>> Suppose that your HW doesn't support pattern matching on tx. But, you do
>>>> support IPsec inline crypto on tx according to user provided pointers that
>>>> you set in the set_pkt_metadata callback. The user will call rte_create_flow
>>>> with some pattern, in response you check that the driver's set_pkt_metadata
>>>> could handle such patterns and actions on tx. If yes, then return success,
>>>> otherwise return false. The successful creation of the flow will indicate to
>>>> the user that packets with this format will be offloaded. Packets with other
>>>> formats will not get offload and set_pkt_metadata for such packets shouldn't
>>>> be called!
>>>>
>>>> When using rte_flow with IPsec, it is used not to indicate that HW must do
>>>> this pattern matching. But rather to indicate that software will send
>>>> packets that match a pattern with proper metadata and expect an action to be
>>>> applied. Software cannot expect this action to be applied unless the packet
>>>> matches the pattern and the proper metadata is provided. For example,
>>>> packets with IPv6 extension headers should not go through IPsec inline
>>>> crypto offload if the pattern is IPv6/ESP because the next IP protocol must
>>>> be ESP.
>>> I think there's already a way to satisfy everyone regarding context
>>> requirements on TX without the huge penalty of SW parsing in case HW doesn't
>>> support matching on egress.
>>>
>>> While seldom used at the moment, rte_flow patterns can match packet
>>> meta-data (see meta pattern items); for instance RTE_FLOW_ITEM_TYPE_PORT
>>> matches a physical port of the underlying device. This works both with
>>> ingress and egress.
>>>
>>> For ingress, RTE_FLOW_ACTION_TYPE_MARK can be used to add meta data to
>>> selected packets. While for egress such an action doesn't make much sense at
>>> the moment, the converse meta pattern item (RTE_FLOW_ITEM_TYPE_MARK) could
>>> be useful to let PMD know what needs to be done with packets submitted by
>>> the application and containing a given mark.
>> Good suggestion. For ingress this would help. The only problem with this is
>> the size of ID. Since it is 32 bit, this particular mark cannot be used to
>> save and retrieve application pointers. If it was 64 bit, this could've
>> solved all the metadata problems.
>>
>> For egress, PMD would still need to do a lookup as, the mark ID would be 32
>> bit and has to be matched with the same in various flows. Though this is
>> better than what we have right now(with flows), this may still be more
>> costlier than "set_pkt_metadata" method.
> It depends on how you implement it, it can be a simple table using it as
> an index, it can be a simple hash based on the tunnel informations,
> etc...
Yeah. I see your point. It can be an index that is set per packet. This 
index will be obtained from the driver as the flow is created, and will 
be saved along with SA. This can be used to uniquely associate a packet 
with a flow.
>
> set_pkt_metada() has two issues:
>
>   1. it is device specific [1] as described in the API.  For an
>      application it is unusable.
>
>   2. it is a function called per packet which is costly and don't give
>      any benefit for an application which can directly store the pointer
>      in the mbuf without calling such function.
For inline crypto mode, the only thing that set_pkt_metadata does is 
associate a packet with a particular security session. The above 
discussed model could replace that. But for inline protocol, that may 
not be the case. In fact I'm in the middle of drafting a patch which 
will enable the application to set & retrieve sequence number & IV used 
while protocol offload. For such cases, there will be more use cases for 
set_pkt_metadata than just storing a pointer.

The idea is, in such cases application would need to send per packet 
information. In inline crypto, it wasn't necessarily per packet (setting 
security session pointer, which was per SA), but for inline protocol, 
there will be per packet variables that the application would need to 
communicate.

For inline crypto, the "MARK" idea should be fine, but for inline 
protocol that may not suffice the requirement.
>>> That way, PMDs that do not support egress packet matching wouldn't need to
>>> lie and would reject rules such as:
>>>
>>>    flow create X egress pattern ip / udp / end actions whatever / end
>>>
>>> But would accept:
>>>
>>>    flow create X egress pattern mark is 42 / end actions whatever / end
>>>
>>> PMDs could also support combinations (remember the position of meta items
>>> within a pattern is not significant) to only perform whatever for UDPv4
>>> packets marked with 42. Non-marked packets would go through unmodified:
>>>
>>>    flow create X egress pattern ip / udp / mark is 42 / end actions whatever / end
>>>
>>> This is just to point out how leveraging meta pattern items on egress is one
>>> possibility using MARK as an example.
>>>
> Regards,
>
> [1] https://dpdk.org/browse/next/dpdk-next-crypto/tree/lib/librte_security/rte_security.h#n331
>