From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 0AC985686 for ; Wed, 1 Apr 2015 15:48:25 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga101.fm.intel.com with ESMTP; 01 Apr 2015 06:48:25 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,503,1422950400"; d="scan'208";a="549562329" Received: from irsmsx104.ger.corp.intel.com ([163.33.3.159]) by orsmga003.jf.intel.com with ESMTP; 01 Apr 2015 06:48:22 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.2]) by IRSMSX104.ger.corp.intel.com ([169.254.3.49]) with mapi id 14.03.0224.002; Wed, 1 Apr 2015 14:48:20 +0100 From: "Ananyev, Konstantin" To: Olivier MATZ Thread-Topic: [dpdk-dev] [PATCH v2 1/5] mbuf: fix clone support when application uses private mbuf data Thread-Index: AQHQaG2kxHNOjCcpFECqfV5gntl+K50wWsQAgAACpICAABPugIAEdP1wgAB/GQCAAEV+oIABPaYAgAFJGTA= Date: Wed, 1 Apr 2015 13:48:20 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772582141368A@irsmsx105.ger.corp.intel.com> References: <1427302838-8285-1-git-send-email-olivier.matz@6wind.com> <1427385595-15011-1-git-send-email-olivier.matz@6wind.com> <1427385595-15011-2-git-send-email-olivier.matz@6wind.com> <2601191342CEEE43887BDE71AB97725821407D4D@irsmsx105.ger.corp.intel.com> <55151DDE.8040301@6wind.com> <55156188.6040101@6wind.com> <2601191342CEEE43887BDE71AB9772582140814C@irsmsx105.ger.corp.intel.com> <55157477.7090207@6wind.com> <2601191342CEEE43887BDE71AB9772582140FB21@irsmsx105.ger.corp.intel.com> <5519AA46.1090409@6wind.com> <2601191342CEEE43887BDE71AB97725821412EE9@irsmsx105.ger.corp.intel.com> <551AEF08.8020009@6wind.com> In-Reply-To: <551AEF08.8020009@6wind.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2 1/5] mbuf: fix clone support when application uses private mbuf data 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, 01 Apr 2015 13:48:27 -0000 > -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Tuesday, March 31, 2015 8:01 PM > To: Ananyev, Konstantin > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 1/5] mbuf: fix clone support when appli= cation uses private mbuf data >=20 > Hi Konstantin, >=20 > On 03/31/2015 01:17 AM, Ananyev, Konstantin wrote: > >>>> With this solution, there are 2 options: > >>>> - no mempool modification, so each application/driver has to add > >>>> priv_size bytes to the object to get the mbuf pointer. This does n= ot > >>>> look feasible. > >>>> - change the mempool to support private area before each object. I > >>>> think mempool API is already quite complex, and I think it's not > >>>> the role of the mempool library to provide such features. > >>> > >>> > >>> In fact, I think the changes would be minimal. > >>> All we have to do, is to make changes in rte_pktmbuf_init(): > >>> > >>> void > >>> rte_pktmbuf_init(struct rte_mempool *mp, > >>> __attribute__((unused)) void *opaque_arg, > >>> void *_m, > >>> __attribute__((unused)) unsigned i) > >>> { > >>> > >>> //extract priv_size from mempool (discussed above). > >>> uint16_t priv_size =3D rte_mbufpool_get_priv_size(mp); > >>> > >>> struct rte_mbuf *m =3D _m + priv_size; > >>> uint32_t buf_len =3D mp->elt_size - sizeof(struct rte_mbuf) - p= riv_size; > >>> > >>> .... > >>> > >>> > >>> With that way priv_size inside mbuf would always be the size its own = private space, > >>> for both direct and indirect mbus., so we don't require to set priv_s= ize at attach/detach. > >>> Again RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() would just work w= ithout any modifications. > >> > >> I'm not sure I'm getting it. The argument '_m' of your > >> rte_pktmbuf_init() is the pointer to the priv data, right? > >> So it means that the mbuf is located priv_size bytes after. > >> > >> The rte_pktmbuf_init() function is called by mempool_create(), > >> and the _m parameter is a pointer to a mempool object. So > >> in my understanding, mempool_get() would not return a rte_mbuf > >> but a pointer to the application private data. > > > > Ah my bad, forgot that mempool's obj_init() returns void now :( > > To make this approach work also need to change it, so it return a point= er to the new object. > > So, rte_pktmbuf_init() would return m and then in mempool_add_elem(): > > > > if (obj_init) > > obj =3D obj_init(mp, obj_init_arg, obj, obj_idx); > > > > rte_ring_sp_enqueue(mp->ring, obj); >=20 > Yes, but modififying mempool is what I wanted to avoid for several > reasons. First, I think that changing the mempool_create() API here > (actually the obj_init prototype) would impact existing applications. >=20 > Also, I'm afraid that storing a different address than the start > address of the object would have additional impacts. For instance, > some data is supposed to be stored before the object, see > __mempool_from_obj() or mempool_obj_audit(). mempool_obj_audit() should be ok, I think, but yes - rte_mempool_from_obj() would change the behaviour and can't be used by mempool_obj_audit() anymore. Ok, I am convinced - let's stick with private space between rte_mbuf and bu= f_adddr, as you suggested. =20 >=20 > Finally, I believe that mempool is not the proper place to do > modifications that are only needed for mbufs. If we really want > to implement mbuf private data in that way, maybe it would be > better to add a new layer above mempool (a mbuf pool layer). Not that I am against it, but seems like even more massive change - every application would need to be changed to use rte_mbufpool_create(), no= ? Konstantin >=20 >=20 > Regards > Olivier