On Mon, Oct 26, 2026 at 9:14 PM Stephen Hemminger
<[email protected]> wrote:
>
> On Mon, 26 Oct 2026 13:23:24 -0700
> Joshua Washington <[email protected]> wrote:
>
> > https://inbox.dpdk.org/dev/[email protected]/
> > introduced descriptor limit checks for TSO packets in DPDK.
> > Unfortunately, a couple of bugs have been found in the implementation.
> >
> > This patch series introduces fixes for those bugs while correcting the
> > overly restrictive behavior recently introduced mandating that all
> > headers be in the same buffer.
> >
> > The final patch in the series introduces tx_pkt_prepare as an option to
> > ensure that there is a way to get a clear signal of when packets violate
> > hardware limitations. Before a recently introduced patch, the GVE
> > behavior was to stop transmitting at the first invalid packet, which
> > would allow the application to do any necessary linearization or packet
> > modification before attempting to transmit again. It turns out that that
> > behavior is considered incorrect, so offer tx_pkt_prepare as an
> > alternative.
> >
> > Depends-on: series-39707 ("net/gve: fix DQO Tx queue stalls")
>
> AI spotted issues here.
>
> Subject: Re: [PATCH 0/3] net/gve: DQO TSO descriptor validation fixes
>
> Series summary
>
> Patch 1 is correct. Patch 2 relaxes the requirement that TSO headers
> be in the first mbuf, but patch 3 then calls
> rte_net_intel_cksum_prepare(), which rejects exactly those packets.
> The series needs to pick one behaviour.
>
>
> Patch 2/3 + 3/3
>
> Error: rte_eth_tx_prepare() rejects split-header TSO packets that
> patch 2 now allows.
>
> rte_net_intel_cksum_flags_prepare() in lib/net/rte_net.h has:
>
> if (unlikely(rte_pktmbuf_data_len(m) <
> inner_l3_offset + m->l3_len + m->l4_len))
> return -ENOTSUP;
>
> So after patch 3, any application that follows the documented
> prepare-then-burst model (e.g. testpmd csum engine) still cannot send
> a TSO packet whose headers span mbufs. Only applications that skip
> tx_prepare get the relaxation from patch 2.
>
> Does DQO actually need the TCP pseudo-header checksum seeded? If not,
> drop the rte_net_intel_cksum_prepare() call. If it does, either keep
> the data_len < headlen check for TSO in gve_tx_validate_pkt_limits()
> and drop the relaxation from patch 2, or do the fixup across segments
> with rte_pktmbuf_read() and copy the result back.
>
TCP pseudo header checksum must be seeded for TSO packets. However, in
the case that the application performs that seeding itself, I would
like to leave it as an option to still send the packet out as long as
it meets the hardware descriptor and packet length limitations. A
packet that is dropped on the receiver due to invalid checksum is not
as severe as a packet that stalls the entire queue. To that end, I am
thinking to only call into rte_net_intel_cksum_prepare() if checksum
offloading is actually requested. In that case, the expectation would
be that all headers are in the same mbuf, matching other drivers with
this behavior. I will make this change in v2 if there are no
objections.
>
> Patch 3/3
>
> Warning: prepare and burst count descriptors differently.
>
> prepare uses nb_segs for non-TSO packets, but gve_tx_pkt_nb_data_descs()
> (and the burst fill loop) skip zero-length segments. A non-TSO packet
> with empty segments can fail prepare with ENOTSUP while burst would
> send it. Use the same count as burst:
>
> tso = !!(tx_pkt->ol_flags & RTE_MBUF_F_TX_TCP_SEG);
> ...
> nb_descs = gve_tx_pkt_nb_data_descs(tx_pkt) + tso;
>
> and drop the comment about nb_segs.
Ah, this is a good point. I was hoping to avoid the packet walk when
it is not necessary. I was planning to make that optimization to the
TX path for multi-mbuf non-TSO packets as well, as a non-fix patch.
While it is technically still a departure, I will trust nb_segs as
long as nb_segs is less than the hardware descriptor limit. If nb_segs
is higher than the limit, the logic will defer to the mbuf walk to get
the actual number of descriptors needed. A future patch will introduce
the same behavior to the datapath.
>
> Info: adding a tx_pkt_prepare op is a behaviour change for stable
> users; stable maintainers may not want it backported with patches 1
> and 2.
>
>
> Patch 2/3
>
> Info: the header walk can stop as soon as the limit is exceeded
> rather than walking the rest of a chain of empty mbufs:
>
> if (likely(tx_pkt->data_len > 0)) {
> header_remain -= tx_pkt->data_len;
> if (++num_header_descs > GVE_TX_MAX_HEADER_DESCS_TSO)
> return false;
> }
>
>
> Patch 1/3
>
> Info: subject has "segment" twice ("fix segment TSO segment offset").
