From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 7BBD65A73 for ; Fri, 27 Mar 2015 15:25:24 +0100 (CET) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP; 27 Mar 2015 07:25:23 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,479,1422950400"; d="scan'208";a="705109223" Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by orsmga002.jf.intel.com with ESMTP; 27 Mar 2015 07:25:22 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.47]) by IRSMSX101.ger.corp.intel.com ([163.33.3.153]) with mapi id 14.03.0224.002; Fri, 27 Mar 2015 14:25:21 +0000 From: "Ananyev, Konstantin" To: Olivier MATZ , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v2 1/5] mbuf: fix clone support when application uses private mbuf data Thread-Index: AQHQaG2kxHNOjCcpFECqfV5gntl+K50wWsQAgAACpIA= Date: Fri, 27 Mar 2015 14:25:21 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772582140814C@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> In-Reply-To: <55156188.6040101@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.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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: Fri, 27 Mar 2015 14:25:25 -0000 Hi Olivier, > -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Friday, March 27, 2015 1:56 PM > To: Ananyev, Konstantin; 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/27/2015 10:07 AM, Olivier MATZ wrote: > >> I think that to support ability to setup priv_size on a mempool basis, > >> and reserve private space between struct rte_mbuf and rte_mbuf. buf_ad= dr, > >> we need to: > >> > >> 1. Store priv_size both inside the mempool and inside the mbuf. > >> > >> 2. rte_pktmbuf_attach() should change the value of priv_size to the pr= iv_size of the direct mbuf we are going to attach to: > >> rte_pktmbuf_attach(struct rte_mbuf *mi, struct rte_mbuf *md) {... mi->= priv_size =3D md->priv_size; ...} > >> > >> 3. rte_pktmbuf_detach() should restore original value of mbuf's priv_s= ize: > >> rte_pktmbuf_detach(struct rte_mbuf *m) > >> { > >> ... > >> m->priv_size =3D rte_mempool_get_privsize(m->pool); > >> m->buf _addr=3D rte_mbuf_to_baddr(m); > >> ... > >> } > >> > >> Also I think we need to provide a way to specify priv_size for all mbu= fs of the mepool at init time: > >> - either force people to specify it at rte_mempool_create() time (prob= ably use init_arg for that), > >> - or provide separate function that could be called straight after rte= _mempool_create() , that > >> would setup priv_size for the pool and for all its mbufs. > >> - or some sort of combination of these 2 approaches - introduce a wrap= per function > >> (rte_mbuf_pool_create() or something) that would take priv_size as par= ameter, > >> create a new mempool and then setup priv_size. >=20 > I though a bit more about this solution, and I realized that doing > mi->priv_size =3D md->priv_size in rte_pktmbuf_attach() is probably not > a good idea, as there is no garantee that the size of mi is large enough > to store the priv of md. >=20 > Having the same priv_size for mi and md is maybe a good constraint. > I can add this in the API comments and assertions in the code to > check this condition, what do you think? Probably we have a different concepts of what is mbuf's private space in m= ind. >>From my point, even indirect buffer should use it's own private space and leave contents of direct mbuf it attached to unmodified. =20 After attach() operation, only contents of the buffer are shared between mb= ufs, but not the mbuf's metadata.=20 Otherwise on detach(), you'll have to copy contents of private space back, = from direct to indirect mbuf?=20 Again how to deal with the case, when 2 or more mbufs will attach to the sa= me direct one? So let say, if we'll have a macro: #define RTE_MBUF_PRIV_PTR(mb) ((void *)((struct rte_mbuf *)(mb)) + 1)) No matter is mb a direct or indirect mbuf. Do you have something else in mind here? >=20 >=20 > > Introducing rte_mbuf_pool_create() seems a good idea to me, it > > would hide 'rte_pktmbuf_pool_private' from the user and force > > to initialize all the required fields (mbuf_data_room_size only > > today, and maybe mbuf_priv_size). > > > > The API would be: > > > > struct rte_mempool * > > rte_mbuf_pool_create(const char *name, unsigned n, unsigned elt_size, > > unsigned cache_size, size_t mbuf_priv_size, > > rte_mempool_obj_ctor_t *obj_init, void *obj_init_arg, > > int socket_id, unsigned flags) > > > > I can give it a try and send a patch for this. >=20 > About this, it is not required anymore for this patch series if we > agree with my comment above. I still think we need some way to setup priv_size on a per-mempool basis. Doing that in rte_mbuf_pool_create() seems like a good thing to me. Not sure, why you decided to drop it? Konstantin >=20 > I'll send a separate patch for that. It's probably a good occasion > to get rid of the pointer casted into an integer for > mbuf_data_room_size. >=20 > Regards, > Olivier