From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 995716942 for ; Thu, 26 Mar 2015 18:34:22 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga101.jf.intel.com with ESMTP; 26 Mar 2015 10:34:08 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,473,1422950400"; d="scan'208";a="546828772" Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by orsmga003.jf.intel.com with ESMTP; 26 Mar 2015 10:34:05 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.47]) by IRSMSX102.ger.corp.intel.com ([169.254.2.9]) with mapi id 14.03.0224.002; Thu, 26 Mar 2015 17:34:05 +0000 From: "Ananyev, Konstantin" To: Zoltan Kiss , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] examples/vhost: use library routines instead of local copies Thread-Index: AQHQZ05yIqg0RM8e4U+higqVDKY9yp0t76+ggAEKcgCAAAmPUA== Date: Thu, 26 Mar 2015 17:34:04 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725821407CAA@irsmsx105.ger.corp.intel.com> References: <1427309006-26590-1-git-send-email-zoltan.kiss@linaro.org> <2601191342CEEE43887BDE71AB97725821407936@irsmsx105.ger.corp.intel.com> <551437D6.80503@linaro.org> In-Reply-To: <551437D6.80503@linaro.org> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] examples/vhost: use library routines instead of local copies 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 17:34:23 -0000 > -----Original Message----- > From: Zoltan Kiss [mailto:zoltan.kiss@linaro.org] > Sent: Thursday, March 26, 2015 4:46 PM > To: Ananyev, Konstantin; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] examples/vhost: use library routines inst= ead of local copies >=20 >=20 >=20 > On 26/03/15 01:20, Ananyev, Konstantin wrote: > > > > > >> -----Original Message----- > >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zoltan Kiss > >> Sent: Wednesday, March 25, 2015 6:43 PM > >> To: dev@dpdk.org > >> Cc: Zoltan Kiss > >> Subject: [dpdk-dev] [PATCH] examples/vhost: use library routines inste= ad of local copies > >> > >> This macro and function were copies from the mbuf library, no reason t= o keep > >> them. > > > > NACK > > You can't use RTE_MBUF_INDIRECT macro here. > > If you'll look at vhost code carefully, you'll realise that we don't us= e standard rte_pktmbuf_attach() here. > > As we attach mbuf not to another mbuf but to external memory buffer, pa= ssed to us by virtio device. > > Look at attach_rxmbuf_zcp(). > Yes, I think the proper fix is to set the flag in attach_rxmbuf_zcp() > and virtio_tx_route_zcp(), then you can use the library macro here. No, it is not. IND_ATTACHED_MBUF flag indicates that that mbuf attached to another mbuf an= d __rte_pktmbuf_prefree_seg() would try to do mbuf detach. We definetly don't want to set IND_ATTACHED_MBUF here. I think there is no need to fix anything here. Konstantin >=20 > > Though I suppose, we can replace pktmbuf_detach_zcp() , with rte_pktmb= uf_detach() - they are doing identical things. > Yes, the only difference is that the latter do "m->ol_flags =3D 0" as wel= l. >=20 > > BTW, I wonder did you ever test your patch? > Indeed I did not, shame on me. I don't have a KVM setup at hand. This > fix were born as a side effect of the cleaning up in the library, > and I'm afraid I don't have the time right now to create a KVM setup. > Could anyone who has it at hand help out to run a quick test? (for the > v2 of this patch, which I'll send in shortly) >=20 > Regards, >=20 > Zoltan >=20 > > My guess it would cause vhost with '--zero-copy' to crash or corrupt t= he packets straightway. > > > > Konstantin > > > >> > >> Signed-off-by: Zoltan Kiss > >> --- > >> examples/vhost/main.c | 38 +++++--------------------------------- > >> 1 file changed, 5 insertions(+), 33 deletions(-) > >> > >> diff --git a/examples/vhost/main.c b/examples/vhost/main.c > >> index c3fcb80..1c998a5 100644 > >> --- a/examples/vhost/main.c > >> +++ b/examples/vhost/main.c > >> @@ -139,8 +139,6 @@ > >> /* Number of descriptors per cacheline. */ > >> #define DESC_PER_CACHELINE (RTE_CACHE_LINE_SIZE / sizeof(struct vrin= g_desc)) > >> > >> -#define MBUF_EXT_MEM(mb) (RTE_MBUF_FROM_BADDR((mb)->buf_addr) !=3D = (mb)) > >> - > >> /* mask of enabled ports */ > >> static uint32_t enabled_port_mask =3D 0; > >> > >> @@ -1538,32 +1536,6 @@ attach_rxmbuf_zcp(struct virtio_net *dev) > >> return; > >> } > >> > >> -/* > >> - * Detach an attched packet mbuf - > >> - * - restore original mbuf address and length values. > >> - * - reset pktmbuf data and data_len to their default values. > >> - * All other fields of the given packet mbuf will be left intact. > >> - * > >> - * @param m > >> - * The attached packet mbuf. > >> - */ > >> -static inline void pktmbuf_detach_zcp(struct rte_mbuf *m) > >> -{ > >> - const struct rte_mempool *mp =3D m->pool; > >> - void *buf =3D RTE_MBUF_TO_BADDR(m); > >> - uint32_t buf_ofs; > >> - uint32_t buf_len =3D mp->elt_size - sizeof(*m); > >> - m->buf_physaddr =3D rte_mempool_virt2phy(mp, m) + sizeof(*m); > >> - > >> - m->buf_addr =3D buf; > >> - m->buf_len =3D (uint16_t)buf_len; > >> - > >> - buf_ofs =3D (RTE_PKTMBUF_HEADROOM <=3D m->buf_len) ? > >> - RTE_PKTMBUF_HEADROOM : m->buf_len; > >> - m->data_off =3D buf_ofs; > >> - > >> - m->data_len =3D 0; > >> -} > >> > >> /* > >> * This function is called after packets have been transimited. It f= etchs mbuf > >> @@ -1590,8 +1562,8 @@ txmbuf_clean_zcp(struct virtio_net *dev, struct = vpool *vpool) > >> > >> for (index =3D 0; index < mbuf_count; index++) { > >> mbuf =3D __rte_mbuf_raw_alloc(vpool->pool); > >> - if (likely(MBUF_EXT_MEM(mbuf))) > >> - pktmbuf_detach_zcp(mbuf); > >> + if (likely(RTE_MBUF_INDIRECT(mbuf))) > >> + rte_pktmbuf_detach(mbuf); > >> rte_ring_sp_enqueue(vpool->ring, mbuf); > >> > >> /* Update used index buffer information. */ > >> @@ -1653,8 +1625,8 @@ static void mbuf_destroy_zcp(struct vpool *vpool= ) > >> for (index =3D 0; index < mbuf_count; index++) { > >> mbuf =3D __rte_mbuf_raw_alloc(vpool->pool); > >> if (likely(mbuf !=3D NULL)) { > >> - if (likely(MBUF_EXT_MEM(mbuf))) > >> - pktmbuf_detach_zcp(mbuf); > >> + if (likely(RTE_MBUF_INDIRECT(mbuf))) > >> + rte_pktmbuf_detach(mbuf); > >> rte_ring_sp_enqueue(vpool->ring, (void *)mbuf); > >> } > >> } > >> @@ -2149,7 +2121,7 @@ switch_worker_zcp(__attribute__((unused)) void *= arg) > >> } > >> while (likely(rx_count)) { > >> rx_count--; > >> - pktmbuf_detach_zcp( > >> + rte_pktmbuf_detach( > >> pkts_burst[rx_count]); > >> rte_ring_sp_enqueue( > >> vpool_array[index].ring, > >> -- > >> 1.9.1 > >