<div dir="ltr">Hi Stephen, I've revised the patch per your feedback and sent out v4: net/cksum: compute raw cksum for several segments. Your review is appreciated.</div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Tue, Jun 9, 2026 at 1:02 AM Stephen Hemminger <<a href="mailto:stephen@networkplumber.org">stephen@networkplumber.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon,  4 Aug 2025 11:54:30 +0800<br>
Su Sai <<a href="mailto:spiderdetective.ss@gmail.com" target="_blank">spiderdetective.ss@gmail.com</a>> wrote:<br>
<br>
> The rte_raw_cksum_mbuf function is used to compute<br>
> the raw checksum of a packet.<br>
> If the packet payload stored in multi mbuf, the function<br>
> will goto the hard case. In hard case,<br>
> the variable 'tmp' is a type of uint32_t,<br>
> so rte_bswap16 will drop high 16 bit.<br>
> Meanwhile, the variable 'sum' is a type of uint32_t,<br>
> so 'sum += tmp' will drop the carry when overflow.<br>
> Both drop will make cksum incorrect.<br>
> This commit fixes the above bug.<br>
> <br>
> Signed-off-by: Su Sai <<a href="mailto:spiderdetective.ss@gmail.com" target="_blank">spiderdetective.ss@gmail.com</a>><br>
> ---<br>
>  .mailmap              |   1 +<br>
>  app/test/test_cksum.c | 106 ++++++++++++++++++++++++++++++++++++++++++<br>
>  lib/net/rte_cksum.h   |  27 +++++++++--<br>
>  3 files changed, 130 insertions(+), 4 deletions(-)<br>
> <br>
> diff --git a/.mailmap b/.mailmap<br>
> index 34a99f93a1..1da1d9f8e1 100644<br>
> --- a/.mailmap<br>
> +++ b/.mailmap<br>
> @@ -1552,6 +1552,7 @@ Sunil Kumar Kori <<a href="mailto:skori@marvell.com" target="_blank">skori@marvell.com</a>> <<a href="mailto:sunil.kori@nxp.com" target="_blank">sunil.kori@nxp.com</a>><br>
>  Sunil Pai G <<a href="mailto:sunil.pai.g@intel.com" target="_blank">sunil.pai.g@intel.com</a>><br>
>  Sunil Uttarwar <<a href="mailto:sunilprakashrao.uttarwar@amd.com" target="_blank">sunilprakashrao.uttarwar@amd.com</a>><br>
>  Sun Jiajia <<a href="mailto:sunx.jiajia@intel.com" target="_blank">sunx.jiajia@intel.com</a>><br>
> +Su Sai <<a href="mailto:spiderdetective.ss@gmail.com" target="_blank">spiderdetective.ss@gmail.com</a>> <<a href="mailto:susai.ss@bytedance.com" target="_blank">susai.ss@bytedance.com</a>><br>
>  Sunyang Wu <<a href="mailto:sunyang.wu@jaguarmicro.com" target="_blank">sunyang.wu@jaguarmicro.com</a>><br>
>  Surabhi Boob <<a href="mailto:surabhi.boob@intel.com" target="_blank">surabhi.boob@intel.com</a>><br>
>  Suyang Ju <<a href="mailto:sju@paloaltonetworks.com" target="_blank">sju@paloaltonetworks.com</a>><br>
> diff --git a/app/test/test_cksum.c b/app/test/test_cksum.c<br>
> index f2ab5af5a7..fb2e3cf9e6 100644<br>
> --- a/app/test/test_cksum.c<br>
> +++ b/app/test/test_cksum.c<br>
> @@ -85,6 +85,42 @@ static const char test_cksum_ipv4_opts_udp[] = {<br>
>       0x00, 0x35, 0x00, 0x09, 0x89, 0x6f, 0x78,<br>
>  };<br>
>  <br>
> +/*<br>
> + * generated in scapy with<br>
> + * Ether()/IP()/TCP(options=[NOP,NOP,Timestamps])/os.urandom(113))<br>
> + */<br>
> +static const char test_cksum_ipv4_tcp_multi_segs[] = {<br>
> +     0x00, 0x16, 0x3e, 0x0b, 0x6b, 0xd2, 0xee, 0xff,<br>
> +     0xff, 0xff, 0xff, 0xff, 0x08, 0x00, 0x45, 0x00,<br>
> +     0x00, 0xa5, 0x46, 0x10, 0x40, 0x00, 0x40, 0x06,<br>
> +     0x80, 0xb5, 0xc0, 0xa8, 0xf9, 0x1d, 0xc0, 0xa8,<br>
> +     0xf9, 0x1e, 0xdc, 0xa2, 0x14, 0x51, 0xbb, 0x8f,<br>
> +     0xa0, 0x00, 0xe4, 0x7c, 0xe4, 0xb8, 0x80, 0x10,<br>
> +     0x02, 0x00, 0x4b, 0xc1, 0x00, 0x00, 0x01, 0x01,<br>
> +     0x08, 0x0a, 0x90, 0x60, 0xf4, 0xff, 0x03, 0xc5,<br>
> +     0xb4, 0x19, 0x77, 0x34, 0xd4, 0xdc, 0x84, 0x86,<br>
> +     0xff, 0x44, 0x09, 0x63, 0x36, 0x2e, 0x26, 0x9b,<br>
> +     0x90, 0x70, 0xf2, 0xed, 0xc8, 0x5b, 0x87, 0xaa,<br>
> +     0xb4, 0x67, 0x6b, 0x32, 0x3d, 0xc4, 0xbf, 0x15,<br>
> +     0xa9, 0x16, 0x6c, 0x2a, 0x9d, 0xb2, 0xb7, 0x6b,<br>
> +     0x58, 0x44, 0x58, 0x12, 0x4b, 0x8f, 0xe5, 0x12,<br>
> +     0x11, 0x90, 0x94, 0x68, 0x37, 0xad, 0x0a, 0x9b,<br>
> +     0xd6, 0x79, 0xf2, 0xb7, 0x31, 0xcf, 0x44, 0x22,<br>
> +     0xc8, 0x99, 0x3f, 0xe5, 0xe7, 0xac, 0xc7, 0x0b,<br>
> +     0x86, 0xdf, 0xda, 0xed, 0x0a, 0x0f, 0x86, 0xd7,<br>
> +     0x48, 0xe2, 0xf1, 0xc2, 0x43, 0xed, 0x47, 0x3a,<br>
> +     0xea, 0x25, 0x2d, 0xd6, 0x60, 0x38, 0x30, 0x07,<br>
> +     0x28, 0xdd, 0x1f, 0x0c, 0xdd, 0x7b, 0x7c, 0xd9,<br>
> +     0x35, 0x9d, 0x14, 0xaa, 0xc6, 0x35, 0xd1, 0x03,<br>
> +     0x38, 0xb1, 0xf5,<br>
> +};<br>
> +<br>
> +static const uint8_t test_cksum_ipv4_tcp_multi_segs_len[] = {<br>
> +     66,  /* the first seg contains all headers, including L2 to L4 */<br>
> +     61,  /* the second seg length is odd, test byte order independent */<br>
> +     52,  /* three segs are sufficient to test the most complex scenarios */<br>
> +};<br>
> +<br>
>  /* test l3/l4 checksum api */<br>
>  static int<br>
>  test_l4_cksum(struct rte_mempool *pktmbuf_pool, const char *pktdata, size_t len)<br>
> @@ -223,6 +259,70 @@ test_l4_cksum(struct rte_mempool *pktmbuf_pool, const char *pktdata, size_t len)<br>
>       return -1;<br>
>  }<br>
>  <br>
> +/* test l4 checksum api for a packet with multiple mbufs */<br>
> +static int<br>
> +test_l4_cksum_multi_mbufs(struct rte_mempool *pktmbuf_pool, const char *pktdata, size_t len,<br>
> +                          const uint8_t *segs, size_t segs_len)<br>
> +{<br>
> +     struct rte_mbuf *m[NB_MBUF] = {0};<br>
> +     struct rte_mbuf *m_hdr = NULL;<br>
> +     struct rte_net_hdr_lens hdr_lens;<br>
> +     size_t i, off = 0;<br>
> +     uint32_t packet_type, l3;<br>
> +     void *l3_hdr;<br>
> +     char *data;<br>
> +<br>
> +     for (i = 0; i < segs_len; i++) {<br>
> +             m[i] = rte_pktmbuf_alloc(pktmbuf_pool);<br>
> +             if (m[i] == NULL)<br>
> +                     GOTO_FAIL("Cannot allocate mbuf");<br>
> +<br>
> +             data = rte_pktmbuf_append(m[i], segs[i]);<br>
> +             if (data == NULL)<br>
> +                     GOTO_FAIL("Cannot append data");<br>
> +<br>
> +             rte_memcpy(data, pktdata + off, segs[i]);<br>
<br>
Tests (except rte_memcpy test) should not use rte_memcpy, instead use<br>
regular memcpy which has better coverage from analyzers.<br>
<br>
> +             off += segs[i];<br>
> +<br>
> +             if (m_hdr) {<br>
> +                     if (rte_pktmbuf_chain(m_hdr, m[i]))<br>
> +                             GOTO_FAIL("Cannot chain mbuf");<br>
> +             } else {<br>
> +                     m_hdr = m[i];<br>
> +             }<br>
> +     }<br>
> +<br>
> +     if (off != len)<br>
> +             GOTO_FAIL("Invalid segs");<br>
> +<br>
> +     packet_type = rte_net_get_ptype(m_hdr, &hdr_lens, RTE_PTYPE_ALL_MASK);<br>
> +     l3 = packet_type & RTE_PTYPE_L3_MASK;<br>
> +<br>
> +     l3_hdr = rte_pktmbuf_mtod_offset(m_hdr, void *, hdr_lens.l2_len);<br>
> +     off = hdr_lens.l2_len + hdr_lens.l3_len;<br>
> +<br>
> +     if (l3 == RTE_PTYPE_L3_IPV4 || l3 == RTE_PTYPE_L3_IPV4_EXT) {<br>
> +             if (rte_ipv4_udptcp_cksum_mbuf_verify(m_hdr, l3_hdr, off) != 0)<br>
> +                     GOTO_FAIL("Invalid L4 checksum verification for multiple mbufs");<br>
> +     } else if (l3 == RTE_PTYPE_L3_IPV6 || l3 == RTE_PTYPE_L3_IPV6_EXT) {<br>
> +             if (rte_ipv6_udptcp_cksum_mbuf_verify(m_hdr, l3_hdr, off) != 0)<br>
> +                     GOTO_FAIL("Invalid L4 checksum verification for multiple mbufs");<br>
> +     }<br>
> +<br>
> +     for (i = 0; i < segs_len; i++)<br>
> +             rte_pktmbuf_free(m[i]);<br>
<br>
Can avoid the loop here and elsewhere by using rte_pktmbuf_free_bulk()<br>
<br>
> +     return 0;<br>
> +<br>
> +fail:<br>
> +     for (i = 0; i < segs_len; i++) {<br>
> +             if (m[i])<br>
> +                     rte_pktmbuf_free(m[i]);<br>
> +     }<br>
<br>
Freebulk will work here<br>
<br>
> +     return -1;<br>
> +}<br>
> +<br>
>  static int<br>
>  test_cksum(void)<br>
>  {<br>
> @@ -256,6 +356,12 @@ test_cksum(void)<br>
>                         sizeof(test_cksum_ipv4_opts_udp)) < 0)<br>
>               GOTO_FAIL("checksum error on ipv4_opts_udp");<br>
>  <br>
> +     if (test_l4_cksum_multi_mbufs(pktmbuf_pool, test_cksum_ipv4_tcp_multi_segs,<br>
> +                       sizeof(test_cksum_ipv4_tcp_multi_segs),<br>
> +                       test_cksum_ipv4_tcp_multi_segs_len,<br>
> +                       sizeof(test_cksum_ipv4_tcp_multi_segs_len)) < 0)<br>
> +             GOTO_FAIL("checksum error on multi mbufs check");<br>
> +<br>
>       rte_mempool_free(pktmbuf_pool);<br>
>  <br>
>       return 0;<br>
> diff --git a/lib/net/rte_cksum.h b/lib/net/rte_cksum.h<br>
> index a8e8927952..679ba82eb6 100644<br>
> --- a/lib/net/rte_cksum.h<br>
> +++ b/lib/net/rte_cksum.h<br>
> @@ -80,6 +80,25 @@ __rte_raw_cksum_reduce(uint32_t sum)<br>
>       return (uint16_t)sum;<br>
>  }<br>
>  <br>
> +/**<br>
> + * @internal Reduce a sum to the non-complemented checksum.<br>
> + * Helper routine for the rte_raw_cksum_mbuf().<br>
> + *<br>
> + * @param sum<br>
> + *   Value of the sum.<br>
> + * @return<br>
> + *   The non-complemented checksum.<br>
> + */<br>
> +static inline uint16_t<br>
> +__rte_raw_cksum_reduce_u64(uint64_t sum)<br>
> +{<br>
> +     uint32_t tmp;<br>
> +<br>
> +     tmp = __rte_raw_cksum_reduce((uint32_t)sum);<br>
> +     tmp += __rte_raw_cksum_reduce((uint32_t)(sum >> 32));<br>
> +     return __rte_raw_cksum_reduce(tmp);<br>
> +}<br>
> +<br>
>  /**<br>
>   * Process the non-complemented checksum of a buffer.<br>
>   *<br>
> @@ -119,8 +138,8 @@ rte_raw_cksum_mbuf(const struct rte_mbuf *m, uint32_t off, uint32_t len,<br>
>  {<br>
>       const struct rte_mbuf *seg;<br>
>       const char *buf;<br>
> -     uint32_t sum, tmp;<br>
> -     uint32_t seglen, done;<br>
> +     uint32_t seglen, done, tmp;<br>
> +     uint64_t sum;<br>
>  <br>
>       /* easy case: all data in the first segment */<br>
>       if (off + len <= rte_pktmbuf_data_len(m)) {<br>
> @@ -157,7 +176,7 @@ rte_raw_cksum_mbuf(const struct rte_mbuf *m, uint32_t off, uint32_t len,<br>
>       for (;;) {<br>
>               tmp = __rte_raw_cksum(buf, seglen, 0);<br>
>               if (done & 1)<br>
> -                     tmp = rte_bswap16((uint16_t)tmp);<br>
> +                     tmp = rte_bswap32(tmp);<br>
>               sum += tmp;<br>
>               done += seglen;<br>
>               if (done == len)<br>
> @@ -169,7 +188,7 @@ rte_raw_cksum_mbuf(const struct rte_mbuf *m, uint32_t off, uint32_t len,<br>
>                       seglen = len - done;<br>
>       }<br>
>  <br>
> -     *cksum = __rte_raw_cksum_reduce(sum);<br>
> +     *cksum = __rte_raw_cksum_reduce_u64(sum);<br>
>       return 0;<br>
>  }<br>
>  <br>
<br>
</blockquote></div>