Re: [dpdk-stable] [PATCH v1] gso: fix marking TCP checksum flag in TCP segments

["Hu, Jiayu" <jiayu.hu@intel.com>]

Hi Ophir,

In the GSO design, the GSO library doesn't care about checksums, which
means it doesn't check if input packets have correct checksums, and it
doesn't do any checksum related work for the output GSO segments. It
depends on the callers to use HW or SW checksum calculation for output
packets. This is why the GSO library doesn't set PKT_TX_TCP_CKSUM. So
I don't think it's a bug.

In my opinion, it's not a good idea to enable HW TCP checksum calculation
silently, and without the aware of the caller. In fact, the caller always know it
does SW TSO (i.e. GSO), instead of real HW TSO. If the caller wants HW checksum
calculation, it can add PKT_TX_TCP_CKSUM to ol_flags before or after calling the
GSO library.

Add Konstantin for more suggestions.

Thanks,
Jiayu

> -----Original Message-----
> From: Ophir Munk [mailto:ophirmu@mellanox.com]
> Sent: Sunday, April 22, 2018 10:21 PM
> To: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>; Olga Shern
> <olgas@mellanox.com>; Pascal Mazon <pascal.mazon@6wind.com>; Ophir
> Munk <ophirmu@mellanox.com>; stable@dpdk.org
> Subject: [PATCH v1] gso: fix marking TCP checksum flag in TCP segments
> 
> Large TCP packets which are marked with PKT_TX_TCP_SEG flag are
> segmented and the flag is cleared in the resulting segments, however,
> the segments checksum is not updated. It is therefore required to set
> the PKT_TX_TCP_CKSUM flag in each TCP segment in order to mark for the
> sending driver the need to update the TCP checksum before transmitting
> the segment.
> 
> Fixes: 119583797b6a ("gso: support TCP/IPv4 GSO")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> ---
>  lib/librte_gso/rte_gso.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/librte_gso/rte_gso.c b/lib/librte_gso/rte_gso.c
> index a44e3d4..e9ce9ce 100644
> --- a/lib/librte_gso/rte_gso.c
> +++ b/lib/librte_gso/rte_gso.c
> @@ -50,12 +50,14 @@ rte_gso_segment(struct rte_mbuf *pkt,
>  			((IS_IPV4_GRE_TCP4(pkt->ol_flags) &&
>  			 (gso_ctx->gso_types &
> DEV_TX_OFFLOAD_GRE_TNL_TSO)))) {
>  		pkt->ol_flags &= (~PKT_TX_TCP_SEG);
> +		pkt->ol_flags |= PKT_TX_TCP_CKSUM;
>  		ret = gso_tunnel_tcp4_segment(pkt, gso_size, ipid_delta,
>  				direct_pool, indirect_pool,
>  				pkts_out, nb_pkts_out);
>  	} else if (IS_IPV4_TCP(pkt->ol_flags) &&
>  			(gso_ctx->gso_types &
> DEV_TX_OFFLOAD_TCP_TSO)) {
>  		pkt->ol_flags &= (~PKT_TX_TCP_SEG);
> +		pkt->ol_flags |= PKT_TX_TCP_CKSUM;
>  		ret = gso_tcp4_segment(pkt, gso_size, ipid_delta,
>  				direct_pool, indirect_pool,
>  				pkts_out, nb_pkts_out);
> --
> 2.7.4