Re: [dpdk-dev] [EXT] Re: [PATCH] ipsec: include high order bytes of esn in pkt len

[Lukas Bartosik <lbartosik@marvell.com>]

On 14.05.2019 15:52, Ananyev, Konstantin wrote:
> Hi Lukasz,
> 
>>>>
>>>> When esn is used then high-order 32 bits are included in ICV
>>>> calculation however are not transmitted. Update packet length
>>>> to be consistent with auth data offset and length before crypto
>>>> operation. High-order 32 bits of esn will be removed from packet
>>>> length in crypto post processing.
>>>
>>> Hi Lukasz,
>>> Why you want to do it?
>>> I deliberately didn't include SQH bits into the pkt_len/data_len,
>>> because it is a temporary data and we are going to drop it anyway.
>>> Konstantin
>>>
>>> Hi Konstantin,
>>> Our OcteonTx crypto driver validates pkt_len with auth data length/offset and it complains
>>> because it is told to authenticate more data that a packet holds (according to pkt_len).
> 
> Thanks for explanation, just to confirm about the check in your PMD:
> You are talking about struct rte_crypto_sym_op auth.data.offset and auth.data.length,
> i.e: auth.data.offset + auth.data.length > pkt_len
> Or something else?
> 
> find drivers/*/octeon* -type f | xargs grep -l 'auth\.data\.'
> returns no results.
> 
> Konstantin
> 

Hi Konstantin

This is exactly auth.data.length and auth.data.offset from rte_crypto_sym_op.
The check takes place in drivers/common/cpt/cpt_ucode.h in cpt_dec_hmac_prep function
although there is no direct check for auth.data.offset + auth.data.length > pkt_len
as at this point auth.data.offset, auth.data.length and pkt_len are stored in 
internal structures related to how we process crypto requests.

Thanks,
Lukasz

>>> I came across this when running IPSec tests which use esn.
>>> I understand that sqh 32 bits are temporary and included only for ICV calculation however
>>> not including them in pkt_len for crypto processing is inconsistent in my opinion.
>>> Thanks,
>>> Lukasz
>>>
>>
>> Hi Konstantin,
>>
>> I should have elaborated more. When 32 high bits of esn are not included in
>> packet length then auth offset and data point to data which is outside packet
>> (according to packet length).
>> This makes crypto request (auth data length and offset) incoherent with a packet
>> which the crypto request points to.
>>
>> This is my argument for including 32 high bits of esn into packet length even
>> though the inclusion is only temporary.
>>
>> Thanks,
>> Lukasz
>>