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

["Li, Xiaoyun" <xiaoyun.li@intel.com>]

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