Re: [PATCH v1] app/testpmd: use Tx preparation in txonly engine

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed,  3 Jan 2024 09:29:12 +0800
Kaiwen Deng <kaiwenx.deng@intel.com> wrote:

> Txonly forwarding engine does not call the Tx preparation API
> before transmitting packets. This may cause some problems.
> 
> TSO breaks when MSS spans more than 8 data fragments. Those
> packets will be dropped by Tx preparation API, but it will cause
> MDD event if txonly forwarding engine does not call the Tx preparation
> API before transmitting packets.
> 
> We can reproduce this issue by these steps list blow on ICE and I40e.
> 
> ./x86_64-native-linuxapp-gcc/app/dpdk-testpmd -c 0xf -n 4 -- -i
> --tx-offloads=0x00008000
> 
> testpmd>set txpkts 64,128,256,512,64,128,256,512,512
> testpmd>set burst 1
> testpmd>start tx_first 1  
> 
> This commit will use Tx preparation API in txonly forwarding engine.
> 
> Fixes: 655131ccf727 ("app/testpmd: factorize fwd engines Tx")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
> ---
>  app/test-pmd/txonly.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
> index c2b88764be..60d69be3f6 100644
> --- a/app/test-pmd/txonly.c
> +++ b/app/test-pmd/txonly.c
> @@ -339,6 +339,7 @@ pkt_burst_transmit(struct fwd_stream *fs)
>  	struct rte_ether_hdr eth_hdr;
>  	uint16_t nb_tx;
>  	uint16_t nb_pkt;
> +	uint16_t nb_prep;
>  	uint16_t vlan_tci, vlan_tci_outer;
>  	uint64_t ol_flags = 0;
>  	uint64_t tx_offloads;
> @@ -396,7 +397,17 @@ pkt_burst_transmit(struct fwd_stream *fs)
>  	if (nb_pkt == 0)
>  		return false;
>  
> -	nb_tx = common_fwd_stream_transmit(fs, pkts_burst, nb_pkt);
> +	nb_prep = rte_eth_tx_prepare(fs->tx_port, fs->tx_queue,
> +		pkts_burst, nb_pkt);
> +	if (unlikely(nb_prep != nb_pkt)) {
> +		fprintf(stderr,
> +			"Preparing packet burst to transmit failed: %s\n",
> +			rte_strerror(rte_errno));
> +		fs->fwd_dropped += (nb_pkt - nb_prep);
> +		rte_pktmbuf_free_bulk(&pkts_burst[nb_prep], nb_pkt - nb_prep);
> +	}
> +
> +	nb_tx = common_fwd_stream_transmit(fs, pkts_burst, nb_prep);

The comment section on this patch raises lots of good points.

1. Testpmd and example applications are not calling tx_prepare.
2. Testpmd and examples are not checking descriptor limits.
3. It is not clear from documentation when tx_prepare is required.

On a practical level, if testpmd was being used in txonly mode,
if the condition ever triggered, you really really don't want to
print a message there since it would likely be endless stream of messages.

Please consider the comments and come back with a better solution in v2.