[dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments

[dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments

[Xiaoyun Li]

In csum forwarding mode, software UDP/TCP csum calculation only takes
the first segment into account while using the whole packet length so
the calculation will read invalid memory region with multi-segments
packets and will get wrong value.
This patch fixes this issue.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
 app/test-pmd/csumonly.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 090797318a..5df3be0a6f 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -18,7 +18,7 @@
 #include <rte_log.h>
 #include <rte_debug.h>
 #include <rte_cycles.h>
-#include <rte_memory.h>
+#include <rte_malloc.h>
 #include <rte_memcpy.h>
 #include <rte_launch.h>
 #include <rte_eal.h>
@@ -56,6 +56,11 @@
 #define GRE_SUPPORTED_FIELDS	(GRE_CHECKSUM_PRESENT | GRE_KEY_PRESENT |\
 				 GRE_SEQUENCE_PRESENT)
 
+/* When UDP or TCP or outer UDP csum offload is off, sw l4 csum is needed */
+#define UDP_TCP_CSUM            (DEV_TX_OFFLOAD_UDP_CKSUM |\
+				 DEV_TX_OFFLOAD_TCP_CKSUM |\
+				 DEV_TX_OFFLOAD_OUTER_UDP_CKSUM)
+
 /* We cannot use rte_cpu_to_be_16() on a constant in a switch/case */
 #if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
 #define _htons(x) ((uint16_t)((((x) & 0x00ffU) << 8) | (((x) & 0xff00U) >> 8)))
@@ -602,12 +607,8 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 	/* do not recalculate udp cksum if it was 0 */
 	if (udp_hdr->dgram_cksum != 0) {
 		udp_hdr->dgram_cksum = 0;
-		if (info->outer_ethertype == _htons(RTE_ETHER_TYPE_IPV4))
-			udp_hdr->dgram_cksum =
-				rte_ipv4_udptcp_cksum(ipv4_hdr, udp_hdr);
-		else
-			udp_hdr->dgram_cksum =
-				rte_ipv6_udptcp_cksum(ipv6_hdr, udp_hdr);
+		udp_hdr->dgram_cksum = get_udptcp_checksum(outer_l3_hdr,
+					udp_hdr, info->outer_ethertype);
 	}
 
 	return ol_flags;
@@ -802,6 +803,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 	struct rte_mbuf *m, *p;
 	struct rte_ether_hdr *eth_hdr;
 	void *l3_hdr = NULL, *outer_l3_hdr = NULL; /* can be IPv4 or IPv6 */
+	uint8_t *l3_buf = NULL;
 	void **gro_ctx;
 	uint16_t gro_pkts_num;
 	uint8_t gro_enable;
@@ -877,7 +879,19 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		rte_ether_addr_copy(&ports[fs->tx_port].eth_addr,
 				&eth_hdr->src_addr);
 		parse_ethernet(eth_hdr, &info);
-		l3_hdr = (char *)eth_hdr + info.l2_len;
+		/* When sw csum is needed, multi-segs needs a buf to contain
+		 * the whole packet for later UDP/TCP csum calculation.
+		 */
+		if (m->nb_segs > 1 && !(tx_ol_flags & PKT_TX_TCP_SEG) &&
+		    !(tx_offloads & UDP_TCP_CSUM)) {
+			l3_buf = rte_zmalloc("csum l3_buf",
+					     info.pkt_len - info.l2_len,
+					     RTE_CACHE_LINE_SIZE);
+			rte_pktmbuf_read(m, info.l2_len,
+					 info.pkt_len - info.l2_len, l3_buf);
+			l3_hdr = l3_buf;
+		} else
+			l3_hdr = (char *)eth_hdr + info.l2_len;
 
 		/* check if it's a supported tunnel */
 		if (txp->parse_tunnel) {
@@ -1051,6 +1065,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 			printf("tx: flags=%s", buf);
 			printf("\n");
 		}
+		rte_free(l3_buf);
 	}
 
 	if (unlikely(gro_enable)) {
-- 
2.25.1

Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments

[David Marchand]

Hello,

On Fri, Oct 15, 2021 at 7:27 AM Xiaoyun Li <xiaoyun.li@intel.com> wrote:
>
> In csum forwarding mode, software UDP/TCP csum calculation only takes
> the first segment into account while using the whole packet length so
> the calculation will read invalid memory region with multi-segments
> packets and will get wrong value.
> This patch fixes this issue.
>
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
>
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> ---
>  app/test-pmd/csumonly.c | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> index 090797318a..5df3be0a6f 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -18,7 +18,7 @@
>  #include <rte_log.h>
>  #include <rte_debug.h>
>  #include <rte_cycles.h>
> -#include <rte_memory.h>
> +#include <rte_malloc.h>

This include caught my eye.


>  #include <rte_memcpy.h>
>  #include <rte_launch.h>
>  #include <rte_eal.h>
> @@ -56,6 +56,11 @@
>  #define GRE_SUPPORTED_FIELDS   (GRE_CHECKSUM_PRESENT | GRE_KEY_PRESENT |\
>                                  GRE_SEQUENCE_PRESENT)
>
> +/* When UDP or TCP or outer UDP csum offload is off, sw l4 csum is needed */
> +#define UDP_TCP_CSUM            (DEV_TX_OFFLOAD_UDP_CKSUM |\
> +                                DEV_TX_OFFLOAD_TCP_CKSUM |\
> +                                DEV_TX_OFFLOAD_OUTER_UDP_CKSUM)
> +
>  /* We cannot use rte_cpu_to_be_16() on a constant in a switch/case */
>  #if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
>  #define _htons(x) ((uint16_t)((((x) & 0x00ffU) << 8) | (((x) & 0xff00U) >> 8)))
> @@ -602,12 +607,8 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
>         /* do not recalculate udp cksum if it was 0 */
>         if (udp_hdr->dgram_cksum != 0) {
>                 udp_hdr->dgram_cksum = 0;
> -               if (info->outer_ethertype == _htons(RTE_ETHER_TYPE_IPV4))
> -                       udp_hdr->dgram_cksum =
> -                               rte_ipv4_udptcp_cksum(ipv4_hdr, udp_hdr);
> -               else
> -                       udp_hdr->dgram_cksum =
> -                               rte_ipv6_udptcp_cksum(ipv6_hdr, udp_hdr);
> +               udp_hdr->dgram_cksum = get_udptcp_checksum(outer_l3_hdr,
> +                                       udp_hdr, info->outer_ethertype);
>         }
>
>         return ol_flags;
> @@ -802,6 +803,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>         struct rte_mbuf *m, *p;
>         struct rte_ether_hdr *eth_hdr;
>         void *l3_hdr = NULL, *outer_l3_hdr = NULL; /* can be IPv4 or IPv6 */
> +       uint8_t *l3_buf = NULL;
>         void **gro_ctx;
>         uint16_t gro_pkts_num;
>         uint8_t gro_enable;
> @@ -877,7 +879,19 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>                 rte_ether_addr_copy(&ports[fs->tx_port].eth_addr,
>                                 &eth_hdr->src_addr);
>                 parse_ethernet(eth_hdr, &info);
> -               l3_hdr = (char *)eth_hdr + info.l2_len;
> +               /* When sw csum is needed, multi-segs needs a buf to contain
> +                * the whole packet for later UDP/TCP csum calculation.
> +                */
> +               if (m->nb_segs > 1 && !(tx_ol_flags & PKT_TX_TCP_SEG) &&
> +                   !(tx_offloads & UDP_TCP_CSUM)) {
> +                       l3_buf = rte_zmalloc("csum l3_buf",
> +                                            info.pkt_len - info.l2_len,
> +                                            RTE_CACHE_LINE_SIZE);

Rather than call a dyn allocation in datapath, can't we have a static
buffer on the stack?


> +                       rte_pktmbuf_read(m, info.l2_len,
> +                                        info.pkt_len - info.l2_len, l3_buf);
> +                       l3_hdr = l3_buf;
> +               } else
> +                       l3_hdr = (char *)eth_hdr + info.l2_len;
>
>                 /* check if it's a supported tunnel */
>                 if (txp->parse_tunnel) {
> @@ -1051,6 +1065,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>                         printf("tx: flags=%s", buf);
>                         printf("\n");
>                 }
> +               rte_free(l3_buf);
>         }
>
>         if (unlikely(gro_enable)) {
> --
> 2.25.1
>


-- 
David Marchand

Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, October 15, 2021 16:10
> To: Li, Xiaoyun <xiaoyun.li@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev <dev@dpdk.org>; dpdk stable
> <stable@dpdk.org>
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi
> segments
> 
> Hello,
> 
> On Fri, Oct 15, 2021 at 7:27 AM Xiaoyun Li <xiaoyun.li@intel.com> wrote:
> >
> > In csum forwarding mode, software UDP/TCP csum calculation only takes
> > the first segment into account while using the whole packet length so
> > the calculation will read invalid memory region with multi-segments
> > packets and will get wrong value.
> > This patch fixes this issue.
> >
> > Fixes: af75078fece3 ("first public release")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> > ---
> >  app/test-pmd/csumonly.c | 31 +++++++++++++++++++++++--------
> >  1 file changed, 23 insertions(+), 8 deletions(-)
> >
> > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> > 090797318a..5df3be0a6f 100644
> > --- a/app/test-pmd/csumonly.c
> > +++ b/app/test-pmd/csumonly.c
> > @@ -18,7 +18,7 @@
> >  #include <rte_log.h>
> >  #include <rte_debug.h>
> >  #include <rte_cycles.h>
> > -#include <rte_memory.h>
> > +#include <rte_malloc.h>
> 
> This include caught my eye.
> 
> 
> >  #include <rte_memcpy.h>
> >  #include <rte_launch.h>
> >  #include <rte_eal.h>
> > @@ -56,6 +56,11 @@
> >  #define GRE_SUPPORTED_FIELDS   (GRE_CHECKSUM_PRESENT |
> GRE_KEY_PRESENT |\
> >                                  GRE_SEQUENCE_PRESENT)
> >
> > +/* When UDP or TCP or outer UDP csum offload is off, sw l4 csum is needed
> */
> > +#define UDP_TCP_CSUM            (DEV_TX_OFFLOAD_UDP_CKSUM |\
> > +                                DEV_TX_OFFLOAD_TCP_CKSUM |\
> > +                                DEV_TX_OFFLOAD_OUTER_UDP_CKSUM)
> > +
> >  /* We cannot use rte_cpu_to_be_16() on a constant in a switch/case */
> > #if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN  #define _htons(x)
> > ((uint16_t)((((x) & 0x00ffU) << 8) | (((x) & 0xff00U) >> 8))) @@
> > -602,12 +607,8 @@ process_outer_cksums(void *outer_l3_hdr, struct
> testpmd_offload_info *info,
> >         /* do not recalculate udp cksum if it was 0 */
> >         if (udp_hdr->dgram_cksum != 0) {
> >                 udp_hdr->dgram_cksum = 0;
> > -               if (info->outer_ethertype == _htons(RTE_ETHER_TYPE_IPV4))
> > -                       udp_hdr->dgram_cksum =
> > -                               rte_ipv4_udptcp_cksum(ipv4_hdr, udp_hdr);
> > -               else
> > -                       udp_hdr->dgram_cksum =
> > -                               rte_ipv6_udptcp_cksum(ipv6_hdr, udp_hdr);
> > +               udp_hdr->dgram_cksum = get_udptcp_checksum(outer_l3_hdr,
> > +                                       udp_hdr,
> > + info->outer_ethertype);
> >         }
> >
> >         return ol_flags;
> > @@ -802,6 +803,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> >         struct rte_mbuf *m, *p;
> >         struct rte_ether_hdr *eth_hdr;
> >         void *l3_hdr = NULL, *outer_l3_hdr = NULL; /* can be IPv4 or
> > IPv6 */
> > +       uint8_t *l3_buf = NULL;
> >         void **gro_ctx;
> >         uint16_t gro_pkts_num;
> >         uint8_t gro_enable;
> > @@ -877,7 +879,19 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> >                 rte_ether_addr_copy(&ports[fs->tx_port].eth_addr,
> >                                 &eth_hdr->src_addr);
> >                 parse_ethernet(eth_hdr, &info);
> > -               l3_hdr = (char *)eth_hdr + info.l2_len;
> > +               /* When sw csum is needed, multi-segs needs a buf to contain
> > +                * the whole packet for later UDP/TCP csum calculation.
> > +                */
> > +               if (m->nb_segs > 1 && !(tx_ol_flags & PKT_TX_TCP_SEG) &&
> > +                   !(tx_offloads & UDP_TCP_CSUM)) {
> > +                       l3_buf = rte_zmalloc("csum l3_buf",
> > +                                            info.pkt_len - info.l2_len,
> > +                                            RTE_CACHE_LINE_SIZE);
> 
> Rather than call a dyn allocation in datapath, can't we have a static buffer on
> the stack?

I wanted to do that. But the issue only happens when it's a large packet. Each hw has its own limitation on max packet size but it grows fast.
I'm not sure how large array should I use. 64K? Since total length in IP hdr is 16 bit.

BRs
Xiaoyun

> 
> 
> > +                       rte_pktmbuf_read(m, info.l2_len,
> > +                                        info.pkt_len - info.l2_len, l3_buf);
> > +                       l3_hdr = l3_buf;
> > +               } else
> > +                       l3_hdr = (char *)eth_hdr + info.l2_len;
> >
> >                 /* check if it's a supported tunnel */
> >                 if (txp->parse_tunnel) { @@ -1051,6 +1065,7 @@
> > pkt_burst_checksum_forward(struct fwd_stream *fs)
> >                         printf("tx: flags=%s", buf);
> >                         printf("\n");
> >                 }
> > +               rte_free(l3_buf);
> >         }
> >
> >         if (unlikely(gro_enable)) {
> > --
> > 2.25.1
> >
> 
> 
> --
> David Marchand

[dpdk-dev] [PATCH v2] app/testpmd: fix l4 sw csum over multi segments

[Xiaoyun Li]

In csum forwarding mode, software UDP/TCP csum calculation only takes
the first segment into account while using the whole packet length so
the calculation will read invalid memory region with multi-segments
packets and will get wrong value.
This patch fixes this issue.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
v2:
 * Use static stack memory instead of dynamic allocating in datapath.
---
 app/test-pmd/csumonly.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 090797318a..9f78ac74e1 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -56,6 +56,11 @@
 #define GRE_SUPPORTED_FIELDS	(GRE_CHECKSUM_PRESENT | GRE_KEY_PRESENT |\
 				 GRE_SEQUENCE_PRESENT)
 
+/* When UDP or TCP or outer UDP csum offload is off, sw l4 csum is needed */
+#define UDP_TCP_CSUM            (DEV_TX_OFFLOAD_UDP_CKSUM |\
+				 DEV_TX_OFFLOAD_TCP_CKSUM |\
+				 DEV_TX_OFFLOAD_OUTER_UDP_CKSUM)
+
 /* We cannot use rte_cpu_to_be_16() on a constant in a switch/case */
 #if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
 #define _htons(x) ((uint16_t)((((x) & 0x00ffU) << 8) | (((x) & 0xff00U) >> 8)))
@@ -602,12 +607,8 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 	/* do not recalculate udp cksum if it was 0 */
 	if (udp_hdr->dgram_cksum != 0) {
 		udp_hdr->dgram_cksum = 0;
-		if (info->outer_ethertype == _htons(RTE_ETHER_TYPE_IPV4))
-			udp_hdr->dgram_cksum =
-				rte_ipv4_udptcp_cksum(ipv4_hdr, udp_hdr);
-		else
-			udp_hdr->dgram_cksum =
-				rte_ipv6_udptcp_cksum(ipv6_hdr, udp_hdr);
+		udp_hdr->dgram_cksum = get_udptcp_checksum(outer_l3_hdr,
+					udp_hdr, info->outer_ethertype);
 	}
 
 	return ol_flags;
@@ -877,7 +878,17 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		rte_ether_addr_copy(&ports[fs->tx_port].eth_addr,
 				&eth_hdr->src_addr);
 		parse_ethernet(eth_hdr, &info);
-		l3_hdr = (char *)eth_hdr + info.l2_len;
+		/* When sw csum is needed, multi-segs needs a buf to contain
+		 * the whole packet for later UDP/TCP csum calculation.
+		 */
+		if (m->nb_segs > 1 && !(tx_ol_flags & PKT_TX_TCP_SEG) &&
+		    !(tx_offloads & UDP_TCP_CSUM)) {
+			char l3_buf[RTE_IPV4_MAX_PKT_LEN + 1];
+			rte_pktmbuf_read(m, info.l2_len,
+					 info.pkt_len - info.l2_len, l3_buf);
+			l3_hdr = l3_buf;
+		} else
+			l3_hdr = (char *)eth_hdr + info.l2_len;
 
 		/* check if it's a supported tunnel */
 		if (txp->parse_tunnel) {
-- 
2.25.1

Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments

[Stephen Hemminger]

On Fri, 15 Oct 2021 13:13:06 +0800
Xiaoyun Li <xiaoyun.li@intel.com> wrote:

> +		/* When sw csum is needed, multi-segs needs a buf to contain
> +		 * the whole packet for later UDP/TCP csum calculation.
> +		 */
> +		if (m->nb_segs > 1 && !(tx_ol_flags & PKT_TX_TCP_SEG) &&
> +		    !(tx_offloads & UDP_TCP_CSUM)) {
> +			l3_buf = rte_zmalloc("csum l3_buf",
> +					     info.pkt_len - info.l2_len,
> +					     RTE_CACHE_LINE_SIZE);
> +			rte_pktmbuf_read(m, info.l2_len,
> +					 info.pkt_len - info.l2_len, l3_buf);
> +			l3_hdr = l3_buf;
> +		} else
> +			l3_hdr = (char *)eth_hdr + info.l2_len;
>  

Rather than copying whole packet, make the code handle checksum streaming.

Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Monday, October 18, 2021 11:00
> To: Li, Xiaoyun <xiaoyun.li@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi
> segments
> 
> On Fri, 15 Oct 2021 13:13:06 +0800
> Xiaoyun Li <xiaoyun.li@intel.com> wrote:
> 
> > +		/* When sw csum is needed, multi-segs needs a buf to contain
> > +		 * the whole packet for later UDP/TCP csum calculation.
> > +		 */
> > +		if (m->nb_segs > 1 && !(tx_ol_flags & PKT_TX_TCP_SEG) &&
> > +		    !(tx_offloads & UDP_TCP_CSUM)) {
> > +			l3_buf = rte_zmalloc("csum l3_buf",
> > +					     info.pkt_len - info.l2_len,
> > +					     RTE_CACHE_LINE_SIZE);
> > +			rte_pktmbuf_read(m, info.l2_len,
> > +					 info.pkt_len - info.l2_len, l3_buf);
> > +			l3_hdr = l3_buf;
> > +		} else
> > +			l3_hdr = (char *)eth_hdr + info.l2_len;
> >
> 
> Rather than copying whole packet, make the code handle checksum streaming.

Copying is the easiest way to do this.

The problem of handling checksum streaming is that in the first segment, l2 and l3 hdr len is 14 bytes when checksum takes 4 bytes each time.
If the datalen of the first segment is 4 bytes aligned (usual case), for the second segment and the following segments, they may need to add a special 2 bytes 0x0 at the start.
Also, mbuf is not passed down to process_inner/outer_chksum so the change will be a lot.

Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments

[Li, Xiaoyun]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Li, Xiaoyun
> Sent: Monday, October 18, 2021 11:17
> To: Stephen Hemminger <stephen@networkplumber.org>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi
> segments
> 
> Hi
> 
> > -----Original Message-----
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > Sent: Monday, October 18, 2021 11:00
> > To: Li, Xiaoyun <xiaoyun.li@intel.com>
> > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org;
> > stable@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi
> > segments
> >
> > On Fri, 15 Oct 2021 13:13:06 +0800
> > Xiaoyun Li <xiaoyun.li@intel.com> wrote:
> >
> > > +		/* When sw csum is needed, multi-segs needs a buf to contain
> > > +		 * the whole packet for later UDP/TCP csum calculation.
> > > +		 */
> > > +		if (m->nb_segs > 1 && !(tx_ol_flags & PKT_TX_TCP_SEG) &&
> > > +		    !(tx_offloads & UDP_TCP_CSUM)) {
> > > +			l3_buf = rte_zmalloc("csum l3_buf",
> > > +					     info.pkt_len - info.l2_len,
> > > +					     RTE_CACHE_LINE_SIZE);
> > > +			rte_pktmbuf_read(m, info.l2_len,
> > > +					 info.pkt_len - info.l2_len, l3_buf);
> > > +			l3_hdr = l3_buf;
> > > +		} else
> > > +			l3_hdr = (char *)eth_hdr + info.l2_len;
> > >
> >
> > Rather than copying whole packet, make the code handle checksum streaming.
> 
> Copying is the easiest way to do this.
> 
> The problem of handling checksum streaming is that in the first segment, l2 and
> l3 hdr len is 14 bytes when checksum takes 4 bytes each time.
> If the datalen of the first segment is 4 bytes aligned (usual case), for the second
> segment and the following segments, they may need to add a special 2 bytes
> 0x0 at the start.
> Also, mbuf is not passed down to process_inner/outer_chksum so the change
> will be a lot.

Also, rte_ipv4/6_udptcp_cksum can't be directly called. Because it only takes ip_hdr and whole packet buffer as input.

Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments

[Ananyev, Konstantin]

> > > +		/* When sw csum is needed, multi-segs needs a buf to contain
> > > +		 * the whole packet for later UDP/TCP csum calculation.
> > > +		 */
> > > +		if (m->nb_segs > 1 && !(tx_ol_flags & PKT_TX_TCP_SEG) &&
> > > +		    !(tx_offloads & UDP_TCP_CSUM)) {
> > > +			l3_buf = rte_zmalloc("csum l3_buf",
> > > +					     info.pkt_len - info.l2_len,
> > > +					     RTE_CACHE_LINE_SIZE);
> > > +			rte_pktmbuf_read(m, info.l2_len,
> > > +					 info.pkt_len - info.l2_len, l3_buf);
> > > +			l3_hdr = l3_buf;
> > > +		} else
> > > +			l3_hdr = (char *)eth_hdr + info.l2_len;
> > >
> >
> > Rather than copying whole packet, make the code handle checksum streaming.
> 
> Copying is the easiest way to do this.
> 
> The problem of handling checksum streaming is that in the first segment, l2 and l3 hdr len is 14 bytes when checksum takes 4 bytes each
> time.
> If the datalen of the first segment is 4 bytes aligned (usual case), for the second segment and the following segments, they may need to add
> a special 2 bytes 0x0 at the start.

Didn't understand that one...
Why you suddenly need to pad non-first segments with zeroes?
Why simply rte_raw_cksum() can't be used for multi-seg case?

> Also, mbuf is not passed down to process_inner/outer_chksum so the change will be a lot.

I also think that copying whole packet just to calculate a checksum - way too much overhead. 

Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments

[Li, Xiaoyun]

> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Monday, October 18, 2021 18:16
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi
> segments
> 
> 
> > > > +		/* When sw csum is needed, multi-segs needs a buf to contain
> > > > +		 * the whole packet for later UDP/TCP csum calculation.
> > > > +		 */
> > > > +		if (m->nb_segs > 1 && !(tx_ol_flags & PKT_TX_TCP_SEG) &&
> > > > +		    !(tx_offloads & UDP_TCP_CSUM)) {
> > > > +			l3_buf = rte_zmalloc("csum l3_buf",
> > > > +					     info.pkt_len - info.l2_len,
> > > > +					     RTE_CACHE_LINE_SIZE);
> > > > +			rte_pktmbuf_read(m, info.l2_len,
> > > > +					 info.pkt_len - info.l2_len, l3_buf);
> > > > +			l3_hdr = l3_buf;
> > > > +		} else
> > > > +			l3_hdr = (char *)eth_hdr + info.l2_len;
> > > >
> > >
> > > Rather than copying whole packet, make the code handle checksum
> streaming.
> >
> > Copying is the easiest way to do this.
> >
> > The problem of handling checksum streaming is that in the first
> > segment, l2 and l3 hdr len is 14 bytes when checksum takes 4 bytes each time.
> > If the datalen of the first segment is 4 bytes aligned (usual case),
> > for the second segment and the following segments, they may need to add a
> special 2 bytes 0x0 at the start.
> 
> Didn't understand that one...
> Why you suddenly need to pad non-first segments with zeroes?
> Why simply rte_raw_cksum() can't be used for multi-seg case?

Normal udp/tcp packets:
The first segment: eth hdr + ip hdr + udp/tcp packet (The total length of this is mbuf data len so like 2048, 4 bytes aligned)
The second segment: continue udp/tcp packet

Now, udp/tcp checksum is calculated. It will take the whole udp/tcp packet. 4 bytes + 4 bytes + 4 bytes...
Then
1st segment: udp/tcp packet (size = 2048 - 14 = 2034, not 4 bytes aligned, 2 bytes left, if use rte_raw_cksum(), the last 2 bytes will be combined with 2 bytes zeros)
2nd segment: continue udp/tcp packet (size = data_len)

For 2nd segment, if don't add 2 bytes zeros first, the checksum value will be wrong.
Because it should be for example 0x1234 (0x12 is left in 1st, 0x34 is in 2nd), 0x1200+0x0034 is correct but 0x1200+0x3400 is not correct.

That's why I think all of the following segments needs zero padding first.

And above is only the usual case of normal tcp/udp packets. The issue also exists for tunnel packets which will calculate outer udp and inner udp/tcp checksum.

> 
> > Also, mbuf is not passed down to process_inner/outer_chksum so the change
> will be a lot.
> 
> I also think that copying whole packet just to calculate a checksum - way too
> much overhead.

Yes. I agree. But it only happens when users don't enable checksum offload, don't enable TSO and the packet crosses multi-segments.

[dpdk-dev] [PATCH v3] app/testpmd: fix l4 sw csum over multi segments

[Xiaoyun Li]

In csum forwarding mode, software UDP/TCP csum calculation only takes
the first segment into account while using the whole packet length so
the calculation will read invalid memory region with multi-segments
packets and will get wrong value.
This patch fixes this issue.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
v3:
 * Use rte_raw_cksum() for multi-segs case instead of copying the whole
 * packet.
v2:
 * Use static stack memory instead of dynamic allocating in datapath
---
 app/test-pmd/csumonly.c | 68 ++++++++++++++++++++++++++++++++---------
 1 file changed, 53 insertions(+), 15 deletions(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 090797318a..f3e60eb3c3 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -91,12 +91,41 @@ struct simple_gre_hdr {
 } __rte_packed;
 
 static uint16_t
-get_udptcp_checksum(void *l3_hdr, void *l4_hdr, uint16_t ethertype)
+get_udptcp_checksum(void *l3_hdr, struct rte_mbuf *m, uint16_t l4_off,
+		    uint16_t ethertype)
 {
+	uint16_t off = l4_off;
+	uint32_t cksum = 0;
+	char *buf;
+
+	while (m != NULL) {
+		buf = rte_pktmbuf_mtod_offset(m, char *, off);
+		cksum += rte_raw_cksum(buf, m->data_len - off);
+		off = 0;
+		m = m->next;
+	}
 	if (ethertype == _htons(RTE_ETHER_TYPE_IPV4))
-		return rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
+		cksum += rte_ipv4_phdr_cksum(l3_hdr, 0);
 	else /* assume ethertype == RTE_ETHER_TYPE_IPV6 */
-		return rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
+		cksum += rte_ipv6_phdr_cksum(l3_hdr, 0);
+
+	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
+	cksum = (~cksum) & 0xffff;
+
+	/*
+	 * Per RFC 768:If the computed checksum is zero for UDP,
+	 * it is transmitted as all ones
+	 * (the equivalent in one's complement arithmetic).
+	 */
+	if (cksum == 0 && ethertype == _htons(RTE_ETHER_TYPE_IPV4) &&
+	    ((struct rte_ipv4_hdr *)l3_hdr)->next_proto_id == IPPROTO_UDP)
+		cksum = 0xffff;
+
+	if (cksum == 0 && ethertype == _htons(RTE_ETHER_TYPE_IPV6) &&
+	    ((struct rte_ipv6_hdr *)l3_hdr)->proto == IPPROTO_UDP)
+		cksum = 0xffff;
+
+	return (uint16_t)cksum;
 }
 
 /* Parse an IPv4 header to fill l3_len, l4_len, and l4_proto */
@@ -455,7 +484,7 @@ parse_encap_ip(void *encap_ip, struct testpmd_offload_info *info)
  * depending on the testpmd command line configuration */
 static uint64_t
 process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
-	uint64_t tx_offloads)
+	uint64_t tx_offloads, struct rte_mbuf *m)
 {
 	struct rte_ipv4_hdr *ipv4_hdr = l3_hdr;
 	struct rte_udp_hdr *udp_hdr;
@@ -463,6 +492,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 	struct rte_sctp_hdr *sctp_hdr;
 	uint64_t ol_flags = 0;
 	uint32_t max_pkt_len, tso_segsz = 0;
+	uint16_t l4_off;
 
 	/* ensure packet is large enough to require tso */
 	if (!info->is_tunnel) {
@@ -505,9 +535,15 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 			if (tx_offloads & DEV_TX_OFFLOAD_UDP_CKSUM) {
 				ol_flags |= PKT_TX_UDP_CKSUM;
 			} else {
+				if (info->is_tunnel)
+					l4_off = info->l2_len +
+						info->outer_l3_len +
+						info->l2_len + info->l3_len;
+				else
+					l4_off = info->l2_len +	info->l3_len;
 				udp_hdr->dgram_cksum = 0;
 				udp_hdr->dgram_cksum =
-					get_udptcp_checksum(l3_hdr, udp_hdr,
+					get_udptcp_checksum(l3_hdr, m, l4_off,
 						info->ethertype);
 			}
 		}
@@ -520,9 +556,14 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 		else if (tx_offloads & DEV_TX_OFFLOAD_TCP_CKSUM) {
 			ol_flags |= PKT_TX_TCP_CKSUM;
 		} else {
+			if (info->is_tunnel)
+				l4_off = info->l2_len + info->outer_l3_len +
+					info->l2_len + info->l3_len;
+			else
+				l4_off = info->l2_len + info->l3_len;
 			tcp_hdr->cksum = 0;
 			tcp_hdr->cksum =
-				get_udptcp_checksum(l3_hdr, tcp_hdr,
+				get_udptcp_checksum(l3_hdr, m, l4_off,
 					info->ethertype);
 		}
 		if (info->gso_enable)
@@ -548,7 +589,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 /* Calculate the checksum of outer header */
 static uint64_t
 process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
-	uint64_t tx_offloads, int tso_enabled)
+	uint64_t tx_offloads, int tso_enabled, struct rte_mbuf *m)
 {
 	struct rte_ipv4_hdr *ipv4_hdr = outer_l3_hdr;
 	struct rte_ipv6_hdr *ipv6_hdr = outer_l3_hdr;
@@ -602,12 +643,9 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 	/* do not recalculate udp cksum if it was 0 */
 	if (udp_hdr->dgram_cksum != 0) {
 		udp_hdr->dgram_cksum = 0;
-		if (info->outer_ethertype == _htons(RTE_ETHER_TYPE_IPV4))
-			udp_hdr->dgram_cksum =
-				rte_ipv4_udptcp_cksum(ipv4_hdr, udp_hdr);
-		else
-			udp_hdr->dgram_cksum =
-				rte_ipv6_udptcp_cksum(ipv6_hdr, udp_hdr);
+		udp_hdr->dgram_cksum = get_udptcp_checksum(outer_l3_hdr,
+					m, info->l2_len + info->outer_l3_len,
+					info->outer_ethertype);
 	}
 
 	return ol_flags;
@@ -942,7 +980,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 
 		/* process checksums of inner headers first */
 		tx_ol_flags |= process_inner_cksums(l3_hdr, &info,
-			tx_offloads);
+			tx_offloads, m);
 
 		/* Then process outer headers if any. Note that the software
 		 * checksum will be wrong if one of the inner checksums is
@@ -950,7 +988,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		if (info.is_tunnel == 1) {
 			tx_ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
 					tx_offloads,
-					!!(tx_ol_flags & PKT_TX_TCP_SEG));
+					!!(tx_ol_flags & PKT_TX_TCP_SEG), m);
 		}
 
 		/* step 3: fill the mbuf meta data (flags and header lengths) */
-- 
2.25.1

Re: [dpdk-dev] [PATCH v3] app/testpmd: fix l4 sw csum over multi segments

[Ferruh Yigit]

On 10/20/2021 11:12 AM, Xiaoyun Li wrote:
> In csum forwarding mode, software UDP/TCP csum calculation only takes
> the first segment into account while using the whole packet length so
> the calculation will read invalid memory region with multi-segments
> packets and will get wrong value.
> This patch fixes this issue.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> ---
> v3:
>   * Use rte_raw_cksum() for multi-segs case instead of copying the whole
>   * packet.
> v2:
>   * Use static stack memory instead of dynamic allocating in datapath
> ---
>   app/test-pmd/csumonly.c | 68 ++++++++++++++++++++++++++++++++---------
>   1 file changed, 53 insertions(+), 15 deletions(-)
> 
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> index 090797318a..f3e60eb3c3 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -91,12 +91,41 @@ struct simple_gre_hdr {
>   } __rte_packed;
>   
>   static uint16_t
> -get_udptcp_checksum(void *l3_hdr, void *l4_hdr, uint16_t ethertype)
> +get_udptcp_checksum(void *l3_hdr, struct rte_mbuf *m, uint16_t l4_off,
> +		    uint16_t ethertype)
>   {
> +	uint16_t off = l4_off;
> +	uint32_t cksum = 0;
> +	char *buf;
> +
> +	while (m != NULL) {
> +		buf = rte_pktmbuf_mtod_offset(m, char *, off);
> +		cksum += rte_raw_cksum(buf, m->data_len - off);
> +		off = 0;
> +		m = m->next;
> +	}
>   	if (ethertype == _htons(RTE_ETHER_TYPE_IPV4))
> -		return rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
> +		cksum += rte_ipv4_phdr_cksum(l3_hdr, 0);
>   	else /* assume ethertype == RTE_ETHER_TYPE_IPV6 */
> -		return rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
> +		cksum += rte_ipv6_phdr_cksum(l3_hdr, 0);
> +

Hi Xiaoyun,

I can see 'rte_ipv[46]_udptcp_cksum()' is not taking multi segment mbuf
into account, so this fix is required,
but instead of implementing this logic into testpmd, what do you think
to have APIs to support multi segment mbufs?
This way other applications also benefit from it and we don't need to
maintain ip4/6 checksum related code in testpmd.

btw, how are you testing this?

Re: [dpdk-dev] [PATCH v3] app/testpmd: fix l4 sw csum over multi segments

[Morten Brørup]

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> Sent: Wednesday, 27 October 2021 12.49
> 
> On 10/20/2021 11:12 AM, Xiaoyun Li wrote:
> > In csum forwarding mode, software UDP/TCP csum calculation only takes
> > the first segment into account while using the whole packet length so
> > the calculation will read invalid memory region with multi-segments
> > packets and will get wrong value.
> > This patch fixes this issue.
> >
> > Fixes: af75078fece3 ("first public release")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> > ---
> > v3:
> >   * Use rte_raw_cksum() for multi-segs case instead of copying the
> whole
> >   * packet.
> > v2:
> >   * Use static stack memory instead of dynamic allocating in datapath
> > ---
> >   app/test-pmd/csumonly.c | 68 ++++++++++++++++++++++++++++++++------
> ---
> >   1 file changed, 53 insertions(+), 15 deletions(-)
> >
> > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> > index 090797318a..f3e60eb3c3 100644
> > --- a/app/test-pmd/csumonly.c
> > +++ b/app/test-pmd/csumonly.c
> > @@ -91,12 +91,41 @@ struct simple_gre_hdr {
> >   } __rte_packed;
> >
> >   static uint16_t
> > -get_udptcp_checksum(void *l3_hdr, void *l4_hdr, uint16_t ethertype)
> > +get_udptcp_checksum(void *l3_hdr, struct rte_mbuf *m, uint16_t
> l4_off,
> > +		    uint16_t ethertype)
> >   {
> > +	uint16_t off = l4_off;
> > +	uint32_t cksum = 0;
> > +	char *buf;
> > +
> > +	while (m != NULL) {
> > +		buf = rte_pktmbuf_mtod_offset(m, char *, off);
> > +		cksum += rte_raw_cksum(buf, m->data_len - off);
> > +		off = 0;
> > +		m = m->next;
> > +	}
> >   	if (ethertype == _htons(RTE_ETHER_TYPE_IPV4))
> > -		return rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
> > +		cksum += rte_ipv4_phdr_cksum(l3_hdr, 0);
> >   	else /* assume ethertype == RTE_ETHER_TYPE_IPV6 */
> > -		return rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
> > +		cksum += rte_ipv6_phdr_cksum(l3_hdr, 0);
> > +
> 
> Hi Xiaoyun,
> 
> I can see 'rte_ipv[46]_udptcp_cksum()' is not taking multi segment mbuf
> into account, so this fix is required,
> but instead of implementing this logic into testpmd, what do you think
> to have APIs to support multi segment mbufs?
> This way other applications also benefit from it and we don't need to
> maintain ip4/6 checksum related code in testpmd.

+1

Also, there is no need to implement the multi-segment raw checksum loop in test-pmd.

You can use the multi-segment raw checksum function in the net library instead:
http://code.dpdk.org/dpdk/latest/source/lib/net/rte_ip.h#L224

> 
> btw, how are you testing this?

Re: [dpdk-dev] [PATCH v3] app/testpmd: fix l4 sw csum over multi segments

[Olivier Matz]

On Wed, Oct 27, 2021 at 01:29:52PM +0200, Morten Brørup wrote:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> > Sent: Wednesday, 27 October 2021 12.49
> > 
> > On 10/20/2021 11:12 AM, Xiaoyun Li wrote:
> > > In csum forwarding mode, software UDP/TCP csum calculation only takes
> > > the first segment into account while using the whole packet length so
> > > the calculation will read invalid memory region with multi-segments
> > > packets and will get wrong value.
> > > This patch fixes this issue.
> > >
> > > Fixes: af75078fece3 ("first public release")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> > > ---
> > > v3:
> > >   * Use rte_raw_cksum() for multi-segs case instead of copying the
> > whole
> > >   * packet.
> > > v2:
> > >   * Use static stack memory instead of dynamic allocating in datapath
> > > ---
> > >   app/test-pmd/csumonly.c | 68 ++++++++++++++++++++++++++++++++------
> > ---
> > >   1 file changed, 53 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> > > index 090797318a..f3e60eb3c3 100644
> > > --- a/app/test-pmd/csumonly.c
> > > +++ b/app/test-pmd/csumonly.c
> > > @@ -91,12 +91,41 @@ struct simple_gre_hdr {
> > >   } __rte_packed;
> > >
> > >   static uint16_t
> > > -get_udptcp_checksum(void *l3_hdr, void *l4_hdr, uint16_t ethertype)
> > > +get_udptcp_checksum(void *l3_hdr, struct rte_mbuf *m, uint16_t
> > l4_off,
> > > +		    uint16_t ethertype)
> > >   {
> > > +	uint16_t off = l4_off;
> > > +	uint32_t cksum = 0;
> > > +	char *buf;
> > > +
> > > +	while (m != NULL) {
> > > +		buf = rte_pktmbuf_mtod_offset(m, char *, off);
> > > +		cksum += rte_raw_cksum(buf, m->data_len - off);
> > > +		off = 0;
> > > +		m = m->next;
> > > +	}
> > >   	if (ethertype == _htons(RTE_ETHER_TYPE_IPV4))
> > > -		return rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
> > > +		cksum += rte_ipv4_phdr_cksum(l3_hdr, 0);
> > >   	else /* assume ethertype == RTE_ETHER_TYPE_IPV6 */
> > > -		return rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
> > > +		cksum += rte_ipv6_phdr_cksum(l3_hdr, 0);
> > > +
> > 
> > Hi Xiaoyun,
> > 
> > I can see 'rte_ipv[46]_udptcp_cksum()' is not taking multi segment mbuf
> > into account, so this fix is required,
> > but instead of implementing this logic into testpmd, what do you think
> > to have APIs to support multi segment mbufs?
> > This way other applications also benefit from it and we don't need to
> > maintain ip4/6 checksum related code in testpmd.
> 
> +1
> 
> Also, there is no need to implement the multi-segment raw checksum loop in test-pmd.
> 
> You can use the multi-segment raw checksum function in the net library instead:
> http://code.dpdk.org/dpdk/latest/source/lib/net/rte_ip.h#L224

+1

We can have mbuf variants of udptcp checksum functions:

rte_ipv4_udptcp_cksum()
rte_ipv4_udptcp_cksum_verify()
rte_ipv6_udptcp_cksum()
rte_ipv6_udptcp_cksum_verify()

Adding a "_mbuf" suffix would be consistent with rte_raw_cksum_mbuf().


Olivier

RE: [dpdk-dev] [PATCH v3] app/testpmd: fix l4 sw csum over multi segments

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Friday, October 29, 2021 09:29
> To: Morten Brørup <mb@smartsharesystems.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> stephen@networkplumber.org; dev@dpdk.org; stable@dpdk.org;
> Medvedkin, Vladimir <vladimir.medvedkin@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3] app/testpmd: fix l4 sw csum over multi
> segments
> 
> On Wed, Oct 27, 2021 at 01:29:52PM +0200, Morten Brørup wrote:
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> > > Sent: Wednesday, 27 October 2021 12.49
> > >
> > > On 10/20/2021 11:12 AM, Xiaoyun Li wrote:
> > > > In csum forwarding mode, software UDP/TCP csum calculation only
> > > > takes the first segment into account while using the whole packet
> > > > length so the calculation will read invalid memory region with
> > > > multi-segments packets and will get wrong value.
> > > > This patch fixes this issue.
> > > >
> > > > Fixes: af75078fece3 ("first public release")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> > > > ---
> > > > v3:
> > > >   * Use rte_raw_cksum() for multi-segs case instead of copying the
> > > whole
> > > >   * packet.
> > > > v2:
> > > >   * Use static stack memory instead of dynamic allocating in
> > > > datapath
> > > > ---
> > > >   app/test-pmd/csumonly.c | 68
> > > > ++++++++++++++++++++++++++++++++------
> > > ---
> > > >   1 file changed, 53 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> > > > index 090797318a..f3e60eb3c3 100644
> > > > --- a/app/test-pmd/csumonly.c
> > > > +++ b/app/test-pmd/csumonly.c
> > > > @@ -91,12 +91,41 @@ struct simple_gre_hdr {
> > > >   } __rte_packed;
> > > >
> > > >   static uint16_t
> > > > -get_udptcp_checksum(void *l3_hdr, void *l4_hdr, uint16_t
> > > > ethertype)
> > > > +get_udptcp_checksum(void *l3_hdr, struct rte_mbuf *m, uint16_t
> > > l4_off,
> > > > +		    uint16_t ethertype)
> > > >   {
> > > > +	uint16_t off = l4_off;
> > > > +	uint32_t cksum = 0;
> > > > +	char *buf;
> > > > +
> > > > +	while (m != NULL) {
> > > > +		buf = rte_pktmbuf_mtod_offset(m, char *, off);
> > > > +		cksum += rte_raw_cksum(buf, m->data_len - off);
> > > > +		off = 0;
> > > > +		m = m->next;
> > > > +	}
> > > >   	if (ethertype == _htons(RTE_ETHER_TYPE_IPV4))
> > > > -		return rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
> > > > +		cksum += rte_ipv4_phdr_cksum(l3_hdr, 0);
> > > >   	else /* assume ethertype == RTE_ETHER_TYPE_IPV6 */
> > > > -		return rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
> > > > +		cksum += rte_ipv6_phdr_cksum(l3_hdr, 0);
> > > > +
> > >
> > > Hi Xiaoyun,
> > >
> > > I can see 'rte_ipv[46]_udptcp_cksum()' is not taking multi segment
> > > mbuf into account, so this fix is required, but instead of
> > > implementing this logic into testpmd, what do you think to have APIs
> > > to support multi segment mbufs?
> > > This way other applications also benefit from it and we don't need
> > > to maintain ip4/6 checksum related code in testpmd.
> >
> > +1
> >
> > Also, there is no need to implement the multi-segment raw checksum loop
> in test-pmd.
> >
> > You can use the multi-segment raw checksum function in the net library
> instead:
> > http://code.dpdk.org/dpdk/latest/source/lib/net/rte_ip.h#L224
> 
> +1
> 
> We can have mbuf variants of udptcp checksum functions:
> 
> rte_ipv4_udptcp_cksum()
> rte_ipv4_udptcp_cksum_verify()
> rte_ipv6_udptcp_cksum()
> rte_ipv6_udptcp_cksum_verify()
> 
> Adding a "_mbuf" suffix would be consistent with rte_raw_cksum_mbuf().

Thanks for the suggestion. Since it's API change, I'll send these in 22.03 release with release note.
V4 will come soon.

> 
> 
> Olivier
--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

[PATCH v4 0/2] Add functions to calculate UDP/TCP cksum in mbuf

[Xiaoyun Li]

Added functions to calculate UDP/TCP checksum for packets which may be
over multi-segments and fix the checksum issue with testpmd csum
forwarding mode.

Xiaoyun Li (2):
  net: add functions to calculate UDP/TCP cksum in mbuf
  testpmd: fix l4 sw csum over multi segments

---
v4:
 * Called rte_raw_cksum_mbuf() to calculate cksum in lib instead of
 * implementing it in testpmd for better maintenance.
 * Removed fix tag for testpmd since it relies on the lib change.
v3:
 * Use rte_raw_cksum() for multi-segs case instead of copying the whole
 * packet.
v2:
 * Use static stack memory instead of dynamic allocating in datapath
---

 app/test-pmd/csumonly.c                |  41 ++++--
 doc/guides/rel_notes/release_22_03.rst |  10 ++
 lib/net/rte_ip.h                       | 186 +++++++++++++++++++++++++
 lib/net/version.map                    |  10 ++
 4 files changed, 232 insertions(+), 15 deletions(-)

-- 
2.25.1

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

[PATCH v4 1/2] net: add functions to calculate UDP/TCP cksum in mbuf

[Xiaoyun Li]

Add functions to call rte_raw_cksum_mbuf() to calculate IPv4/6
UDP/TCP checksum in mbuf which can be over multi-segments.

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
 doc/guides/rel_notes/release_22_03.rst |  10 ++
 lib/net/rte_ip.h                       | 186 +++++++++++++++++++++++++
 lib/net/version.map                    |  10 ++
 3 files changed, 206 insertions(+)

diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
index 6d99d1eaa9..7a082c4427 100644
--- a/doc/guides/rel_notes/release_22_03.rst
+++ b/doc/guides/rel_notes/release_22_03.rst
@@ -55,6 +55,13 @@ New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Added functions to calculate UDP/TCP checksum in mbuf.**
+  * Added the following functions to calculate UDP/TCP checksum of packets
+    which can be over multi-segments:
+    - ``rte_ipv4_udptcp_cksum_mbuf()``
+    - ``rte_ipv4_udptcp_cksum_mbuf_verify()``
+    - ``rte_ipv6_udptcp_cksum_mbuf()``
+    - ``rte_ipv6_udptcp_cksum_mbuf_verify()``
 
 Removed Items
 -------------
@@ -84,6 +91,9 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* net: added experimental functions ``rte_ipv4_udptcp_cksum_mbuf()``,
+  ``rte_ipv4_udptcp_cksum_mbuf_verify()``, ``rte_ipv6_udptcp_cksum_mbuf()``,
+  ``rte_ipv6_udptcp_cksum_mbuf_verify()``
 
 ABI Changes
 -----------
diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h
index c575250852..534f401d26 100644
--- a/lib/net/rte_ip.h
+++ b/lib/net/rte_ip.h
@@ -400,6 +400,65 @@ rte_ipv4_udptcp_cksum(const struct rte_ipv4_hdr *ipv4_hdr, const void *l4_hdr)
 	return cksum;
 }
 
+/**
+ * @internal Calculate the non-complemented IPv4 L4 checksum of a packet
+ */
+static inline uint16_t
+__rte_ipv4_udptcp_cksum_mbuf(const struct rte_mbuf *m,
+			     const struct rte_ipv4_hdr *ipv4_hdr,
+			     uint16_t l4_off)
+{
+	uint16_t raw_cksum;
+	uint32_t cksum;
+
+	if (l4_off > m->pkt_len)
+		return 0;
+
+	if (rte_raw_cksum_mbuf(m, l4_off, m->pkt_len - l4_off, &raw_cksum))
+		return 0;
+
+	cksum = raw_cksum + rte_ipv4_phdr_cksum(ipv4_hdr, 0);
+
+	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
+
+	return (uint16_t)cksum;
+}
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Compute the IPv4 UDP/TCP checksum of a packet.
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param ipv4_hdr
+ *   The pointer to the contiguous IPv4 header.
+ * @param l4_off
+ *   The offset in bytes to start L4 checksum.
+ * @return
+ *   The complemented checksum to set in the L4 header.
+ */
+__rte_experimental
+static inline uint16_t
+rte_ipv4_udptcp_cksum_mbuf(const struct rte_mbuf *m,
+			   const struct rte_ipv4_hdr *ipv4_hdr, uint16_t l4_off)
+{
+	uint16_t cksum = __rte_ipv4_udptcp_cksum_mbuf(m, ipv4_hdr, l4_off);
+
+	cksum = ~cksum;
+
+	/*
+	 * Per RFC 768: If the computed checksum is zero for UDP,
+	 * it is transmitted as all ones
+	 * (the equivalent in one's complement arithmetic).
+	 */
+	if (cksum == 0 && ipv4_hdr->next_proto_id == IPPROTO_UDP)
+		cksum = 0xffff;
+
+	return cksum;
+}
+
 /**
  * Validate the IPv4 UDP or TCP checksum.
  *
@@ -426,6 +485,38 @@ rte_ipv4_udptcp_cksum_verify(const struct rte_ipv4_hdr *ipv4_hdr,
 	return 0;
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Verify the IPv4 UDP/TCP checksum of a packet.
+ *
+ * In case of UDP, the caller must first check if udp_hdr->dgram_cksum is 0
+ * (i.e. no checksum).
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param ipv4_hdr
+ *   The pointer to the contiguous IPv4 header.
+ * @param l4_off
+ *   The offset in bytes to start L4 checksum.
+ * @return
+ *   Return 0 if the checksum is correct, else -1.
+ */
+__rte_experimental
+static inline uint16_t
+rte_ipv4_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
+				  const struct rte_ipv4_hdr *ipv4_hdr,
+				  uint16_t l4_off)
+{
+	uint16_t cksum = __rte_ipv4_udptcp_cksum_mbuf(m, ipv4_hdr, l4_off);
+
+	if (cksum != 0xffff)
+		return -1;
+
+	return 0;
+}
+
 /**
  * IPv6 Header
  */
@@ -538,6 +629,68 @@ rte_ipv6_udptcp_cksum(const struct rte_ipv6_hdr *ipv6_hdr, const void *l4_hdr)
 	return cksum;
 }
 
+/**
+ * @internal Calculate the non-complemented IPv6 L4 checksum of a packet
+ */
+static inline uint16_t
+__rte_ipv6_udptcp_cksum_mbuf(const struct rte_mbuf *m,
+			     const struct rte_ipv6_hdr *ipv6_hdr,
+			     uint16_t l4_off)
+{
+	uint16_t raw_cksum;
+	uint32_t cksum;
+
+	if (l4_off > m->pkt_len)
+		return 0;
+
+	if (rte_raw_cksum_mbuf(m, l4_off, m->pkt_len - l4_off, &raw_cksum))
+		return 0;
+
+	cksum = raw_cksum + rte_ipv6_phdr_cksum(ipv6_hdr, 0);
+
+	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
+
+	return (uint16_t)cksum;
+}
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Process the IPv6 UDP or TCP checksum of a packet.
+ *
+ * The IPv6 header must not be followed by extension headers. The layer 4
+ * checksum must be set to 0 in the L4 header by the caller.
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param ipv6_hdr
+ *   The pointer to the contiguous IPv6 header.
+ * @param l4_off
+ *   The offset in bytes to start L4 checksum.
+ * @return
+ *   The complemented checksum to set in the L4 header.
+ */
+__rte_experimental
+static inline uint16_t
+rte_ipv6_udptcp_cksum_mbuf(const struct rte_mbuf *m,
+			   const struct rte_ipv6_hdr *ipv6_hdr, uint16_t l4_off)
+{
+	uint16_t cksum = __rte_ipv6_udptcp_cksum_mbuf(m, ipv6_hdr, l4_off);
+
+	cksum = ~cksum;
+
+	/*
+	 * Per RFC 768: If the computed checksum is zero for UDP,
+	 * it is transmitted as all ones
+	 * (the equivalent in one's complement arithmetic).
+	 */
+	if (cksum == 0 && ipv6_hdr->proto == IPPROTO_UDP)
+		cksum = 0xffff;
+
+	return cksum;
+}
+
 /**
  * Validate the IPv6 UDP or TCP checksum.
  *
@@ -565,6 +718,39 @@ rte_ipv6_udptcp_cksum_verify(const struct rte_ipv6_hdr *ipv6_hdr,
 	return 0;
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Validate the IPv6 UDP or TCP checksum of a packet.
+ *
+ * In case of UDP, the caller must first check if udp_hdr->dgram_cksum is 0:
+ * this is either invalid or means no checksum in some situations. See 8.1
+ * (Upper-Layer Checksums) in RFC 8200.
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param ipv6_hdr
+ *   The pointer to the contiguous IPv6 header.
+ * @param l4_off
+ *   The offset in bytes to start L4 checksum.
+ * @return
+ *   Return 0 if the checksum is correct, else -1.
+ */
+__rte_experimental
+static inline int
+rte_ipv6_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
+				  const struct rte_ipv6_hdr *ipv6_hdr,
+				  uint16_t l4_off)
+{
+	uint16_t cksum = __rte_ipv6_udptcp_cksum_mbuf(m, ipv6_hdr, l4_off);
+
+	if (cksum != 0xffff)
+		return -1;
+
+	return 0;
+}
+
 /** IPv6 fragment extension header. */
 #define	RTE_IPV6_EHDR_MF_SHIFT	0
 #define	RTE_IPV6_EHDR_MF_MASK	1
diff --git a/lib/net/version.map b/lib/net/version.map
index 4f4330d1c4..0f2aacdef8 100644
--- a/lib/net/version.map
+++ b/lib/net/version.map
@@ -12,3 +12,13 @@ DPDK_22 {
 
 	local: *;
 };
+
+EXPERIMENTAL {
+	global:
+
+	# added in 22.03
+	rte_ipv4_udptcp_cksum_mbuf;
+	rte_ipv4_udptcp_cksum_mbuf_verify;
+	rte_ipv6_udptcp_cksum_mbuf;
+	rte_ipv6_udptcp_cksum_mbuf_verify;
+};
-- 
2.25.1

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

Re: [PATCH v4 1/2] net: add functions to calculate UDP/TCP cksum in mbuf

[Singh, Aman Deep]

On 12/3/2021 5:08 PM, Xiaoyun Li wrote:
> Add functions to call rte_raw_cksum_mbuf() to calculate IPv4/6
> UDP/TCP checksum in mbuf which can be over multi-segments.
>
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> ---
>   doc/guides/rel_notes/release_22_03.rst |  10 ++
>   lib/net/rte_ip.h                       | 186 +++++++++++++++++++++++++
>   lib/net/version.map                    |  10 ++
>   3 files changed, 206 insertions(+)
>
> diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
> index 6d99d1eaa9..7a082c4427 100644
> --- a/doc/guides/rel_notes/release_22_03.rst
> +++ b/doc/guides/rel_notes/release_22_03.rst
> @@ -55,6 +55,13 @@ New Features
>        Also, make sure to start the actual text at the margin.
>        =======================================================
>   
> +* **Added functions to calculate UDP/TCP checksum in mbuf.**
> +  * Added the following functions to calculate UDP/TCP checksum of packets
> +    which can be over multi-segments:
> +    - ``rte_ipv4_udptcp_cksum_mbuf()``
> +    - ``rte_ipv4_udptcp_cksum_mbuf_verify()``
> +    - ``rte_ipv6_udptcp_cksum_mbuf()``
> +    - ``rte_ipv6_udptcp_cksum_mbuf_verify()``
>   
>   Removed Items
>   -------------
> @@ -84,6 +91,9 @@ API Changes
>      Also, make sure to start the actual text at the margin.
>      =======================================================
>   
> +* net: added experimental functions ``rte_ipv4_udptcp_cksum_mbuf()``,
> +  ``rte_ipv4_udptcp_cksum_mbuf_verify()``, ``rte_ipv6_udptcp_cksum_mbuf()``,
> +  ``rte_ipv6_udptcp_cksum_mbuf_verify()``
>   
>   ABI Changes
>   -----------
> diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h
> index c575250852..534f401d26 100644
> --- a/lib/net/rte_ip.h
> +++ b/lib/net/rte_ip.h
> @@ -400,6 +400,65 @@ rte_ipv4_udptcp_cksum(const struct rte_ipv4_hdr *ipv4_hdr, const void *l4_hdr)
>   	return cksum;
>   }
>   
> +/**
> + * @internal Calculate the non-complemented IPv4 L4 checksum of a packet
> + */
> +static inline uint16_t
> +__rte_ipv4_udptcp_cksum_mbuf(const struct rte_mbuf *m,
> +			     const struct rte_ipv4_hdr *ipv4_hdr,
> +			     uint16_t l4_off)
> +{
> +	uint16_t raw_cksum;
> +	uint32_t cksum;
> +
> +	if (l4_off > m->pkt_len)
> +		return 0;
> +
> +	if (rte_raw_cksum_mbuf(m, l4_off, m->pkt_len - l4_off, &raw_cksum))
> +		return 0;
> +
> +	cksum = raw_cksum + rte_ipv4_phdr_cksum(ipv4_hdr, 0);
> +
> +	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
At times, even after above operation "cksum" might stay above 16-bits, 
ex "cksum = 0x1FFFF" to start with.
Can we consider using "return __rte_raw_cksum_reduce(cksum);"
> +
> +	return (uint16_t)cksum;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Compute the IPv4 UDP/TCP checksum of a packet.
> + *
> + * @param m
> + *   The pointer to the mbuf.
> + * @param ipv4_hdr
> + *   The pointer to the contiguous IPv4 header.
> + * @param l4_off
> + *   The offset in bytes to start L4 checksum.
> + * @return
> + *   The complemented checksum to set in the L4 header.
> + */
> +__rte_experimental
> +static inline uint16_t
> +rte_ipv4_udptcp_cksum_mbuf(const struct rte_mbuf *m,
> +			   const struct rte_ipv4_hdr *ipv4_hdr, uint16_t l4_off)
> +{
> +	uint16_t cksum = __rte_ipv4_udptcp_cksum_mbuf(m, ipv4_hdr, l4_off);
> +
> +	cksum = ~cksum;
> +
> +	/*
> +	 * Per RFC 768: If the computed checksum is zero for UDP,
> +	 * it is transmitted as all ones
> +	 * (the equivalent in one's complement arithmetic).
> +	 */
> +	if (cksum == 0 && ipv4_hdr->next_proto_id == IPPROTO_UDP)
> +		cksum = 0xffff;
> +
> +	return cksum;
> +}
> +
>   /**
>    * Validate the IPv4 UDP or TCP checksum.
>    *
> @@ -426,6 +485,38 @@ rte_ipv4_udptcp_cksum_verify(const struct rte_ipv4_hdr *ipv4_hdr,
>   	return 0;
>   }
>   
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Verify the IPv4 UDP/TCP checksum of a packet.
> + *
> + * In case of UDP, the caller must first check if udp_hdr->dgram_cksum is 0
> + * (i.e. no checksum).
> + *
> + * @param m
> + *   The pointer to the mbuf.
> + * @param ipv4_hdr
> + *   The pointer to the contiguous IPv4 header.
> + * @param l4_off
> + *   The offset in bytes to start L4 checksum.
> + * @return
> + *   Return 0 if the checksum is correct, else -1.
> + */
> +__rte_experimental
> +static inline uint16_t
> +rte_ipv4_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
> +				  const struct rte_ipv4_hdr *ipv4_hdr,
> +				  uint16_t l4_off)
> +{
> +	uint16_t cksum = __rte_ipv4_udptcp_cksum_mbuf(m, ipv4_hdr, l4_off);
> +
> +	if (cksum != 0xffff)
> +		return -1;
cksum other than 0xffff, should return error. Is that the intent or I am 
missing something obvious.
> +
> +	return 0;
> +}
> +
>   /**
>    * IPv6 Header
>    */
> @@ -538,6 +629,68 @@ rte_ipv6_udptcp_cksum(const struct rte_ipv6_hdr *ipv6_hdr, const void *l4_hdr)
>   	return cksum;
>   }
>   
> +/**
> + * @internal Calculate the non-complemented IPv6 L4 checksum of a packet
> + */
> +static inline uint16_t
> +__rte_ipv6_udptcp_cksum_mbuf(const struct rte_mbuf *m,
> +			     const struct rte_ipv6_hdr *ipv6_hdr,
> +			     uint16_t l4_off)
> +{
> +	uint16_t raw_cksum;
> +	uint32_t cksum;
> +
> +	if (l4_off > m->pkt_len)
> +		return 0;
> +
> +	if (rte_raw_cksum_mbuf(m, l4_off, m->pkt_len - l4_off, &raw_cksum))
> +		return 0;
> +
> +	cksum = raw_cksum + rte_ipv6_phdr_cksum(ipv6_hdr, 0);
> +
> +	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
Same, please check if we can opt for __rte_raw_cksum_reduce(cksum)
> +
> +	return (uint16_t)cksum;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Process the IPv6 UDP or TCP checksum of a packet.
> + *
> + * The IPv6 header must not be followed by extension headers. The layer 4
> + * checksum must be set to 0 in the L4 header by the caller.
> + *
> + * @param m
> + *   The pointer to the mbuf.
> + * @param ipv6_hdr
> + *   The pointer to the contiguous IPv6 header.
> + * @param l4_off
> + *   The offset in bytes to start L4 checksum.
> + * @return
> + *   The complemented checksum to set in the L4 header.
> + */
> +__rte_experimental
> +static inline uint16_t
> +rte_ipv6_udptcp_cksum_mbuf(const struct rte_mbuf *m,
> +			   const struct rte_ipv6_hdr *ipv6_hdr, uint16_t l4_off)
> +{
> +	uint16_t cksum = __rte_ipv6_udptcp_cksum_mbuf(m, ipv6_hdr, l4_off);
> +
> +	cksum = ~cksum;
> +
> +	/*
> +	 * Per RFC 768: If the computed checksum is zero for UDP,
> +	 * it is transmitted as all ones
> +	 * (the equivalent in one's complement arithmetic).
> +	 */
> +	if (cksum == 0 && ipv6_hdr->proto == IPPROTO_UDP)
> +		cksum = 0xffff;
> +
> +	return cksum;
> +}
> +
>   /**
>    * Validate the IPv6 UDP or TCP checksum.
>    *
> @@ -565,6 +718,39 @@ rte_ipv6_udptcp_cksum_verify(const struct rte_ipv6_hdr *ipv6_hdr,
>   	return 0;
>   }
>   
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Validate the IPv6 UDP or TCP checksum of a packet.
> + *
> + * In case of UDP, the caller must first check if udp_hdr->dgram_cksum is 0:
> + * this is either invalid or means no checksum in some situations. See 8.1
> + * (Upper-Layer Checksums) in RFC 8200.
> + *
> + * @param m
> + *   The pointer to the mbuf.
> + * @param ipv6_hdr
> + *   The pointer to the contiguous IPv6 header.
> + * @param l4_off
> + *   The offset in bytes to start L4 checksum.
> + * @return
> + *   Return 0 if the checksum is correct, else -1.
> + */
> +__rte_experimental
> +static inline int
> +rte_ipv6_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
> +				  const struct rte_ipv6_hdr *ipv6_hdr,
> +				  uint16_t l4_off)
> +{
> +	uint16_t cksum = __rte_ipv6_udptcp_cksum_mbuf(m, ipv6_hdr, l4_off);
> +
> +	if (cksum != 0xffff)
> +		return -1;
> +
> +	return 0;
> +}
> +
>   /** IPv6 fragment extension header. */
>   #define	RTE_IPV6_EHDR_MF_SHIFT	0
>   #define	RTE_IPV6_EHDR_MF_MASK	1
> diff --git a/lib/net/version.map b/lib/net/version.map
> index 4f4330d1c4..0f2aacdef8 100644
> --- a/lib/net/version.map
> +++ b/lib/net/version.map
> @@ -12,3 +12,13 @@ DPDK_22 {
>   
>   	local: *;
>   };
> +
> +EXPERIMENTAL {
> +	global:
> +
> +	# added in 22.03
> +	rte_ipv4_udptcp_cksum_mbuf;
> +	rte_ipv4_udptcp_cksum_mbuf_verify;
> +	rte_ipv6_udptcp_cksum_mbuf;
> +	rte_ipv6_udptcp_cksum_mbuf_verify;
> +};

RE: [PATCH v4 1/2] net: add functions to calculate UDP/TCP cksum in mbuf

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: Singh, Aman Deep <aman.deep.singh@intel.com>
> Sent: Wednesday, December 15, 2021 11:34
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; olivier.matz@6wind.com;
> mb@smartsharesystems.com; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> Medvedkin, Vladimir <vladimir.medvedkin@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v4 1/2] net: add functions to calculate UDP/TCP cksum in
> mbuf
> 
> 
> On 12/3/2021 5:08 PM, Xiaoyun Li wrote:
> > Add functions to call rte_raw_cksum_mbuf() to calculate IPv4/6 UDP/TCP
> > checksum in mbuf which can be over multi-segments.
> >
> > Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> > ---
> >   doc/guides/rel_notes/release_22_03.rst |  10 ++
> >   lib/net/rte_ip.h                       | 186 +++++++++++++++++++++++++
> >   lib/net/version.map                    |  10 ++
> >   3 files changed, 206 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/release_22_03.rst
> > b/doc/guides/rel_notes/release_22_03.rst
> > index 6d99d1eaa9..7a082c4427 100644
> > --- a/doc/guides/rel_notes/release_22_03.rst
> > +++ b/doc/guides/rel_notes/release_22_03.rst
> > @@ -55,6 +55,13 @@ New Features
> >        Also, make sure to start the actual text at the margin.
> >        =======================================================
> >
> > +* **Added functions to calculate UDP/TCP checksum in mbuf.**
> > +  * Added the following functions to calculate UDP/TCP checksum of
> packets
> > +    which can be over multi-segments:
> > +    - ``rte_ipv4_udptcp_cksum_mbuf()``
> > +    - ``rte_ipv4_udptcp_cksum_mbuf_verify()``
> > +    - ``rte_ipv6_udptcp_cksum_mbuf()``
> > +    - ``rte_ipv6_udptcp_cksum_mbuf_verify()``
> >
> >   Removed Items
> >   -------------
> > @@ -84,6 +91,9 @@ API Changes
> >      Also, make sure to start the actual text at the margin.
> >      =======================================================
> >
> > +* net: added experimental functions ``rte_ipv4_udptcp_cksum_mbuf()``,
> > +  ``rte_ipv4_udptcp_cksum_mbuf_verify()``,
> > +``rte_ipv6_udptcp_cksum_mbuf()``,
> > +  ``rte_ipv6_udptcp_cksum_mbuf_verify()``
> >
> >   ABI Changes
> >   -----------
> > diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h index
> > c575250852..534f401d26 100644
> > --- a/lib/net/rte_ip.h
> > +++ b/lib/net/rte_ip.h
> > @@ -400,6 +400,65 @@ rte_ipv4_udptcp_cksum(const struct rte_ipv4_hdr
> *ipv4_hdr, const void *l4_hdr)
> >   	return cksum;
> >   }
> >
> > +/**
> > + * @internal Calculate the non-complemented IPv4 L4 checksum of a
> > +packet  */ static inline uint16_t __rte_ipv4_udptcp_cksum_mbuf(const
> > +struct rte_mbuf *m,
> > +			     const struct rte_ipv4_hdr *ipv4_hdr,
> > +			     uint16_t l4_off)
> > +{
> > +	uint16_t raw_cksum;
> > +	uint32_t cksum;
> > +
> > +	if (l4_off > m->pkt_len)
> > +		return 0;
> > +
> > +	if (rte_raw_cksum_mbuf(m, l4_off, m->pkt_len - l4_off,
> &raw_cksum))
> > +		return 0;
> > +
> > +	cksum = raw_cksum + rte_ipv4_phdr_cksum(ipv4_hdr, 0);
> > +
> > +	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
> At times, even after above operation "cksum" might stay above 16-bits, ex
> "cksum = 0x1FFFF" to start with.
> Can we consider using "return __rte_raw_cksum_reduce(cksum);"

Will use it in next version. Thanks.

Also, not related to this patch. It means that __rte_ipv4_udptcp_cksum and __rte_ipv6_udptcp_cksum have the same issue, right?
Should anyone fix that?

> > +
> > +	return (uint16_t)cksum;
> > +}
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Compute the IPv4 UDP/TCP checksum of a packet.
> > + *
> > + * @param m
> > + *   The pointer to the mbuf.
> > + * @param ipv4_hdr
> > + *   The pointer to the contiguous IPv4 header.
> > + * @param l4_off
> > + *   The offset in bytes to start L4 checksum.
> > + * @return
> > + *   The complemented checksum to set in the L4 header.
> > + */
> > +__rte_experimental
> > +static inline uint16_t
> > +rte_ipv4_udptcp_cksum_mbuf(const struct rte_mbuf *m,
> > +			   const struct rte_ipv4_hdr *ipv4_hdr, uint16_t l4_off)
> {
> > +	uint16_t cksum = __rte_ipv4_udptcp_cksum_mbuf(m, ipv4_hdr,
> l4_off);
> > +
> > +	cksum = ~cksum;
> > +
> > +	/*
> > +	 * Per RFC 768: If the computed checksum is zero for UDP,
> > +	 * it is transmitted as all ones
> > +	 * (the equivalent in one's complement arithmetic).
> > +	 */
> > +	if (cksum == 0 && ipv4_hdr->next_proto_id == IPPROTO_UDP)
> > +		cksum = 0xffff;
> > +
> > +	return cksum;
> > +}
> > +
> >   /**
> >    * Validate the IPv4 UDP or TCP checksum.
> >    *
> > @@ -426,6 +485,38 @@ rte_ipv4_udptcp_cksum_verify(const struct
> rte_ipv4_hdr *ipv4_hdr,
> >   	return 0;
> >   }
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Verify the IPv4 UDP/TCP checksum of a packet.
> > + *
> > + * In case of UDP, the caller must first check if
> > +udp_hdr->dgram_cksum is 0
> > + * (i.e. no checksum).
> > + *
> > + * @param m
> > + *   The pointer to the mbuf.
> > + * @param ipv4_hdr
> > + *   The pointer to the contiguous IPv4 header.
> > + * @param l4_off
> > + *   The offset in bytes to start L4 checksum.
> > + * @return
> > + *   Return 0 if the checksum is correct, else -1.
> > + */
> > +__rte_experimental
> > +static inline uint16_t
> > +rte_ipv4_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
> > +				  const struct rte_ipv4_hdr *ipv4_hdr,
> > +				  uint16_t l4_off)
> > +{
> > +	uint16_t cksum = __rte_ipv4_udptcp_cksum_mbuf(m, ipv4_hdr,
> l4_off);
> > +
> > +	if (cksum != 0xffff)
> > +		return -1;
> cksum other than 0xffff, should return error. Is that the intent or I am
> missing something obvious.

This is the intent. This function is to verify if the cksum in the packet is correct.

It's different from calling rte_ipv4/6_udptcp_cksum_mbuf(). When calling rte_ipv4/6_udptcp_cksum_mbuf(), you need to set the cksum in udp/tcp header as 0. Then calculate the cksum.

But here, user should directly call this function with the original packet. Then if the udp/tcp cksum is correct, after the calculation (please note that, this is calling __rte_ipv4_udptcp_cksum_mbuf(), so the result needs to be ~), it should be 0xffff, namely, ~cksum = 0 which means cksum is correct. You can see rte_ipv4/6_udptcp_cksum_verify() is doing the same.

> > +
> > +	return 0;
> > +}
> > +
> >   /**
> >    * IPv6 Header
> >    */
> > @@ -538,6 +629,68 @@ rte_ipv6_udptcp_cksum(const struct rte_ipv6_hdr
> *ipv6_hdr, const void *l4_hdr)
> >   	return cksum;
> >   }
> >
> > +/**
> > + * @internal Calculate the non-complemented IPv6 L4 checksum of a
> > +packet  */ static inline uint16_t __rte_ipv6_udptcp_cksum_mbuf(const
> > +struct rte_mbuf *m,
> > +			     const struct rte_ipv6_hdr *ipv6_hdr,
> > +			     uint16_t l4_off)
> > +{
> > +	uint16_t raw_cksum;
> > +	uint32_t cksum;
> > +
> > +	if (l4_off > m->pkt_len)
> > +		return 0;
> > +
> > +	if (rte_raw_cksum_mbuf(m, l4_off, m->pkt_len - l4_off,
> &raw_cksum))
> > +		return 0;
> > +
> > +	cksum = raw_cksum + rte_ipv6_phdr_cksum(ipv6_hdr, 0);
> > +
> > +	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
> Same, please check if we can opt for __rte_raw_cksum_reduce(cksum)
> > +
> > +	return (uint16_t)cksum;
> > +}
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Process the IPv6 UDP or TCP checksum of a packet.
> > + *
> > + * The IPv6 header must not be followed by extension headers. The
> > +layer 4
> > + * checksum must be set to 0 in the L4 header by the caller.
> > + *
> > + * @param m
> > + *   The pointer to the mbuf.
> > + * @param ipv6_hdr
> > + *   The pointer to the contiguous IPv6 header.
> > + * @param l4_off
> > + *   The offset in bytes to start L4 checksum.
> > + * @return
> > + *   The complemented checksum to set in the L4 header.
> > + */
> > +__rte_experimental
> > +static inline uint16_t
> > +rte_ipv6_udptcp_cksum_mbuf(const struct rte_mbuf *m,
> > +			   const struct rte_ipv6_hdr *ipv6_hdr, uint16_t l4_off)
> {
> > +	uint16_t cksum = __rte_ipv6_udptcp_cksum_mbuf(m, ipv6_hdr,
> l4_off);
> > +
> > +	cksum = ~cksum;
> > +
> > +	/*
> > +	 * Per RFC 768: If the computed checksum is zero for UDP,
> > +	 * it is transmitted as all ones
> > +	 * (the equivalent in one's complement arithmetic).
> > +	 */
> > +	if (cksum == 0 && ipv6_hdr->proto == IPPROTO_UDP)
> > +		cksum = 0xffff;
> > +
> > +	return cksum;
> > +}
> > +
> >   /**
> >    * Validate the IPv6 UDP or TCP checksum.
> >    *
> > @@ -565,6 +718,39 @@ rte_ipv6_udptcp_cksum_verify(const struct
> rte_ipv6_hdr *ipv6_hdr,
> >   	return 0;
> >   }
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Validate the IPv6 UDP or TCP checksum of a packet.
> > + *
> > + * In case of UDP, the caller must first check if udp_hdr->dgram_cksum is 0:
> > + * this is either invalid or means no checksum in some situations.
> > +See 8.1
> > + * (Upper-Layer Checksums) in RFC 8200.
> > + *
> > + * @param m
> > + *   The pointer to the mbuf.
> > + * @param ipv6_hdr
> > + *   The pointer to the contiguous IPv6 header.
> > + * @param l4_off
> > + *   The offset in bytes to start L4 checksum.
> > + * @return
> > + *   Return 0 if the checksum is correct, else -1.
> > + */
> > +__rte_experimental
> > +static inline int
> > +rte_ipv6_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
> > +				  const struct rte_ipv6_hdr *ipv6_hdr,
> > +				  uint16_t l4_off)
> > +{
> > +	uint16_t cksum = __rte_ipv6_udptcp_cksum_mbuf(m, ipv6_hdr,
> l4_off);
> > +
> > +	if (cksum != 0xffff)
> > +		return -1;
> > +
> > +	return 0;
> > +}
> > +
> >   /** IPv6 fragment extension header. */
> >   #define	RTE_IPV6_EHDR_MF_SHIFT	0
> >   #define	RTE_IPV6_EHDR_MF_MASK	1
> > diff --git a/lib/net/version.map b/lib/net/version.map index
> > 4f4330d1c4..0f2aacdef8 100644
> > --- a/lib/net/version.map
> > +++ b/lib/net/version.map
> > @@ -12,3 +12,13 @@ DPDK_22 {
> >
> >   	local: *;
> >   };
> > +
> > +EXPERIMENTAL {
> > +	global:
> > +
> > +	# added in 22.03
> > +	rte_ipv4_udptcp_cksum_mbuf;
> > +	rte_ipv4_udptcp_cksum_mbuf_verify;
> > +	rte_ipv6_udptcp_cksum_mbuf;
> > +	rte_ipv6_udptcp_cksum_mbuf_verify;
> > +};

RE: [PATCH v4 1/2] net: add functions to calculate UDP/TCP cksum in mbuf

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: Li, Xiaoyun <xiaoyun.li@intel.com>
> Sent: Tuesday, January 4, 2022 15:19
> To: Singh, Aman Deep <aman.deep.singh@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; olivier.matz@6wind.com;
> mb@smartsharesystems.com; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> Medvedkin, Vladimir <vladimir.medvedkin@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v4 1/2] net: add functions to calculate UDP/TCP cksum in
> mbuf
> 
> Hi
> 
> > -----Original Message-----
> > From: Singh, Aman Deep <aman.deep.singh@intel.com>
> > Sent: Wednesday, December 15, 2021 11:34
> > To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh
> > <ferruh.yigit@intel.com>; olivier.matz@6wind.com;
> > mb@smartsharesystems.com; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> Medvedkin,
> > Vladimir <vladimir.medvedkin@intel.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [PATCH v4 1/2] net: add functions to calculate UDP/TCP
> > cksum in mbuf
> >
> >
> > On 12/3/2021 5:08 PM, Xiaoyun Li wrote:
> > > Add functions to call rte_raw_cksum_mbuf() to calculate IPv4/6
> > > UDP/TCP checksum in mbuf which can be over multi-segments.
> > >
> > > Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> > > ---
> > >   doc/guides/rel_notes/release_22_03.rst |  10 ++
> > >   lib/net/rte_ip.h                       | 186 +++++++++++++++++++++++++
> > >   lib/net/version.map                    |  10 ++
> > >   3 files changed, 206 insertions(+)
> > >
> > > diff --git a/doc/guides/rel_notes/release_22_03.rst
> > > b/doc/guides/rel_notes/release_22_03.rst
> > > index 6d99d1eaa9..7a082c4427 100644
> > > --- a/doc/guides/rel_notes/release_22_03.rst
> > > +++ b/doc/guides/rel_notes/release_22_03.rst
> > > @@ -55,6 +55,13 @@ New Features
> > >        Also, make sure to start the actual text at the margin.
> > >        =======================================================
> > >
> > > +* **Added functions to calculate UDP/TCP checksum in mbuf.**
> > > +  * Added the following functions to calculate UDP/TCP checksum of
> > packets
> > > +    which can be over multi-segments:
> > > +    - ``rte_ipv4_udptcp_cksum_mbuf()``
> > > +    - ``rte_ipv4_udptcp_cksum_mbuf_verify()``
> > > +    - ``rte_ipv6_udptcp_cksum_mbuf()``
> > > +    - ``rte_ipv6_udptcp_cksum_mbuf_verify()``
> > >
> > >   Removed Items
> > >   -------------
> > > @@ -84,6 +91,9 @@ API Changes
> > >      Also, make sure to start the actual text at the margin.
> > >      =======================================================
> > >
> > > +* net: added experimental functions
> > > +``rte_ipv4_udptcp_cksum_mbuf()``,
> > > +  ``rte_ipv4_udptcp_cksum_mbuf_verify()``,
> > > +``rte_ipv6_udptcp_cksum_mbuf()``,
> > > +  ``rte_ipv6_udptcp_cksum_mbuf_verify()``
> > >
> > >   ABI Changes
> > >   -----------
> > > diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h index
> > > c575250852..534f401d26 100644
> > > --- a/lib/net/rte_ip.h
> > > +++ b/lib/net/rte_ip.h
> > > @@ -400,6 +400,65 @@ rte_ipv4_udptcp_cksum(const struct
> rte_ipv4_hdr
> > *ipv4_hdr, const void *l4_hdr)
> > >   	return cksum;
> > >   }
> > >
> > > +/**
> > > + * @internal Calculate the non-complemented IPv4 L4 checksum of a
> > > +packet  */ static inline uint16_t
> > > +__rte_ipv4_udptcp_cksum_mbuf(const
> > > +struct rte_mbuf *m,
> > > +			     const struct rte_ipv4_hdr *ipv4_hdr,
> > > +			     uint16_t l4_off)
> > > +{
> > > +	uint16_t raw_cksum;
> > > +	uint32_t cksum;
> > > +
> > > +	if (l4_off > m->pkt_len)
> > > +		return 0;
> > > +
> > > +	if (rte_raw_cksum_mbuf(m, l4_off, m->pkt_len - l4_off,
> > &raw_cksum))
> > > +		return 0;
> > > +
> > > +	cksum = raw_cksum + rte_ipv4_phdr_cksum(ipv4_hdr, 0);
> > > +
> > > +	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
> > At times, even after above operation "cksum" might stay above 16-bits,
> > ex "cksum = 0x1FFFF" to start with.
> > Can we consider using "return __rte_raw_cksum_reduce(cksum);"
> 
> Will use it in next version. Thanks.
> 
> Also, not related to this patch. It means that __rte_ipv4_udptcp_cksum and
> __rte_ipv6_udptcp_cksum have the same issue, right?
> Should anyone fix that?

Forgot the intent here.
rte_raw_cksum_mbuf() already calls __rte_raw_cksum_reduce().
So actually, it's a result of uint16_t + uint16_t. So it's impossible of your case. There's no need to call __rte_raw_cksum_reduce().

> 
> > > +
> > > +	return (uint16_t)cksum;
> > > +}
> > > +
> > > +/**
> > > + * @warning
> > > + * @b EXPERIMENTAL: this API may change without prior notice.
> > > + *
> > > + * Compute the IPv4 UDP/TCP checksum of a packet.
> > > + *
> > > + * @param m
> > > + *   The pointer to the mbuf.
> > > + * @param ipv4_hdr
> > > + *   The pointer to the contiguous IPv4 header.
> > > + * @param l4_off
> > > + *   The offset in bytes to start L4 checksum.
> > > + * @return
> > > + *   The complemented checksum to set in the L4 header.
> > > + */
> > > +__rte_experimental
> > > +static inline uint16_t
> > > +rte_ipv4_udptcp_cksum_mbuf(const struct rte_mbuf *m,
> > > +			   const struct rte_ipv4_hdr *ipv4_hdr, uint16_t l4_off)
> > {
> > > +	uint16_t cksum = __rte_ipv4_udptcp_cksum_mbuf(m, ipv4_hdr,
> > l4_off);
> > > +
> > > +	cksum = ~cksum;
> > > +
> > > +	/*
> > > +	 * Per RFC 768: If the computed checksum is zero for UDP,
> > > +	 * it is transmitted as all ones
> > > +	 * (the equivalent in one's complement arithmetic).
> > > +	 */
> > > +	if (cksum == 0 && ipv4_hdr->next_proto_id == IPPROTO_UDP)
> > > +		cksum = 0xffff;
> > > +
> > > +	return cksum;
> > > +}
> > > +
> > >   /**
> > >    * Validate the IPv4 UDP or TCP checksum.
> > >    *
> > > @@ -426,6 +485,38 @@ rte_ipv4_udptcp_cksum_verify(const struct
> > rte_ipv4_hdr *ipv4_hdr,
> > >   	return 0;
> > >   }
> > >
> > > +/**
> > > + * @warning
> > > + * @b EXPERIMENTAL: this API may change without prior notice.
> > > + *
> > > + * Verify the IPv4 UDP/TCP checksum of a packet.
> > > + *
> > > + * In case of UDP, the caller must first check if
> > > +udp_hdr->dgram_cksum is 0
> > > + * (i.e. no checksum).
> > > + *
> > > + * @param m
> > > + *   The pointer to the mbuf.
> > > + * @param ipv4_hdr
> > > + *   The pointer to the contiguous IPv4 header.
> > > + * @param l4_off
> > > + *   The offset in bytes to start L4 checksum.
> > > + * @return
> > > + *   Return 0 if the checksum is correct, else -1.
> > > + */
> > > +__rte_experimental
> > > +static inline uint16_t
> > > +rte_ipv4_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
> > > +				  const struct rte_ipv4_hdr *ipv4_hdr,
> > > +				  uint16_t l4_off)
> > > +{
> > > +	uint16_t cksum = __rte_ipv4_udptcp_cksum_mbuf(m, ipv4_hdr,
> > l4_off);
> > > +
> > > +	if (cksum != 0xffff)
> > > +		return -1;
> > cksum other than 0xffff, should return error. Is that the intent or I
> > am missing something obvious.
> 
> This is the intent. This function is to verify if the cksum in the packet is correct.
> 
> It's different from calling rte_ipv4/6_udptcp_cksum_mbuf(). When calling
> rte_ipv4/6_udptcp_cksum_mbuf(), you need to set the cksum in udp/tcp
> header as 0. Then calculate the cksum.
> 
> But here, user should directly call this function with the original packet. Then
> if the udp/tcp cksum is correct, after the calculation (please note that, this is
> calling __rte_ipv4_udptcp_cksum_mbuf(), so the result needs to be ~), it
> should be 0xffff, namely, ~cksum = 0 which means cksum is correct. You can
> see rte_ipv4/6_udptcp_cksum_verify() is doing the same.
> 
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >   /**
> > >    * IPv6 Header
> > >    */
> > > @@ -538,6 +629,68 @@ rte_ipv6_udptcp_cksum(const struct
> rte_ipv6_hdr
> > *ipv6_hdr, const void *l4_hdr)
> > >   	return cksum;
> > >   }
> > >
> > > +/**
> > > + * @internal Calculate the non-complemented IPv6 L4 checksum of a
> > > +packet  */ static inline uint16_t
> > > +__rte_ipv6_udptcp_cksum_mbuf(const
> > > +struct rte_mbuf *m,
> > > +			     const struct rte_ipv6_hdr *ipv6_hdr,
> > > +			     uint16_t l4_off)
> > > +{
> > > +	uint16_t raw_cksum;
> > > +	uint32_t cksum;
> > > +
> > > +	if (l4_off > m->pkt_len)
> > > +		return 0;
> > > +
> > > +	if (rte_raw_cksum_mbuf(m, l4_off, m->pkt_len - l4_off,
> > &raw_cksum))
> > > +		return 0;
> > > +
> > > +	cksum = raw_cksum + rte_ipv6_phdr_cksum(ipv6_hdr, 0);
> > > +
> > > +	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
> > Same, please check if we can opt for __rte_raw_cksum_reduce(cksum)
> > > +
> > > +	return (uint16_t)cksum;
> > > +}
> > > +
> > > +/**
> > > + * @warning
> > > + * @b EXPERIMENTAL: this API may change without prior notice.
> > > + *
> > > + * Process the IPv6 UDP or TCP checksum of a packet.
> > > + *
> > > + * The IPv6 header must not be followed by extension headers. The
> > > +layer 4
> > > + * checksum must be set to 0 in the L4 header by the caller.
> > > + *
> > > + * @param m
> > > + *   The pointer to the mbuf.
> > > + * @param ipv6_hdr
> > > + *   The pointer to the contiguous IPv6 header.
> > > + * @param l4_off
> > > + *   The offset in bytes to start L4 checksum.
> > > + * @return
> > > + *   The complemented checksum to set in the L4 header.
> > > + */
> > > +__rte_experimental
> > > +static inline uint16_t
> > > +rte_ipv6_udptcp_cksum_mbuf(const struct rte_mbuf *m,
> > > +			   const struct rte_ipv6_hdr *ipv6_hdr, uint16_t l4_off)
> > {
> > > +	uint16_t cksum = __rte_ipv6_udptcp_cksum_mbuf(m, ipv6_hdr,
> > l4_off);
> > > +
> > > +	cksum = ~cksum;
> > > +
> > > +	/*
> > > +	 * Per RFC 768: If the computed checksum is zero for UDP,
> > > +	 * it is transmitted as all ones
> > > +	 * (the equivalent in one's complement arithmetic).
> > > +	 */
> > > +	if (cksum == 0 && ipv6_hdr->proto == IPPROTO_UDP)
> > > +		cksum = 0xffff;
> > > +
> > > +	return cksum;
> > > +}
> > > +
> > >   /**
> > >    * Validate the IPv6 UDP or TCP checksum.
> > >    *
> > > @@ -565,6 +718,39 @@ rte_ipv6_udptcp_cksum_verify(const struct
> > rte_ipv6_hdr *ipv6_hdr,
> > >   	return 0;
> > >   }
> > >
> > > +/**
> > > + * @warning
> > > + * @b EXPERIMENTAL: this API may change without prior notice.
> > > + *
> > > + * Validate the IPv6 UDP or TCP checksum of a packet.
> > > + *
> > > + * In case of UDP, the caller must first check if udp_hdr->dgram_cksum is
> 0:
> > > + * this is either invalid or means no checksum in some situations.
> > > +See 8.1
> > > + * (Upper-Layer Checksums) in RFC 8200.
> > > + *
> > > + * @param m
> > > + *   The pointer to the mbuf.
> > > + * @param ipv6_hdr
> > > + *   The pointer to the contiguous IPv6 header.
> > > + * @param l4_off
> > > + *   The offset in bytes to start L4 checksum.
> > > + * @return
> > > + *   Return 0 if the checksum is correct, else -1.
> > > + */
> > > +__rte_experimental
> > > +static inline int
> > > +rte_ipv6_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
> > > +				  const struct rte_ipv6_hdr *ipv6_hdr,
> > > +				  uint16_t l4_off)
> > > +{
> > > +	uint16_t cksum = __rte_ipv6_udptcp_cksum_mbuf(m, ipv6_hdr,
> > l4_off);
> > > +
> > > +	if (cksum != 0xffff)
> > > +		return -1;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >   /** IPv6 fragment extension header. */
> > >   #define	RTE_IPV6_EHDR_MF_SHIFT	0
> > >   #define	RTE_IPV6_EHDR_MF_MASK	1
> > > diff --git a/lib/net/version.map b/lib/net/version.map index
> > > 4f4330d1c4..0f2aacdef8 100644
> > > --- a/lib/net/version.map
> > > +++ b/lib/net/version.map
> > > @@ -12,3 +12,13 @@ DPDK_22 {
> > >
> > >   	local: *;
> > >   };
> > > +
> > > +EXPERIMENTAL {
> > > +	global:
> > > +
> > > +	# added in 22.03
> > > +	rte_ipv4_udptcp_cksum_mbuf;
> > > +	rte_ipv4_udptcp_cksum_mbuf_verify;
> > > +	rte_ipv6_udptcp_cksum_mbuf;
> > > +	rte_ipv6_udptcp_cksum_mbuf_verify;
> > > +};

Re: [PATCH v4 1/2] net: add functions to calculate UDP/TCP cksum in mbuf

[Singh, Aman Deep]

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


On 1/4/2022 9:10 PM, Li, Xiaoyun wrote:
> Hi
>
>> -----Original Message-----
>> From: Li, Xiaoyun<xiaoyun.li@intel.com>
>> Sent: Tuesday, January 4, 2022 15:19
>> To: Singh, Aman Deep<aman.deep.singh@intel.com>; Yigit, Ferruh
>> <ferruh.yigit@intel.com>;olivier.matz@6wind.com;
>> mb@smartsharesystems.com; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>;stephen@networkplumber.org;
>> Medvedkin, Vladimir<vladimir.medvedkin@intel.com>
>> Cc:dev@dpdk.org
>> Subject: RE: [PATCH v4 1/2] net: add functions to calculate UDP/TCP cksum in
>> mbuf
>>
>> Hi
>>
>>> -----Original Message-----
>>> From: Singh, Aman Deep<aman.deep.singh@intel.com>
>>> Sent: Wednesday, December 15, 2021 11:34
>>> To: Li, Xiaoyun<xiaoyun.li@intel.com>; Yigit, Ferruh
>>> <ferruh.yigit@intel.com>;olivier.matz@6wind.com;
>>> mb@smartsharesystems.com; Ananyev, Konstantin
>>> <konstantin.ananyev@intel.com>;stephen@networkplumber.org;
>> Medvedkin,
>>> Vladimir<vladimir.medvedkin@intel.com>
>>> Cc:dev@dpdk.org
>>> Subject: Re: [PATCH v4 1/2] net: add functions to calculate UDP/TCP
>>> cksum in mbuf
>>>
>>>
>>> On 12/3/2021 5:08 PM, Xiaoyun Li wrote:
>>>> Add functions to call rte_raw_cksum_mbuf() to calculate IPv4/6
>>>> UDP/TCP checksum in mbuf which can be over multi-segments.
>>>>
>>>> Signed-off-by: Xiaoyun Li<xiaoyun.li@intel.com>

Acked-by: Aman Singh <aman.deep.singh@intel.com>

>>>> ---
>>>>    doc/guides/rel_notes/release_22_03.rst |  10 ++
>>>>    lib/net/rte_ip.h                       | 186 +++++++++++++++++++++++++
>>>>    lib/net/version.map                    |  10 ++
>>>>    3 files changed, 206 insertions(+)
>>>>
>>>> diff --git a/doc/guides/rel_notes/release_22_03.rst
>>>> b/doc/guides/rel_notes/release_22_03.rst
>>>> index 6d99d1eaa9..7a082c4427 100644
>>>> --- a/doc/guides/rel_notes/release_22_03.rst
>>>> +++ b/doc/guides/rel_notes/release_22_03.rst
>>>> @@ -55,6 +55,13 @@ New Features
>>>>         Also, make sure to start the actual text at the margin.
>>>>         =======================================================
>>>>
>>>> +* **Added functions to calculate UDP/TCP checksum in mbuf.**
>>>> +  * Added the following functions to calculate UDP/TCP checksum of
>>> packets
>>>> +    which can be over multi-segments:
>>>> +    - ``rte_ipv4_udptcp_cksum_mbuf()``
>>>> +    - ``rte_ipv4_udptcp_cksum_mbuf_verify()``
>>>> +    - ``rte_ipv6_udptcp_cksum_mbuf()``
>>>> +    - ``rte_ipv6_udptcp_cksum_mbuf_verify()``
>>>>
>>>>    Removed Items
>>>>    -------------
>>>> @@ -84,6 +91,9 @@ API Changes
>>>>       Also, make sure to start the actual text at the margin.
>>>>       =======================================================
>>>>
>>>> +* net: added experimental functions
>>>> +``rte_ipv4_udptcp_cksum_mbuf()``,
>>>> +  ``rte_ipv4_udptcp_cksum_mbuf_verify()``,
>>>> +``rte_ipv6_udptcp_cksum_mbuf()``,
>>>> +  ``rte_ipv6_udptcp_cksum_mbuf_verify()``
>>>>
>>>>    ABI Changes
>>>>    -----------
>>>> diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h index
>>>> c575250852..534f401d26 100644
>>>> --- a/lib/net/rte_ip.h
>>>> +++ b/lib/net/rte_ip.h
>>>> @@ -400,6 +400,65 @@ rte_ipv4_udptcp_cksum(const struct
>> rte_ipv4_hdr
>>> *ipv4_hdr, const void *l4_hdr)
>>>>    	return cksum;
>>>>    }
>>>>
>>>> +/**
>>>> + * @internal Calculate the non-complemented IPv4 L4 checksum of a
>>>> +packet  */ static inline uint16_t
>>>> +__rte_ipv4_udptcp_cksum_mbuf(const
>>>> +struct rte_mbuf *m,
>>>> +			     const struct rte_ipv4_hdr *ipv4_hdr,
>>>> +			     uint16_t l4_off)
>>>> +{
>>>> +	uint16_t raw_cksum;
>>>> +	uint32_t cksum;
>>>> +
>>>> +	if (l4_off > m->pkt_len)
>>>> +		return 0;
>>>> +
>>>> +	if (rte_raw_cksum_mbuf(m, l4_off, m->pkt_len - l4_off,
>>> &raw_cksum))
>>>> +		return 0;
>>>> +
>>>> +	cksum = raw_cksum + rte_ipv4_phdr_cksum(ipv4_hdr, 0);
>>>> +
>>>> +	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
>>> At times, even after above operation "cksum" might stay above 16-bits,
>>> ex "cksum = 0x1FFFF" to start with.
>>> Can we consider using "return __rte_raw_cksum_reduce(cksum);"
>> Will use it in next version. Thanks.
>>
>> Also, not related to this patch. It means that __rte_ipv4_udptcp_cksum and
>> __rte_ipv6_udptcp_cksum have the same issue, right?
>> Should anyone fix that?
> Forgot the intent here.
> rte_raw_cksum_mbuf() already calls __rte_raw_cksum_reduce().
> So actually, it's a result of uint16_t + uint16_t. So it's impossible of your case. There's no need to call __rte_raw_cksum_reduce().
Got it, Thanks. With u16 + u16, max 1-bit overflow only possible. So 
effective operation here reduces to-

cksum = ((cksum & 0x10000) >> 16) + (cksum & 0xffff);

>>>> +
>>>> +	return (uint16_t)cksum;
>>>> +}
>>>> +
>>>> +/**
>>>> + * @warning
>>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>>> + *
>>>> + * Compute the IPv4 UDP/TCP checksum of a packet.
>>>> + *
>>>> + * @param m
>>>> + *   The pointer to the mbuf.
>>>> + * @param ipv4_hdr
>>>> + *   The pointer to the contiguous IPv4 header.
>>>> + * @param l4_off
>>>> + *   The offset in bytes to start L4 checksum.
>>>> + * @return
>>>> + *   The complemented checksum to set in the L4 header.
>>>> + */
>>>> +__rte_experimental
>>>> +static inline uint16_t
>>>> +rte_ipv4_udptcp_cksum_mbuf(const struct rte_mbuf *m,
>>>> +			   const struct rte_ipv4_hdr *ipv4_hdr, uint16_t l4_off)
>>> {
>>>> +	uint16_t cksum = __rte_ipv4_udptcp_cksum_mbuf(m, ipv4_hdr,
>>> l4_off);
>>>> +
>>>> +	cksum = ~cksum;
>>>> +
>>>> +	/*
>>>> +	 * Per RFC 768: If the computed checksum is zero for UDP,
>>>> +	 * it is transmitted as all ones
>>>> +	 * (the equivalent in one's complement arithmetic).
>>>> +	 */
>>>> +	if (cksum == 0 && ipv4_hdr->next_proto_id == IPPROTO_UDP)
>>>> +		cksum = 0xffff;
>>>> +
>>>> +	return cksum;
>>>> +}
>>>> +
>>>>    /**
>>>>     * Validate the IPv4 UDP or TCP checksum.
>>>>     *
>>>> @@ -426,6 +485,38 @@ rte_ipv4_udptcp_cksum_verify(const struct
>>> rte_ipv4_hdr *ipv4_hdr,
>>>>    	return 0;
>>>>    }
>>>>
>>>> +/**
>>>> + * @warning
>>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>>> + *
>>>> + * Verify the IPv4 UDP/TCP checksum of a packet.
>>>> + *
>>>> + * In case of UDP, the caller must first check if
>>>> +udp_hdr->dgram_cksum is 0
>>>> + * (i.e. no checksum).
>>>> + *
>>>> + * @param m
>>>> + *   The pointer to the mbuf.
>>>> + * @param ipv4_hdr
>>>> + *   The pointer to the contiguous IPv4 header.
>>>> + * @param l4_off
>>>> + *   The offset in bytes to start L4 checksum.
>>>> + * @return
>>>> + *   Return 0 if the checksum is correct, else -1.
>>>> + */
>>>> +__rte_experimental
>>>> +static inline uint16_t
>>>> +rte_ipv4_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
>>>> +				  const struct rte_ipv4_hdr *ipv4_hdr,
>>>> +				  uint16_t l4_off)
>>>> +{
>>>> +	uint16_t cksum = __rte_ipv4_udptcp_cksum_mbuf(m, ipv4_hdr,
>>> l4_off);
>>>> +
>>>> +	if (cksum != 0xffff)
>>>> +		return -1;
>>> cksum other than 0xffff, should return error. Is that the intent or I
>>> am missing something obvious.
>> This is the intent. This function is to verify if the cksum in the packet is correct.
>>
>> It's different from calling rte_ipv4/6_udptcp_cksum_mbuf(). When calling
>> rte_ipv4/6_udptcp_cksum_mbuf(), you need to set the cksum in udp/tcp
>> header as 0. Then calculate the cksum.
>>
>> But here, user should directly call this function with the original packet. Then
>> if the udp/tcp cksum is correct, after the calculation (please note that, this is
>> calling __rte_ipv4_udptcp_cksum_mbuf(), so the result needs to be ~), it
>> should be 0xffff, namely, ~cksum = 0 which means cksum is correct. You can
>> see rte_ipv4/6_udptcp_cksum_verify() is doing the same.
>>
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>>    /**
>>>>     * IPv6 Header
>>>>     */
>>>> @@ -538,6 +629,68 @@ rte_ipv6_udptcp_cksum(const struct
>> rte_ipv6_hdr
>>> *ipv6_hdr, const void *l4_hdr)
>>>>    	return cksum;
>>>>    }
>>>>
>>>> +/**
>>>> + * @internal Calculate the non-complemented IPv6 L4 checksum of a
>>>> +packet  */ static inline uint16_t
>>>> +__rte_ipv6_udptcp_cksum_mbuf(const
>>>> +struct rte_mbuf *m,
>>>> +			     const struct rte_ipv6_hdr *ipv6_hdr,
>>>> +			     uint16_t l4_off)
>>>> +{
>>>> +	uint16_t raw_cksum;
>>>> +	uint32_t cksum;
>>>> +
>>>> +	if (l4_off > m->pkt_len)
>>>> +		return 0;
>>>> +
>>>> +	if (rte_raw_cksum_mbuf(m, l4_off, m->pkt_len - l4_off,
>>> &raw_cksum))
>>>> +		return 0;
>>>> +
>>>> +	cksum = raw_cksum + rte_ipv6_phdr_cksum(ipv6_hdr, 0);
>>>> +
>>>> +	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
>>> Same, please check if we can opt for __rte_raw_cksum_reduce(cksum)
>>>> +
>>>> +	return (uint16_t)cksum;
>>>> +}
>>>> +
>>>> +/**
>>>> + * @warning
>>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>>> + *
>>>> + * Process the IPv6 UDP or TCP checksum of a packet.
>>>> + *
>>>> + * The IPv6 header must not be followed by extension headers. The
>>>> +layer 4
>>>> + * checksum must be set to 0 in the L4 header by the caller.
>>>> + *
>>>> + * @param m
>>>> + *   The pointer to the mbuf.
>>>> + * @param ipv6_hdr
>>>> + *   The pointer to the contiguous IPv6 header.
>>>> + * @param l4_off
>>>> + *   The offset in bytes to start L4 checksum.
>>>> + * @return
>>>> + *   The complemented checksum to set in the L4 header.
>>>> + */
>>>> +__rte_experimental
>>>> +static inline uint16_t
>>>> +rte_ipv6_udptcp_cksum_mbuf(const struct rte_mbuf *m,
>>>> +			   const struct rte_ipv6_hdr *ipv6_hdr, uint16_t l4_off)
>>> {
>>>> +	uint16_t cksum = __rte_ipv6_udptcp_cksum_mbuf(m, ipv6_hdr,
>>> l4_off);
>>>> +
>>>> +	cksum = ~cksum;
>>>> +
>>>> +	/*
>>>> +	 * Per RFC 768: If the computed checksum is zero for UDP,
>>>> +	 * it is transmitted as all ones
>>>> +	 * (the equivalent in one's complement arithmetic).
>>>> +	 */
>>>> +	if (cksum == 0 && ipv6_hdr->proto == IPPROTO_UDP)
>>>> +		cksum = 0xffff;
>>>> +
>>>> +	return cksum;
>>>> +}
>>>> +
>>>>    /**
>>>>     * Validate the IPv6 UDP or TCP checksum.
>>>>     *
>>>> @@ -565,6 +718,39 @@ rte_ipv6_udptcp_cksum_verify(const struct
>>> rte_ipv6_hdr *ipv6_hdr,
>>>>    	return 0;
>>>>    }
>>>>
>>>> +/**
>>>> + * @warning
>>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>>> + *
>>>> + * Validate the IPv6 UDP or TCP checksum of a packet.
>>>> + *
>>>> + * In case of UDP, the caller must first check if udp_hdr->dgram_cksum is
>> 0:
>>>> + * this is either invalid or means no checksum in some situations.
>>>> +See 8.1
>>>> + * (Upper-Layer Checksums) in RFC 8200.
>>>> + *
>>>> + * @param m
>>>> + *   The pointer to the mbuf.
>>>> + * @param ipv6_hdr
>>>> + *   The pointer to the contiguous IPv6 header.
>>>> + * @param l4_off
>>>> + *   The offset in bytes to start L4 checksum.
>>>> + * @return
>>>> + *   Return 0 if the checksum is correct, else -1.
>>>> + */
>>>> +__rte_experimental
>>>> +static inline int
>>>> +rte_ipv6_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
>>>> +				  const struct rte_ipv6_hdr *ipv6_hdr,
>>>> +				  uint16_t l4_off)
>>>> +{
>>>> +	uint16_t cksum = __rte_ipv6_udptcp_cksum_mbuf(m, ipv6_hdr,
>>> l4_off);
>>>> +
>>>> +	if (cksum != 0xffff)
>>>> +		return -1;
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>>    /** IPv6 fragment extension header. */
>>>>    #define	RTE_IPV6_EHDR_MF_SHIFT	0
>>>>    #define	RTE_IPV6_EHDR_MF_MASK	1
>>>> diff --git a/lib/net/version.map b/lib/net/version.map index
>>>> 4f4330d1c4..0f2aacdef8 100644
>>>> --- a/lib/net/version.map
>>>> +++ b/lib/net/version.map
>>>> @@ -12,3 +12,13 @@ DPDK_22 {
>>>>
>>>>    	local: *;
>>>>    };
>>>> +
>>>> +EXPERIMENTAL {
>>>> +	global:
>>>> +
>>>> +	# added in 22.03
>>>> +	rte_ipv4_udptcp_cksum_mbuf;
>>>> +	rte_ipv4_udptcp_cksum_mbuf_verify;
>>>> +	rte_ipv6_udptcp_cksum_mbuf;
>>>> +	rte_ipv6_udptcp_cksum_mbuf_verify;
>>>> +};

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

[PATCH v4 2/2] testpmd: fix l4 sw csum over multi segments

[Xiaoyun Li]

In csum forwarding mode, software UDP/TCP csum calculation only takes
the first segment into account while using the whole packet length so
the calculation will read invalid memory region with multi-segments
packets and will get wrong value.
This patch fixes this issue.

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
 app/test-pmd/csumonly.c | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 2aeea243b6..0fbe1f1be7 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -96,12 +96,13 @@ struct simple_gre_hdr {
 } __rte_packed;
 
 static uint16_t
-get_udptcp_checksum(void *l3_hdr, void *l4_hdr, uint16_t ethertype)
+get_udptcp_checksum(struct rte_mbuf *m, void *l3_hdr, uint16_t l4_off,
+		    uint16_t ethertype)
 {
 	if (ethertype == _htons(RTE_ETHER_TYPE_IPV4))
-		return rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
+		return rte_ipv4_udptcp_cksum_mbuf(m, l3_hdr, l4_off);
 	else /* assume ethertype == RTE_ETHER_TYPE_IPV6 */
-		return rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
+		return rte_ipv6_udptcp_cksum_mbuf(m, l3_hdr, l4_off);
 }
 
 /* Parse an IPv4 header to fill l3_len, l4_len, and l4_proto */
@@ -460,7 +461,7 @@ parse_encap_ip(void *encap_ip, struct testpmd_offload_info *info)
  * depending on the testpmd command line configuration */
 static uint64_t
 process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
-	uint64_t tx_offloads)
+	uint64_t tx_offloads, struct rte_mbuf *m)
 {
 	struct rte_ipv4_hdr *ipv4_hdr = l3_hdr;
 	struct rte_udp_hdr *udp_hdr;
@@ -468,6 +469,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 	struct rte_sctp_hdr *sctp_hdr;
 	uint64_t ol_flags = 0;
 	uint32_t max_pkt_len, tso_segsz = 0;
+	uint16_t l4_off;
 
 	/* ensure packet is large enough to require tso */
 	if (!info->is_tunnel) {
@@ -510,9 +512,15 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 			if (tx_offloads & RTE_ETH_TX_OFFLOAD_UDP_CKSUM) {
 				ol_flags |= RTE_MBUF_F_TX_UDP_CKSUM;
 			} else {
+				if (info->is_tunnel)
+					l4_off = info->l2_len +
+						 info->outer_l3_len +
+						 info->l2_len + info->l3_len;
+				else
+					l4_off = info->l2_len +	info->l3_len;
 				udp_hdr->dgram_cksum = 0;
 				udp_hdr->dgram_cksum =
-					get_udptcp_checksum(l3_hdr, udp_hdr,
+					get_udptcp_checksum(m, l3_hdr, l4_off,
 						info->ethertype);
 			}
 		}
@@ -527,9 +535,14 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 		else if (tx_offloads & RTE_ETH_TX_OFFLOAD_TCP_CKSUM) {
 			ol_flags |= RTE_MBUF_F_TX_TCP_CKSUM;
 		} else {
+			if (info->is_tunnel)
+				l4_off = info->l2_len + info->outer_l3_len +
+					 info->l2_len + info->l3_len;
+			else
+				l4_off = info->l2_len + info->l3_len;
 			tcp_hdr->cksum = 0;
 			tcp_hdr->cksum =
-				get_udptcp_checksum(l3_hdr, tcp_hdr,
+				get_udptcp_checksum(m, l3_hdr, l4_off,
 					info->ethertype);
 		}
 #ifdef RTE_LIB_GSO
@@ -557,7 +570,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 /* Calculate the checksum of outer header */
 static uint64_t
 process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
-	uint64_t tx_offloads, int tso_enabled)
+	uint64_t tx_offloads, int tso_enabled, struct rte_mbuf *m)
 {
 	struct rte_ipv4_hdr *ipv4_hdr = outer_l3_hdr;
 	struct rte_ipv6_hdr *ipv6_hdr = outer_l3_hdr;
@@ -611,12 +624,9 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 	/* do not recalculate udp cksum if it was 0 */
 	if (udp_hdr->dgram_cksum != 0) {
 		udp_hdr->dgram_cksum = 0;
-		if (info->outer_ethertype == _htons(RTE_ETHER_TYPE_IPV4))
-			udp_hdr->dgram_cksum =
-				rte_ipv4_udptcp_cksum(ipv4_hdr, udp_hdr);
-		else
-			udp_hdr->dgram_cksum =
-				rte_ipv6_udptcp_cksum(ipv6_hdr, udp_hdr);
+		udp_hdr->dgram_cksum = get_udptcp_checksum(m, outer_l3_hdr,
+					info->l2_len + info->outer_l3_len,
+					info->outer_ethertype);
 	}
 
 	return ol_flags;
@@ -957,7 +967,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 
 		/* process checksums of inner headers first */
 		tx_ol_flags |= process_inner_cksums(l3_hdr, &info,
-			tx_offloads);
+			tx_offloads, m);
 
 		/* Then process outer headers if any. Note that the software
 		 * checksum will be wrong if one of the inner checksums is
@@ -965,7 +975,8 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		if (info.is_tunnel == 1) {
 			tx_ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
 					tx_offloads,
-					!!(tx_ol_flags & RTE_MBUF_F_TX_TCP_SEG));
+					!!(tx_ol_flags & RTE_MBUF_F_TX_TCP_SEG),
+					m);
 		}
 
 		/* step 3: fill the mbuf meta data (flags and header lengths) */
-- 
2.25.1

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

RE: [PATCH v4 0/2] Add functions to calculate UDP/TCP cksum in mbuf

[Pai G, Sunil]

Tested-by: Sunil Pai G <sunil.pai.g@intel.com>

Hi , 

Tested the series with OVS  - with both TCP and UDP packets by printing out the csum before sending and after reception of packets from testpmd.
Getting expected csum values with this patch.

Thanks and regards
Sunil

<snipped>

[PATCH v5 0/2] Add functions to calculate UDP/TCP cksum in mbuf

[Xiaoyun Li]

Added functions to calculate UDP/TCP checksum for packets which may be
over multi-segments and fix the checksum issue with testpmd csum
forwarding mode.

Xiaoyun Li (2):
  net: add functions to calculate UDP/TCP cksum in mbuf
  testpmd: fix l4 sw csum over multi segments

---
v5:
 * Fixed commit log intendation issue.
 * Removed added inline experimental APIs in version.map to fix windows
 * linkage issue.
 * Added Sunil's Tested-by tag and Aman's Acked-by tag.
v4:
 * Called rte_raw_cksum_mbuf() to calculate cksum in lib instead of
 * implementing it in testpmd for better maintenance.
 * Removed fix tag for testpmd since it relies on the lib change.
v3:
 * Use rte_raw_cksum() for multi-segs case instead of copying the whole
 * packet.
v2:
 * Use static stack memory instead of dynamic allocating in datapath
---

 app/test-pmd/csumonly.c                |  41 ++++--
 doc/guides/rel_notes/release_22_03.rst |  11 ++
 lib/net/rte_ip.h                       | 186 +++++++++++++++++++++++++
 3 files changed, 223 insertions(+), 15 deletions(-)

-- 
2.25.1

[PATCH v5 1/2] net: add functions to calculate UDP/TCP cksum in mbuf

[Xiaoyun Li]

Add functions to call rte_raw_cksum_mbuf() to calculate IPv4/6
UDP/TCP checksum in mbuf which can be over multi-segments.

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
Acked-by: Aman Singh <aman.deep.singh@intel.com>
Tested-by: Sunil Pai G <sunil.pai.g@intel.com>
---
 doc/guides/rel_notes/release_22_03.rst |  11 ++
 lib/net/rte_ip.h                       | 186 +++++++++++++++++++++++++
 2 files changed, 197 insertions(+)

diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
index 6d99d1eaa9..785fd22001 100644
--- a/doc/guides/rel_notes/release_22_03.rst
+++ b/doc/guides/rel_notes/release_22_03.rst
@@ -55,6 +55,14 @@ New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Added functions to calculate UDP/TCP checksum in mbuf.**
+
+  * Added the following functions to calculate UDP/TCP checksum of packets
+    which can be over multi-segments:
+    - ``rte_ipv4_udptcp_cksum_mbuf()``
+    - ``rte_ipv4_udptcp_cksum_mbuf_verify()``
+    - ``rte_ipv6_udptcp_cksum_mbuf()``
+    - ``rte_ipv6_udptcp_cksum_mbuf_verify()``
 
 Removed Items
 -------------
@@ -84,6 +92,9 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* net: added experimental functions ``rte_ipv4_udptcp_cksum_mbuf()``,
+  ``rte_ipv4_udptcp_cksum_mbuf_verify()``, ``rte_ipv6_udptcp_cksum_mbuf()``,
+  ``rte_ipv6_udptcp_cksum_mbuf_verify()``
 
 ABI Changes
 -----------
diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h
index c575250852..534f401d26 100644
--- a/lib/net/rte_ip.h
+++ b/lib/net/rte_ip.h
@@ -400,6 +400,65 @@ rte_ipv4_udptcp_cksum(const struct rte_ipv4_hdr *ipv4_hdr, const void *l4_hdr)
 	return cksum;
 }
 
+/**
+ * @internal Calculate the non-complemented IPv4 L4 checksum of a packet
+ */
+static inline uint16_t
+__rte_ipv4_udptcp_cksum_mbuf(const struct rte_mbuf *m,
+			     const struct rte_ipv4_hdr *ipv4_hdr,
+			     uint16_t l4_off)
+{
+	uint16_t raw_cksum;
+	uint32_t cksum;
+
+	if (l4_off > m->pkt_len)
+		return 0;
+
+	if (rte_raw_cksum_mbuf(m, l4_off, m->pkt_len - l4_off, &raw_cksum))
+		return 0;
+
+	cksum = raw_cksum + rte_ipv4_phdr_cksum(ipv4_hdr, 0);
+
+	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
+
+	return (uint16_t)cksum;
+}
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Compute the IPv4 UDP/TCP checksum of a packet.
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param ipv4_hdr
+ *   The pointer to the contiguous IPv4 header.
+ * @param l4_off
+ *   The offset in bytes to start L4 checksum.
+ * @return
+ *   The complemented checksum to set in the L4 header.
+ */
+__rte_experimental
+static inline uint16_t
+rte_ipv4_udptcp_cksum_mbuf(const struct rte_mbuf *m,
+			   const struct rte_ipv4_hdr *ipv4_hdr, uint16_t l4_off)
+{
+	uint16_t cksum = __rte_ipv4_udptcp_cksum_mbuf(m, ipv4_hdr, l4_off);
+
+	cksum = ~cksum;
+
+	/*
+	 * Per RFC 768: If the computed checksum is zero for UDP,
+	 * it is transmitted as all ones
+	 * (the equivalent in one's complement arithmetic).
+	 */
+	if (cksum == 0 && ipv4_hdr->next_proto_id == IPPROTO_UDP)
+		cksum = 0xffff;
+
+	return cksum;
+}
+
 /**
  * Validate the IPv4 UDP or TCP checksum.
  *
@@ -426,6 +485,38 @@ rte_ipv4_udptcp_cksum_verify(const struct rte_ipv4_hdr *ipv4_hdr,
 	return 0;
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Verify the IPv4 UDP/TCP checksum of a packet.
+ *
+ * In case of UDP, the caller must first check if udp_hdr->dgram_cksum is 0
+ * (i.e. no checksum).
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param ipv4_hdr
+ *   The pointer to the contiguous IPv4 header.
+ * @param l4_off
+ *   The offset in bytes to start L4 checksum.
+ * @return
+ *   Return 0 if the checksum is correct, else -1.
+ */
+__rte_experimental
+static inline uint16_t
+rte_ipv4_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
+				  const struct rte_ipv4_hdr *ipv4_hdr,
+				  uint16_t l4_off)
+{
+	uint16_t cksum = __rte_ipv4_udptcp_cksum_mbuf(m, ipv4_hdr, l4_off);
+
+	if (cksum != 0xffff)
+		return -1;
+
+	return 0;
+}
+
 /**
  * IPv6 Header
  */
@@ -538,6 +629,68 @@ rte_ipv6_udptcp_cksum(const struct rte_ipv6_hdr *ipv6_hdr, const void *l4_hdr)
 	return cksum;
 }
 
+/**
+ * @internal Calculate the non-complemented IPv6 L4 checksum of a packet
+ */
+static inline uint16_t
+__rte_ipv6_udptcp_cksum_mbuf(const struct rte_mbuf *m,
+			     const struct rte_ipv6_hdr *ipv6_hdr,
+			     uint16_t l4_off)
+{
+	uint16_t raw_cksum;
+	uint32_t cksum;
+
+	if (l4_off > m->pkt_len)
+		return 0;
+
+	if (rte_raw_cksum_mbuf(m, l4_off, m->pkt_len - l4_off, &raw_cksum))
+		return 0;
+
+	cksum = raw_cksum + rte_ipv6_phdr_cksum(ipv6_hdr, 0);
+
+	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
+
+	return (uint16_t)cksum;
+}
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Process the IPv6 UDP or TCP checksum of a packet.
+ *
+ * The IPv6 header must not be followed by extension headers. The layer 4
+ * checksum must be set to 0 in the L4 header by the caller.
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param ipv6_hdr
+ *   The pointer to the contiguous IPv6 header.
+ * @param l4_off
+ *   The offset in bytes to start L4 checksum.
+ * @return
+ *   The complemented checksum to set in the L4 header.
+ */
+__rte_experimental
+static inline uint16_t
+rte_ipv6_udptcp_cksum_mbuf(const struct rte_mbuf *m,
+			   const struct rte_ipv6_hdr *ipv6_hdr, uint16_t l4_off)
+{
+	uint16_t cksum = __rte_ipv6_udptcp_cksum_mbuf(m, ipv6_hdr, l4_off);
+
+	cksum = ~cksum;
+
+	/*
+	 * Per RFC 768: If the computed checksum is zero for UDP,
+	 * it is transmitted as all ones
+	 * (the equivalent in one's complement arithmetic).
+	 */
+	if (cksum == 0 && ipv6_hdr->proto == IPPROTO_UDP)
+		cksum = 0xffff;
+
+	return cksum;
+}
+
 /**
  * Validate the IPv6 UDP or TCP checksum.
  *
@@ -565,6 +718,39 @@ rte_ipv6_udptcp_cksum_verify(const struct rte_ipv6_hdr *ipv6_hdr,
 	return 0;
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Validate the IPv6 UDP or TCP checksum of a packet.
+ *
+ * In case of UDP, the caller must first check if udp_hdr->dgram_cksum is 0:
+ * this is either invalid or means no checksum in some situations. See 8.1
+ * (Upper-Layer Checksums) in RFC 8200.
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param ipv6_hdr
+ *   The pointer to the contiguous IPv6 header.
+ * @param l4_off
+ *   The offset in bytes to start L4 checksum.
+ * @return
+ *   Return 0 if the checksum is correct, else -1.
+ */
+__rte_experimental
+static inline int
+rte_ipv6_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
+				  const struct rte_ipv6_hdr *ipv6_hdr,
+				  uint16_t l4_off)
+{
+	uint16_t cksum = __rte_ipv6_udptcp_cksum_mbuf(m, ipv6_hdr, l4_off);
+
+	if (cksum != 0xffff)
+		return -1;
+
+	return 0;
+}
+
 /** IPv6 fragment extension header. */
 #define	RTE_IPV6_EHDR_MF_SHIFT	0
 #define	RTE_IPV6_EHDR_MF_MASK	1
-- 
2.25.1

Re: [PATCH v5 1/2] net: add functions to calculate UDP/TCP cksum in mbuf

[Ferruh Yigit]

On 1/6/2022 4:03 PM, Xiaoyun Li wrote:
> Add functions to call rte_raw_cksum_mbuf() to calculate IPv4/6
> UDP/TCP checksum in mbuf which can be over multi-segments.
> 
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> Acked-by: Aman Singh <aman.deep.singh@intel.com>
> Tested-by: Sunil Pai G <sunil.pai.g@intel.com>


Thanks Xiaoyun, patch looks good to me:
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>


Olivier,
Just reminder of this patch if you want to to review.

[PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments

[Xiaoyun Li]

In csum forwarding mode, software UDP/TCP csum calculation only takes
the first segment into account while using the whole packet length so
the calculation will read invalid memory region with multi-segments
packets and will get wrong value.
This patch fixes this issue.

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
Tested-by: Sunil Pai G <sunil.pai.g@intel.com>
---
 app/test-pmd/csumonly.c | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 2aeea243b6..0fbe1f1be7 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -96,12 +96,13 @@ struct simple_gre_hdr {
 } __rte_packed;
 
 static uint16_t
-get_udptcp_checksum(void *l3_hdr, void *l4_hdr, uint16_t ethertype)
+get_udptcp_checksum(struct rte_mbuf *m, void *l3_hdr, uint16_t l4_off,
+		    uint16_t ethertype)
 {
 	if (ethertype == _htons(RTE_ETHER_TYPE_IPV4))
-		return rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
+		return rte_ipv4_udptcp_cksum_mbuf(m, l3_hdr, l4_off);
 	else /* assume ethertype == RTE_ETHER_TYPE_IPV6 */
-		return rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
+		return rte_ipv6_udptcp_cksum_mbuf(m, l3_hdr, l4_off);
 }
 
 /* Parse an IPv4 header to fill l3_len, l4_len, and l4_proto */
@@ -460,7 +461,7 @@ parse_encap_ip(void *encap_ip, struct testpmd_offload_info *info)
  * depending on the testpmd command line configuration */
 static uint64_t
 process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
-	uint64_t tx_offloads)
+	uint64_t tx_offloads, struct rte_mbuf *m)
 {
 	struct rte_ipv4_hdr *ipv4_hdr = l3_hdr;
 	struct rte_udp_hdr *udp_hdr;
@@ -468,6 +469,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 	struct rte_sctp_hdr *sctp_hdr;
 	uint64_t ol_flags = 0;
 	uint32_t max_pkt_len, tso_segsz = 0;
+	uint16_t l4_off;
 
 	/* ensure packet is large enough to require tso */
 	if (!info->is_tunnel) {
@@ -510,9 +512,15 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 			if (tx_offloads & RTE_ETH_TX_OFFLOAD_UDP_CKSUM) {
 				ol_flags |= RTE_MBUF_F_TX_UDP_CKSUM;
 			} else {
+				if (info->is_tunnel)
+					l4_off = info->l2_len +
+						 info->outer_l3_len +
+						 info->l2_len + info->l3_len;
+				else
+					l4_off = info->l2_len +	info->l3_len;
 				udp_hdr->dgram_cksum = 0;
 				udp_hdr->dgram_cksum =
-					get_udptcp_checksum(l3_hdr, udp_hdr,
+					get_udptcp_checksum(m, l3_hdr, l4_off,
 						info->ethertype);
 			}
 		}
@@ -527,9 +535,14 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 		else if (tx_offloads & RTE_ETH_TX_OFFLOAD_TCP_CKSUM) {
 			ol_flags |= RTE_MBUF_F_TX_TCP_CKSUM;
 		} else {
+			if (info->is_tunnel)
+				l4_off = info->l2_len + info->outer_l3_len +
+					 info->l2_len + info->l3_len;
+			else
+				l4_off = info->l2_len + info->l3_len;
 			tcp_hdr->cksum = 0;
 			tcp_hdr->cksum =
-				get_udptcp_checksum(l3_hdr, tcp_hdr,
+				get_udptcp_checksum(m, l3_hdr, l4_off,
 					info->ethertype);
 		}
 #ifdef RTE_LIB_GSO
@@ -557,7 +570,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 /* Calculate the checksum of outer header */
 static uint64_t
 process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
-	uint64_t tx_offloads, int tso_enabled)
+	uint64_t tx_offloads, int tso_enabled, struct rte_mbuf *m)
 {
 	struct rte_ipv4_hdr *ipv4_hdr = outer_l3_hdr;
 	struct rte_ipv6_hdr *ipv6_hdr = outer_l3_hdr;
@@ -611,12 +624,9 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 	/* do not recalculate udp cksum if it was 0 */
 	if (udp_hdr->dgram_cksum != 0) {
 		udp_hdr->dgram_cksum = 0;
-		if (info->outer_ethertype == _htons(RTE_ETHER_TYPE_IPV4))
-			udp_hdr->dgram_cksum =
-				rte_ipv4_udptcp_cksum(ipv4_hdr, udp_hdr);
-		else
-			udp_hdr->dgram_cksum =
-				rte_ipv6_udptcp_cksum(ipv6_hdr, udp_hdr);
+		udp_hdr->dgram_cksum = get_udptcp_checksum(m, outer_l3_hdr,
+					info->l2_len + info->outer_l3_len,
+					info->outer_ethertype);
 	}
 
 	return ol_flags;
@@ -957,7 +967,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 
 		/* process checksums of inner headers first */
 		tx_ol_flags |= process_inner_cksums(l3_hdr, &info,
-			tx_offloads);
+			tx_offloads, m);
 
 		/* Then process outer headers if any. Note that the software
 		 * checksum will be wrong if one of the inner checksums is
@@ -965,7 +975,8 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		if (info.is_tunnel == 1) {
 			tx_ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
 					tx_offloads,
-					!!(tx_ol_flags & RTE_MBUF_F_TX_TCP_SEG));
+					!!(tx_ol_flags & RTE_MBUF_F_TX_TCP_SEG),
+					m);
 		}
 
 		/* step 3: fill the mbuf meta data (flags and header lengths) */
-- 
2.25.1

Re: [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments

[Ferruh Yigit]

On 1/6/2022 4:03 PM, Xiaoyun Li wrote:
> In csum forwarding mode, software UDP/TCP csum calculation only takes
> the first segment into account while using the whole packet length so
> the calculation will read invalid memory region with multi-segments
> packets and will get wrong value.
> This patch fixes this issue.
> 
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> Tested-by: Sunil Pai G <sunil.pai.g@intel.com>

Can you please check following check-git-log.sh warnings:

./devtools/check-git-log.sh -2
Wrong headline label:
         testpmd: fix l4 sw csum over multi segments
Wrong headline case:
                         "testpmd: fix l4 sw csum over multi segments": l4 --> L4
Wrong headline case:
                         "testpmd: fix l4 sw csum over multi segments": sw --> SW
Missing 'Fixes' tag:
         testpmd: fix l4 sw csum over multi segments



> ---
>   app/test-pmd/csumonly.c | 41 ++++++++++++++++++++++++++---------------
>   1 file changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> index 2aeea243b6..0fbe1f1be7 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -96,12 +96,13 @@ struct simple_gre_hdr {
>   } __rte_packed;
>   
>   static uint16_t
> -get_udptcp_checksum(void *l3_hdr, void *l4_hdr, uint16_t ethertype)
> +get_udptcp_checksum(struct rte_mbuf *m, void *l3_hdr, uint16_t l4_off,
> +		    uint16_t ethertype)
>   {
>   	if (ethertype == _htons(RTE_ETHER_TYPE_IPV4))
> -		return rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
> +		return rte_ipv4_udptcp_cksum_mbuf(m, l3_hdr, l4_off);
>   	else /* assume ethertype == RTE_ETHER_TYPE_IPV6 */
> -		return rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
> +		return rte_ipv6_udptcp_cksum_mbuf(m, l3_hdr, l4_off);
>   }
>   
>   /* Parse an IPv4 header to fill l3_len, l4_len, and l4_proto */
> @@ -460,7 +461,7 @@ parse_encap_ip(void *encap_ip, struct testpmd_offload_info *info)
>    * depending on the testpmd command line configuration */
>   static uint64_t
>   process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
> -	uint64_t tx_offloads)
> +	uint64_t tx_offloads, struct rte_mbuf *m)
>   {
>   	struct rte_ipv4_hdr *ipv4_hdr = l3_hdr;
>   	struct rte_udp_hdr *udp_hdr;
> @@ -468,6 +469,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
>   	struct rte_sctp_hdr *sctp_hdr;
>   	uint64_t ol_flags = 0;
>   	uint32_t max_pkt_len, tso_segsz = 0;
> +	uint16_t l4_off;
>   
>   	/* ensure packet is large enough to require tso */
>   	if (!info->is_tunnel) {
> @@ -510,9 +512,15 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
>   			if (tx_offloads & RTE_ETH_TX_OFFLOAD_UDP_CKSUM) {
>   				ol_flags |= RTE_MBUF_F_TX_UDP_CKSUM;
>   			} else {
> +				if (info->is_tunnel)
> +					l4_off = info->l2_len +
> +						 info->outer_l3_len +
> +						 info->l2_len + info->l3_len;
> +				else
> +					l4_off = info->l2_len +	info->l3_len;
>   				udp_hdr->dgram_cksum = 0;
>   				udp_hdr->dgram_cksum =
> -					get_udptcp_checksum(l3_hdr, udp_hdr,
> +					get_udptcp_checksum(m, l3_hdr, l4_off,
>   						info->ethertype);
>   			}
>   		}
> @@ -527,9 +535,14 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
>   		else if (tx_offloads & RTE_ETH_TX_OFFLOAD_TCP_CKSUM) {
>   			ol_flags |= RTE_MBUF_F_TX_TCP_CKSUM;
>   		} else {
> +			if (info->is_tunnel)
> +				l4_off = info->l2_len + info->outer_l3_len +
> +					 info->l2_len + info->l3_len;
> +			else
> +				l4_off = info->l2_len + info->l3_len;
>   			tcp_hdr->cksum = 0;
>   			tcp_hdr->cksum =
> -				get_udptcp_checksum(l3_hdr, tcp_hdr,
> +				get_udptcp_checksum(m, l3_hdr, l4_off,
>   					info->ethertype);
>   		}
>   #ifdef RTE_LIB_GSO
> @@ -557,7 +570,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
>   /* Calculate the checksum of outer header */
>   static uint64_t
>   process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
> -	uint64_t tx_offloads, int tso_enabled)
> +	uint64_t tx_offloads, int tso_enabled, struct rte_mbuf *m)
>   {
>   	struct rte_ipv4_hdr *ipv4_hdr = outer_l3_hdr;
>   	struct rte_ipv6_hdr *ipv6_hdr = outer_l3_hdr;
> @@ -611,12 +624,9 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
>   	/* do not recalculate udp cksum if it was 0 */
>   	if (udp_hdr->dgram_cksum != 0) {
>   		udp_hdr->dgram_cksum = 0;
> -		if (info->outer_ethertype == _htons(RTE_ETHER_TYPE_IPV4))
> -			udp_hdr->dgram_cksum =
> -				rte_ipv4_udptcp_cksum(ipv4_hdr, udp_hdr);
> -		else
> -			udp_hdr->dgram_cksum =
> -				rte_ipv6_udptcp_cksum(ipv6_hdr, udp_hdr);
> +		udp_hdr->dgram_cksum = get_udptcp_checksum(m, outer_l3_hdr,
> +					info->l2_len + info->outer_l3_len,
> +					info->outer_ethertype);
>   	}
>   
>   	return ol_flags;
> @@ -957,7 +967,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>   
>   		/* process checksums of inner headers first */
>   		tx_ol_flags |= process_inner_cksums(l3_hdr, &info,
> -			tx_offloads);
> +			tx_offloads, m);
>   
>   		/* Then process outer headers if any. Note that the software
>   		 * checksum will be wrong if one of the inner checksums is
> @@ -965,7 +975,8 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>   		if (info.is_tunnel == 1) {
>   			tx_ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
>   					tx_offloads,
> -					!!(tx_ol_flags & RTE_MBUF_F_TX_TCP_SEG));
> +					!!(tx_ol_flags & RTE_MBUF_F_TX_TCP_SEG),
> +					m);
>   		}
>   
>   		/* step 3: fill the mbuf meta data (flags and header lengths) */

RE: [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Friday, January 21, 2022 15:17
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Singh, Aman Deep
> <aman.deep.singh@intel.com>; olivier.matz@6wind.com;
> mb@smartsharesystems.com; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> Medvedkin, Vladimir <vladimir.medvedkin@intel.com>
> Cc: dev@dpdk.org; Pai G, Sunil <sunil.pai.g@intel.com>
> Subject: Re: [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments
> 
> On 1/6/2022 4:03 PM, Xiaoyun Li wrote:
> > In csum forwarding mode, software UDP/TCP csum calculation only takes
> > the first segment into account while using the whole packet length so
> > the calculation will read invalid memory region with multi-segments
> > packets and will get wrong value.
> > This patch fixes this issue.
> >
> > Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> > Tested-by: Sunil Pai G <sunil.pai.g@intel.com>
> 
> Can you please check following check-git-log.sh warnings:
> 
> ./devtools/check-git-log.sh -2
> Wrong headline label:
>          testpmd: fix l4 sw csum over multi segments Wrong headline case:
>                          "testpmd: fix l4 sw csum over multi segments": l4 --> L4 Wrong
> headline case:
>                          "testpmd: fix l4 sw csum over multi segments": sw --> SW
> Missing 'Fixes' tag:
>          testpmd: fix l4 sw csum over multi segments
> 

Will fix this in next version.
BTW, should I change the patch name to "app/testpmd: enable L4 SW csum over multi segments"? or ignore the fixline complaining?

> 
> 
> > ---
> >   app/test-pmd/csumonly.c | 41 ++++++++++++++++++++++++++-----------
> ----
> >   1 file changed, 26 insertions(+), 15 deletions(-)
> >
> > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> > 2aeea243b6..0fbe1f1be7 100644
> > --- a/app/test-pmd/csumonly.c
> > +++ b/app/test-pmd/csumonly.c
> > @@ -96,12 +96,13 @@ struct simple_gre_hdr {
> >   } __rte_packed;
> >
> >   static uint16_t
> > -get_udptcp_checksum(void *l3_hdr, void *l4_hdr, uint16_t ethertype)
> > +get_udptcp_checksum(struct rte_mbuf *m, void *l3_hdr, uint16_t l4_off,
> > +		    uint16_t ethertype)
> >   {
> >   	if (ethertype == _htons(RTE_ETHER_TYPE_IPV4))
> > -		return rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
> > +		return rte_ipv4_udptcp_cksum_mbuf(m, l3_hdr, l4_off);
> >   	else /* assume ethertype == RTE_ETHER_TYPE_IPV6 */
> > -		return rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
> > +		return rte_ipv6_udptcp_cksum_mbuf(m, l3_hdr, l4_off);
> >   }
> >
> >   /* Parse an IPv4 header to fill l3_len, l4_len, and l4_proto */ @@
> > -460,7 +461,7 @@ parse_encap_ip(void *encap_ip, struct
> testpmd_offload_info *info)
> >    * depending on the testpmd command line configuration */
> >   static uint64_t
> >   process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info
> *info,
> > -	uint64_t tx_offloads)
> > +	uint64_t tx_offloads, struct rte_mbuf *m)
> >   {
> >   	struct rte_ipv4_hdr *ipv4_hdr = l3_hdr;
> >   	struct rte_udp_hdr *udp_hdr;
> > @@ -468,6 +469,7 @@ process_inner_cksums(void *l3_hdr, const struct
> testpmd_offload_info *info,
> >   	struct rte_sctp_hdr *sctp_hdr;
> >   	uint64_t ol_flags = 0;
> >   	uint32_t max_pkt_len, tso_segsz = 0;
> > +	uint16_t l4_off;
> >
> >   	/* ensure packet is large enough to require tso */
> >   	if (!info->is_tunnel) {
> > @@ -510,9 +512,15 @@ process_inner_cksums(void *l3_hdr, const struct
> testpmd_offload_info *info,
> >   			if (tx_offloads &
> RTE_ETH_TX_OFFLOAD_UDP_CKSUM) {
> >   				ol_flags |= RTE_MBUF_F_TX_UDP_CKSUM;
> >   			} else {
> > +				if (info->is_tunnel)
> > +					l4_off = info->l2_len +
> > +						 info->outer_l3_len +
> > +						 info->l2_len + info->l3_len;
> > +				else
> > +					l4_off = info->l2_len +	info->l3_len;
> >   				udp_hdr->dgram_cksum = 0;
> >   				udp_hdr->dgram_cksum =
> > -					get_udptcp_checksum(l3_hdr,
> udp_hdr,
> > +					get_udptcp_checksum(m, l3_hdr,
> l4_off,
> >   						info->ethertype);
> >   			}
> >   		}
> > @@ -527,9 +535,14 @@ process_inner_cksums(void *l3_hdr, const struct
> testpmd_offload_info *info,
> >   		else if (tx_offloads & RTE_ETH_TX_OFFLOAD_TCP_CKSUM) {
> >   			ol_flags |= RTE_MBUF_F_TX_TCP_CKSUM;
> >   		} else {
> > +			if (info->is_tunnel)
> > +				l4_off = info->l2_len + info->outer_l3_len +
> > +					 info->l2_len + info->l3_len;
> > +			else
> > +				l4_off = info->l2_len + info->l3_len;
> >   			tcp_hdr->cksum = 0;
> >   			tcp_hdr->cksum =
> > -				get_udptcp_checksum(l3_hdr, tcp_hdr,
> > +				get_udptcp_checksum(m, l3_hdr, l4_off,
> >   					info->ethertype);
> >   		}
> >   #ifdef RTE_LIB_GSO
> > @@ -557,7 +570,7 @@ process_inner_cksums(void *l3_hdr, const struct
> testpmd_offload_info *info,
> >   /* Calculate the checksum of outer header */
> >   static uint64_t
> >   process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info
> *info,
> > -	uint64_t tx_offloads, int tso_enabled)
> > +	uint64_t tx_offloads, int tso_enabled, struct rte_mbuf *m)
> >   {
> >   	struct rte_ipv4_hdr *ipv4_hdr = outer_l3_hdr;
> >   	struct rte_ipv6_hdr *ipv6_hdr = outer_l3_hdr; @@ -611,12 +624,9
> @@
> > process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info
> *info,
> >   	/* do not recalculate udp cksum if it was 0 */
> >   	if (udp_hdr->dgram_cksum != 0) {
> >   		udp_hdr->dgram_cksum = 0;
> > -		if (info->outer_ethertype == _htons(RTE_ETHER_TYPE_IPV4))
> > -			udp_hdr->dgram_cksum =
> > -				rte_ipv4_udptcp_cksum(ipv4_hdr, udp_hdr);
> > -		else
> > -			udp_hdr->dgram_cksum =
> > -				rte_ipv6_udptcp_cksum(ipv6_hdr, udp_hdr);
> > +		udp_hdr->dgram_cksum = get_udptcp_checksum(m,
> outer_l3_hdr,
> > +					info->l2_len + info->outer_l3_len,
> > +					info->outer_ethertype);
> >   	}
> >
> >   	return ol_flags;
> > @@ -957,7 +967,7 @@ pkt_burst_checksum_forward(struct fwd_stream
> *fs)
> >
> >   		/* process checksums of inner headers first */
> >   		tx_ol_flags |= process_inner_cksums(l3_hdr, &info,
> > -			tx_offloads);
> > +			tx_offloads, m);
> >
> >   		/* Then process outer headers if any. Note that the software
> >   		 * checksum will be wrong if one of the inner checksums is
> @@
> > -965,7 +975,8 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> >   		if (info.is_tunnel == 1) {
> >   			tx_ol_flags |= process_outer_cksums(outer_l3_hdr,
> &info,
> >   					tx_offloads,
> > -					!!(tx_ol_flags &
> RTE_MBUF_F_TX_TCP_SEG));
> > +					!!(tx_ol_flags &
> RTE_MBUF_F_TX_TCP_SEG),
> > +					m);
> >   		}
> >
> >   		/* step 3: fill the mbuf meta data (flags and header lengths)
> */

Re: [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments

[Ferruh Yigit]

On 1/24/2022 9:43 AM, Li, Xiaoyun wrote:
> Hi
> 
>> -----Original Message-----
>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>> Sent: Friday, January 21, 2022 15:17
>> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Singh, Aman Deep
>> <aman.deep.singh@intel.com>; olivier.matz@6wind.com;
>> mb@smartsharesystems.com; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
>> Medvedkin, Vladimir <vladimir.medvedkin@intel.com>
>> Cc: dev@dpdk.org; Pai G, Sunil <sunil.pai.g@intel.com>
>> Subject: Re: [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments
>>
>> On 1/6/2022 4:03 PM, Xiaoyun Li wrote:
>>> In csum forwarding mode, software UDP/TCP csum calculation only takes
>>> the first segment into account while using the whole packet length so
>>> the calculation will read invalid memory region with multi-segments
>>> packets and will get wrong value.
>>> This patch fixes this issue.
>>>
>>> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
>>> Tested-by: Sunil Pai G <sunil.pai.g@intel.com>
>>
>> Can you please check following check-git-log.sh warnings:
>>
>> ./devtools/check-git-log.sh -2
>> Wrong headline label:
>>           testpmd: fix l4 sw csum over multi segments Wrong headline case:
>>                           "testpmd: fix l4 sw csum over multi segments": l4 --> L4 Wrong
>> headline case:
>>                           "testpmd: fix l4 sw csum over multi segments": sw --> SW
>> Missing 'Fixes' tag:
>>           testpmd: fix l4 sw csum over multi segments
>>
> 
> Will fix this in next version.
> BTW, should I change the patch name to "app/testpmd: enable L4 SW csum over multi segments"? or ignore the fixline complaining?
> 

Please change the patch title, and it would be good to provide a fix patch,
thanks.

>>
>>
>>> ---
>>>    app/test-pmd/csumonly.c | 41 ++++++++++++++++++++++++++-----------
>> ----
>>>    1 file changed, 26 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
>>> 2aeea243b6..0fbe1f1be7 100644
>>> --- a/app/test-pmd/csumonly.c
>>> +++ b/app/test-pmd/csumonly.c
>>> @@ -96,12 +96,13 @@ struct simple_gre_hdr {
>>>    } __rte_packed;
>>>
>>>    static uint16_t
>>> -get_udptcp_checksum(void *l3_hdr, void *l4_hdr, uint16_t ethertype)
>>> +get_udptcp_checksum(struct rte_mbuf *m, void *l3_hdr, uint16_t l4_off,
>>> +		    uint16_t ethertype)
>>>    {
>>>    	if (ethertype == _htons(RTE_ETHER_TYPE_IPV4))
>>> -		return rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
>>> +		return rte_ipv4_udptcp_cksum_mbuf(m, l3_hdr, l4_off);
>>>    	else /* assume ethertype == RTE_ETHER_TYPE_IPV6 */
>>> -		return rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
>>> +		return rte_ipv6_udptcp_cksum_mbuf(m, l3_hdr, l4_off);
>>>    }
>>>
>>>    /* Parse an IPv4 header to fill l3_len, l4_len, and l4_proto */ @@
>>> -460,7 +461,7 @@ parse_encap_ip(void *encap_ip, struct
>> testpmd_offload_info *info)
>>>     * depending on the testpmd command line configuration */
>>>    static uint64_t
>>>    process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info
>> *info,
>>> -	uint64_t tx_offloads)
>>> +	uint64_t tx_offloads, struct rte_mbuf *m)
>>>    {
>>>    	struct rte_ipv4_hdr *ipv4_hdr = l3_hdr;
>>>    	struct rte_udp_hdr *udp_hdr;
>>> @@ -468,6 +469,7 @@ process_inner_cksums(void *l3_hdr, const struct
>> testpmd_offload_info *info,
>>>    	struct rte_sctp_hdr *sctp_hdr;
>>>    	uint64_t ol_flags = 0;
>>>    	uint32_t max_pkt_len, tso_segsz = 0;
>>> +	uint16_t l4_off;
>>>
>>>    	/* ensure packet is large enough to require tso */
>>>    	if (!info->is_tunnel) {
>>> @@ -510,9 +512,15 @@ process_inner_cksums(void *l3_hdr, const struct
>> testpmd_offload_info *info,
>>>    			if (tx_offloads &
>> RTE_ETH_TX_OFFLOAD_UDP_CKSUM) {
>>>    				ol_flags |= RTE_MBUF_F_TX_UDP_CKSUM;
>>>    			} else {
>>> +				if (info->is_tunnel)
>>> +					l4_off = info->l2_len +
>>> +						 info->outer_l3_len +
>>> +						 info->l2_len + info->l3_len;
>>> +				else
>>> +					l4_off = info->l2_len +	info->l3_len;
>>>    				udp_hdr->dgram_cksum = 0;
>>>    				udp_hdr->dgram_cksum =
>>> -					get_udptcp_checksum(l3_hdr,
>> udp_hdr,
>>> +					get_udptcp_checksum(m, l3_hdr,
>> l4_off,
>>>    						info->ethertype);
>>>    			}
>>>    		}
>>> @@ -527,9 +535,14 @@ process_inner_cksums(void *l3_hdr, const struct
>> testpmd_offload_info *info,
>>>    		else if (tx_offloads & RTE_ETH_TX_OFFLOAD_TCP_CKSUM) {
>>>    			ol_flags |= RTE_MBUF_F_TX_TCP_CKSUM;
>>>    		} else {
>>> +			if (info->is_tunnel)
>>> +				l4_off = info->l2_len + info->outer_l3_len +
>>> +					 info->l2_len + info->l3_len;
>>> +			else
>>> +				l4_off = info->l2_len + info->l3_len;
>>>    			tcp_hdr->cksum = 0;
>>>    			tcp_hdr->cksum =
>>> -				get_udptcp_checksum(l3_hdr, tcp_hdr,
>>> +				get_udptcp_checksum(m, l3_hdr, l4_off,
>>>    					info->ethertype);
>>>    		}
>>>    #ifdef RTE_LIB_GSO
>>> @@ -557,7 +570,7 @@ process_inner_cksums(void *l3_hdr, const struct
>> testpmd_offload_info *info,
>>>    /* Calculate the checksum of outer header */
>>>    static uint64_t
>>>    process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info
>> *info,
>>> -	uint64_t tx_offloads, int tso_enabled)
>>> +	uint64_t tx_offloads, int tso_enabled, struct rte_mbuf *m)
>>>    {
>>>    	struct rte_ipv4_hdr *ipv4_hdr = outer_l3_hdr;
>>>    	struct rte_ipv6_hdr *ipv6_hdr = outer_l3_hdr; @@ -611,12 +624,9
>> @@
>>> process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info
>> *info,
>>>    	/* do not recalculate udp cksum if it was 0 */
>>>    	if (udp_hdr->dgram_cksum != 0) {
>>>    		udp_hdr->dgram_cksum = 0;
>>> -		if (info->outer_ethertype == _htons(RTE_ETHER_TYPE_IPV4))
>>> -			udp_hdr->dgram_cksum =
>>> -				rte_ipv4_udptcp_cksum(ipv4_hdr, udp_hdr);
>>> -		else
>>> -			udp_hdr->dgram_cksum =
>>> -				rte_ipv6_udptcp_cksum(ipv6_hdr, udp_hdr);
>>> +		udp_hdr->dgram_cksum = get_udptcp_checksum(m,
>> outer_l3_hdr,
>>> +					info->l2_len + info->outer_l3_len,
>>> +					info->outer_ethertype);
>>>    	}
>>>
>>>    	return ol_flags;
>>> @@ -957,7 +967,7 @@ pkt_burst_checksum_forward(struct fwd_stream
>> *fs)
>>>
>>>    		/* process checksums of inner headers first */
>>>    		tx_ol_flags |= process_inner_cksums(l3_hdr, &info,
>>> -			tx_offloads);
>>> +			tx_offloads, m);
>>>
>>>    		/* Then process outer headers if any. Note that the software
>>>    		 * checksum will be wrong if one of the inner checksums is
>> @@
>>> -965,7 +975,8 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>>>    		if (info.is_tunnel == 1) {
>>>    			tx_ol_flags |= process_outer_cksums(outer_l3_hdr,
>> &info,
>>>    					tx_offloads,
>>> -					!!(tx_ol_flags &
>> RTE_MBUF_F_TX_TCP_SEG));
>>> +					!!(tx_ol_flags &
>> RTE_MBUF_F_TX_TCP_SEG),
>>> +					m);
>>>    		}
>>>
>>>    		/* step 3: fill the mbuf meta data (flags and header lengths)
>> */
> 

Re: [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments

[Kevin Traynor]

On 06/01/2022 16:03, Xiaoyun Li wrote:
> In csum forwarding mode, software UDP/TCP csum calculation only takes
> the first segment into account while using the whole packet length so
> the calculation will read invalid memory region with multi-segments
> packets and will get wrong value.
> This patch fixes this issue.

There should be a 'Fixes:' tag and assuming you want this patch 
backported to LTS, 'Cc: stable@dpdk.org' too.

Re: [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments

[Ferruh Yigit]

On 1/21/2022 5:09 PM, Kevin Traynor wrote:
> On 06/01/2022 16:03, Xiaoyun Li wrote:
>> In csum forwarding mode, software UDP/TCP csum calculation only takes
>> the first segment into account while using the whole packet length so
>> the calculation will read invalid memory region with multi-segments
>> packets and will get wrong value.
>> This patch fixes this issue.
> 
> There should be a 'Fixes:' tag and assuming you want this patch backported to LTS, 'Cc: stable@dpdk.org' too.
> 

It can't be, because fix relies on new APIs added in this release (patch 1/2).

Re: [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments

[Kevin Traynor]

On 24/01/2022 09:16, Ferruh Yigit wrote:
> On 1/21/2022 5:09 PM, Kevin Traynor wrote:
>> On 06/01/2022 16:03, Xiaoyun Li wrote:
>>> In csum forwarding mode, software UDP/TCP csum calculation only takes
>>> the first segment into account while using the whole packet length so
>>> the calculation will read invalid memory region with multi-segments
>>> packets and will get wrong value.
>>> This patch fixes this issue.
>>
>> There should be a 'Fixes:' tag and assuming you want this patch backported to LTS, 'Cc: stable@dpdk.org' too.
>>
> 
> It can't be, because fix relies on new APIs added in this release (patch 1/2).
> 

ok, makes sense, thanks. Do you think it worth adding a Bz for it and 
listing it as a known issue in 21.11.1?

Re: [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments

[Ferruh Yigit]

On 1/24/2022 10:30 AM, Kevin Traynor wrote:
> On 24/01/2022 09:16, Ferruh Yigit wrote:
>> On 1/21/2022 5:09 PM, Kevin Traynor wrote:
>>> On 06/01/2022 16:03, Xiaoyun Li wrote:
>>>> In csum forwarding mode, software UDP/TCP csum calculation only takes
>>>> the first segment into account while using the whole packet length so
>>>> the calculation will read invalid memory region with multi-segments
>>>> packets and will get wrong value.
>>>> This patch fixes this issue.
>>>
>>> There should be a 'Fixes:' tag and assuming you want this patch backported to LTS, 'Cc: stable@dpdk.org' too.
>>>
>>
>> It can't be, because fix relies on new APIs added in this release (patch 1/2).
>>
> 
> ok, makes sense, thanks. Do you think it worth adding a Bz for it and listing it as a known issue in 21.11.1?
> 

Was discussing with Xialong, it is in the gray area that if this is a fix
or new feature.

It is not the case that initial implementation says that multi segment
mbufs are supported, in that case this is not a fix but enabling multi
segment support.

So perhaps better to update commit log in this way to prevent confusion
for the fixing and fix tag..

[PATCH v6 0/2] Add functions to calculate UDP/TCP cksum in mbuf

[Xiaoyun Li]

Added functions to calculate UDP/TCP checksum for packets which may be
over multi-segments and called the functions in testpmd csum forwarding
mode to support UDP/TCP sotfware checksum over multi-segments.

Xiaoyun Li (2):
  net: add functions to calculate UDP/TCP cksum in mbuf
  app/testpmd: enable L4 SW csum over multi segments

---
v6:
 * Fixed testpmd patch name issue.
 * Added release note for enabling L4 SW checksum in testpmd.
 * Added Ferruh's Acked-by tag.
v5:
 * Fixed commit log intendation issue.
 * Removed added inline experimental APIs in version.map to fix windows
 * linkage issue.
 * Added Sunil's Tested-by tag and Aman's Acked-by tag.
v4:
 * Called rte_raw_cksum_mbuf() to calculate cksum in lib instead of
 * implementing it in testpmd for better maintenance.
 * Removed fix tag for testpmd since it relies on the lib change.
v3:
 * Use rte_raw_cksum() for multi-segs case instead of copying the whole
 * packet.
v2:
 * Use static stack memory instead of dynamic allocating in datapath.
---

 app/test-pmd/csumonly.c                |  41 ++++--
 doc/guides/rel_notes/release_22_03.rst |  13 ++
 lib/net/rte_ip.h                       | 186 +++++++++++++++++++++++++
 3 files changed, 225 insertions(+), 15 deletions(-)

-- 
2.25.1

[PATCH v6 1/2] net: add functions to calculate UDP/TCP cksum in mbuf

[Xiaoyun Li]

Add functions to call rte_raw_cksum_mbuf() to calculate IPv4/6
UDP/TCP checksum in mbuf which can be over multi-segments.

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
Acked-by: Aman Singh <aman.deep.singh@intel.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
Tested-by: Sunil Pai G <sunil.pai.g@intel.com>
---
 doc/guides/rel_notes/release_22_03.rst |  11 ++
 lib/net/rte_ip.h                       | 186 +++++++++++++++++++++++++
 2 files changed, 197 insertions(+)

diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
index 6d99d1eaa9..785fd22001 100644
--- a/doc/guides/rel_notes/release_22_03.rst
+++ b/doc/guides/rel_notes/release_22_03.rst
@@ -55,6 +55,14 @@ New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Added functions to calculate UDP/TCP checksum in mbuf.**
+
+  * Added the following functions to calculate UDP/TCP checksum of packets
+    which can be over multi-segments:
+    - ``rte_ipv4_udptcp_cksum_mbuf()``
+    - ``rte_ipv4_udptcp_cksum_mbuf_verify()``
+    - ``rte_ipv6_udptcp_cksum_mbuf()``
+    - ``rte_ipv6_udptcp_cksum_mbuf_verify()``
 
 Removed Items
 -------------
@@ -84,6 +92,9 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* net: added experimental functions ``rte_ipv4_udptcp_cksum_mbuf()``,
+  ``rte_ipv4_udptcp_cksum_mbuf_verify()``, ``rte_ipv6_udptcp_cksum_mbuf()``,
+  ``rte_ipv6_udptcp_cksum_mbuf_verify()``
 
 ABI Changes
 -----------
diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h
index c575250852..534f401d26 100644
--- a/lib/net/rte_ip.h
+++ b/lib/net/rte_ip.h
@@ -400,6 +400,65 @@ rte_ipv4_udptcp_cksum(const struct rte_ipv4_hdr *ipv4_hdr, const void *l4_hdr)
 	return cksum;
 }
 
+/**
+ * @internal Calculate the non-complemented IPv4 L4 checksum of a packet
+ */
+static inline uint16_t
+__rte_ipv4_udptcp_cksum_mbuf(const struct rte_mbuf *m,
+			     const struct rte_ipv4_hdr *ipv4_hdr,
+			     uint16_t l4_off)
+{
+	uint16_t raw_cksum;
+	uint32_t cksum;
+
+	if (l4_off > m->pkt_len)
+		return 0;
+
+	if (rte_raw_cksum_mbuf(m, l4_off, m->pkt_len - l4_off, &raw_cksum))
+		return 0;
+
+	cksum = raw_cksum + rte_ipv4_phdr_cksum(ipv4_hdr, 0);
+
+	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
+
+	return (uint16_t)cksum;
+}
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Compute the IPv4 UDP/TCP checksum of a packet.
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param ipv4_hdr
+ *   The pointer to the contiguous IPv4 header.
+ * @param l4_off
+ *   The offset in bytes to start L4 checksum.
+ * @return
+ *   The complemented checksum to set in the L4 header.
+ */
+__rte_experimental
+static inline uint16_t
+rte_ipv4_udptcp_cksum_mbuf(const struct rte_mbuf *m,
+			   const struct rte_ipv4_hdr *ipv4_hdr, uint16_t l4_off)
+{
+	uint16_t cksum = __rte_ipv4_udptcp_cksum_mbuf(m, ipv4_hdr, l4_off);
+
+	cksum = ~cksum;
+
+	/*
+	 * Per RFC 768: If the computed checksum is zero for UDP,
+	 * it is transmitted as all ones
+	 * (the equivalent in one's complement arithmetic).
+	 */
+	if (cksum == 0 && ipv4_hdr->next_proto_id == IPPROTO_UDP)
+		cksum = 0xffff;
+
+	return cksum;
+}
+
 /**
  * Validate the IPv4 UDP or TCP checksum.
  *
@@ -426,6 +485,38 @@ rte_ipv4_udptcp_cksum_verify(const struct rte_ipv4_hdr *ipv4_hdr,
 	return 0;
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Verify the IPv4 UDP/TCP checksum of a packet.
+ *
+ * In case of UDP, the caller must first check if udp_hdr->dgram_cksum is 0
+ * (i.e. no checksum).
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param ipv4_hdr
+ *   The pointer to the contiguous IPv4 header.
+ * @param l4_off
+ *   The offset in bytes to start L4 checksum.
+ * @return
+ *   Return 0 if the checksum is correct, else -1.
+ */
+__rte_experimental
+static inline uint16_t
+rte_ipv4_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
+				  const struct rte_ipv4_hdr *ipv4_hdr,
+				  uint16_t l4_off)
+{
+	uint16_t cksum = __rte_ipv4_udptcp_cksum_mbuf(m, ipv4_hdr, l4_off);
+
+	if (cksum != 0xffff)
+		return -1;
+
+	return 0;
+}
+
 /**
  * IPv6 Header
  */
@@ -538,6 +629,68 @@ rte_ipv6_udptcp_cksum(const struct rte_ipv6_hdr *ipv6_hdr, const void *l4_hdr)
 	return cksum;
 }
 
+/**
+ * @internal Calculate the non-complemented IPv6 L4 checksum of a packet
+ */
+static inline uint16_t
+__rte_ipv6_udptcp_cksum_mbuf(const struct rte_mbuf *m,
+			     const struct rte_ipv6_hdr *ipv6_hdr,
+			     uint16_t l4_off)
+{
+	uint16_t raw_cksum;
+	uint32_t cksum;
+
+	if (l4_off > m->pkt_len)
+		return 0;
+
+	if (rte_raw_cksum_mbuf(m, l4_off, m->pkt_len - l4_off, &raw_cksum))
+		return 0;
+
+	cksum = raw_cksum + rte_ipv6_phdr_cksum(ipv6_hdr, 0);
+
+	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
+
+	return (uint16_t)cksum;
+}
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Process the IPv6 UDP or TCP checksum of a packet.
+ *
+ * The IPv6 header must not be followed by extension headers. The layer 4
+ * checksum must be set to 0 in the L4 header by the caller.
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param ipv6_hdr
+ *   The pointer to the contiguous IPv6 header.
+ * @param l4_off
+ *   The offset in bytes to start L4 checksum.
+ * @return
+ *   The complemented checksum to set in the L4 header.
+ */
+__rte_experimental
+static inline uint16_t
+rte_ipv6_udptcp_cksum_mbuf(const struct rte_mbuf *m,
+			   const struct rte_ipv6_hdr *ipv6_hdr, uint16_t l4_off)
+{
+	uint16_t cksum = __rte_ipv6_udptcp_cksum_mbuf(m, ipv6_hdr, l4_off);
+
+	cksum = ~cksum;
+
+	/*
+	 * Per RFC 768: If the computed checksum is zero for UDP,
+	 * it is transmitted as all ones
+	 * (the equivalent in one's complement arithmetic).
+	 */
+	if (cksum == 0 && ipv6_hdr->proto == IPPROTO_UDP)
+		cksum = 0xffff;
+
+	return cksum;
+}
+
 /**
  * Validate the IPv6 UDP or TCP checksum.
  *
@@ -565,6 +718,39 @@ rte_ipv6_udptcp_cksum_verify(const struct rte_ipv6_hdr *ipv6_hdr,
 	return 0;
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Validate the IPv6 UDP or TCP checksum of a packet.
+ *
+ * In case of UDP, the caller must first check if udp_hdr->dgram_cksum is 0:
+ * this is either invalid or means no checksum in some situations. See 8.1
+ * (Upper-Layer Checksums) in RFC 8200.
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param ipv6_hdr
+ *   The pointer to the contiguous IPv6 header.
+ * @param l4_off
+ *   The offset in bytes to start L4 checksum.
+ * @return
+ *   Return 0 if the checksum is correct, else -1.
+ */
+__rte_experimental
+static inline int
+rte_ipv6_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
+				  const struct rte_ipv6_hdr *ipv6_hdr,
+				  uint16_t l4_off)
+{
+	uint16_t cksum = __rte_ipv6_udptcp_cksum_mbuf(m, ipv6_hdr, l4_off);
+
+	if (cksum != 0xffff)
+		return -1;
+
+	return 0;
+}
+
 /** IPv6 fragment extension header. */
 #define	RTE_IPV6_EHDR_MF_SHIFT	0
 #define	RTE_IPV6_EHDR_MF_MASK	1
-- 
2.25.1

Re: [PATCH v6 1/2] net: add functions to calculate UDP/TCP cksum in mbuf

[Ferruh Yigit]

On 1/24/2022 12:28 PM, Xiaoyun Li wrote:
> Add functions to call rte_raw_cksum_mbuf() to calculate IPv4/6
> UDP/TCP checksum in mbuf which can be over multi-segments.
> 
> Signed-off-by: Xiaoyun Li<xiaoyun.li@intel.com>
> Acked-by: Aman Singh<aman.deep.singh@intel.com>
> Acked-by: Ferruh Yigit<ferruh.yigit@intel.com>
> Tested-by: Sunil Pai G<sunil.pai.g@intel.com>

Hi Olivier,

I am planning to proceed with the patch if you don't have any objection.

[PATCH v6 2/2] app/testpmd: enable L4 SW csum over multi segments

[Xiaoyun Li]

Csum forwarding mode only supports software UDP/TCP csum calculation
for single segment packets when hardware offload is not enabled.
This patch enables software UDP/TCP csum calculation over multiple
segments.

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
Tested-by: Sunil Pai G <sunil.pai.g@intel.com>
---
 app/test-pmd/csumonly.c                | 41 ++++++++++++++++----------
 doc/guides/rel_notes/release_22_03.rst |  2 ++
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 2aeea243b6..0fbe1f1be7 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -96,12 +96,13 @@ struct simple_gre_hdr {
 } __rte_packed;
 
 static uint16_t
-get_udptcp_checksum(void *l3_hdr, void *l4_hdr, uint16_t ethertype)
+get_udptcp_checksum(struct rte_mbuf *m, void *l3_hdr, uint16_t l4_off,
+		    uint16_t ethertype)
 {
 	if (ethertype == _htons(RTE_ETHER_TYPE_IPV4))
-		return rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
+		return rte_ipv4_udptcp_cksum_mbuf(m, l3_hdr, l4_off);
 	else /* assume ethertype == RTE_ETHER_TYPE_IPV6 */
-		return rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
+		return rte_ipv6_udptcp_cksum_mbuf(m, l3_hdr, l4_off);
 }
 
 /* Parse an IPv4 header to fill l3_len, l4_len, and l4_proto */
@@ -460,7 +461,7 @@ parse_encap_ip(void *encap_ip, struct testpmd_offload_info *info)
  * depending on the testpmd command line configuration */
 static uint64_t
 process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
-	uint64_t tx_offloads)
+	uint64_t tx_offloads, struct rte_mbuf *m)
 {
 	struct rte_ipv4_hdr *ipv4_hdr = l3_hdr;
 	struct rte_udp_hdr *udp_hdr;
@@ -468,6 +469,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 	struct rte_sctp_hdr *sctp_hdr;
 	uint64_t ol_flags = 0;
 	uint32_t max_pkt_len, tso_segsz = 0;
+	uint16_t l4_off;
 
 	/* ensure packet is large enough to require tso */
 	if (!info->is_tunnel) {
@@ -510,9 +512,15 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 			if (tx_offloads & RTE_ETH_TX_OFFLOAD_UDP_CKSUM) {
 				ol_flags |= RTE_MBUF_F_TX_UDP_CKSUM;
 			} else {
+				if (info->is_tunnel)
+					l4_off = info->l2_len +
+						 info->outer_l3_len +
+						 info->l2_len + info->l3_len;
+				else
+					l4_off = info->l2_len +	info->l3_len;
 				udp_hdr->dgram_cksum = 0;
 				udp_hdr->dgram_cksum =
-					get_udptcp_checksum(l3_hdr, udp_hdr,
+					get_udptcp_checksum(m, l3_hdr, l4_off,
 						info->ethertype);
 			}
 		}
@@ -527,9 +535,14 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 		else if (tx_offloads & RTE_ETH_TX_OFFLOAD_TCP_CKSUM) {
 			ol_flags |= RTE_MBUF_F_TX_TCP_CKSUM;
 		} else {
+			if (info->is_tunnel)
+				l4_off = info->l2_len + info->outer_l3_len +
+					 info->l2_len + info->l3_len;
+			else
+				l4_off = info->l2_len + info->l3_len;
 			tcp_hdr->cksum = 0;
 			tcp_hdr->cksum =
-				get_udptcp_checksum(l3_hdr, tcp_hdr,
+				get_udptcp_checksum(m, l3_hdr, l4_off,
 					info->ethertype);
 		}
 #ifdef RTE_LIB_GSO
@@ -557,7 +570,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 /* Calculate the checksum of outer header */
 static uint64_t
 process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
-	uint64_t tx_offloads, int tso_enabled)
+	uint64_t tx_offloads, int tso_enabled, struct rte_mbuf *m)
 {
 	struct rte_ipv4_hdr *ipv4_hdr = outer_l3_hdr;
 	struct rte_ipv6_hdr *ipv6_hdr = outer_l3_hdr;
@@ -611,12 +624,9 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 	/* do not recalculate udp cksum if it was 0 */
 	if (udp_hdr->dgram_cksum != 0) {
 		udp_hdr->dgram_cksum = 0;
-		if (info->outer_ethertype == _htons(RTE_ETHER_TYPE_IPV4))
-			udp_hdr->dgram_cksum =
-				rte_ipv4_udptcp_cksum(ipv4_hdr, udp_hdr);
-		else
-			udp_hdr->dgram_cksum =
-				rte_ipv6_udptcp_cksum(ipv6_hdr, udp_hdr);
+		udp_hdr->dgram_cksum = get_udptcp_checksum(m, outer_l3_hdr,
+					info->l2_len + info->outer_l3_len,
+					info->outer_ethertype);
 	}
 
 	return ol_flags;
@@ -957,7 +967,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 
 		/* process checksums of inner headers first */
 		tx_ol_flags |= process_inner_cksums(l3_hdr, &info,
-			tx_offloads);
+			tx_offloads, m);
 
 		/* Then process outer headers if any. Note that the software
 		 * checksum will be wrong if one of the inner checksums is
@@ -965,7 +975,8 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		if (info.is_tunnel == 1) {
 			tx_ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
 					tx_offloads,
-					!!(tx_ol_flags & RTE_MBUF_F_TX_TCP_SEG));
+					!!(tx_ol_flags & RTE_MBUF_F_TX_TCP_SEG),
+					m);
 		}
 
 		/* step 3: fill the mbuf meta data (flags and header lengths) */
diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
index 785fd22001..3c57555535 100644
--- a/doc/guides/rel_notes/release_22_03.rst
+++ b/doc/guides/rel_notes/release_22_03.rst
@@ -63,6 +63,8 @@ New Features
     - ``rte_ipv4_udptcp_cksum_mbuf_verify()``
     - ``rte_ipv6_udptcp_cksum_mbuf()``
     - ``rte_ipv6_udptcp_cksum_mbuf_verify()``
+  * Called ``rte_ipv4/6_udptcp_cksum_mbuf()`` functions in testpmd csum mode
+    to support software UDP/TCP checksum over multiple segments.
 
 Removed Items
 -------------
-- 
2.25.1

Re: [PATCH v6 2/2] app/testpmd: enable L4 SW csum over multi segments

[Ferruh Yigit]

On 1/24/2022 12:28 PM, Xiaoyun Li wrote:
> Csum forwarding mode only supports software UDP/TCP csum calculation
> for single segment packets when hardware offload is not enabled.
> This patch enables software UDP/TCP csum calculation over multiple
> segments.
> 
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> Tested-by: Sunil Pai G <sunil.pai.g@intel.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [PATCH v6 0/2] Add functions to calculate UDP/TCP cksum in mbuf

[Ferruh Yigit]

On 1/24/2022 12:28 PM, Xiaoyun Li wrote:
> Added functions to calculate UDP/TCP checksum for packets which may be
> over multi-segments and called the functions in testpmd csum forwarding
> mode to support UDP/TCP sotfware checksum over multi-segments.
> 
> Xiaoyun Li (2):
>    net: add functions to calculate UDP/TCP cksum in mbuf
>    app/testpmd: enable L4 SW csum over multi segments

Series applied to dpdk-next-net/main, thanks.

[dpdk-dev] [PATCH v3] app/testpmd: fix l4 sw csum over multi segments

[Xiaoyun Li]

In csum forwarding mode, software UDP/TCP csum calculation only takes
the first segment into account while using the whole packet length so
the calculation will read invalid memory region with multi-segments
packets and will get wrong value.
This patch fixes this issue.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
v3:
 * Use rte_raw_cksum() for multi-segs case instead of copying the whole
 * packet.
v2:
 * Use static stack memory instead of dynamic allocating in datapath
---
 app/test-pmd/csumonly.c | 68 ++++++++++++++++++++++++++++++++---------
 1 file changed, 53 insertions(+), 15 deletions(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 090797318a..f3e60eb3c3 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -91,12 +91,41 @@ struct simple_gre_hdr {
 } __rte_packed;
 
 static uint16_t
-get_udptcp_checksum(void *l3_hdr, void *l4_hdr, uint16_t ethertype)
+get_udptcp_checksum(void *l3_hdr, struct rte_mbuf *m, uint16_t l4_off,
+		    uint16_t ethertype)
 {
+	uint16_t off = l4_off;
+	uint32_t cksum = 0;
+	char *buf;
+
+	while (m != NULL) {
+		buf = rte_pktmbuf_mtod_offset(m, char *, off);
+		cksum += rte_raw_cksum(buf, m->data_len - off);
+		off = 0;
+		m = m->next;
+	}
 	if (ethertype == _htons(RTE_ETHER_TYPE_IPV4))
-		return rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
+		cksum += rte_ipv4_phdr_cksum(l3_hdr, 0);
 	else /* assume ethertype == RTE_ETHER_TYPE_IPV6 */
-		return rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
+		cksum += rte_ipv6_phdr_cksum(l3_hdr, 0);
+
+	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
+	cksum = (~cksum) & 0xffff;
+
+	/*
+	 * Per RFC 768:If the computed checksum is zero for UDP,
+	 * it is transmitted as all ones
+	 * (the equivalent in one's complement arithmetic).
+	 */
+	if (cksum == 0 && ethertype == _htons(RTE_ETHER_TYPE_IPV4) &&
+	    ((struct rte_ipv4_hdr *)l3_hdr)->next_proto_id == IPPROTO_UDP)
+		cksum = 0xffff;
+
+	if (cksum == 0 && ethertype == _htons(RTE_ETHER_TYPE_IPV6) &&
+	    ((struct rte_ipv6_hdr *)l3_hdr)->proto == IPPROTO_UDP)
+		cksum = 0xffff;
+
+	return (uint16_t)cksum;
 }
 
 /* Parse an IPv4 header to fill l3_len, l4_len, and l4_proto */
@@ -455,7 +484,7 @@ parse_encap_ip(void *encap_ip, struct testpmd_offload_info *info)
  * depending on the testpmd command line configuration */
 static uint64_t
 process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
-	uint64_t tx_offloads)
+	uint64_t tx_offloads, struct rte_mbuf *m)
 {
 	struct rte_ipv4_hdr *ipv4_hdr = l3_hdr;
 	struct rte_udp_hdr *udp_hdr;
@@ -463,6 +492,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 	struct rte_sctp_hdr *sctp_hdr;
 	uint64_t ol_flags = 0;
 	uint32_t max_pkt_len, tso_segsz = 0;
+	uint16_t l4_off;
 
 	/* ensure packet is large enough to require tso */
 	if (!info->is_tunnel) {
@@ -505,9 +535,15 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 			if (tx_offloads & DEV_TX_OFFLOAD_UDP_CKSUM) {
 				ol_flags |= PKT_TX_UDP_CKSUM;
 			} else {
+				if (info->is_tunnel)
+					l4_off = info->l2_len +
+						info->outer_l3_len +
+						info->l2_len + info->l3_len;
+				else
+					l4_off = info->l2_len +	info->l3_len;
 				udp_hdr->dgram_cksum = 0;
 				udp_hdr->dgram_cksum =
-					get_udptcp_checksum(l3_hdr, udp_hdr,
+					get_udptcp_checksum(l3_hdr, m, l4_off,
 						info->ethertype);
 			}
 		}
@@ -520,9 +556,14 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 		else if (tx_offloads & DEV_TX_OFFLOAD_TCP_CKSUM) {
 			ol_flags |= PKT_TX_TCP_CKSUM;
 		} else {
+			if (info->is_tunnel)
+				l4_off = info->l2_len + info->outer_l3_len +
+					info->l2_len + info->l3_len;
+			else
+				l4_off = info->l2_len + info->l3_len;
 			tcp_hdr->cksum = 0;
 			tcp_hdr->cksum =
-				get_udptcp_checksum(l3_hdr, tcp_hdr,
+				get_udptcp_checksum(l3_hdr, m, l4_off,
 					info->ethertype);
 		}
 		if (info->gso_enable)
@@ -548,7 +589,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 /* Calculate the checksum of outer header */
 static uint64_t
 process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
-	uint64_t tx_offloads, int tso_enabled)
+	uint64_t tx_offloads, int tso_enabled, struct rte_mbuf *m)
 {
 	struct rte_ipv4_hdr *ipv4_hdr = outer_l3_hdr;
 	struct rte_ipv6_hdr *ipv6_hdr = outer_l3_hdr;
@@ -602,12 +643,9 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 	/* do not recalculate udp cksum if it was 0 */
 	if (udp_hdr->dgram_cksum != 0) {
 		udp_hdr->dgram_cksum = 0;
-		if (info->outer_ethertype == _htons(RTE_ETHER_TYPE_IPV4))
-			udp_hdr->dgram_cksum =
-				rte_ipv4_udptcp_cksum(ipv4_hdr, udp_hdr);
-		else
-			udp_hdr->dgram_cksum =
-				rte_ipv6_udptcp_cksum(ipv6_hdr, udp_hdr);
+		udp_hdr->dgram_cksum = get_udptcp_checksum(outer_l3_hdr,
+					m, info->l2_len + info->outer_l3_len,
+					info->outer_ethertype);
 	}
 
 	return ol_flags;
@@ -942,7 +980,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 
 		/* process checksums of inner headers first */
 		tx_ol_flags |= process_inner_cksums(l3_hdr, &info,
-			tx_offloads);
+			tx_offloads, m);
 
 		/* Then process outer headers if any. Note that the software
 		 * checksum will be wrong if one of the inner checksums is
@@ -950,7 +988,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		if (info.is_tunnel == 1) {
 			tx_ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
 					tx_offloads,
-					!!(tx_ol_flags & PKT_TX_TCP_SEG));
+					!!(tx_ol_flags & PKT_TX_TCP_SEG), m);
 		}
 
 		/* step 3: fill the mbuf meta data (flags and header lengths) */
-- 
2.25.1