DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Xiaoyun Li <xiaoyun.li@intel.com>, <konstantin.ananyev@intel.com>,
	<stephen@networkplumber.org>,
	Olivier Matz <olivier.matz@6wind.com>
Cc: dev@dpdk.org, stable@dpdk.org,
	"Vladimir Medvedkin" <vladimir.medvedkin@intel.com>,
	"Morten Brørup" <mb@smartsharesystems.com>
Subject: Re: [dpdk-dev] [PATCH v3] app/testpmd: fix l4 sw csum over multi segments
Date: Wed, 27 Oct 2021 11:48:35 +0100	[thread overview]
Message-ID: <66ba870b-947f-29aa-45da-0e6f930b2398@intel.com> (raw)
In-Reply-To: <20211020101243.203063-1-xiaoyun.li@intel.com>

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?

  reply	other threads:[~2021-10-27 10:48 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-15  5:13 [dpdk-dev] [PATCH] " 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 [this message]
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
2021-10-20 10:10 [dpdk-dev] [PATCH v3] app/testpmd: fix l4 sw csum over multi segments Xiaoyun Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=66ba870b-947f-29aa-45da-0e6f930b2398@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=dev@dpdk.org \
    --cc=konstantin.ananyev@intel.com \
    --cc=mb@smartsharesystems.com \
    --cc=olivier.matz@6wind.com \
    --cc=stable@dpdk.org \
    --cc=stephen@networkplumber.org \
    --cc=vladimir.medvedkin@intel.com \
    --cc=xiaoyun.li@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).