patches for DPDK stable branches
 help / color / mirror / Atom feed
From: "Hu, Jiayu" <jiayu.hu@intel.com>
To: Ophir Munk <ophirmu@mellanox.com>, "dev@dpdk.org" <dev@dpdk.org>,
	"Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Cc: Thomas Monjalon <thomas@monjalon.net>,
	Olga Shern <olgas@mellanox.com>,
	Pascal Mazon <pascal.mazon@6wind.com>,
	"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-stable] [PATCH v1] gso: fix marking TCP checksum flag in TCP segments
Date: Mon, 23 Apr 2018 04:13:48 +0000	[thread overview]
Message-ID: <ED946F0BEFE0A141B63BABBD629A2A9B388B3D55@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <1524406859-29585-1-git-send-email-ophirmu@mellanox.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

  parent reply	other threads:[~2018-04-23  4:13 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-22 14:20 Ophir Munk
2018-04-22 14:47 ` Ophir Munk
2018-04-23  4:13 ` Hu, Jiayu [this message]
2018-04-24  9:44   ` Ophir Munk
2018-04-24 10:56     ` Ananyev, Konstantin
2018-04-24 11:45       ` Ophir Munk
2018-04-24 12:31         ` Ananyev, Konstantin
2018-04-24 12:55           ` Hu, Jiayu
2018-04-24 13:53             ` Ophir Munk
2018-04-25  1:51               ` Hu, Jiayu
2018-04-24 13:41           ` Ophir Munk
2018-04-24 14:26             ` Ananyev, Konstantin
  -- strict thread matches above, loose matches on Subject: below --
2018-04-22 14:19 Ophir Munk
2018-04-30 13:38 ` Luca Boccassi
2018-04-22 14:15 Ophir Munk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ED946F0BEFE0A141B63BABBD629A2A9B388B3D55@shsmsx102.ccr.corp.intel.com \
    --to=jiayu.hu@intel.com \
    --cc=dev@dpdk.org \
    --cc=konstantin.ananyev@intel.com \
    --cc=olgas@mellanox.com \
    --cc=ophirmu@mellanox.com \
    --cc=pascal.mazon@6wind.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).