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 3C8D36A87 for ; Tue, 31 Mar 2015 21:01:40 +0200 (CEST) Received: from [185.65.185.242] (helo=[192.168.1.30]) by mail.droids-corp.org with esmtpsa (TLS1.2:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1Yd1Tv-0005rs-Eg; Tue, 31 Mar 2015 21:05:50 +0200 Message-ID: <551AEF08.8020009@6wind.com> Date: Tue, 31 Mar 2015 21:01:28 +0200 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" 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> In-Reply-To: <2601191342CEEE43887BDE71AB97725821412EE9@irsmsx105.ger.corp.intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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: Tue, 31 Mar 2015 19:01:40 -0000 Hi Konstantin, 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 not >>>> 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 = rte_mbufpool_get_priv_size(mp); >>> >>> struct rte_mbuf *m = _m + priv_size; >>> uint32_t buf_len = mp->elt_size - sizeof(struct rte_mbuf) - priv_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_size at attach/detach. >>> Again RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() would just work without 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 pointer to the new object. > So, rte_pktmbuf_init() would return m and then in mempool_add_elem(): > > if (obj_init) > obj = obj_init(mp, obj_init_arg, obj, obj_idx); > > rte_ring_sp_enqueue(mp->ring, obj); 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. 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(). 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). Regards Olivier