RE: [PATCH] app/testpmd: fix GTP header parsing in csum FWD engine

[Gregory Etelson <getelson@nvidia.com>]

[-- Attachment #1: Type: text/plain, Size: 2487 bytes --]

Hello,

From: Singh, Aman Deep <aman.deep.singh@intel.com>
Sent: Friday, March 11, 2022 15:35
To: Gregory Etelson <getelson@nvidia.com>; dev@dpdk.org
Cc: Matan Azrad <matan@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; stable@dpdk.org; Xiaoyun Li <xiaoyun.li@intel.com>; Yuying Zhang <yuying.zhang@intel.com>; Ting Xu <ting.xu@intel.com>; Ferruh Yigit <ferruh.yigit@intel.com>
Subject: Re: [PATCH] app/testpmd: fix GTP header parsing in csum FWD engine

External email: Use caution opening links or attachments


Looks good to me.
On 3/10/2022 7:50 PM, Gregory Etelson wrote:

GTP header can be followed by an optional 32 bits extension.

GTP notifies about the extension presence through the E, S or PN

header bits.



Csum GTP header parser did not check the extension bits value.



The patch updates GTP header length if header extension bits are set.
Can we rephrase above line "if at-least one of the extension bit is set."
To make it more clear.

[Gregory] I’ll post v2 with updated comment.







Cc: stable@dpdk.org<mailto:stable@dpdk.org>



Fixes: d8e5e69f3a9b ("app/testpmd: add GTP parsing and Tx checksum offload")

Signed-off-by: Gregory Etelson <getelson@nvidia.com><mailto:getelson@nvidia.com>
Acked-by: Aman Singh <aman.deep.singh@intel.com><mailto:aman.deep.singh@intel.com>



---

 app/test-pmd/csumonly.c | 7 +++----

 1 file changed, 3 insertions(+), 4 deletions(-)



diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c

index 5274d498ee..f8abcded2b 100644

--- a/app/test-pmd/csumonly.c

+++ b/app/test-pmd/csumonly.c

@@ -223,15 +223,14 @@ parse_gtp(struct rte_udp_hdr *udp_hdr,



        gtp_hdr = (struct rte_gtp_hdr *)((char *)udp_hdr +

                  sizeof(struct rte_udp_hdr));

-

+       if (gtp_hdr->e || gtp_hdr->s || gtp_hdr->pn)

+               gtp_len += sizeof(struct rte_gtp_hdr_ext_word);

        /*

         * Check message type. If message type is 0xff, it is

         * a GTP data packet. If not, it is a GTP control packet

         */

        if (gtp_hdr->msg_type == 0xff) {

-               ip_ver = *(uint8_t *)((char *)udp_hdr +

-                        sizeof(struct rte_udp_hdr) +

-                        sizeof(struct rte_gtp_hdr));

+               ip_ver = *(uint8_t *)((char *)gtp_hdr + gtp_len);

                ip_ver = (ip_ver) & 0xf0;



                if (ip_ver == RTE_GTP_TYPE_IPV4) {

[-- Attachment #2: Type: text/html, Size: 7927 bytes --]