From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <danny.zhou@intel.com>
Received: from mga03.intel.com (mga03.intel.com [134.134.136.65])
 by dpdk.org (Postfix) with ESMTP id 8AEFA9AB6
 for <dev@dpdk.org>; Mon, 23 Feb 2015 16:24:19 +0100 (CET)
Received: from orsmga002.jf.intel.com ([10.7.209.21])
 by orsmga103.jf.intel.com with ESMTP; 23 Feb 2015 07:18:40 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.09,631,1418112000"; d="scan'208";a="689470033"
Received: from pgsmsx107.gar.corp.intel.com ([10.221.44.105])
 by orsmga002.jf.intel.com with ESMTP; 23 Feb 2015 07:24:16 -0800
Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by
 PGSMSX107.gar.corp.intel.com (10.221.44.105) with Microsoft SMTP Server (TLS)
 id 14.3.195.1; Mon, 23 Feb 2015 23:24:14 +0800
Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.161]) by
 SHSMSX152.ccr.corp.intel.com ([169.254.6.46]) with mapi id 14.03.0195.001;
 Mon, 23 Feb 2015 23:24:13 +0800
From: "Zhou, Danny" <danny.zhou@intel.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Thread-Topic: [dpdk-dev] [PATCH v4 4/5] eal: add per rx queue interrupt
 handling based on VFIO
Thread-Index: AQHQTV7E/DHJ5wnUz0u1M4UeJBEYn5z+IH3Q//+XyACAAJywAP//glyAgACGbmA=
Date: Mon, 23 Feb 2015 15:24:13 +0000
Message-ID: <DFDF335405C17848924A094BC35766CF0AAB3D4E@SHSMSX104.ccr.corp.intel.com>
References: <1424353698-29837-1-git-send-email-danny.zhou@intel.com>
 <3509738.2W4po8ncMD@xps13>
 <DFDF335405C17848924A094BC35766CF0AAB3C60@SHSMSX104.ccr.corp.intel.com>
 <2740446.fpxNnYhYZp@xps13>
In-Reply-To: <2740446.fpxNnYhYZp@xps13>
Accept-Language: zh-CN, en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-originating-ip: [10.239.127.40]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v4 4/5] eal: add per rx queue interrupt
 handling based on VFIO
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://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: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 23 Feb 2015 15:24:20 -0000


> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Monday, February 23, 2015 11:19 PM
> To: Zhou, Danny
> Cc: Gonzalez Monroy, Sergio; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 4/5] eal: add per rx queue interrupt ha=
ndling based on VFIO
>=20
> 2015-02-23 15:02, Zhou, Danny:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2015-02-23 11:47, Zhou, Danny:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > 2015-02-19 21:48, Zhou Danny:
> > > > > > --- a/lib/librte_eal/linuxapp/eal/Makefile
> > > > > > +++ b/lib/librte_eal/linuxapp/eal/Makefile
> > > > > > @@ -43,6 +43,7 @@ CFLAGS +=3D -I$(SRCDIR)/include
> > > > > >  CFLAGS +=3D -I$(RTE_SDK)/lib/librte_eal/common
> > > > > >  CFLAGS +=3D -I$(RTE_SDK)/lib/librte_eal/common/include
> > > > > >  CFLAGS +=3D -I$(RTE_SDK)/lib/librte_ring
> > > > > > +CFLAGS +=3D -I$(RTE_SDK)/lib/librte_mbuf
> > > > > >  CFLAGS +=3D -I$(RTE_SDK)/lib/librte_mempool
> > > > > >  CFLAGS +=3D -I$(RTE_SDK)/lib/librte_malloc
> > > > > >  CFLAGS +=3D -I$(RTE_SDK)/lib/librte_ether
> > > > >
> > > > > Why do we need mbuf in EAL?
> > > >
> > > > The file eal_interrupts.c includes rte_ethdev.h which defines struc=
ture rte_eth_devices that
> > > > eal needs to use in order to get per-port intr_handle. The rte_ethd=
ev.h includes the rte_mbuf.h
> > > > so the Makefile is updated here.
> > >
> > > I see. You are breaking layer isolation by introducing ethdev in EAL.
> > > The cause seems to be:
> > >
> > > +       struct rte_intr_handle intr_handle =3D
> > > +                               rte_eth_devices[port_id].pci_dev->int=
r_handle;
> > >
> > > Maybe that pci_dev should be a parameter of the function.
> >
> > Adding pci_dev as a parameter has similar problem due to eal does not i=
nclude rte_pci.h which
>=20
> I don't understand your assertion. rte_pci.h is part of EAL.
>=20

rte_eal.h does not include any DPDK header file, adding pci_dev would force=
 it to include rte_pci.h file.
With solution below, those kinds of mess could be completely avoided.

> > defines struct rte_pci_device. It looks the new-added function rte_eal_=
wait_rx_intr(uint8_t port_id, uint8_t queue_id);
> > is not proper to be declared in rte_eal.h, will rename it to
> > 	rte_intr_wait_rx (struct rte_intr_handle *intr_handle, uint8_t queue_i=
d);
> >
> > and then move declaration from rte_eal.h to rte_interrupts.h. So isolat=
ion can be avoided and no need to includes
> rte_ethdev.h
> > and change Makefile.
>=20
> Yes could be better.