app/testpmd: fix GTP header parsing in csum FWD engine
PATCH 1/1 app/testpmd: fix GTP header parsing in csum FWD engine
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.
--- app/test-pmd/csumonly.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
app/test-pmd/csumonly.c +3 -4
@@ -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 ) {
Signature · 1 line
2.35.1
Looks good to me.
Quoted 7 lines · Gregory Etelson
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.
Quoted 5 lines
Fixes d8e5e69f3a9b ("app/testpmd: add GTP parsing and Tx checksum offload")Signed-off-by Gregory Etelson<[email protected]>
Quoted 27 lines
--- app/test-pmd/csumonly.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)app/test-pmd/csumonly.c +3
-4@@ -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
Acked-by Aman SinghHello,
From: Singh, Aman Deep <[email protected]>
Sent: Friday, March 11, 2022 15:35
To: Gregory Etelson <[email protected]>; [email protected]
Cc: Matan Azrad <[email protected]>; Raslan Darawsheh <[email protected]>; [email protected]; Xiaoyun Li <[email protected]>; Yuying Zhang <[email protected]>; Ting Xu <[email protected]>; Ferruh Yigit <[email protected]>
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.
---
app/test-pmd/csumonly.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
app/test-pmd/csumonly.c +0 -0
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) {PATCH v2 1/1 app/testpmd: fix GTP header parsing in csum FWD engine
In reply to Gregory EtelsonGTP 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 at-least one of the extension bits is set.
---
v2: Update the patch comment.
--- app/test-pmd/csumonly.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
app/test-pmd/csumonly.c +3 -4
@@ -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 ) {
Signature · 1 line
2.35.1
13/03/2022 10:01, Gregory Etelson:
Quoted 6 lines
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.
Quoted 8 lines
The patch updates GTP header length if at-least one of the extension bits is set.
Fixes d8e5e69f3a9b ("app/testpmd: add GTP parsing and Tx checksum offload")Signed-off-by Gregory Etelson <[email protected]>Acked-by Aman Singh <[email protected]>
It should be "Fixes" line, "Cc" line, blank line, and "Signed-off/Ack" lines. Applied, thanks.