From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id C16975A85 for ; Fri, 27 Mar 2015 14:56:34 +0100 (CET) Received: from was59-1-82-226-113-214.fbx.proxad.net ([82.226.113.214] helo=[192.168.0.10]) by mail.droids-corp.org with esmtpsa (TLS1.2:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1YbUoS-0005oN-2c; Fri, 27 Mar 2015 15:00:43 +0100 Message-ID: <55156188.6040101@6wind.com> Date: Fri, 27 Mar 2015 14:56:24 +0100 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.5.0 MIME-Version: 1.0 To: "Ananyev, Konstantin" , "dev@dpdk.org" 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> In-Reply-To: <55151DDE.8040301@6wind.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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 13:56:34 -0000 Hi Konstantin, 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_addr, >> 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 priv_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 = md->priv_size; ...} >> >> 3. rte_pktmbuf_detach() should restore original value of mbuf's priv_size: >> rte_pktmbuf_detach(struct rte_mbuf *m) >> { >> ... >> m->priv_size = rte_mempool_get_privsize(m->pool); >> m->buf _addr= rte_mbuf_to_baddr(m); >> ... >> } >> >> Also I think we need to provide a way to specify priv_size for all mbufs of the mepool at init time: >> - either force people to specify it at rte_mempool_create() time (probably 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 wrapper function >> (rte_mbuf_pool_create() or something) that would take priv_size as parameter, >> create a new mempool and then setup priv_size. I though a bit more about this solution, and I realized that doing mi->priv_size = 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. 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? > 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. About this, it is not required anymore for this patch series if we agree with my comment above. 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. Regards, Olivier