From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f181.google.com (mail-wi0-f181.google.com [209.85.212.181]) by dpdk.org (Postfix) with ESMTP id 3BA587F68 for ; Wed, 12 Nov 2014 14:31:05 +0100 (CET) Received: by mail-wi0-f181.google.com with SMTP id n3so4906888wiv.2 for ; Wed, 12 Nov 2014 05:40:59 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=nKEOzJu/dd8vKNdw8XwUmkoVRjAI9Q4NdF/sG04frTg=; b=j2STjMPY7YnT5IElahtyLnTlTy4SH1gaN3BSKQRd468L37GJykFtU8H05GR2C9dsP0 qqId8nhHGJXW/5laUCLaM19DKGDkcrwtBHP6rK+Rk+1sgiyEKwP51nmGdKpG0iSSXv9j RoXUICCtLp1iyMRKLm7Dxo/14I+fo10y8FAV+I9rrWmdhAQL8w6QpRKFWW1tO2dyLmOF OVEoERTUh/4RXdi+vBAFu/h7TyMzEOGO3fDOTC+srrZrv2//pGkuuegh9TiusnmxuPqu mEiIIKj3wJcaH0rQtF4un2o6t7ppnVU18+fIxVjcKlO2c1FMWMwOjweIIcuYSS/pqYlV +DaA== X-Gm-Message-State: ALoCoQnos2eM4rT8dkiK5E9b+53wVHiZEBrl+k7bqFn8GvuRHKYRJyoX2ZuzUva7CWSztKDFv1YS X-Received: by 10.180.90.112 with SMTP id bv16mr5687202wib.53.1415799659010; Wed, 12 Nov 2014 05:40:59 -0800 (PST) Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136]) by mx.google.com with ESMTPSA id j17sm17742817wjn.32.2014.11.12.05.40.57 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 12 Nov 2014 05:40:58 -0800 (PST) From: Thomas Monjalon To: Olivier MATZ , "Ananyev, Konstantin" , dev@dpdk.org Date: Wed, 12 Nov 2014 14:40:40 +0100 Message-ID: <1893731.yh14ESvpZM@xps13> Organization: 6WIND User-Agent: KMail/4.14.2 (Linux/3.17.2-1-ARCH; KDE/4.14.2; x86_64; ; ) In-Reply-To: <54635B2B.5040603@6wind.com> References: <1414376006-31402-1-git-send-email-jijiang.liu@intel.com> <2601191342CEEE43887BDE71AB977258213A3F5F@IRSMSX105.ger.corp.intel.com> <54635B2B.5040603@6wind.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v8 10/10] app/testpmd:test VxLAN Tx checksum offload X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Nov 2014 13:31:05 -0000 2014-11-12 14:05, Olivier MATZ: > 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 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. I feel this question is really important and we need more people to review the API. We'll also need more validation tests and performance checks with several use cases. Release is already late and I'm not comfortable with such change now. The only chance to have dev_prep_tx() in 1.8 would be to quickly have a large consensus and some benchmarks in pipeline and run to completion models. Conclusion: we should integrate TSO without dev_prep_tx (option 2 or 3) and then speed up dev & tests for dev_prep_tx(). This improvement for pipeline model could go in 2.0 if it's considered too short or risky for 1.8. Konstantin, could you be in charge of dev_prep_tx() works? Thanks for the good discussion -- Thomas