Bill Fischofer(Bill-Fischofer-Linaro) 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:
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_r150429846
updated_at 2017-11-13 01:57:53
