Re: [dpdk-dev] [PATCH 09/11] examples/ipsec-secgw: Fixed ip length in case of transport

[Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>]

On 16/10/2017 12:44, Aviad Yehezkel wrote:
> On 10/16/2017 12:43 PM, Sergio Gonzalez Monroy wrote:
>> On 14/10/2017 14:28, aviadye@dev.mellanox.co.il wrote:
>>> From: Aviad Yehezkel <aviadye@mellanox.com>
>>>
>>> IP length was incorrect causing corrupted ICMP packets for example
>>>
>>> Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
>>> ---
>>>   examples/ipsec-secgw/esp.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/examples/ipsec-secgw/esp.c b/examples/ipsec-secgw/esp.c
>>> index 81ebf55..12c6f8c 100644
>>> --- a/examples/ipsec-secgw/esp.c
>>> +++ b/examples/ipsec-secgw/esp.c
>>> @@ -205,13 +205,13 @@ esp_inbound_post(struct rte_mbuf *m, struct 
>>> ipsec_sa *sa,
>>>           if (likely(ip->ip_v == IPVERSION)) {
>>>               memmove(ip4, ip, ip->ip_hl * 4);
>>>               ip4->ip_p = *nexthdr;
>>> -            ip4->ip_len = htons(rte_pktmbuf_data_len(m));
>>> +            ip4->ip_len = htons(rte_pktmbuf_pkt_len(m));
>>>           } else {
>>>               ip6 = (struct ip6_hdr *)ip4;
>>>               /* XXX No option headers supported */
>>>               memmove(ip6, ip, sizeof(struct ip6_hdr));
>>>               ip6->ip6_nxt = *nexthdr;
>>> -            ip6->ip6_plen = htons(rte_pktmbuf_data_len(m));
>>> +            ip6->ip6_plen = htons(rte_pktmbuf_pkt_len(m));
>>>           }
>>>       } else
>>>           ipip_inbound(m, sizeof(struct esp_hdr) + sa->iv_len);
>>
>> AFAIK the app does not support multi-segments (chain mbufs), so 
>> data_len should be the same as pkt_len.
>> Is that not the case?
>>
> This is the inbound function (RX side), so mbufs are allocated by PMD.
> PMD is allocating mbuf with additional 14 bytes for ETH header but 
> trim it before passing the mbuf.
> As a result seg len is 14 bytes smaller than data len.
>

Sorry, I am still missing something here.
rte_pktmbuf_trim updates both data_len and pkt_len, so how can they not 
be the same when we have a single mbuf?

I think to remember using data_len instead of pkt_len so it is easy to 
see that the application does not support multi-segments (aka. chained 
mbufs)

Thanks,
Sergio

> Thanks,
> Aviad
>
>> Thanks,
>> Sergio
>