Re: [PATCH v2 0/2] net/mana: fix tx burst error handling and add TSO support

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 26 Oct 2026 21:21:23 +0000
Tathagat Priyadarshi <tathagat.dpdk@gmail.com> wrote:

> This series fixes error handling in the MANA transmit burst and adds
> hardware TCP segmentation offload (TSO).
> 
> Patch 1 drops a packet that cannot be sent, counting it in stats.errors,
> instead of skipping or stalling on it, so rte_eth_tx_burst() keeps
> returning a contiguous prefix and a short return means only a full ring.
> It also bounds the segment count by the size of the SGL array.
> 
> Patch 2 adds hardware TSO for IPv4 and IPv6 TCP packets.
> 
> Testing on Azure Standard_D8ds_v6 (kernel 6.8.0-1067-azure, MANA PCI
> 7870:00:00.0; the driver reports no firmware version), two VMs on one
> subnet:
> - VM A ran testpmd txonly, modified locally to emit TCP packets with
>   RTE_MBUF_F_TX_TCP_SEG set (not part of this series), after
>   "csum set ip hw 0", "csum set tcp hw 0" and "tso set 1460 0".
> - VM B captured the frames with tcpdump on its kernel interface, GRO off.
> - A 4000-byte IPv4 packet in a single mbuf, in mbufs with a header-only
>   first segment, and in mbufs with payload in the first segment, each
>   arrived as 1514, 1514 and 1080 byte frames with consecutive TCP
>   sequence numbers, per-segment IP IDs and valid IP and TCP checksums.
> - Plain UDP with "tso set 0" was unaffected.
> IPv6 TSO was not tested.
> 
> v2:
> - Patch 1:
>   * Drop unsendable packets and return a separate consumed count instead
>     of breaking out of the burst; break only when the ring is full.
>   * Move the MAX_SGL_ENTRIES_FOR_TRANSMIT check here from patch 2, limit
>     nb_seg_max and nb_mtu_seg_max to it, and add Fixes and Cc stable.
> - Patch 2:
>   * Use the drop path for all per-packet TSO failures.
>   * Honour the TCP checksum implied by RTE_MBUF_F_TX_TCP_SEG when the
>     packet is sent unsegmented.
>   * Bound the TSO segment walk and check the SGE count against the
>     expected one, instead of the unreachable check in the loop.
>   * Move GDMA_WR_OOB_IN_SGL and GDMA_WR_PAD_BY_SGE0 next to
>     struct gdma_work_request.
>   * Document the extra SGE in mana.rst, add a release note, and rewrite
>     the commit message.
> 
> With the drop-and-continue path, CQE suppression is back to the original
> behavior where only an uncompleted final packet of a burst may wait for a
> subsequent burst to generate a CQE.
> 
> Tathagat Priyadarshi (2):
>   net/mana: fix transmit burst error handling
>   net/mana: add hardware TSO support
> 
>  doc/guides/nics/features/mana.ini      |   1 +
>  doc/guides/nics/mana.rst               |   8 ++
>  doc/guides/rel_notes/release_26_11.rst |   4 +
>  drivers/net/mana/gdma.c                |   5 +
>  drivers/net/mana/mana.c                |   4 +-
>  drivers/net/mana/mana.h                |   6 +-
>  drivers/net/mana/tx.c                  | 191 +++++++++++++++++++++----
>  7 files changed, 186 insertions(+), 33 deletions(-)
> 

AI review still turning up errors here.

Review: [PATCH v2 0/2] net/mana: tx error handling and TSO

Patch 1/2 is a good fix. Patch 2/2 has two hardware field width
truncations and a checksum bug in the new non-TSO fallback.


[PATCH v2 1/2] net/mana: fix transmit burst error handling

Info

  Dropping a packet now happens mid-burst, after the previous WQE may
  have been posted with suppress_tx_CQE_generation set (the decision
  is made on pkt_idx + 1 < nb_pkts). If every remaining packet in the
  burst is dropped, the last posted WQE has no CQE and its mbufs (and
  any suppressed ones before it) are not reclaimed until a later burst
  posts another WQE. Same for the post-failure break and the 32-bit
  wqe_count break. Pre-existing, not a blocker here, but worth a
  follow-up: decide suppression after validation of the next packet,
  or never suppress the last WQE before ringing the doorbell.


[PATCH v2 2/2] net/mana: add hardware TSO support

Error

  Non-TSO fallback computes the wrong pseudo-header checksum.
  The fallback branch is entered for TCP_SEG packets with
  tso_segsz == 0 or no payload, and passes m_pkt->ol_flags (with
  RTE_MBUF_F_TX_TCP_SEG still set) to rte_ipv4_phdr_cksum() /
  rte_ipv6_phdr_cksum(). Those helpers use length 0 when TCP_SEG is
  set, so the seed is wrong for an unsegmented frame and the hardware
  emits a bad TCP checksum. Mask the flag:

	uint64_t ol = m_pkt->ol_flags & ~RTE_MBUF_F_TX_TCP_SEG;
	...
	tcp_hdr->cksum = rte_ipv4_phdr_cksum(ip_hdr, ol);

  Also, TCP_SEG with tso_segsz == 0 or l4_len == 0 is invalid input
  per the mbuf API. With gso_hs == 0 the fallback writes cksum at
  l2_len + l3_len == 0, corrupting the Ethernet header. Drop these
  instead of sending them:

	if (m_pkt->ol_flags & RTE_MBUF_F_TX_TCP_SEG) {
		if (unlikely(m_pkt->tso_segsz == 0 ||
			     m_pkt->l4_len == 0 ||
			     m_pkt->data_len < gso_hs))
			goto drop;
		is_tso = m_pkt->pkt_len > gso_hs;
	}

Error

  client_data_unit is a 14-bit field in struct gdma_wqe_dma_oob, but
  tso_segsz is uint16_t. Any tso_segsz >= 16384 is silently
  truncated; 16384 becomes 0 == NOT_USING_CLIENT_DATA_UNIT and a
  64KB frame is handed to the wire unsegmented. Reject it:

	#define MANA_TSO_MAX_SEGSZ ((1u << 14) - 1)

	if (unlikely(m_pkt->tso_segsz > MANA_TSO_MAX_SEGSZ))
		goto drop;

Error

  last_v_bytes is an 8-bit field, but gso_hs is
  l2_len + l3_len + l4_len and can exceed 255 (IPv6 extension headers,
  VLAN plus TCP options). gdma.c writes sgl[0].size into it and the
  value wraps. Drop TSO packets with gso_hs > UINT8_MAX, or state the
  real hardware header limit and check against that.

Warning

  The extra SGE for a TSO header split is documented as a limitation
  but not reflected in dev_info. tx_desc_lim.nb_seg_max is the TSO
  limit (nb_mtu_seg_max is the non-TSO one), so applications and
  rte_gso trusting it build packets that are then dropped. Advertise
  the real limit and drop the mana.rst paragraph:

	uint16_t max_sge = RTE_MIN(priv->max_send_sge,
				   MAX_SGL_ENTRIES_FOR_TRANSMIT);

	dev_info->tx_desc_lim.nb_seg_max = max_sge - 1;
	dev_info->tx_desc_lim.nb_mtu_seg_max = max_sge;

Warning

  No bounds on the TSO request beyond the above. What is the maximum
  payload (pkt_len - gso_hs) and the minimum MSS the adapter accepts?
  Check both in the TSO validation and drop out-of-range packets,
  rather than leaving it to the device.

Info

  The TSO and non-TSO SGL loops duplicate the MR lookup and SGE fill.
  One loop with a start offset on the first segment covers both:

	uint32_t off = 0;

	num_sge = 0;
	if (is_tso) {
		/* SGE0: headers only */
		if (fill_sge(&sgl, num_sge++, m_pkt, 0, gso_hs, mr))
			goto drop;
		off = gso_hs;
	}
	for (m_seg = m_pkt; m_seg; m_seg = m_seg->next, off = 0) {
		if (m_seg->data_len == off)
			continue;
		if (fill_sge(&sgl, num_sge++, m_seg, off,
			     m_seg->data_len - off, mr))
			goto drop;
	}

  With expected_sge already checked against the array size, the
  m_seg != NULL || num_sge != expected_sge re-check goes away.

Info

  Header rewriting (total_length, hdr_checksum, cksum) in tx_burst
  modifies data the application may share (refcnt > 1, external
  buffers). The existing TCP_CKSUM path does the same, so not new,
  but the conventional place is tx_pkt_prepare. A mana_tx_prepare()
  using rte_validate_tx_offload() and rte_net_intel_cksum_prepare()
  would also move the segment-count checks out of the fast path.

Info

  Commit message: drop "allowing transmit bursts of up to 64KB TCP
  packets at wire speed". The test log is useful, but belongs below
  the "---" line, not in the permanent history.