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 34AC3B399 for ; Mon, 8 Sep 2014 14:00:53 +0200 (CEST) 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.0:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1XQxk7-0007jZ-8l; Mon, 08 Sep 2014 14:08:27 +0200 Message-ID: <540D9B95.3020504@6wind.com> Date: Mon, 08 Sep 2014 14:05:41 +0200 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.5.0 MIME-Version: 1.0 To: Bruce Richardson , dev@dpdk.org References: <1409759378-10113-1-git-send-email-bruce.richardson@intel.com> <1409759378-10113-8-git-send-email-bruce.richardson@intel.com> In-Reply-To: <1409759378-10113-8-git-send-email-bruce.richardson@intel.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the mbuf metadata 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: Mon, 08 Sep 2014 12:00:53 -0000 Hi Bruce, On 09/03/2014 05:49 PM, Bruce Richardson wrote: > Removed the explicit zero-sized metadata definition at the end of the > mbuf data structure. Updated the metadata macros to take account of this > change so that all existing code which uses those macros still works. > > Signed-off-by: Bruce Richardson > --- > lib/librte_mbuf/rte_mbuf.h | 22 ++++++++-------------- > 1 file changed, 8 insertions(+), 14 deletions(-) > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > index 5260001..ca66d9a 100644 > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -166,31 +166,25 @@ struct rte_mbuf { > struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */ > struct rte_mbuf *next; /**< Next segment of scattered packet. */ > > - union { > - uint8_t metadata[0]; > - uint16_t metadata16[0]; > - uint32_t metadata32[0]; > - uint64_t metadata64[0]; > - } __rte_cache_aligned; > } __rte_cache_aligned; > > #define RTE_MBUF_METADATA_UINT8(mbuf, offset) \ > - (mbuf->metadata[offset]) > + (((uint8_t *)&(mbuf)[1])[offset]) > #define RTE_MBUF_METADATA_UINT16(mbuf, offset) \ > - (mbuf->metadata16[offset/sizeof(uint16_t)]) > + (((uint16_t *)&(mbuf)[1])[offset/sizeof(uint16_t)]) > #define RTE_MBUF_METADATA_UINT32(mbuf, offset) \ > - (mbuf->metadata32[offset/sizeof(uint32_t)]) > + (((uint32_t *)&(mbuf)[1])[offset/sizeof(uint32_t)]) > #define RTE_MBUF_METADATA_UINT64(mbuf, offset) \ > - (mbuf->metadata64[offset/sizeof(uint64_t)]) > + (((uint64_t *)&(mbuf)[1])[offset/sizeof(uint64_t)]) > > #define RTE_MBUF_METADATA_UINT8_PTR(mbuf, offset) \ > - (&mbuf->metadata[offset]) > + (&RTE_MBUF_METADATA_UINT8(mbuf, offset)) > #define RTE_MBUF_METADATA_UINT16_PTR(mbuf, offset) \ > - (&mbuf->metadata16[offset/sizeof(uint16_t)]) > + (&RTE_MBUF_METADATA_UINT16(mbuf, offset)) > #define RTE_MBUF_METADATA_UINT32_PTR(mbuf, offset) \ > - (&mbuf->metadata32[offset/sizeof(uint32_t)]) > + (&RTE_MBUF_METADATA_UINT32(mbuf, offset)) > #define RTE_MBUF_METADATA_UINT64_PTR(mbuf, offset) \ > - (&mbuf->metadata64[offset/sizeof(uint64_t)]) > + (&RTE_MBUF_METADATA_UINT64(mbuf, offset)) > > /** > * Given the buf_addr returns the pointer to corresponding mbuf. > I think it goes in the good direction. So: Acked-by: Olivier Matz Just one question: why not removing RTE_MBUF_METADATA*() macros? I'd just provide one macro that gives a (void*) to the first byte after the mbuf structure. The format of the metadata is up to the application, that usually casts (m + 1) into a private structure, making the macros not very useful. I suggest to move these macros outside rte_mbuf.h, in the application-specific or library-specific header, what do you think? Regards, Olivier