> > > +
> > > +int
> > > +gso_tcp4_segment(struct rte_mbuf *pkt,
> > > + uint16_t gso_size,
> > > + uint8_t ipid_delta,
> > > + struct rte_mempool *direct_pool,
> > > + struct rte_mempool *indirect_pool,
> > > + struct rte_mbuf **pkts_out,
> > > + uint16_t nb_pkts_out)
> > > +{
> > > + struct ipv4_hdr *ipv4_hdr;
> > > + uint16_t tcp_dl;
> > > + uint16_t pyld_unit_size;
> > > + uint16_t hdr_offset;
> > > + int ret = 1;
> > > +
> > > + ipv4_hdr = (struct ipv4_hdr *)(rte_pktmbuf_mtod(pkt, char *) +
> > > + pkt->l2_len);
> > > + /* Don't process the fragmented packet */
> > > + if (unlikely((ipv4_hdr->fragment_offset & rte_cpu_to_be_16(
> > > + IPV4_HDR_DF_MASK)) == 0)) {
> >
> >
> > It is not a check for fragmented packet - it is a check that fragmentation
> > is allowed for that packet.
> > Should be IPV4_HDR_DF_MASK - 1, I think.
DF bit doesn't indicate is packet fragmented or not.
It forbids to fragment packet any further.
To check is packet already fragmented or not, you have to check MF bit and
frag_offset.
Both have to be zero for un-fragmented packets.
>
> IMO, IPV4_HDR_DF_MASK whose value is (1 << 14) is used to get DF bit. It's a
> little-endian value. But ipv4_hdr->fragment_offset is big-endian order.
> So the value of DF bit should be "ipv4_hdr->fragment_offset &
> rte_cpu_to_be_16(
> IPV4_HDR_DF_MASK)". If this value is 0, the input packet is fragmented.
>
> >
> > > + pkts_out[0] = pkt;
> > > + return ret;
> > > + }
> > > +
> > > + tcp_dl = rte_be_to_cpu_16(ipv4_hdr->total_length) - pkt->l3_len -
> > > + pkt->l4_len;
> >
> > Why not use pkt->pkt_len - pkt->l2_len -pkt_l3_len - pkt_l4_len?
>
> Yes, we can use pkt->pkt_len - pkt->l2_len -pkt_l3_len - pkt_l4_len here.
>
> >
> > > + /* Don't process the packet without data */
> > > + if (unlikely(tcp_dl == 0)) {
> > > + pkts_out[0] = pkt;
> > > + return ret;
> > > + }
> > > +
> > > + hdr_offset = pkt->l2_len + pkt->l3_len + pkt->l4_len;
> > > + pyld_unit_size = gso_size - hdr_offset - ETHER_CRC_LEN;
> >
> > Hmm, why do we need to count CRC_LEN here?
>
> Yes, we shouldn't count ETHER_CRC_LEN here. Its length should be
> included in gso_size.
Why?
What is the point to account crc len into this computation?
Why not just assume that gso_size is already a max_frame_size - crc_len
As I remember, when we RX packet crc bytes will be already stripped,
when user populates the packet, he doesn't care about crc bytes too.
Konstantin
