From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0723DA055A; Tue, 25 Feb 2020 13:28:57 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 6E42D1BC24; Tue, 25 Feb 2020 13:28:55 +0100 (CET) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id DBD312C02 for ; Tue, 25 Feb 2020 13:28:53 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Feb 2020 04:28:52 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,484,1574150400"; d="scan'208";a="436244738" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.252.15.135]) ([10.252.15.135]) by fmsmga005.fm.intel.com with ESMTP; 25 Feb 2020 04:28:49 -0800 To: Andrew Rybchenko , Stephen Hemminger Cc: Neil Horman , John McNamara , Marko Kovacevic , dev@dpdk.org, David Marchand , Thomas Monjalon , "Kinsella, Ray" , Olivier MATZ References: <20200217153805.888130-1-ferruh.yigit@intel.com> <20200217220106.25ceb64c@hermes.lan> <6d9cfcfa-cade-8281-37d5-b048b14d8d0f@solarflare.com> From: Ferruh Yigit Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata= mQINBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABtCVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+iQJUBBMBCgA+AhsDAh4BAheABQsJCAcDBRUK CQgLBRYCAwEAFiEE0jZTh0IuwoTjmYHH+TPrQ98TYR8FAl1meboFCQlupOoACgkQ+TPrQ98T YR9ACBAAv2tomhyxY0Tp9Up7mNGLfEdBu/7joB/vIdqMRv63ojkwr9orQq5V16V/25+JEAD0 60cKodBDM6HdUvqLHatS8fooWRueSXHKYwJ3vxyB2tWDyZrLzLI1jxEvunGodoIzUOtum0Ce gPynnfQCelXBja0BwLXJMplM6TY1wXX22ap0ZViC0m714U5U4LQpzjabtFtjT8qOUR6L7hfy YQ72PBuktGb00UR/N5UrR6GqB0x4W41aZBHXfUQnvWIMmmCrRUJX36hOTYBzh+x86ULgg7H2 1499tA4o6rvE13FiGccplBNWCAIroAe/G11rdoN5NBgYVXu++38gTa/MBmIt6zRi6ch15oLA Ln2vHOdqhrgDuxjhMpG2bpNE36DG/V9WWyWdIRlz3NYPCDM/S3anbHlhjStXHOz1uHOnerXM 1jEjcsvmj1vSyYoQMyRcRJmBZLrekvgZeh7nJzbPHxtth8M7AoqiZ/o/BpYU+0xZ+J5/szWZ aYxxmIRu5ejFf+Wn9s5eXNHmyqxBidpCWvcbKYDBnkw2+Y9E5YTpL0mS0dCCOlrO7gca27ux ybtbj84aaW1g0CfIlUnOtHgMCmz6zPXThb+A8H8j3O6qmPoVqT3qnq3Uhy6GOoH8Fdu2Vchh TWiF5yo+pvUagQP6LpslffufSnu+RKAagkj7/RSuZV25Ag0EV9ZMvgEQAKc0Db17xNqtSwEv mfp4tkddwW9XA0tWWKtY4KUdd/jijYqc3fDD54ESYpV8QWj0xK4YM0dLxnDU2IYxjEshSB1T qAatVWz9WtBYvzalsyTqMKP3w34FciuL7orXP4AibPtrHuIXWQOBECcVZTTOdZYGAzaYzxiA ONzF9eTiwIqe9/oaOjTwTLnOarHt16QApTYQSnxDUQljeNvKYt1lZE/gAUUxNLWsYyTT+22/ vU0GDUahsJxs1+f1yEr+OGrFiEAmqrzpF0lCS3f/3HVTU6rS9cK3glVUeaTF4+1SK5ZNO35p iVQCwphmxa+dwTG/DvvHYCtgOZorTJ+OHfvCnSVjsM4kcXGjJPy3JZmUtyL9UxEbYlrffGPQ I3gLXIGD5AN5XdAXFCjjaID/KR1c9RHd7Oaw0Pdcq9UtMLgM1vdX8RlDuMGPrj5sQrRVbgYH fVU/TQCk1C9KhzOwg4Ap2T3tE1umY/DqrXQgsgH71PXFucVjOyHMYXXugLT8YQ0gcBPHy9mZ qw5mgOI5lCl6d4uCcUT0l/OEtPG/rA1lxz8ctdFBVOQOxCvwRG2QCgcJ/UTn5vlivul+cThi 6ERPvjqjblLncQtRg8izj2qgmwQkvfj+h7Ex88bI8iWtu5+I3K3LmNz/UxHBSWEmUnkg4fJl Rr7oItHsZ0ia6wWQ8lQnABEBAAGJAjwEGAEKACYCGwwWIQTSNlOHQi7ChOOZgcf5M+tD3xNh HwUCXWZ5wAUJB3FgggAKCRD5M+tD3xNhH2O+D/9OEz62YuJQLuIuOfL67eFTIB5/1+0j8Tsu o2psca1PUQ61SZJZOMl6VwNxpdvEaolVdrpnSxUF31kPEvR0Igy8HysQ11pj8AcgH0a9FrvU /8k2Roccd2ZIdpNLkirGFZR7LtRw41Kt1Jg+lafI0efkiHKMT/6D/P1EUp1RxOBNtWGV2hrd 0Yg9ds+VMphHHU69fDH02SwgpvXwG8Qm14Zi5WQ66R4CtTkHuYtA63sS17vMl8fDuTCtvfPF HzvdJLIhDYN3Mm1oMjKLlq4PUdYh68Fiwm+boJoBUFGuregJFlO3hM7uHBDhSEnXQr5mqpPM 6R/7Q5BjAxrwVBisH0yQGjsWlnysRWNfExAE2sRePSl0or9q19ddkRYltl6X4FDUXy2DTXa9 a+Fw4e1EvmcF3PjmTYs9IE3Vc64CRQXkhujcN4ZZh5lvOpU8WgyDxFq7bavFnSS6kx7Tk29/ wNJBp+cf9qsQxLbqhW5kfORuZGecus0TLcmpZEFKKjTJBK9gELRBB/zoN3j41hlEl7uTUXTI JQFLhpsFlEdKLujyvT/aCwP3XWT+B2uZDKrMAElF6ltpTxI53JYi22WO7NH7MR16Fhi4R6vh FHNBOkiAhUpoXRZXaCR6+X4qwA8CwHGqHRBfYFSU/Ulq1ZLR+S3hNj2mbnSx0lBs1eEqe2vh cA== Message-ID: <36992cfb-0b39-85f0-a02a-0c49ed9c9eec@intel.com> Date: Tue, 25 Feb 2020 12:28:49 +0000 MIME-Version: 1.0 In-Reply-To: <6d9cfcfa-cade-8281-37d5-b048b14d8d0f@solarflare.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH] doc: plan splitting the ethdev ops struct X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 2/25/2020 10:35 AM, Andrew Rybchenko wrote: > On 2/21/20 1:40 PM, Ferruh Yigit wrote: >> On 2/18/2020 6:01 AM, Stephen Hemminger wrote: >>> On Mon, 17 Feb 2020 15:38:05 +0000 >>> Ferruh Yigit wrote: >>> >>>> For the ABI compatibility it is better to hide internal data structures >>>> from the application as much as possible. But because of some inline >>>> functions 'struct eth_dev_ops' can't be hidden completely. >>>> >>>> Plan is to split the 'struct eth_dev_ops' into two as ones used by >>>> inline functions and ones not used, and hide the second part that not >>>> used by inline functions completely to the application. >>>> >>>> Signed-off-by: Ferruh Yigit >>>> --- >>>> Cc: David Marchand >>>> Cc: Thomas Monjalon >>>> Cc: Andrew Rybchenko >>>> --- >>>> doc/guides/rel_notes/deprecation.rst | 6 ++++++ >>>> 1 file changed, 6 insertions(+) >>>> >>>> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst >>>> index dfcca87ab..2aa431028 100644 >>>> --- a/doc/guides/rel_notes/deprecation.rst >>>> +++ b/doc/guides/rel_notes/deprecation.rst >>>> @@ -72,6 +72,12 @@ Deprecation Notices >>>> In 19.11 PMDs will still update the field even when the offload is not >>>> enabled. >>>> >>>> +* ethdev: Split the ``struct eth_dev_ops`` struct to hide it as much as possible. >>>> + Currently the ``struct eth_dev_ops`` struct is accessible by the application >>>> + because some inline functions, like ``rte_eth_tx_descriptor_status()``, >>>> + access the struct directly. The struct will be separate in two, the ops used >>>> + by inline functions still will be accessible to user but rest will be hidden. >>>> + >>>> * cryptodev: support for using IV with all sizes is added, J0 still can >>>> be used but only when IV length in following structs ``rte_crypto_auth_xform``, >>>> ``rte_crypto_aead_xform`` is set to zero. When IV length is greater or equal >>> >>> Good luck, truely hiding internals is hard. The mbuf structure is already split but not really >>> hidden at all (just look at dwarf output). It doesn't make sense to do it unless >>> you can really hide it. >> >> I believe this can be done, only following [1] dev_ops are used by inline >> functions, rest can be moved into separate struct and moved into ethdev driver >> looking header. >> >> [1] >> rx_queue_count >> rx_descriptor_done >> rx_descriptor_status >> tx_descriptor_status > > I think having 3 places (if I understand the intention > correctly) with ethdev callbacks is too much. Yes, this is the intention. > So, I think > that these ops should be simply moved to nearby Tx/Rx > burst and Tx prepare callbacks (e.g. move into inline_ops > structure which is located at the beginning of rte_eth_dev > in order to preserve 3 existing callback location). I think this is reasonable, but this can be done only in 20.11 with ABI break. What do you think doing the initial hide in 20.05 with three places and do proper splitting in 20.11 as suggested above. Or it is possible to drop the interim work and do the all changes in 20.11, not sure. > > Also I'd consider to deprecate and remove rx_queue_count > and rx_descriptor_done. +1 to deprecate the 'rx_descriptor_done', we have already 'rx_descriptor_status' & 'tx_descriptor_status' to replace the functionality. @Thomas, @Ray, can you please remind how deprecation was done, is it marking API, 'rte_eth_rx_descriptor_done', as deprecated in 20.11 and removing it in 21.11? If so I guess deprecation notice should be send before 20.11? (And make sure all PMDs did the switch before 20.11) Not sure about 'rx_queue_count', if any application may need it or not. > >>> >>> I would attack the rte_device stuff first. Make rte_device opaque to the application >>> that would help for future versions. Then work backwards to rte_tehtdev. >>> >