Re: [PATCH v3 2/2] app/testpmd: support TCP TSO in Tx only mode

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 11/11/2022 9:04 AM, Andrew Rybchenko wrote:
> @@ -459,11 +486,16 @@ tx_only_begin(portid_t pi)
>   {
>   	uint16_t pkt_hdr_len, pkt_data_len;
>   	int dynf;
> -	uint8_t ip_proto = IPPROTO_UDP;
> +	uint8_t ip_proto;
>   
>   	pkt_hdr_len = (uint16_t)(sizeof(struct rte_ether_hdr) +
>   				 sizeof(struct rte_ipv4_hdr));
> +
> +	ip_proto = txonly_tso_segsz > 0 ? IPPROTO_TCP : IPPROTO_UDP;

Instead of detecting protocol via txonly_tso_segsz, what about 
introducing '--tx-tcp' and use it?
I put some more comment to have tcp specific parameters in prev version 
of the patch.

As far as I understand, '--tx-udp'/'--tx-tcp' parameters are optional, 
that is why to detect protocol we can:
a) Use UDP by default if '--tx-udp'/'--tx-tcp' not provided
b) Have another argument to set protocol explicitly, something like 
'--tx-proto='. In this case it is better to have '--tx-udp'/'--tx-tcp' 
parameter as '--tx-l4-port', because otherwise protocol information will 
be duplicated (need to check against '--tx-proto=txp --tx-udp' etc..).

I think both of us think it is not good idea to change existing 
parameter name, that is why I prefer option a) above, what do you think?



>   	switch (ip_proto) {
> +	case IPPROTO_TCP:
> +		pkt_hdr_len += sizeof(struct rte_tcp_hdr);
> +		break;
>   	case IPPROTO_UDP:
>   		pkt_hdr_len += sizeof(struct rte_udp_hdr);
>   		break;