[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.