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

["Morten Brørup" <mb@smartsharesystems.com>]

> 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?