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

[Aviad Yehezkel <aviadye@dev.mellanox.co.il>]

Solved that issue, this was an issue with mlx5 PMD with the new inline 
ipsec code.

The PMD wasn't updating mbuf->data_len correctly.


Thanks!


On 10/16/2017 3:03 PM, Sergio Gonzalez Monroy wrote:
> 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
>>
>