patches for DPDK stable branches
 help / color / mirror / Atom feed
From: "Huang, Wei" <wei.huang@intel.com>
To: "Yigit, Ferruh" <ferruh.yigit@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>, "Xu, Rosen" <rosen.xu@intel.com>,
	"Zhang, Qi Z" <qi.z.zhang@intel.com>
Cc: "stable@dpdk.org" <stable@dpdk.org>,
	"Zhang, Tianfei" <tianfei.zhang@intel.com>,
	Ray Kinsella <mdr@ashroe.eu>
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH v11 3/4] raw/ifpga: add OPAE API for OpenStack Cyborg
Date: Fri, 22 Jan 2021 03:16:47 +0000
Message-ID: <DM6PR11MB35305BAAC693355F86E61CC1EFA00@DM6PR11MB3530.namprd11.prod.outlook.com> (raw)
In-Reply-To: <afcf29f0-be66-a35c-bfd0-9f2c0329a51a@intel.com>

Hi Ferruh,

That's a good question.
The answer is YES, we do need all these functions. We have complete the integrated test with Cyborg, there is no redundant function. Let me show you a use case in Cyborg.
Cyborg will update FPGA flash and reboot it to make it effective, so they will call functions like below.
1. opae_enumerate() to find the target FPGA
2. opae_get_property() to check FPGA version
3. opae_get_image_info() to check the update image
4. opae_update_flash() to update FPGA flash
5. opae_reboot_device() to reboot FPGA
6. After reboot, FPGA kernel driver will not be vfio-pci by default, opae_bind_driver() is used to bind vfio-pci kernel driver.
7. opae_probe_device() to attach ifpga PMD to FPGA, then this FPGA can be managed by Cyborg again.

These functions will be wrapped in Python package. Cyborg require this Python package can be downloaded from PyPI and compiled without DPDK installed.
So there is an independent project which will create a static library file from target DPDK. This library is part of Python package and will be used when compiling Python module. That's why I didn't export these function in map file.
BTW, the header file ifpga_opae_api.h is also integrated into Python package from target DPDK.

Thanks,
Wei

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Friday, January 22, 2021 00:30
To: Huang, Wei <wei.huang@intel.com>; dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Ray Kinsella <mdr@ashroe.eu>
Subject: Re: [dpdk-dev] [PATCH v11 3/4] raw/ifpga: add OPAE API for OpenStack Cyborg

On 1/21/2021 6:03 AM, Wei Huang wrote:
> Cyborg is an OpenStack project that aims to provide a general purpose 
> management framework for acceleration resources (i.e. various types of 
> accelerators such as GPU, FPGA, NP, ODP, DPDK/SPDK and so on).
> It needs some OPAE type APIs to manage PACs (Programmable Acceleration
> Card) with Intel FPGA. Below major functions are added to meets Cyborg 
> requirements.
> 1. opae_init() set up OPAE environment.
> 2. opae_cleanup() clean up OPAE environment.
> 3. opae_enumerate() searches PAC with specific FPGA.
> 4. opae_get_property() gets properties of FPGA.
> 5. opae_partial_reconfigure() perform partial configuration on FPGA.
> 6. opae_get_image_info() gets information of image file.
> 7. opae_update_flash() updates FPGA flash with specific image file.
> 8. opae_cancel_flash_update() cancel process of FPGA flash update.
> 9. opae_probe_device() manually probe specific FPGA with ifpga driver.
> 10. opae_remove_device() manually remove specific FPGA from ifpga driver.
> 11. opae_bind_driver() binds specific FPGA with specified kernel driver.
> 12. opae_unbind_driver() unbinds specific FPGA from kernel driver.
> 13. opae_reboot_device() reboots specific FPGA (do reconfiguration).
> 

Hi Wei,

As far as I understand you are adding above public functions which are on top of raw/ifpga driver functions, so they are like PMD specific APIs, I think there are a few problems with it:

1) Do we really need/want this much PMD specific API? Can't we have them through the rawdev abstraction layer?

2) DPDK public APIs are part of API/ABI policy, so there are a few rules they have to follow, like:
- They should start with 'rte_' prefix, and the PMD specific APIs should start with 'rte_pmd_' prefix
- They should be in the .map file
- They should be experimental at least one release
- They should be fully documented in a doxygen format
   - Header file should be added to index file for API documentation

Please don't update above before 1) is clearified and we are sure new APIs are required.

<...>

> @@ -13,8 +13,10 @@ objs = [base_objs]
>   deps += ['ethdev', 'rawdev', 'pci', 'bus_pci', 'kvargs',
>   	'bus_vdev', 'bus_ifpga', 'net', 'net_i40e', 'net_ipn3ke']
>   
> -sources = files('ifpga_rawdev.c')
> +sources = files('ifpga_rawdev.c', 'ifpga_opae_api.c')
>   
>   includes += include_directories('base')
>   includes += include_directories('../../net/ipn3ke')
>   includes += include_directories('../../net/i40e')
> +
> +install_headers('ifpga_opae_api.h')
> 

There is a 'headers' helper that you can use for meson. Also the header file name should start with 'rte_pmd_'.

Even before this patch, isn't application has to include the rawdev PMD header? 
Why that header was not installed?

  reply	other threads:[~2021-01-22  3:16 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-21  6:03 [dpdk-stable] [PATCH v11 0/4] raw/ifpga: add extra OPAE APIs Wei Huang
2021-01-21  6:03 ` [dpdk-stable] [PATCH v11 1/4] raw/ifpga: add fpga rsu function Wei Huang
2021-01-21 16:30   ` Ferruh Yigit
2021-01-22  2:18     ` Huang, Wei
2021-01-25 10:01       ` Ferruh Yigit
2021-01-25 10:33         ` Xu, Rosen
2021-01-26  1:16           ` Huang, Wei
2021-01-21  6:03 ` [dpdk-stable] [PATCH v11 2/4] raw/ifpga: add fpga property get function Wei Huang
2021-01-21  6:03 ` [dpdk-stable] [PATCH v11 3/4] raw/ifpga: add OPAE API for OpenStack Cyborg Wei Huang
2021-01-21 16:29   ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
2021-01-22  2:43     ` Huang, Wei
2021-01-25 10:07       ` Ferruh Yigit
2021-01-21 16:30   ` Ferruh Yigit
2021-01-22  3:16     ` Huang, Wei [this message]
2021-01-21 16:33   ` Ferruh Yigit
2021-01-22  3:25     ` Huang, Wei
2021-01-21  6:03 ` [dpdk-stable] [PATCH v11 4/4] examples/ifpga: add example for opae ifpga API Wei Huang
2021-01-21 16:49   ` Ferruh Yigit
2021-01-22  3:52     ` Huang, Wei
2021-01-21  7:08 ` [dpdk-stable] [PATCH v11 0/4] raw/ifpga: add extra OPAE APIs Zhang, Qi Z

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6PR11MB35305BAAC693355F86E61CC1EFA00@DM6PR11MB3530.namprd11.prod.outlook.com \
    --to=wei.huang@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=mdr@ashroe.eu \
    --cc=qi.z.zhang@intel.com \
    --cc=rosen.xu@intel.com \
    --cc=stable@dpdk.org \
    --cc=tianfei.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git