DPDK patches and discussions
 help / color / mirror / Atom feed
From: Olivier MATZ <olivier.matz@6wind.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	 Yong Wang <yongwang@vmware.com>,
	"Liu, Jijiang" <jijiang.liu@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v8 10/10] app/testpmd:test VxLAN Tx checksum offload
Date: Wed, 12 Nov 2014 14:05:47 +0100
Message-ID: <54635B2B.5040603@6wind.com> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB977258213A3F5F@IRSMSX105.ger.corp.intel.com>

Hi Konstantin,

On 11/12/2014 10:55 AM, Ananyev, Konstantin wrote:
>>  From an API perspective, it looks a bit more complex to have to call
>> dev_prep_tx() before sending the packets if they have been flagged
>> for offload processing. But I admit I have no other argument. I'll be
>> happy to have more comments from other people on the list.
>>
>> I'm sending a first version of the patchset now as it's ready, it does
>> not take in account this comment, but I'm open to add it in a v2 if
>> there is a consensus on this.
>>
>> Now, knowing that:
>> - adding dev_prep_tx() will also concern hw checksum (TCP L4 checksum
>>    already requires to set the TCP pseudo header checksum), so adding
>>    this will change the API of an existing feature
>> - TSO is a new feature expected for 1.8 (which should be out soon)
>>
>> Do you think we need to include this for 1.8 or can we postpone your
>> proposition for after the 1.8 release?
>
> I'd say it would be good to have it done together with TSO feature.
> About changing API: I think existing applications shouldn't be affected.
> For existing PMDs/TX offloads we don't change  any rules what need to be filled by the app.
> We just add a new function that can do that for user.
> If the app fills required manually (as all apps have to do now) it would keep working as expected.

I agree, this proposition could work without changing the current
applications.

> If you feel like it is too much work for 1.8 timeframe -
> can we at least move fix_tcp_phdr_cksum() out of TX PMD as a temporary measure?
> Let say create a function  get_ipv4_udptcp_checksum(struct rte_mbuf *m) (in librte_net ?).
> It will calculate PSD checksum for both TSO and non-TSO case based on given mbuf flags/fields.
> Then we can update testpmd/csumonly.c to use it.

I'm not sure having get_ipv4_udptcp_checksum() in librte_net would
help. The value we have to set in the TCP checksum field depends on the
PMD (altought only ixgbe is supported now). So, it would require
another parameter <portid> and a new PMD eth_ops... which looks very
similar to dev_prep_tx() (except that dev_prep_tx() can be bulked).
I think a stack will not be able to call get_udptcp_checksum(m ,port)
because it does not know the physical port at the time the packet is
built. Moreover, calling a function through a pointer is more efficient
when bulked. So I think the dev_prep_tx() you initially describe is
a better answer to the problem.

I don't know what is the exact timeframe for 1.8, maybe Thomas can help
on this? Depending on it, we have several options:

- implement dev_prep_tx() for 1.8 in the TSO series: this implies that
   the community agrees on this new API. We need to check that it will
   be faster in a pipeline model (I think this is obvious) but also that
   it does not penalize the run-to-completion model: introducing another
   function dev_prep_tx() can result in duplicated tests in the driver
   (ex: test the offload flag values).

- postpone dev_prep_tx() or similar to next version and push the current
   TSO patchset (including the comments done on the list). It does not
   modify the current offload API, it provides the TSO feature on ixgbe
   based on a similar API concept (set the TCP phdr cksum). The drawback
   is a potential performance loss when using a pipeline model.

- another option that you may prefer is to bind the API behavior to
   ixgbe (for 1.8): we can ask the application to set the pseudo-header
   checksum without the IP len when doing TSO, as required by the ixgbe
   driver. Then, for next release, we can think about dev_prep_tx(). The
   drawback of this solution is that we may go back on this choice if the
   dev_prep_tx() approach is not validated by the community.


Regards,
Olivier

  reply	other threads:[~2014-11-12 12:55 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-27  2:13 [dpdk-dev] [PATCH v8 00/10] Support VxLAN on Fortville Jijiang Liu
2014-10-27  2:13 ` [dpdk-dev] [PATCH v8 01/10] librte_mbuf:the rte_mbuf structure changes Jijiang Liu
2014-10-27  2:13 ` [dpdk-dev] [PATCH v8 02/10] librte_ether:add the basic data structures of VxLAN Jijiang Liu
2014-10-27  2:13 ` [dpdk-dev] [PATCH v8 03/10] librte_ether:add VxLAN packet identification API Jijiang Liu
2014-10-27  2:13 ` [dpdk-dev] [PATCH v8 04/10] i40e:support VxLAN packet identification in i40e Jijiang Liu
2014-10-27  2:13 ` [dpdk-dev] [PATCH v8 05/10] app/test-pmd:test VxLAN packet identification Jijiang Liu
2014-10-27  2:13 ` [dpdk-dev] [PATCH v8 06/10] librte_ether:add data structures of VxLAN filter Jijiang Liu
2014-10-27  2:13 ` [dpdk-dev] [PATCH v8 07/10] i40e:implement the API of VxLAN filter in librte_pmd_i40e Jijiang Liu
2014-10-27  2:13 ` [dpdk-dev] [PATCH v8 08/10] app/testpmd:test VxLAN packet filter Jijiang Liu
2014-10-27  2:13 ` [dpdk-dev] [PATCH v8 09/10] i40e:support VxLAN Tx checksum offload Jijiang Liu
2014-10-27  2:13 ` [dpdk-dev] [PATCH v8 10/10] app/testpmd:test " Jijiang Liu
2014-11-04  8:19   ` Olivier MATZ
2014-11-05  6:02     ` Liu, Jijiang
2014-11-05 10:28       ` Olivier MATZ
2014-11-06 11:24         ` Liu, Jijiang
2014-11-06 13:08           ` Olivier MATZ
2014-11-06 14:27             ` Liu, Jijiang
2014-11-07  0:43         ` Yong Wang
2014-11-07 17:16           ` Olivier MATZ
2014-11-10 11:39             ` Ananyev, Konstantin
2014-11-10 15:57               ` Olivier MATZ
2014-11-12  9:55                 ` Ananyev, Konstantin
2014-11-12 13:05                   ` Olivier MATZ [this message]
2014-11-12 13:40                     ` Thomas Monjalon
2014-11-12 23:14                       ` Ananyev, Konstantin
2014-11-12 14:39                     ` Ananyev, Konstantin
2014-11-12 14:56                       ` Olivier MATZ
     [not found]             ` <D0868B54.24DBB%yongwang@vmware.com>
2014-11-11  0:07               ` [dpdk-dev] FW: " Yong Wang
2014-11-10  6:03         ` [dpdk-dev] " Liu, Jijiang
2014-11-10 16:17           ` Olivier MATZ
     [not found]             ` <1ED644BD7E0A5F4091CF203DAFB8E4CC01D8F7A7@SHSMSX101.ccr.corp.intel.com>
2014-11-12 17:26               ` Thomas Monjalon
2014-11-13  5:35                 ` Liu, Jijiang
2014-11-13  5:39                   ` Liu, Jijiang
2014-11-13  6:51                 ` Liu, Jijiang
2014-11-13  9:10                   ` Thomas Monjalon
2014-11-14  8:15                     ` Liu, Jijiang
2014-11-14  9:09                       ` Olivier MATZ
2014-11-17  6:52                         ` Liu, Jijiang
2014-11-17 11:21                           ` Olivier MATZ
2014-11-20  7:28                             ` Liu, Jijiang
2014-11-20 16:36                               ` Olivier MATZ
2014-11-21  5:40                                 ` Liu, Jijiang
2014-10-27  2:20 ` [dpdk-dev] [PATCH v8 00/10] Support VxLAN on Fortville Liu, Yong
2014-10-27  2:41 ` Zhang, Helin
2014-10-27 13:46   ` Thomas Monjalon
2014-10-27 14:34     ` Liu, Jijiang
2014-10-27 15:15       ` Thomas Monjalon

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=54635B2B.5040603@6wind.com \
    --to=olivier.matz@6wind.com \
    --cc=dev@dpdk.org \
    --cc=jijiang.liu@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=yongwang@vmware.com \
    /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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git