* [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments @ 2021-10-15 5:13 Xiaoyun Li 2021-10-15 8:09 ` David Marchand ` (6 more replies) 0 siblings, 7 replies; 39+ messages in thread From: Xiaoyun Li @ 2021-10-15 5:13 UTC (permalink / raw) To: ferruh.yigit; +Cc: dev, Xiaoyun Li, stable 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, ð_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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments 2021-10-15 5:13 [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments Xiaoyun Li @ 2021-10-15 8:09 ` David Marchand 2021-10-18 2:02 ` Li, Xiaoyun 2021-10-18 2:16 ` [dpdk-dev] [PATCH v2] " Xiaoyun Li ` (5 subsequent siblings) 6 siblings, 1 reply; 39+ messages in thread From: David Marchand @ 2021-10-15 8:09 UTC (permalink / raw) To: Xiaoyun Li; +Cc: Yigit, Ferruh, dev, dpdk stable 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, > ð_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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments 2021-10-15 8:09 ` David Marchand @ 2021-10-18 2:02 ` Li, Xiaoyun 0 siblings, 0 replies; 39+ messages in thread From: Li, Xiaoyun @ 2021-10-18 2:02 UTC (permalink / raw) To: David Marchand; +Cc: Yigit, Ferruh, dev, dpdk stable 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, > > ð_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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [dpdk-dev] [PATCH v2] app/testpmd: fix l4 sw csum over multi segments 2021-10-15 5:13 [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments Xiaoyun Li 2021-10-15 8:09 ` David Marchand @ 2021-10-18 2:16 ` Xiaoyun Li 2021-10-18 3:00 ` [dpdk-dev] [PATCH] " Stephen Hemminger ` (4 subsequent siblings) 6 siblings, 0 replies; 39+ messages in thread From: Xiaoyun Li @ 2021-10-18 2:16 UTC (permalink / raw) To: ferruh.yigit, david.marchand; +Cc: dev, Xiaoyun Li, stable 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, ð_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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments 2021-10-15 5:13 [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments Xiaoyun Li 2021-10-15 8:09 ` David Marchand 2021-10-18 2:16 ` [dpdk-dev] [PATCH v2] " Xiaoyun Li @ 2021-10-18 3:00 ` Stephen Hemminger 2021-10-18 3:16 ` Li, Xiaoyun 2021-10-20 10:12 ` [dpdk-dev] [PATCH v3] " Xiaoyun Li ` (3 subsequent siblings) 6 siblings, 1 reply; 39+ messages in thread From: Stephen Hemminger @ 2021-10-18 3:00 UTC (permalink / raw) To: Xiaoyun Li; +Cc: ferruh.yigit, dev, stable 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments 2021-10-18 3:00 ` [dpdk-dev] [PATCH] " Stephen Hemminger @ 2021-10-18 3:16 ` Li, Xiaoyun 2021-10-18 4:40 ` Li, Xiaoyun 2021-10-18 10:15 ` Ananyev, Konstantin 0 siblings, 2 replies; 39+ messages in thread From: Li, Xiaoyun @ 2021-10-18 3:16 UTC (permalink / raw) To: Stephen Hemminger; +Cc: Yigit, Ferruh, dev, stable 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments 2021-10-18 3:16 ` Li, Xiaoyun @ 2021-10-18 4:40 ` Li, Xiaoyun 2021-10-18 10:15 ` Ananyev, Konstantin 1 sibling, 0 replies; 39+ messages in thread From: Li, Xiaoyun @ 2021-10-18 4:40 UTC (permalink / raw) To: Li, Xiaoyun, Stephen Hemminger; +Cc: Yigit, Ferruh, dev, stable > -----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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments 2021-10-18 3:16 ` Li, Xiaoyun 2021-10-18 4:40 ` Li, Xiaoyun @ 2021-10-18 10:15 ` Ananyev, Konstantin 2021-10-19 1:54 ` Li, Xiaoyun 1 sibling, 1 reply; 39+ messages in thread From: Ananyev, Konstantin @ 2021-10-18 10:15 UTC (permalink / raw) To: Li, Xiaoyun, Stephen Hemminger; +Cc: Yigit, Ferruh, dev, stable > > > + /* 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments 2021-10-18 10:15 ` Ananyev, Konstantin @ 2021-10-19 1:54 ` Li, Xiaoyun 0 siblings, 0 replies; 39+ messages in thread From: Li, Xiaoyun @ 2021-10-19 1:54 UTC (permalink / raw) To: Ananyev, Konstantin, Stephen Hemminger; +Cc: Yigit, Ferruh, dev, stable > -----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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [dpdk-dev] [PATCH v3] app/testpmd: fix l4 sw csum over multi segments 2021-10-15 5:13 [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments Xiaoyun Li ` (2 preceding siblings ...) 2021-10-18 3:00 ` [dpdk-dev] [PATCH] " Stephen Hemminger @ 2021-10-20 10:12 ` Xiaoyun Li 2021-10-27 10:48 ` Ferruh Yigit 2021-12-03 11:38 ` [PATCH v4 0/2] Add functions to calculate UDP/TCP cksum in mbuf Xiaoyun Li ` (2 subsequent siblings) 6 siblings, 1 reply; 39+ messages in thread From: Xiaoyun Li @ 2021-10-20 10:12 UTC (permalink / raw) To: konstantin.ananyev, stephen, ferruh.yigit; +Cc: dev, Xiaoyun Li, stable 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [dpdk-dev] [PATCH v3] app/testpmd: fix l4 sw csum over multi segments 2021-10-20 10:12 ` [dpdk-dev] [PATCH v3] " Xiaoyun Li @ 2021-10-27 10:48 ` Ferruh Yigit 2021-10-27 11:29 ` Morten Brørup 0 siblings, 1 reply; 39+ messages in thread From: Ferruh Yigit @ 2021-10-27 10:48 UTC (permalink / raw) To: Xiaoyun Li, konstantin.ananyev, stephen, Olivier Matz Cc: dev, stable, Vladimir Medvedkin, Morten Brørup 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? ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [dpdk-dev] [PATCH v3] app/testpmd: fix l4 sw csum over multi segments 2021-10-27 10:48 ` Ferruh Yigit @ 2021-10-27 11:29 ` Morten Brørup 2021-10-29 8:29 ` Olivier Matz 0 siblings, 1 reply; 39+ messages in thread From: Morten Brørup @ 2021-10-27 11:29 UTC (permalink / raw) To: Ferruh Yigit, Xiaoyun Li, konstantin.ananyev, stephen, Olivier Matz Cc: dev, stable, Vladimir Medvedkin > 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? ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [dpdk-dev] [PATCH v3] app/testpmd: fix l4 sw csum over multi segments 2021-10-27 11:29 ` Morten Brørup @ 2021-10-29 8:29 ` Olivier Matz 2021-12-03 11:31 ` Li, Xiaoyun 0 siblings, 1 reply; 39+ messages in thread From: Olivier Matz @ 2021-10-29 8:29 UTC (permalink / raw) To: Morten Brørup Cc: Ferruh Yigit, Xiaoyun Li, konstantin.ananyev, stephen, dev, stable, Vladimir Medvedkin 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* RE: [dpdk-dev] [PATCH v3] app/testpmd: fix l4 sw csum over multi segments 2021-10-29 8:29 ` Olivier Matz @ 2021-12-03 11:31 ` Li, Xiaoyun 0 siblings, 0 replies; 39+ messages in thread From: Li, Xiaoyun @ 2021-12-03 11:31 UTC (permalink / raw) To: Olivier Matz, Morten Brørup Cc: Yigit, Ferruh, Ananyev, Konstantin, stephen, dev, stable, Medvedkin, Vladimir 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v4 0/2] Add functions to calculate UDP/TCP cksum in mbuf 2021-10-15 5:13 [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments Xiaoyun Li ` (3 preceding siblings ...) 2021-10-20 10:12 ` [dpdk-dev] [PATCH v3] " Xiaoyun Li @ 2021-12-03 11:38 ` Xiaoyun Li 2021-12-03 11:38 ` [PATCH v4 1/2] net: add " Xiaoyun Li ` (2 more replies) 2022-01-06 16:03 ` [PATCH v5 " Xiaoyun Li 2022-01-24 12:28 ` [PATCH v6 0/2] Add functions to calculate UDP/TCP cksum in mbuf Xiaoyun Li 6 siblings, 3 replies; 39+ messages in thread From: Xiaoyun Li @ 2021-12-03 11:38 UTC (permalink / raw) To: ferruh.yigit, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v4 1/2] net: add functions to calculate UDP/TCP cksum in mbuf 2021-12-03 11:38 ` [PATCH v4 0/2] Add functions to calculate UDP/TCP cksum in mbuf Xiaoyun Li @ 2021-12-03 11:38 ` Xiaoyun Li 2021-12-15 11:33 ` Singh, Aman Deep 2021-12-03 11:38 ` [PATCH v4 2/2] testpmd: fix l4 sw csum over multi segments Xiaoyun Li 2021-12-08 6:10 ` [PATCH v4 0/2] Add functions to calculate UDP/TCP cksum in mbuf Pai G, Sunil 2 siblings, 1 reply; 39+ messages in thread From: Xiaoyun Li @ 2021-12-03 11:38 UTC (permalink / raw) To: ferruh.yigit, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 1/2] net: add functions to calculate UDP/TCP cksum in mbuf 2021-12-03 11:38 ` [PATCH v4 1/2] net: add " Xiaoyun Li @ 2021-12-15 11:33 ` Singh, Aman Deep 2022-01-04 15:18 ` Li, Xiaoyun 0 siblings, 1 reply; 39+ messages in thread From: Singh, Aman Deep @ 2021-12-15 11:33 UTC (permalink / raw) To: Xiaoyun Li, ferruh.yigit, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev 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; > +}; ^ permalink raw reply [flat|nested] 39+ messages in thread
* RE: [PATCH v4 1/2] net: add functions to calculate UDP/TCP cksum in mbuf 2021-12-15 11:33 ` Singh, Aman Deep @ 2022-01-04 15:18 ` Li, Xiaoyun 2022-01-04 15:40 ` Li, Xiaoyun 0 siblings, 1 reply; 39+ messages in thread From: Li, Xiaoyun @ 2022-01-04 15:18 UTC (permalink / raw) To: Singh, Aman Deep, Yigit, Ferruh, olivier.matz, mb, Ananyev, Konstantin, stephen, Medvedkin, Vladimir Cc: dev 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; > > +}; ^ permalink raw reply [flat|nested] 39+ messages in thread
* RE: [PATCH v4 1/2] net: add functions to calculate UDP/TCP cksum in mbuf 2022-01-04 15:18 ` Li, Xiaoyun @ 2022-01-04 15:40 ` Li, Xiaoyun 2022-01-06 12:56 ` Singh, Aman Deep 0 siblings, 1 reply; 39+ messages in thread From: Li, Xiaoyun @ 2022-01-04 15:40 UTC (permalink / raw) To: Li, Xiaoyun, Singh, Aman Deep, Yigit, Ferruh, olivier.matz, mb, Ananyev, Konstantin, stephen, Medvedkin, Vladimir Cc: dev 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; > > > +}; ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v4 1/2] net: add functions to calculate UDP/TCP cksum in mbuf 2022-01-04 15:40 ` Li, Xiaoyun @ 2022-01-06 12:56 ` Singh, Aman Deep 0 siblings, 0 replies; 39+ messages in thread From: Singh, Aman Deep @ 2022-01-06 12:56 UTC (permalink / raw) To: Li, Xiaoyun, Yigit, Ferruh, olivier.matz, mb, Ananyev, Konstantin, stephen, Medvedkin, Vladimir Cc: dev [-- 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 --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v4 2/2] testpmd: fix l4 sw csum over multi segments 2021-12-03 11:38 ` [PATCH v4 0/2] Add functions to calculate UDP/TCP cksum in mbuf Xiaoyun Li 2021-12-03 11:38 ` [PATCH v4 1/2] net: add " Xiaoyun Li @ 2021-12-03 11:38 ` Xiaoyun Li 2021-12-08 6:10 ` [PATCH v4 0/2] Add functions to calculate UDP/TCP cksum in mbuf Pai G, Sunil 2 siblings, 0 replies; 39+ messages in thread From: Xiaoyun Li @ 2021-12-03 11:38 UTC (permalink / raw) To: ferruh.yigit, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* RE: [PATCH v4 0/2] Add functions to calculate UDP/TCP cksum in mbuf 2021-12-03 11:38 ` [PATCH v4 0/2] Add functions to calculate UDP/TCP cksum in mbuf Xiaoyun Li 2021-12-03 11:38 ` [PATCH v4 1/2] net: add " Xiaoyun Li 2021-12-03 11:38 ` [PATCH v4 2/2] testpmd: fix l4 sw csum over multi segments Xiaoyun Li @ 2021-12-08 6:10 ` Pai G, Sunil 2 siblings, 0 replies; 39+ messages in thread From: Pai G, Sunil @ 2021-12-08 6:10 UTC (permalink / raw) To: Li, Xiaoyun, Yigit, Ferruh, olivier.matz, mb, Ananyev, Konstantin, stephen, Medvedkin, Vladimir Cc: dev, Li, Xiaoyun 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> ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 0/2] Add functions to calculate UDP/TCP cksum in mbuf 2021-10-15 5:13 [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments Xiaoyun Li ` (4 preceding siblings ...) 2021-12-03 11:38 ` [PATCH v4 0/2] Add functions to calculate UDP/TCP cksum in mbuf Xiaoyun Li @ 2022-01-06 16:03 ` Xiaoyun Li 2022-01-06 16:03 ` [PATCH v5 1/2] net: add " Xiaoyun Li 2022-01-06 16:03 ` [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments Xiaoyun Li 2022-01-24 12:28 ` [PATCH v6 0/2] Add functions to calculate UDP/TCP cksum in mbuf Xiaoyun Li 6 siblings, 2 replies; 39+ messages in thread From: Xiaoyun Li @ 2022-01-06 16:03 UTC (permalink / raw) To: Aman.Deep.Singh, ferruh.yigit, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 1/2] net: add functions to calculate UDP/TCP cksum in mbuf 2022-01-06 16:03 ` [PATCH v5 " Xiaoyun Li @ 2022-01-06 16:03 ` Xiaoyun Li 2022-01-21 15:16 ` Ferruh Yigit 2022-01-06 16:03 ` [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments Xiaoyun Li 1 sibling, 1 reply; 39+ messages in thread From: Xiaoyun Li @ 2022-01-06 16:03 UTC (permalink / raw) To: Aman.Deep.Singh, ferruh.yigit, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, Xiaoyun Li, Aman Singh, Sunil Pai G 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 1/2] net: add functions to calculate UDP/TCP cksum in mbuf 2022-01-06 16:03 ` [PATCH v5 1/2] net: add " Xiaoyun Li @ 2022-01-21 15:16 ` Ferruh Yigit 0 siblings, 0 replies; 39+ messages in thread From: Ferruh Yigit @ 2022-01-21 15:16 UTC (permalink / raw) To: Xiaoyun Li, Aman.Deep.Singh, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, Sunil Pai G 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments 2022-01-06 16:03 ` [PATCH v5 " Xiaoyun Li 2022-01-06 16:03 ` [PATCH v5 1/2] net: add " Xiaoyun Li @ 2022-01-06 16:03 ` Xiaoyun Li 2022-01-21 15:16 ` Ferruh Yigit 2022-01-21 17:09 ` Kevin Traynor 1 sibling, 2 replies; 39+ messages in thread From: Xiaoyun Li @ 2022-01-06 16:03 UTC (permalink / raw) To: Aman.Deep.Singh, ferruh.yigit, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, Xiaoyun Li, Sunil Pai G 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments 2022-01-06 16:03 ` [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments Xiaoyun Li @ 2022-01-21 15:16 ` Ferruh Yigit 2022-01-24 9:43 ` Li, Xiaoyun 2022-01-21 17:09 ` Kevin Traynor 1 sibling, 1 reply; 39+ messages in thread From: Ferruh Yigit @ 2022-01-21 15:16 UTC (permalink / raw) To: Xiaoyun Li, Aman.Deep.Singh, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, Sunil Pai G 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) */ ^ permalink raw reply [flat|nested] 39+ messages in thread
* RE: [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments 2022-01-21 15:16 ` Ferruh Yigit @ 2022-01-24 9:43 ` Li, Xiaoyun 2022-01-24 10:16 ` Ferruh Yigit 0 siblings, 1 reply; 39+ messages in thread From: Li, Xiaoyun @ 2022-01-24 9:43 UTC (permalink / raw) To: Yigit, Ferruh, Singh, Aman Deep, olivier.matz, mb, Ananyev, Konstantin, stephen, Medvedkin, Vladimir Cc: dev, Pai G, Sunil 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) > */ ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments 2022-01-24 9:43 ` Li, Xiaoyun @ 2022-01-24 10:16 ` Ferruh Yigit 0 siblings, 0 replies; 39+ messages in thread From: Ferruh Yigit @ 2022-01-24 10:16 UTC (permalink / raw) To: Li, Xiaoyun, Singh, Aman Deep, olivier.matz, mb, Ananyev, Konstantin, stephen, Medvedkin, Vladimir Cc: dev, Pai G, Sunil 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) >> */ > ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments 2022-01-06 16:03 ` [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments Xiaoyun Li 2022-01-21 15:16 ` Ferruh Yigit @ 2022-01-21 17:09 ` Kevin Traynor 2022-01-24 9:16 ` Ferruh Yigit 1 sibling, 1 reply; 39+ messages in thread From: Kevin Traynor @ 2022-01-21 17:09 UTC (permalink / raw) To: Xiaoyun Li, Aman.Deep.Singh, ferruh.yigit, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, Sunil Pai G 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments 2022-01-21 17:09 ` Kevin Traynor @ 2022-01-24 9:16 ` Ferruh Yigit 2022-01-24 10:30 ` Kevin Traynor 0 siblings, 1 reply; 39+ messages in thread From: Ferruh Yigit @ 2022-01-24 9:16 UTC (permalink / raw) To: Kevin Traynor, Xiaoyun Li, Aman.Deep.Singh, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, Sunil Pai G 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). ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments 2022-01-24 9:16 ` Ferruh Yigit @ 2022-01-24 10:30 ` Kevin Traynor 2022-01-24 11:02 ` Ferruh Yigit 0 siblings, 1 reply; 39+ messages in thread From: Kevin Traynor @ 2022-01-24 10:30 UTC (permalink / raw) To: Ferruh Yigit, Xiaoyun Li, Aman.Deep.Singh, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, Sunil Pai G 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? ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments 2022-01-24 10:30 ` Kevin Traynor @ 2022-01-24 11:02 ` Ferruh Yigit 0 siblings, 0 replies; 39+ messages in thread From: Ferruh Yigit @ 2022-01-24 11:02 UTC (permalink / raw) To: Kevin Traynor, Xiaoyun Li, Aman.Deep.Singh, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, Sunil Pai G 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.. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v6 0/2] Add functions to calculate UDP/TCP cksum in mbuf 2021-10-15 5:13 [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments Xiaoyun Li ` (5 preceding siblings ...) 2022-01-06 16:03 ` [PATCH v5 " Xiaoyun Li @ 2022-01-24 12:28 ` Xiaoyun Li 2022-01-24 12:28 ` [PATCH v6 1/2] net: add " Xiaoyun Li ` (2 more replies) 6 siblings, 3 replies; 39+ messages in thread From: Xiaoyun Li @ 2022-01-24 12:28 UTC (permalink / raw) To: ktraynor, Aman.Deep.Singh, ferruh.yigit, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v6 1/2] net: add functions to calculate UDP/TCP cksum in mbuf 2022-01-24 12:28 ` [PATCH v6 0/2] Add functions to calculate UDP/TCP cksum in mbuf Xiaoyun Li @ 2022-01-24 12:28 ` Xiaoyun Li 2022-02-03 12:41 ` Ferruh Yigit 2022-01-24 12:28 ` [PATCH v6 2/2] app/testpmd: enable L4 SW csum over multi segments Xiaoyun Li 2022-02-04 13:12 ` [PATCH v6 0/2] Add functions to calculate UDP/TCP cksum in mbuf Ferruh Yigit 2 siblings, 1 reply; 39+ messages in thread From: Xiaoyun Li @ 2022-01-24 12:28 UTC (permalink / raw) To: ktraynor, Aman.Deep.Singh, ferruh.yigit, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, Xiaoyun Li, Aman Singh, Sunil Pai G 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v6 1/2] net: add functions to calculate UDP/TCP cksum in mbuf 2022-01-24 12:28 ` [PATCH v6 1/2] net: add " Xiaoyun Li @ 2022-02-03 12:41 ` Ferruh Yigit 0 siblings, 0 replies; 39+ messages in thread From: Ferruh Yigit @ 2022-02-03 12:41 UTC (permalink / raw) To: Xiaoyun Li, ktraynor, Aman.Deep.Singh, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, Sunil Pai G 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v6 2/2] app/testpmd: enable L4 SW csum over multi segments 2022-01-24 12:28 ` [PATCH v6 0/2] Add functions to calculate UDP/TCP cksum in mbuf Xiaoyun Li 2022-01-24 12:28 ` [PATCH v6 1/2] net: add " Xiaoyun Li @ 2022-01-24 12:28 ` Xiaoyun Li 2022-02-04 13:12 ` Ferruh Yigit 2022-02-04 13:12 ` [PATCH v6 0/2] Add functions to calculate UDP/TCP cksum in mbuf Ferruh Yigit 2 siblings, 1 reply; 39+ messages in thread From: Xiaoyun Li @ 2022-01-24 12:28 UTC (permalink / raw) To: ktraynor, Aman.Deep.Singh, ferruh.yigit, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, Xiaoyun Li, Sunil Pai G 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v6 2/2] app/testpmd: enable L4 SW csum over multi segments 2022-01-24 12:28 ` [PATCH v6 2/2] app/testpmd: enable L4 SW csum over multi segments Xiaoyun Li @ 2022-02-04 13:12 ` Ferruh Yigit 0 siblings, 0 replies; 39+ messages in thread From: Ferruh Yigit @ 2022-02-04 13:12 UTC (permalink / raw) To: Xiaoyun Li, ktraynor, Aman.Deep.Singh, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev, Sunil Pai G 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> ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v6 0/2] Add functions to calculate UDP/TCP cksum in mbuf 2022-01-24 12:28 ` [PATCH v6 0/2] Add functions to calculate UDP/TCP cksum in mbuf Xiaoyun Li 2022-01-24 12:28 ` [PATCH v6 1/2] net: add " Xiaoyun Li 2022-01-24 12:28 ` [PATCH v6 2/2] app/testpmd: enable L4 SW csum over multi segments Xiaoyun Li @ 2022-02-04 13:12 ` Ferruh Yigit 2 siblings, 0 replies; 39+ messages in thread From: Ferruh Yigit @ 2022-02-04 13:12 UTC (permalink / raw) To: Xiaoyun Li, ktraynor, Aman.Deep.Singh, olivier.matz, mb, konstantin.ananyev, stephen, vladimir.medvedkin Cc: dev 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2022-02-04 13:13 UTC | newest] Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-15 5:13 [dpdk-dev] [PATCH] app/testpmd: fix l4 sw csum over multi segments Xiaoyun Li 2021-10-15 8:09 ` David Marchand 2021-10-18 2:02 ` Li, Xiaoyun 2021-10-18 2:16 ` [dpdk-dev] [PATCH v2] " Xiaoyun Li 2021-10-18 3:00 ` [dpdk-dev] [PATCH] " Stephen Hemminger 2021-10-18 3:16 ` Li, Xiaoyun 2021-10-18 4:40 ` Li, Xiaoyun 2021-10-18 10:15 ` Ananyev, Konstantin 2021-10-19 1:54 ` Li, Xiaoyun 2021-10-20 10:12 ` [dpdk-dev] [PATCH v3] " Xiaoyun Li 2021-10-27 10:48 ` Ferruh Yigit 2021-10-27 11:29 ` Morten Brørup 2021-10-29 8:29 ` Olivier Matz 2021-12-03 11:31 ` Li, Xiaoyun 2021-12-03 11:38 ` [PATCH v4 0/2] Add functions to calculate UDP/TCP cksum in mbuf Xiaoyun Li 2021-12-03 11:38 ` [PATCH v4 1/2] net: add " Xiaoyun Li 2021-12-15 11:33 ` Singh, Aman Deep 2022-01-04 15:18 ` Li, Xiaoyun 2022-01-04 15:40 ` Li, Xiaoyun 2022-01-06 12:56 ` Singh, Aman Deep 2021-12-03 11:38 ` [PATCH v4 2/2] testpmd: fix l4 sw csum over multi segments Xiaoyun Li 2021-12-08 6:10 ` [PATCH v4 0/2] Add functions to calculate UDP/TCP cksum in mbuf Pai G, Sunil 2022-01-06 16:03 ` [PATCH v5 " Xiaoyun Li 2022-01-06 16:03 ` [PATCH v5 1/2] net: add " Xiaoyun Li 2022-01-21 15:16 ` Ferruh Yigit 2022-01-06 16:03 ` [PATCH v5 2/2] testpmd: fix l4 sw csum over multi segments Xiaoyun Li 2022-01-21 15:16 ` Ferruh Yigit 2022-01-24 9:43 ` Li, Xiaoyun 2022-01-24 10:16 ` Ferruh Yigit 2022-01-21 17:09 ` Kevin Traynor 2022-01-24 9:16 ` Ferruh Yigit 2022-01-24 10:30 ` Kevin Traynor 2022-01-24 11:02 ` Ferruh Yigit 2022-01-24 12:28 ` [PATCH v6 0/2] Add functions to calculate UDP/TCP cksum in mbuf Xiaoyun Li 2022-01-24 12:28 ` [PATCH v6 1/2] net: add " Xiaoyun Li 2022-02-03 12:41 ` Ferruh Yigit 2022-01-24 12:28 ` [PATCH v6 2/2] app/testpmd: enable L4 SW csum over multi segments Xiaoyun Li 2022-02-04 13:12 ` Ferruh Yigit 2022-02-04 13:12 ` [PATCH v6 0/2] Add functions to calculate UDP/TCP cksum in mbuf Ferruh Yigit
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).