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

[Lukas Bartosik <lbartosik@marvell.com>]

Hi Konstantin

On 30.05.2019 18:51, Ananyev, Konstantin wrote:
> Hi Lukasz,
> 
>> diff --git a/lib/librte_ipsec/esp_outb.c b/lib/librte_ipsec/esp_outb.c
>> index c798bc4..ed5974b 100644
>> --- a/lib/librte_ipsec/esp_outb.c
>> +++ b/lib/librte_ipsec/esp_outb.c
>> @@ -126,11 +126,11 @@ outb_tun_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
>>
>>  	/* pad length + esp tail */
>>  	pdlen = clen - plen;
>> -	tlen = pdlen + sa->icv_len;
>> +	tlen = pdlen + sa->icv_len + sa->sqh_len;
> 
> We probably don't want to increase pkt_len by  sa->sqh_len for inline case.
> That's why I suggested to pass sqh_len as parameter to that function.
> Then for inline we can just pass 0.
> Do you see any obstacles with that approach?
> Same thought for transport mode.
> Konstantin
> 

I agree this is incorrect. I have missed inline case.
I will send revised patch.

Thanks,
Lukasz

>>
>>  	/* do append and prepend */
>>  	ml = rte_pktmbuf_lastseg(mb);
>> -	if (tlen + sa->sqh_len + sa->aad_len > rte_pktmbuf_tailroom(ml))
>> +	if (tlen + sa->aad_len > rte_pktmbuf_tailroom(ml))
>>  		return -ENOSPC;
>>
>>  	/* prepend header */
>> @@ -152,8 +152,8 @@ outb_tun_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
>>  	rte_memcpy(ph, sa->hdr, sa->hdr_len);
>>
>>  	/* update original and new ip header fields */
>> -	update_tun_l3hdr(sa, ph + sa->hdr_l3_off, mb->pkt_len, sa->hdr_l3_off,
>> -			sqn_low16(sqc));
>> +	update_tun_l3hdr(sa, ph + sa->hdr_l3_off, mb->pkt_len - sa->sqh_len,
>> +			sa->hdr_l3_off, sqn_low16(sqc));
>>
>>  	/* update spi, seqn and iv */
>>  	esph = (struct esp_hdr *)(ph + sa->hdr_len);
>> @@ -292,11 +292,11 @@ outb_trs_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
>>
>>  	/* pad length + esp tail */
>>  	pdlen = clen - plen;
>> -	tlen = pdlen + sa->icv_len;
>> +	tlen = pdlen + sa->icv_len + sa->sqh_len;
>>
>>  	/* do append and insert */
>>  	ml = rte_pktmbuf_lastseg(mb);
>> -	if (tlen + sa->sqh_len + sa->aad_len > rte_pktmbuf_tailroom(ml))
>> +	if (tlen + sa->aad_len > rte_pktmbuf_tailroom(ml))
>>  		return -ENOSPC;
>>
>>  	/* prepend space for ESP header */
>> @@ -314,8 +314,8 @@ outb_trs_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
>>  	insert_esph(ph, ph + hlen, uhlen);
>>
>>  	/* update ip  header fields */
>> -	np = update_trs_l3hdr(sa, ph + l2len, mb->pkt_len, l2len, l3len,
>> -			IPPROTO_ESP);
>> +	np = update_trs_l3hdr(sa, ph + l2len, mb->pkt_len - sa->sqh_len, l2len,
>> +			l3len, IPPROTO_ESP);
>>
>>  	/* update spi, seqn and iv */
>>  	esph = (struct esp_hdr *)(ph + uhlen);
>> @@ -425,6 +425,9 @@ esp_outb_sqh_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>>  	for (i = 0; i != num; i++) {
>>  		if ((mb[i]->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) == 0) {
>>  			ml = rte_pktmbuf_lastseg(mb[i]);
>> +			/* remove high-order 32 bits of esn from packet len */
>> +			mb[i]->pkt_len -= sa->sqh_len;
>> +			ml->data_len -= sa->sqh_len;
>>  			icv = rte_pktmbuf_mtod_offset(ml, void *,
>>  				ml->data_len - icv_len);
>>  			remove_sqh(icv, icv_len);