From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 9BEEB8E90 for ; Mon, 23 Nov 2015 14:08:17 +0100 (CET) Received: from was59-1-82-226-113-214.fbx.proxad.net ([82.226.113.214] helo=[192.168.0.10]) by mail.droids-corp.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84) (envelope-from ) id 1a0qrZ-0007T0-Ar; Mon, 23 Nov 2015 14:08:57 +0100 Message-ID: <56530FB1.8000106@6wind.com> Date: Mon, 23 Nov 2015 14:08:01 +0100 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.7.0 MIME-Version: 1.0 To: Declan Doherty , "Ananyev, Konstantin" , "dev@dpdk.org" References: <1447176763-19303-1-git-send-email-declan.doherty@intel.com> <1447441090-8129-1-git-send-email-declan.doherty@intel.com> <1447441090-8129-7-git-send-email-declan.doherty@intel.com> <564F3BC5.90308@6wind.com> <564F57CE.2030203@intel.com> <5652D7E9.7050202@6wind.com> <2601191342CEEE43887BDE71AB97725836ACB2DA@irsmsx105.ger.corp.intel.com> <565303B8.3080202@intel.com> In-Reply-To: <565303B8.3080202@intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v7 06/10] mbuf_offload: library to support attaching offloads to a mbuf 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: Mon, 23 Nov 2015 13:08:18 -0000 Hi, On 11/23/2015 01:16 PM, Declan Doherty wrote: >> I don't think we should start to re-use userdata. >> Userdata was intended for the upper layer app to pass/store it's >> private data associated with mbuf, and we probably should keep it this >> way. If the crypto API PMD takes both mbuf and crypto instead of just the mbuf, the mbuf_offload and the userdata wouldn't be very different, as the metadata would stay inside the application. So, if it's application private (the dpdk does not know how to handle it in case of mbuf free, dup, ...), the content of it should be app-specific. >> While mbuf_offload (or whatever we'll name it) supposed to keep data >> necessary for crypto (and other future type of devices) to operate >> with mbuf. Any idea of future devices? >> All your comments above about that this new field is just ignored by >> most mbuf >> operations (copy/attach/append/free, etc) are valid. >> By I suppose for now we can just state that for mbuf_offload is not >> supported by >> all these mbuf ops. So, who is responsible of freeing the mbuf offload metadata? The crypto pmd ? It means that as soon as the mbuf offload is added to the mbuf, it is not possible to call any other dpdk function that would process the mbuf... if we cannot call anything else before, why not just passing the crypto argument as a parameter? Managing offload data would even be more complex in the future if there are more than one mbuf_offload attached to the mbuf. >> I understand that current API is probably not perfect and might need >> to be revised in future. The problem is that it's not easy to change the dpdk API now. >> Again, as it is completely new feature, I suspect it would be a lot of >> change requests for it anyway. >> But we need some generic way to pass other (not ethdev) specific >> information within mbuf >> between user-level and PMDs for non-ethernet devices (and probably >> between different PMDs too). If a crypto PMD needs a crypto info, why not adding a crypto argument? I feel it's clearer from a user point of view. About PMD to PMD metadata, do you have any use case? > Just to re-iterate Konstantin's point above. I'm aware that a number of > mbuf operations currently are not supported and are not aware of the > rte_mbuf_offload, but we will be continuing development throughout 2.3 > and beyond to address this. Also I hope we will get much more community > feedback and interaction so we can get a more definite feature set for > the library, and by minimizing the features in this release, it will > allow more flexibility on how we can develop this part of handling > offloaded operations and it will limit the ABI issues we will face if it > turns out we need to change this going forwards. I can see the amount of work you've done for making the cryptodev happen in dpdk. I also recognize that I didn't make comments very early. If we really want to have this feature in the next release, maybe there is a way to mark it as experimental, meaning that the API is subject to change? What do you think? Thomas, any comment? Regards, Olivier