diff mbox series

app/testpmd: fix GTP header parsing in csum FWD engine

Message ID 20220310142019.13398-1-getelson@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series app/testpmd: fix GTP header parsing in csum FWD engine |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Gregory Etelson March 10, 2022, 2:20 p.m. UTC 
  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.

Cc: stable@dpdk.org

Fixes: d8e5e69f3a9b ("app/testpmd: add GTP parsing and Tx checksum offload")
Signed-off-by: Gregory Etelson <getelson@nvidia.com>
---
 app/test-pmd/csumonly.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Singh, Aman Deep March 11, 2022, 1:34 p.m. UTC | #1 
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.
>
> Cc:stable@dpdk.org
>
> Fixes: d8e5e69f3a9b ("app/testpmd: add GTP parsing and Tx checksum offload")
> Signed-off-by: Gregory Etelson<getelson@nvidia.com>
Acked-by: Aman Singh <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) {
Gregory Etelson March 13, 2022, 8:44 a.m. UTC | #2 
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) {
diff mbox series

Patch

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) {