diff mbox

[dpdk-dev,2/3] bus/pci: expose sysfs parsing API

Message ID 20180309230809.63361-3-xiao.w.wang@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Xiao Wang March 9, 2018, 11:08 p.m. UTC 
  Some existing sysfs parsing functions are helpful for the later vDPA
driver, this patch make them global and expose them to shared lib.

Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
---
 drivers/bus/pci/linux/pci.c             | 9 ++++-----
 drivers/bus/pci/linux/pci_init.h        | 8 ++++++++
 drivers/bus/pci/rte_bus_pci_version.map | 8 ++++++++
 3 files changed, 20 insertions(+), 5 deletions(-)

Comments

Anatoly Burakov March 14, 2018, 11:19 a.m. UTC | #1 
On 09-Mar-18 11:08 PM, Xiao Wang wrote:
> Some existing sysfs parsing functions are helpful for the later vDPA
> driver, this patch make them global and expose them to shared lib.
> 
> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> ---
>   drivers/bus/pci/linux/pci.c             | 9 ++++-----
>   drivers/bus/pci/linux/pci_init.h        | 8 ++++++++
>   drivers/bus/pci/rte_bus_pci_version.map | 8 ++++++++
>   3 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> index abde64119..81e5e5650 100644
> --- a/drivers/bus/pci/linux/pci.c
> +++ b/drivers/bus/pci/linux/pci.c
> @@ -32,7 +32,7 @@
>   
>   extern struct rte_pci_bus rte_pci_bus;
>   
> -static int
> +int
>   pci_get_kernel_driver_by_path(const char *filename, char *dri_name)

Here and in other places - shouldn't this too be prefixed with rte_?
Gaëtan Rivet March 14, 2018, 1:30 p.m. UTC | #2 
Hi,

On Wed, Mar 14, 2018 at 11:19:31AM +0000, Burakov, Anatoly wrote:
> On 09-Mar-18 11:08 PM, Xiao Wang wrote:
> > Some existing sysfs parsing functions are helpful for the later vDPA
> > driver, this patch make them global and expose them to shared lib.
> > 
> > Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> > ---
> >   drivers/bus/pci/linux/pci.c             | 9 ++++-----
> >   drivers/bus/pci/linux/pci_init.h        | 8 ++++++++
> >   drivers/bus/pci/rte_bus_pci_version.map | 8 ++++++++
> >   3 files changed, 20 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> > index abde64119..81e5e5650 100644
> > --- a/drivers/bus/pci/linux/pci.c
> > +++ b/drivers/bus/pci/linux/pci.c
> > @@ -32,7 +32,7 @@
> >   extern struct rte_pci_bus rte_pci_bus;
> > -static int
> > +int
> >   pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
> 
> Here and in other places - shouldn't this too be prefixed with rte_?
> 

A public PCI function should be prefixed by rte_pci_ yes.

Additionally, if this function was to be exposed, then there should be a
BSD implementation as well (shared map file).

I don't know how BSD works, I'm not sure parsing the filesystem is the
way to get a PCI driver name. If so, maybe the function should be called
another, generic, way, that would work for both linux and BSD (and
ideally, having a real BSD implementation).

> 
> -- 
> Thanks,
> Anatoly
Xiao Wang March 15, 2018, 4:49 p.m. UTC | #3 
Hi Rivet,

> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> Sent: Wednesday, March 14, 2018 9:31 PM
> To: Burakov, Anatoly <anatoly.burakov@intel.com>
> Cc: Wang, Xiao W <xiao.w.wang@intel.com>; dev@dpdk.org; Wang, Zhihong
> <zhihong.wang@intel.com>; maxime.coquelin@redhat.com;
> yliu@fridaylinux.org; Liang, Cunming <cunming.liang@intel.com>; Xu, Rosen
> <rosen.xu@intel.com>; Chen, Junjie J <junjie.j.chen@intel.com>; Daly, Dan
> <dan.daly@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 2/3] bus/pci: expose sysfs parsing API
> 
> Hi,
> 
> On Wed, Mar 14, 2018 at 11:19:31AM +0000, Burakov, Anatoly wrote:
> > On 09-Mar-18 11:08 PM, Xiao Wang wrote:
> > > Some existing sysfs parsing functions are helpful for the later vDPA
> > > driver, this patch make them global and expose them to shared lib.
> > >
> > > Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> > > ---
> > >   drivers/bus/pci/linux/pci.c             | 9 ++++-----
> > >   drivers/bus/pci/linux/pci_init.h        | 8 ++++++++
> > >   drivers/bus/pci/rte_bus_pci_version.map | 8 ++++++++
> > >   3 files changed, 20 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> > > index abde64119..81e5e5650 100644
> > > --- a/drivers/bus/pci/linux/pci.c
> > > +++ b/drivers/bus/pci/linux/pci.c
> > > @@ -32,7 +32,7 @@
> > >   extern struct rte_pci_bus rte_pci_bus;
> > > -static int
> > > +int
> > >   pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
> >
> > Here and in other places - shouldn't this too be prefixed with rte_?
> >
> 
> A public PCI function should be prefixed by rte_pci_ yes.

OK, will add this prefix.

> 
> Additionally, if this function was to be exposed, then there should be a
> BSD implementation as well (shared map file).
> 
> I don't know how BSD works, I'm not sure parsing the filesystem is the
> way to get a PCI driver name. If so, maybe the function should be called
> another, generic, way, that would work for both linux and BSD (and
> ideally, having a real BSD implementation).

BSD is not parsing the filesystem, it uses PCIOCGETCONF ioctl to retrieve
PCI device information.
This function is quite linux, especially for the API name. I'm afraid we can
only return err on BSD for this API.

BRs,
Xiao

> 
> >
> > --
> > Thanks,
> > Anatoly
> 
> --
> Gaëtan Rivet
> 6WIND
Gaëtan Rivet March 15, 2018, 5:19 p.m. UTC | #4 
On Thu, Mar 15, 2018 at 04:49:41PM +0000, Wang, Xiao W wrote:
> Hi Rivet,
> 
> > -----Original Message-----
> > From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> > Sent: Wednesday, March 14, 2018 9:31 PM
> > To: Burakov, Anatoly <anatoly.burakov@intel.com>
> > Cc: Wang, Xiao W <xiao.w.wang@intel.com>; dev@dpdk.org; Wang, Zhihong
> > <zhihong.wang@intel.com>; maxime.coquelin@redhat.com;
> > yliu@fridaylinux.org; Liang, Cunming <cunming.liang@intel.com>; Xu, Rosen
> > <rosen.xu@intel.com>; Chen, Junjie J <junjie.j.chen@intel.com>; Daly, Dan
> > <dan.daly@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH 2/3] bus/pci: expose sysfs parsing API
> > 
> > Hi,
> > 
> > On Wed, Mar 14, 2018 at 11:19:31AM +0000, Burakov, Anatoly wrote:
> > > On 09-Mar-18 11:08 PM, Xiao Wang wrote:
> > > > Some existing sysfs parsing functions are helpful for the later vDPA
> > > > driver, this patch make them global and expose them to shared lib.
> > > >
> > > > Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> > > > ---
> > > >   drivers/bus/pci/linux/pci.c             | 9 ++++-----
> > > >   drivers/bus/pci/linux/pci_init.h        | 8 ++++++++
> > > >   drivers/bus/pci/rte_bus_pci_version.map | 8 ++++++++
> > > >   3 files changed, 20 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> > > > index abde64119..81e5e5650 100644
> > > > --- a/drivers/bus/pci/linux/pci.c
> > > > +++ b/drivers/bus/pci/linux/pci.c
> > > > @@ -32,7 +32,7 @@
> > > >   extern struct rte_pci_bus rte_pci_bus;
> > > > -static int
> > > > +int
> > > >   pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
> > >
> > > Here and in other places - shouldn't this too be prefixed with rte_?
> > >
> > 
> > A public PCI function should be prefixed by rte_pci_ yes.
> 
> OK, will add this prefix.
> 
> > 
> > Additionally, if this function was to be exposed, then there should be a
> > BSD implementation as well (shared map file).
> > 
> > I don't know how BSD works, I'm not sure parsing the filesystem is the
> > way to get a PCI driver name. If so, maybe the function should be called
> > another, generic, way, that would work for both linux and BSD (and
> > ideally, having a real BSD implementation).
> 
> BSD is not parsing the filesystem, it uses PCIOCGETCONF ioctl to retrieve
> PCI device information.
> This function is quite linux, especially for the API name. I'm afraid we can
> only return err on BSD for this API.

How about renaming the function to something like
rte_pci_device_kdriver_name();

and allowing for a sensible BSD implementation to happen if someone
needs it?
Xiao Wang March 19, 2018, 1:31 a.m. UTC | #5 
Hi Rivet,

> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> Sent: Friday, March 16, 2018 1:19 AM
> To: Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: Burakov, Anatoly <anatoly.burakov@intel.com>; dev@dpdk.org; Wang,
> Zhihong <zhihong.wang@intel.com>; maxime.coquelin@redhat.com;
> yliu@fridaylinux.org; Liang, Cunming <cunming.liang@intel.com>; Xu, Rosen
> <rosen.xu@intel.com>; Chen, Junjie J <junjie.j.chen@intel.com>; Daly, Dan
> <dan.daly@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 2/3] bus/pci: expose sysfs parsing API
> 
> On Thu, Mar 15, 2018 at 04:49:41PM +0000, Wang, Xiao W wrote:
> > Hi Rivet,
> >
> > > -----Original Message-----
> > > From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> > > Sent: Wednesday, March 14, 2018 9:31 PM
> > > To: Burakov, Anatoly <anatoly.burakov@intel.com>
> > > Cc: Wang, Xiao W <xiao.w.wang@intel.com>; dev@dpdk.org; Wang,
> Zhihong
> > > <zhihong.wang@intel.com>; maxime.coquelin@redhat.com;
> > > yliu@fridaylinux.org; Liang, Cunming <cunming.liang@intel.com>; Xu,
> Rosen
> > > <rosen.xu@intel.com>; Chen, Junjie J <junjie.j.chen@intel.com>; Daly, Dan
> > > <dan.daly@intel.com>
> > > Subject: Re: [dpdk-dev] [PATCH 2/3] bus/pci: expose sysfs parsing API
> > >
> > > Hi,
> > >
> > > On Wed, Mar 14, 2018 at 11:19:31AM +0000, Burakov, Anatoly wrote:
> > > > On 09-Mar-18 11:08 PM, Xiao Wang wrote:
> > > > > Some existing sysfs parsing functions are helpful for the later vDPA
> > > > > driver, this patch make them global and expose them to shared lib.
> > > > >
> > > > > Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> > > > > ---
> > > > >   drivers/bus/pci/linux/pci.c             | 9 ++++-----
> > > > >   drivers/bus/pci/linux/pci_init.h        | 8 ++++++++
> > > > >   drivers/bus/pci/rte_bus_pci_version.map | 8 ++++++++
> > > > >   3 files changed, 20 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> > > > > index abde64119..81e5e5650 100644
> > > > > --- a/drivers/bus/pci/linux/pci.c
> > > > > +++ b/drivers/bus/pci/linux/pci.c
> > > > > @@ -32,7 +32,7 @@
> > > > >   extern struct rte_pci_bus rte_pci_bus;
> > > > > -static int
> > > > > +int
> > > > >   pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
> > > >
> > > > Here and in other places - shouldn't this too be prefixed with rte_?
> > > >
> > >
> > > A public PCI function should be prefixed by rte_pci_ yes.
> >
> > OK, will add this prefix.
> >
> > >
> > > Additionally, if this function was to be exposed, then there should be a
> > > BSD implementation as well (shared map file).
> > >
> > > I don't know how BSD works, I'm not sure parsing the filesystem is the
> > > way to get a PCI driver name. If so, maybe the function should be called
> > > another, generic, way, that would work for both linux and BSD (and
> > > ideally, having a real BSD implementation).
> >
> > BSD is not parsing the filesystem, it uses PCIOCGETCONF ioctl to retrieve
> > PCI device information.
> > This function is quite linux, especially for the API name. I'm afraid we can
> > only return err on BSD for this API.
> 
> How about renaming the function to something like
> rte_pci_device_kdriver_name();
> 
> and allowing for a sensible BSD implementation to happen if someone
> needs it?

Yes, it looks more generic, and allows a BSD implementation to happen.
I will rename it as below in next version.
rte_pci_device_kdriver_name(const struct rte_pci_addr *addr, char *dri_name)

BRs,
Xiao

> 
> --
> Gaëtan Rivet
> 6WIND
Xiao Wang March 21, 2018, 1:21 p.m. UTC | #6 
This patch set has dependency on http://dpdk.org/dev/patchwork/patch/36241/
(vhost: support selective datapath);

ifc VF is virtio vring compatible device, it can be used to accelerate vhost
data path. This patch implements vDPA driver ops which configures ifc VF to
be a vhost data path accelerator.

ifcvf driver uses vdev as a control domain to manage ifc VFs that belong
to it. It registers vDPA device ops to vhost lib to enable these VFs to be
used as vhost data path accelerator.

Live migration feature is supported by ifc VF and this driver enables
it based on vhost lib.

vDPA needs to create different containers for different devices, thus this
patch set adds some APIs in eal/vfio to support multiple container, e.g.
- rte_vfio_create_container
- rte_vfio_destroy_container
- rte_vfio_bind_group_no
- rte_vfio_unbind_group_no
By this extension, a device can be put into a new specific container, rather
than the previous default container.

v2:
- Rename function pci_get_kernel_driver_by_path to rte_pci_device_kdriver_name
  to make the API generic cross Linux and BSD, make it as EXPERIMENTAL.
- Rebase on Zhihong's vDPA v3 patch set.
- Minor code cleanup on vfio extension.

Junjie Chen (1):
  eal/vfio: add support for multiple container

Xiao Wang (2):
  bus/pci: expose sysfs parsing API
  net/ifcvf: add ifcvf driver

 config/common_base                       |    6 +
 config/common_linuxapp                   |    1 +
 drivers/bus/pci/Makefile                 |    2 +
 drivers/bus/pci/bsd/pci.c                |   14 +
 drivers/bus/pci/linux/pci.c              |   22 +-
 drivers/bus/pci/rte_bus_pci.h            |   32 +
 drivers/bus/pci/rte_bus_pci_version.map  |    8 +
 drivers/net/Makefile                     |    1 +
 drivers/net/ifcvf/Makefile               |   40 +
 drivers/net/ifcvf/base/ifcvf.c           |  329 ++++++++
 drivers/net/ifcvf/base/ifcvf.h           |  156 ++++
 drivers/net/ifcvf/base/ifcvf_osdep.h     |   52 ++
 drivers/net/ifcvf/ifcvf_ethdev.c         | 1240 ++++++++++++++++++++++++++++++
 drivers/net/ifcvf/rte_ifcvf_version.map  |    4 +
 lib/librte_eal/bsdapp/eal/eal.c          |   51 +-
 lib/librte_eal/common/include/rte_vfio.h |  117 ++-
 lib/librte_eal/linuxapp/eal/eal_vfio.c   |  553 ++++++++++---
 lib/librte_eal/linuxapp/eal/eal_vfio.h   |    2 +
 lib/librte_eal/rte_eal_version.map       |    7 +
 mk/rte.app.mk                            |    1 +
 20 files changed, 2527 insertions(+), 111 deletions(-)
 create mode 100644 drivers/net/ifcvf/Makefile
 create mode 100644 drivers/net/ifcvf/base/ifcvf.c
 create mode 100644 drivers/net/ifcvf/base/ifcvf.h
 create mode 100644 drivers/net/ifcvf/base/ifcvf_osdep.h
 create mode 100644 drivers/net/ifcvf/ifcvf_ethdev.c
 create mode 100644 drivers/net/ifcvf/rte_ifcvf_version.map
diff mbox

Patch

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index abde64119..81e5e5650 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -32,7 +32,7 @@ 
 
 extern struct rte_pci_bus rte_pci_bus;
 
-static int
+int
 pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
 {
 	int count;
@@ -168,9 +168,8 @@  pci_parse_one_sysfs_resource(char *line, size_t len, uint64_t *phys_addr,
 	return 0;
 }
 
-/* parse the "resource" sysfs file */
-static int
-pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
+int
+rte_pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
 {
 	FILE *f;
 	char buf[BUFSIZ];
@@ -302,7 +301,7 @@  pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 
 	/* parse resources */
 	snprintf(filename, sizeof(filename), "%s/resource", dirname);
-	if (pci_parse_sysfs_resource(filename, dev) < 0) {
+	if (rte_pci_parse_sysfs_resource(filename, dev) < 0) {
 		RTE_LOG(ERR, EAL, "%s(): cannot parse resource\n", __func__);
 		free(dev);
 		return -1;
diff --git a/drivers/bus/pci/linux/pci_init.h b/drivers/bus/pci/linux/pci_init.h
index c2e603a37..e871c3942 100644
--- a/drivers/bus/pci/linux/pci_init.h
+++ b/drivers/bus/pci/linux/pci_init.h
@@ -83,6 +83,14 @@  int pci_vfio_unmap_resource(struct rte_pci_device *dev);
 
 int pci_vfio_is_enabled(void);
 
+/* parse sysfs file path */
+int
+pci_get_kernel_driver_by_path(const char *filename, char *dri_name);
+
+/* parse the "resource" sysfs file */
+int
+rte_pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev);
+
 #endif
 
 #endif /* EAL_PCI_INIT_H_ */
diff --git a/drivers/bus/pci/rte_bus_pci_version.map b/drivers/bus/pci/rte_bus_pci_version.map
index 27e9c4f10..dff2b52e8 100644
--- a/drivers/bus/pci/rte_bus_pci_version.map
+++ b/drivers/bus/pci/rte_bus_pci_version.map
@@ -16,3 +16,11 @@  DPDK_17.11 {
 
 	local: *;
 };
+
+DPDK_18.05 {
+	global:
+
+	pci_get_kernel_driver_by_path;
+	rte_pci_parse_sysfs_resource;
+
+} DPDK_17.11;