From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Joseph, Anoob" Subject: Re: [PATCH 1/2] cryptodev: add min headroom and tailroom requirement Date: Thu, 28 Jun 2018 17:29:56 +0530 Message-ID: <3407576b-a5c1-a919-f4ca-04041c4a88c2@caviumnetworks.com> References: <1529389574-6643-1-git-send-email-anoob.joseph@caviumnetworks.com> <1529389574-6643-2-git-send-email-anoob.joseph@caviumnetworks.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Akhil Goyal , Ankur Dwivedi , Jerin Jacob , Narayana Prasad , "dev@dpdk.org" To: "De Lara Guarch, Pablo" , "Doherty, Declan" Return-path: Received: from NAM02-BL2-obe.outbound.protection.outlook.com (mail-bl2nam02on0049.outbound.protection.outlook.com [104.47.38.49]) by dpdk.org (Postfix) with ESMTP id 05CF65F1A for ; Thu, 28 Jun 2018 14:00:16 +0200 (CEST) In-Reply-To: Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Pablo, On 28-06-2018 17:11, De Lara Guarch, Pablo wrote: > External Email > > Hi Anoob, > >> -----Original Message----- >> From: Joseph, Anoob [mailto:Anoob.Joseph@caviumnetworks.com] >> Sent: Thursday, June 28, 2018 3:56 AM >> To: Doherty, Declan ; De Lara Guarch, Pablo >> >> Cc: Akhil Goyal ; Ankur Dwivedi >> ; Jerin Jacob >> ; Narayana Prasad >> ; dev@dpdk.org >> Subject: Re: [PATCH 1/2] cryptodev: add min headroom and tailroom >> requirement >> >> Hi Declan, >> >> Please see inline. >> >> Thanks, >> >> Anoob >> >> >> On 26-06-2018 15:42, Doherty, Declan wrote: >>> External Email >>> >>> On 19/06/2018 7:26 AM, Anoob Joseph wrote: >>>> Enabling crypto devs to specify the minimum headroom and tailroom it >>>> expects in the mbuf. For net PMDs, standard headroom has to be >>>> honoured by applications, which is not strictly followed for crypto >>>> devs. This >>> How is this done for NET PMDs, I don't see anything explicit in the >>> ehtdev API for specification of headroom requirements. >> In rte_mbuf.h, the minimum size required for packets involved in rx/tx is >> specified and that considers headroom also. Applications usually use these >> default macros while creating mbufs which are involved in rx/tx. >> https://git.dpdk.org/dpdk/tree/lib/librte_mbuf/rte_mbuf.h#n411 >>>> prevents crypto devs from using free space in mbuf (available as >>>> head/tailroom) for internal requirements in crypto operations. >>>> Addition of head/tailroom requirement will help PMDs to communicate >>>> such requirements to the application. >>>> >>>> The availability and use of head/tailroom is an optimization if the >>>> hardware supports use of head/tailroom for crypto-op info. For >>>> devices that do not support using the head/tailroom, they can >>>> continue to operate without any performance-drop. >>>> >>> Is there any variations in requirements for terms headroom/tailroom on >>> a per algorithmic basis or is it purely for the device? >> It is purely per device basis. The device can specify upper bounds for the >> head/tailroom. A device that even specified the room, may not even use the >> entire room in all cases. So it doesn't have to be algo specific. >>>> Signed-off-by: Anoob Joseph >>>> --- >>>> doc/guides/rel_notes/deprecation.rst | 4 ++++ >>>> lib/librte_cryptodev/rte_cryptodev.h | 6 ++++++ >>>> 2 files changed, 10 insertions(+) >>>> >>>> diff --git a/doc/guides/rel_notes/deprecation.rst >>>> b/doc/guides/rel_notes/deprecation.rst >>>> index 1ce692e..a547289 100644 >>>> --- a/doc/guides/rel_notes/deprecation.rst >>>> +++ b/doc/guides/rel_notes/deprecation.rst >>>> @@ -122,3 +122,7 @@ Deprecation Notices >>>> - Function ``rte_cryptodev_get_private_session_size()`` will be >>>> deprecated >>>> in 18.05, and it gets replaced with >>>> ``rte_cryptodev_sym_get_private_session_size()``. >>>> It will be removed in 18.08. >>>> + - New field, ``min_headroom_req``, added in ``rte_cryptodev_info`` >>>> structure. It will be >>>> + added in 18.11. >>>> + - New field, ``min_tailroom_req``, added in ``rte_cryptodev_info`` >>>> structure. It will be >>>> + added in 18.11. >>>> diff --git a/lib/librte_cryptodev/rte_cryptodev.h >>>> b/lib/librte_cryptodev/rte_cryptodev.h >>>> index 92ce6d4..fa944b8 100644 >>>> --- a/lib/librte_cryptodev/rte_cryptodev.h >>>> +++ b/lib/librte_cryptodev/rte_cryptodev.h >>>> @@ -382,6 +382,12 @@ struct rte_cryptodev_info { >>>> unsigned max_nb_queue_pairs; >>>> /**< Maximum number of queues pairs supported by device. */ >>>> >>>> + uint32_t min_headroom_req; >>>> + /**< Minimum mbuf headroom required by device */ >>>> + >>>> + uint32_t min_tailroom_req; >>>> + /**< Minimum mbuf tailroom required by device */ > I would add the word "mbuf" here, in the variable names (e.g. min_mbuf_headroom_req), > to be more explicit. Will revise the patch with this change. > > Also, just let you know that we are currently modifying the info structure in this release. > Therefore, I think we could make these changes in now and then you don't need to add deprecation notices on this, > but better to add the API Change in release notes. Will do so. Thanks, Anoob