[PATCH 1/2] net: fix offset calculation for GENEVE packet

[PATCH 1/2] net: fix offset calculation for GENEVE packet

[skori]

From: Sunil Kumar Kori <skori@marvell.com>

While parsing packet headers, offset must be added to get next
header but for geneve header parsing offset is overwritten.
Also inner_l2_len is not set in case of geneve packets.

Fixes: 64ed7f854cf4 ("net: add tunnel packet type parsing")

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
 lib/net/rte_net.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/net/rte_net.c b/lib/net/rte_net.c
index be24690fdf..8a3ebf8478 100644
--- a/lib/net/rte_net.c
+++ b/lib/net/rte_net.c
@@ -251,7 +251,8 @@ ptype_tunnel_with_udp(uint16_t *proto, const struct rte_mbuf *m,
 		if (unlikely(gnh == NULL))
 			return 0;
 		geneve_len = sizeof(*gnh) + gnh->opt_len * 4;
-		*off = geneve_len;
+		hdr_lens->inner_l2_len = sizeof(struct rte_udp_hdr) + geneve_len;
+		*off += geneve_len;
 		*proto = gnh->proto;
 		if (gnh->proto == 0)
 			*proto = rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV4);
-- 
2.43.0

[PATCH 2/2] app/testpmd: clear stale internal len information

[skori]

From: Sunil Kumar Kori <skori@marvell.com>

hdr_lens is used to maintain header lengths after parsing packets.
When port receives different type of packets (say first is VXLAN
packet and second is GRE packet).

For first packet, L2/L3/L4 lengths are set for inner and outer header
alongwith tunnel_len.

Now for second packet, tunnel_len is added more than its size it
contains stale value which further leads to wrong header pointers.

Hence clearing stale information before processing each packet.

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
 app/test-pmd/csumonly.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index fa0002d321..4d02d622d6 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -670,6 +670,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 			rte_prefetch0(rte_pktmbuf_mtod(pkts_burst[i + 1],
 						       void *));
 
+		memset(&hdr_lens, 0, sizeof(struct rte_net_hdr_lens));
 		m = pkts_burst[i];
 		info.is_tunnel = 0;
 		info.pkt_len = rte_pktmbuf_pkt_len(m);
-- 
2.43.0

Re: [PATCH 2/2] app/testpmd: clear stale internal len information

[Stephen Hemminger]

On Mon, 19 May 2025 21:36:56 +0530
<skori@marvell.com> wrote:

> From: Sunil Kumar Kori <skori@marvell.com>
> 
> hdr_lens is used to maintain header lengths after parsing packets.
> When port receives different type of packets (say first is VXLAN
> packet and second is GRE packet).
> 
> For first packet, L2/L3/L4 lengths are set for inner and outer header
> alongwith tunnel_len.
> 
> Now for second packet, tunnel_len is added more than its size it
> contains stale value which further leads to wrong header pointers.
> 
> Hence clearing stale information before processing each packet.
> 
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>

The initialization of hdr_lens then becomes redundant.

Maybe better to just move it into the loop.

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index fa0002d321..203af35cf0 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -627,7 +627,6 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
        uint32_t rx_bad_outer_l4_csum;
        uint32_t rx_bad_outer_ip_csum;
        struct testpmd_offload_info info;
-       struct rte_net_hdr_lens hdr_lens = {0};
        uint32_t ptype;
 
        /* receive a burst of packet */
@@ -666,6 +665,8 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 #endif
 
        for (i = 0; i < nb_rx; i++) {
+               struct rte_net_hdr_lens hdr_lens = {0};
+
                if (likely(i < nb_rx - 1))
                        rte_prefetch0(rte_pktmbuf_mtod(pkts_burst[i + 1],
                                                       void *));
~/DPDK/main $ 

Re: [PATCH 1/2] net: fix offset calculation for GENEVE packet

[Stephen Hemminger]

On Mon, 19 May 2025 21:36:55 +0530
<skori@marvell.com> wrote:

> From: Sunil Kumar Kori <skori@marvell.com>
> 
> While parsing packet headers, offset must be added to get next
> header but for geneve header parsing offset is overwritten.
> Also inner_l2_len is not set in case of geneve packets.
> 
> Fixes: 64ed7f854cf4 ("net: add tunnel packet type parsing")
> 
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>

Looks good, but why reorder the assignment to *off ?

Could just be:

diff --git a/lib/net/rte_net.c b/lib/net/rte_net.c
index be24690fdf..cdad463398 100644
--- a/lib/net/rte_net.c
+++ b/lib/net/rte_net.c
@@ -252,6 +252,7 @@ ptype_tunnel_with_udp(uint16_t *proto, const struct rte_mbuf *m,
                        return 0;
                geneve_len = sizeof(*gnh) + gnh->opt_len * 4;
                *off = geneve_len;
+               hdr_lens->inner_l2_len = sizeof(struct rte_udp_hdr) + geneve_len;
                *proto = gnh->proto;
                if (gnh->proto == 0)
                        *proto = rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV4);

RE: [EXTERNAL] Re: [PATCH 1/2] net: fix offset calculation for GENEVE packet

[Sunil Kumar Kori]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Wednesday, May 21, 2025 2:52 AM
> To: Sunil Kumar Kori <skori@marvell.com>
> Cc: Jie Hai <haijie1@huawei.com>; dev@dpdk.org
> Subject: [EXTERNAL] Re: [PATCH 1/2] net: fix offset calculation for GENEVE
> packet
> 
> On Mon, 19 May 2025 21: 36: 55 +0530 <skori@ marvell. com> wrote: > From: Sunil
> Kumar Kori <skori@ marvell. com> > > While parsing packet headers, offset must
> be added to get next > header but for geneve header parsing offset
> 
> On Mon, 19 May 2025 21:36:55 +0530
> <skori@marvell.com> wrote:
> 
> > From: Sunil Kumar Kori <skori@marvell.com>
> >
> > While parsing packet headers, offset must be added to get next header
> > but for geneve header parsing offset is overwritten.
> > Also inner_l2_len is not set in case of geneve packets.
> >
> > Fixes: 64ed7f854cf4 ("net: add tunnel packet type parsing")
> >
> > Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> 
> Looks good, but why reorder the assignment to *off ?
> 
> Could just be:
> 
No specific reason. It can be kept as it is. I will fix in next version.

RE: [EXTERNAL] Re: [PATCH 2/2] app/testpmd: clear stale internal len information

[Sunil Kumar Kori]

> On Mon, 19 May 2025 21:36:56 +0530
> <skori@marvell.com> wrote:
> 
> > From: Sunil Kumar Kori <skori@marvell.com>
> >
> > hdr_lens is used to maintain header lengths after parsing packets.
> > When port receives different type of packets (say first is VXLAN
> > packet and second is GRE packet).
> >
> > For first packet, L2/L3/L4 lengths are set for inner and outer header
> > alongwith tunnel_len.
> >
> > Now for second packet, tunnel_len is added more than its size it
> > contains stale value which further leads to wrong header pointers.
> >
> > Hence clearing stale information before processing each packet.
> >
> > Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> 
> The initialization of hdr_lens then becomes redundant.
> 
> Maybe better to just move it into the loop.
> 
Yes, that will not make any difference. I will fix in next version.

[PATCH v2 1/2] net: fix offset calculation for GENEVE packet

[skori]

From: Sunil Kumar Kori <skori@marvell.com>

While parsing packet headers, offset must be added to get next
header but for geneve header parsing offset is overwritten.
Also inner_l2_len is not set in case of geneve packets.

Fixes: 64ed7f854cf4 ("net: add tunnel packet type parsing")

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
 lib/net/rte_net.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/net/rte_net.c b/lib/net/rte_net.c
index be24690fdf..1264f33d61 100644
--- a/lib/net/rte_net.c
+++ b/lib/net/rte_net.c
@@ -251,7 +251,8 @@ ptype_tunnel_with_udp(uint16_t *proto, const struct rte_mbuf *m,
 		if (unlikely(gnh == NULL))
 			return 0;
 		geneve_len = sizeof(*gnh) + gnh->opt_len * 4;
-		*off = geneve_len;
+		*off += geneve_len;
+		hdr_lens->inner_l2_len = sizeof(struct rte_udp_hdr) + geneve_len;
 		*proto = gnh->proto;
 		if (gnh->proto == 0)
 			*proto = rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV4);
-- 
2.43.0

[PATCH v2 2/2] app/testpmd: clear stale internal len information

[skori]

From: Sunil Kumar Kori <skori@marvell.com>

hdr_lens is used to maintain header lengths after parsing packets.
When port receives different type of packets (say first is VXLAN
packet and second is GRE packet).

For first packet, L2/L3/L4 lengths are set for inner and outer header
alongwith tunnel_len.

Now for second packet, tunnel_len is added more than its size it
contains stale value which further leads to wrong header pointers.

Hence clearing stale information before processing each packet.

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
 app/test-pmd/csumonly.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index fa0002d321..203af35cf0 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -627,7 +627,6 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 	uint32_t rx_bad_outer_l4_csum;
 	uint32_t rx_bad_outer_ip_csum;
 	struct testpmd_offload_info info;
-	struct rte_net_hdr_lens hdr_lens = {0};
 	uint32_t ptype;
 
 	/* receive a burst of packet */
@@ -666,6 +665,8 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 #endif
 
 	for (i = 0; i < nb_rx; i++) {
+		struct rte_net_hdr_lens hdr_lens = {0};
+
 		if (likely(i < nb_rx - 1))
 			rte_prefetch0(rte_pktmbuf_mtod(pkts_burst[i + 1],
 						       void *));
-- 
2.43.0

Re: [PATCH v2 1/2] net: fix offset calculation for GENEVE packet

[huangdengdui]

I have also found some bugs about the tunnel packet type parsing API[1], including one of the problems fixed by this patch.

[1] https://patchwork.dpdk.org/project/dpdk/list/?series=35233

On 2025/5/21 13:11, skori@marvell.com wrote:
> From: Sunil Kumar Kori <skori@marvell.com>
> 
> While parsing packet headers, offset must be added to get next
> header but for geneve header parsing offset is overwritten.
> Also inner_l2_len is not set in case of geneve packets.
> 
> Fixes: 64ed7f854cf4 ("net: add tunnel packet type parsing")
> 
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> ---
>  lib/net/rte_net.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/net/rte_net.c b/lib/net/rte_net.c
> index be24690fdf..1264f33d61 100644
> --- a/lib/net/rte_net.c
> +++ b/lib/net/rte_net.c
> @@ -251,7 +251,8 @@ ptype_tunnel_with_udp(uint16_t *proto, const struct rte_mbuf *m,
>  		if (unlikely(gnh == NULL))
>  			return 0;
>  		geneve_len = sizeof(*gnh) + gnh->opt_len * 4;
> -		*off = geneve_len;
> +		*off += geneve_len;
> +		hdr_lens->inner_l2_len = sizeof(struct rte_udp_hdr) + geneve_len;

The l2_len in mbuf is also calculated from the outer L4 header, so it is calculated in the same way here.
However, this is easy to be misunderstood, can we add a note to inner_l2_len as follows?

struct rte_net_hdr_lens {
	uint8_t l2_len;
	/* Outer_L4_len + ... + inner L2_len for tunneling pkt. */
	uint8_t inner_l2_len;
	uint16_t l3_len;
	uint16_t inner_l3_len;
	uint16_t tunnel_len;
	uint8_t l4_len;
	uint8_t inner_l4_len;
};


>  		*proto = gnh->proto;
>  		if (gnh->proto == 0)
>  			*proto = rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV4);