From mboxrd@z Thu Jan  1 00:00:00 1970
From: Olivier Matz <olivier.matz@6wind.com>
Subject: Re: [PATCH v6 1/2] mbuf: support attaching external
	buffer to mbuf
Date: Thu, 26 Apr 2018 21:45:17 +0200
Message-ID: <20180426194517.enoqdbe25jgi5mjm@neon>
References: <20180310012532.15809-1-yskoh@mellanox.com>
 <20180426011010.28078-1-yskoh@mellanox.com>
 <1a65f081-4c92-78d5-b00c-08e66fdef5c8@solarflare.com>
 <20180426171813.GA8437@yongseok-MBP.local>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Andrew Rybchenko <arybchenko@solarflare.com>, wenzhuo.lu@intel.com,
 jingjing.wu@intel.com, dev@dpdk.org, konstantin.ananyev@intel.com,
 stephen@networkplumber.org, thomas@monjalon.net,
 adrien.mazarguil@6wind.com, nelio.laranjeiro@6wind.com
To: Yongseok Koh <yskoh@mellanox.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67])
 by dpdk.org (Postfix) with ESMTP id 01E205F2C
 for <dev@dpdk.org>; Thu, 26 Apr 2018 21:45:25 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <20180426171813.GA8437@yongseok-MBP.local>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Thu, Apr 26, 2018 at 10:18:14AM -0700, Yongseok Koh wrote:
>

[...]

> > >   /** Mbuf prefetch */
> > >   #define RTE_MBUF_PREFETCH_TO_FREE(m) do {       \
> > >   	if ((m) != NULL)                        \
> > > @@ -1213,11 +1307,157 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
> > >   }
> > >   /**
> > > + * Initialize shared data at the end of an external buffer before attaching
> > > + * to a mbuf by ``rte_pktmbuf_attach_extbuf()``. This is not a mandatory
> > > + * initialization but a helper function to simply spare a few bytes at the
> > > + * end of the buffer for shared data. If shared data is allocated
> > > + * separately, this should not be called but application has to properly
> > > + * initialize the shared data according to its need.
> > > + *
> > > + * Free callback and its argument is saved and the refcnt is set to 1.
> > > + *
> > > + * @warning
> > > + * buf_len must be adjusted to RTE_PTR_DIFF(shinfo, buf_addr) after this
> > > + * initialization. For example,
> > 
> > May be buf_len should be inout and it should be done by the function?
> > Just a question since current approach looks fragile.
> 
> Yeah, I thought about that but I didn't want to alter user's variable, I thought
> it could be error-prone. Anyway either way is okay to me. Will wait for a day to
> get input because I will send out a new version (hopefully last :-) to fix the
> nit you mentioned below.

+1, I had exactly the same comment than Andrew in mind.
To me, it looks better to have buf_len as in/out.

I don't think it's a problem to have this change for rc2.

So,
Acked-by: Olivier Matz <olivier.matz@6wind.com>

Thank you Yongseok for this nice improvement.