From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f42.google.com (mail-wg0-f42.google.com [74.125.82.42]) by dpdk.org (Postfix) with ESMTP id 02D2AA6A for ; Thu, 26 Mar 2015 09:44:51 +0100 (CET) Received: by wgs2 with SMTP id 2so56158861wgs.1 for ; Thu, 26 Mar 2015 01:44:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=5mPTVmt7kMbym8cDufRyxhRvxX3RqCrrFAE8L+ZE9hQ=; b=Kz4sgTTVqQ2dBRtLjaFV0AljtCWurmyDZacXB6OsxNY9hcI6gpviZUHP8vSmdepltC Mg5e+2w600TRaLHPKTgQqTrLYEyzI2S+0kgwT6O8bEYtXJ3KdO6nHIesIibQHiUvPDDI rXuumS4cKphcszrc6AodxEEj+TTUm72Yu3NiQfz8zjM3dL35rqMaMnrjNB5+b8jyC2Re jeQwgMiwfwaTsIdUCdE52AHx7u8Q6WM0u++ptE/TV5y8fBJ+WYqiwB0ChHTW1jazGi9p zC56KTU8BzPFymr2+CIGexzAR4iaSwq5/9WDY2q+mCFAa3VfmF4IBqgoWTDu16/TsaVV XRvA== X-Gm-Message-State: ALoCoQl4VfVFPADHLxG4rCzIMSsUog7IxrtpXSXOhhuBnJVby70E+2v6SgQmwvPnDA7JpczGV52c X-Received: by 10.194.60.104 with SMTP id g8mr25538063wjr.96.1427359490787; Thu, 26 Mar 2015 01:44:50 -0700 (PDT) Received: from [10.16.0.195] (6wind.net2.nerim.net. [213.41.180.237]) by mx.google.com with ESMTPSA id bx3sm7316518wjc.21.2015.03.26.01.44.49 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 26 Mar 2015 01:44:50 -0700 (PDT) Message-ID: <5513C703.6040908@6wind.com> Date: Thu, 26 Mar 2015 09:44:51 +0100 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.3.0 MIME-Version: 1.0 To: Zoltan Kiss , dev@dpdk.org, dev@openvswitch.org References: <5511A1F0.40605@linaro.org> <5512EA87.1020707@6wind.com> <55130506.4090000@linaro.org> In-Reply-To: <55130506.4090000@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] ovs-dpdk: placing the 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: Thu, 26 Mar 2015 08:44:51 -0000 Hi Zoltan, On 03/25/2015 07:57 PM, Zoltan Kiss wrote: > I have some comments for the first patch: > >> diff --git a/examples/vhost/main.c b/examples/vhost/main.c >> index c3fcb80..050f3ac 100644 >> --- a/examples/vhost/main.c >> +++ b/examples/vhost/main.c > I've sent in a separate patch for this file, I think it's just easier to > ditch the old copy-pasted code, see "[PATCH] examples/vhost: use library > routines instead of local copies" As Konstantin stated in the other thread, this modification is wrong, sorry. I don't see why I changed this code instead of keeping the initial logic. I'll fix that in the v2. >> /** >> - * Given the buf_addr returns the pointer to corresponding mbuf. >> + * Return the mbuf owning the given data buffer address. >> + * >> + * @param mi >> + * The pointer to the indirect mbuf. >> + * @param buffer_addr >> + * The address of the data buffer of the direct mbuf. > You don't need this parameter, it's mi->buf_addr. That's correct. In this case, I propose to change the name of the function in: struct rte_mbuf *rte_mbuf_from_indirect(struct rte_mbuf *mi) >> @@ -744,9 +767,11 @@ static inline void rte_pktmbuf_attach(struct >> rte_mbuf *mi, struct rte_mbuf *md) >> static inline void rte_pktmbuf_detach(struct rte_mbuf *m) >> { >> const struct rte_mempool *mp = m->pool; >> - void *buf = RTE_MBUF_TO_BADDR(m); >> + void *buf = rte_mbuf_to_baddr(m); >> uint32_t buf_len = mp->elt_size - sizeof(*m); > I don't see any reason to keep buf and buf_len, just assign straight to > m->buf_addr and *len. > Besides that, you need to deduct m->priv_size from buf_len. Agree. I suggest something like: static inline void rte_pktmbuf_detach(struct rte_mbuf *m) { const struct rte_mempool *mp = m->pool; void *buf = rte_mbuf_to_baddr(m); unsigned mhdr_size = (char *)buf - (char *)m; m->buf_addr = buf; m->buf_physaddr = rte_mempool_virt2phy(mp, m) + mhdr_size; m->buf_len = (uint16_t)(mp->elt_size - mhdr_size); ... > The rest of the series looks good, > > Reviewed-by: Zoltan Kiss Thank you for your review. I'll send a v2 today. Regards, Olivier