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 DFEE5A00E6 for ; Thu, 11 Jul 2019 16:37:27 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id AE2461B464; Thu, 11 Jul 2019 16:37:27 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 901493772 for ; Thu, 11 Jul 2019 16:37:25 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Jul 2019 07:37:24 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.63,478,1557212400"; d="scan'208";a="168040600" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga007.fm.intel.com with ESMTP; 11 Jul 2019 07:37:24 -0700 Received: from fmsmsx118.amr.corp.intel.com (10.18.116.18) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 11 Jul 2019 07:37:24 -0700 Received: from fmsmsx117.amr.corp.intel.com ([169.254.3.206]) by fmsmsx118.amr.corp.intel.com ([169.254.1.247]) with mapi id 14.03.0439.000; Thu, 11 Jul 2019 07:37:24 -0700 From: "Wiles, Keith" To: Olivier Matz CC: dpdk dev community , Stephen Hemminger Thread-Topic: [dpdk-dev] [RFC] mbuf: support dynamic fields and flags Thread-Index: AQHVNwH87YnOXs5oQkeR+lrxtZZvp6bEluuAgAAGbACAAOVnAIAAcOSA Date: Thu, 11 Jul 2019 14:37:23 +0000 Message-ID: References: <20190710092907.5565-1-olivier.matz@6wind.com> <20190710104917.73bc9201@hermes.lan> <3117C805-492A-4DA9-BE8F-E119E057C80D@intel.com> <20190711075320.woswl6fhl6szgdqb@glumotte.dev.6wind.com> In-Reply-To: <20190711075320.woswl6fhl6szgdqb@glumotte.dev.6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.251.155.216] Content-Type: text/plain; charset="us-ascii" Content-ID: <32933AEE97765641B7E33453724A3A98@intel.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [RFC] mbuf: support dynamic fields and flags 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 Jul 11, 2019, at 2:53 AM, Olivier Matz wrote: >=20 > Hi Keith, >=20 > On Wed, Jul 10, 2019 at 06:12:16PM +0000, Wiles, Keith wrote: >>=20 >>=20 >>> On Jul 10, 2019, at 12:49 PM, Stephen Hemminger wrote: >>>=20 >>> On Wed, 10 Jul 2019 11:29:07 +0200 >>> Olivier Matz wrote: >>>=20 >>>> /** >>>> * Indicate that the metadata field in the mbuf is in use. >>>> @@ -738,6 +741,8 @@ struct rte_mbuf { >>>> */ >>>> struct rte_mbuf_ext_shared_info *shinfo; >>>>=20 >>>> + uint64_t dynfield1; /**< Reserved for dynamic fields. */ >>>> + uint64_t dynfield2; /**< Reserved for dynamic fields. */ >>>> } __rte_cache_aligned; >>>=20 >>> Growing mbuf is a fundamental ABI break and this needs >>> higher level approval. Why not one pointer? >>>=20 >>> It looks like you are creating something like FreeBSD m_tag. >>> Why not use that? >>=20 >> Changing the mbuf structure causes a big problem for a number reasons as= Stephen states. >=20 > Can you elaborate? >=20 > This is indeed an ABI break, but I think this is only due to the adding > of rte_mbuf_dynfield_copy() in rte_pktmbuf_attach(). The size of the > mbuf does not change and the fields are not initialized when creating a > new mbuf. So I think there is no ABI change for code that is not using > rte_pktmbuf_attach(). >=20 > I don't think it's a problem to have one ABI change, if it avoids many > others in the future. >=20 >> If we leave the mbuf stucture alone and add this feature to the >> headroom space between the mbuf structure and the packet. When setting >> up the mempool/mbuf pool we define a headroom to hold the extra data >> when the mbuf pool is created or just use the current headroom >> space. Using this method we can eliminate the mbuf structure change >> and add the data to the packet buffer. We can do away with dynfield1 >> and 2 as we know where headroom space begins and ends. Just a thought. >=20 > The size of the mbuf metadata (between the mbuf structure and the > buffer) is configured per pool, so it can be different accross > mbufs. So, the access to the dynamic field would be slower: > *(mbuf + dynfield_offset + metadata_size(mbuf)) We can force that space to be a minimum size when the mempool is created in= the case of a cloned mbuf. The cloned mbuf is a small use case, but am imp= ortant one and increasing the size for those special mbufs by a cache line = should not be a huge problem. I think most allocations do not change the size from the default value of t= he headroom (128). The mbuf + buffer are normally rounded to 2K or a bit bi= gger, which gives a bit more space in those cases of a packet size of 1518-= 1522. Jumbo frames are the same. Using the headroom size for an application= needs to be defined and setup for the max size anyway for the application = needs, so normally all mbuf creates should contain the same size to account= for mbuf moments within the system. That is my $0.02. >=20 > Also, the size of the data buffer can be 0: it happens for mbuf pools > that are dedicated to mbuf clones (that reference data in another mbuf > or in an external buffer). In this case, there is no room after metadata > to store the dynamic fields. >=20 > Thanks, > Olivier Regards, Keith