Dmitry Eremin-Solenikov(lumag) replied on github web page:
platform/linux-generic/odp_ipsec.c
@@ -1055,7 +1069,16 @@ int odp_ipsec_in_enq(const odp_packet_t pkt_in[], int
num_in,
result->status = status;
if (NULL != ipsec_sa) {
result->sa = ipsec_sa->ipsec_sa_hdl;
- queue = ipsec_sa->queue;
+ if (ipsec_sa->in.cos && !status.error.all) {
+ odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
+ const uint8_t *base = odp_packet_data(pkt);
+
+ queue = cls_pkt_get_queue(pkt_hdr,
+ ipsec_sa->in.cos,
+ base);
+ } else {
Comment:
OK, sorry, I was confused by the text. Dropped these patches for now.
> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
> Read that again. It specifies a CoS that begins classification, not a queue.
> Putting a packet on a queue does not trigger the classifier. The current
> linux-generic classifier implementation doesn't support this type of
> "alternate entry point", nor does it support decrypted packets that probably
> don't start with L2 headers, etc. That's why I suggest this should probably
> be its own PR because it will involve enhancements to the classifier as well
> as IPsec to achieve the desired result.
>> Dmitry Eremin-Solenikov(lumag) wrote:
>> @Bill-Fischofer-Linaro quoting spec:
>> ```
>> * Successfully decapsulated packets are sent to
>> * classification through this CoS. Other resulting
>> * events are sent to 'dest_queue'. This field is
>> * considered only when 'pipeline' is
>> * ODP_IPSEC_PIPELINE_CLS. The CoS must not be
>> shared
>> * between any pktio interface default CoS. The
>> maximum
>> * number of different CoS supported is defined by
>> * IPSEC capability max_cls_cos.
>>
>> ```
>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>> Ack, will change.
>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>> @Bill-Fischofer-Linaro the problem here is not in shifting, but in locking
>>>> (this implementation is lock-free). I will take a deeper look on the RFC
>>>> 6479 after finishing IPv6.
>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>> @Bill-Fischofer-Linaro well, this is not what API says. Currently if
>>>>> `ODP_PIPELINE_CLS` is selected, IPsec packets go to predefined (per-SA)
>>>>> `odp_cos_t`.
>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>> This test is unnecessary as you're not going to overflow a 64-bit
>>>>>> counter.
>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>> This test is going to need revision since the actual CoS is selected as
>>>>>>> a result of filtering the decrypted packet through PMRs that don't
>>>>>>> exist in this version of the test. Another reason to make pipeline
>>>>>>> support its own PR.
>>>>>>>
>>>>>>> The test we'd want is
>>>>>>>
>>>>>>> - Create a couple of CoS and associated queue objects.
>>>>>>> - Create PMRs that filter packets into these CoSes.
>>>>>>> - Create plaintext packets that will match the PMRs defined.
>>>>>>> - Encrypt these packets via `odp_ipsec_out()`
>>>>>>> - Now decrypt them via `odp_ipsec_in_enq()` with pipelining enabled and
>>>>>>> verify that the results show up on the expected queue based on the
>>>>>>> previously defined PMRs.
>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>> Pipelining isn't about sending the result to an alternate queue that's
>>>>>>>> known ahead of time, but rather about sending the result to the
>>>>>>>> `cls_classify_packet()` function so that classification can be run
>>>>>>>> against the decrypted packet and PMRs can filter it into the correct
>>>>>>>> CoS (and associated queue) that is to receive the packet.
>>>>>>>>
>>>>>>>> This is likely to be a sufficiently large change in itself that it may
>>>>>>>> be better to be its own PR, in which case this can be pulled from the
>>>>>>>> rest of this PR so that the other changes can be merged first.
>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>> Not urgent, but at some point we should look at [RFC
>>>>>>>>> 6479](https://tools.ietf.org/html/rfc6479) to handle larger windows
>>>>>>>>> needed for large multi-core systems and to do this more efficiently.
>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>> It seems inefficient to incur those two atomic loads on every call.
>>>>>>>>>> How about:
>>>>>>>>>> ```
>>>>>>>>>> if (ipsec_sa->hard_limit_bytes > 0 &&
>>>>>>>>>> odp_atomic_load_u64(&ipsec_sa->bytes) >
>>>>>>>>>> ipsec_sa->hard_limit_bytes) etc.
>>>>>>>>>> ```
https://github.com/Linaro/odp/pull/243#discussion_r150434050
updated_at 2017-11-13 01:57:53
