> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.anan...@intel.com>
> Sent: Monday, October 18, 2021 18:16
> To: Li, Xiaoyun <xiaoyun...@intel.com>; Stephen Hemminger
> <step...@networkplumber.org>
> Cc: Yigit, Ferruh <ferruh.yi...@intel.com>; dev@dpdk.org; sta...@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.

Reply via email to