Re: [dpdk-dev] [PATCH v2 0/7] Removal of PCI bus ABIs

["Liu, Changpeng" <changpeng.liu@intel.com>]

I tried the above DPDK patches, and got the following errors:

pci.c:115:7: error: call to ‘rte_pci_read_config’ declared with attribute error: Symbol is not public ABI
  115 |  rc = rte_pci_read_config(dev->dev_handle, value, len, offset);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pci.c: In function ‘cfg_write_rte’:
pci.c:125:7: error: call to ‘rte_pci_write_config’ declared with attribute error: Symbol is not public ABI
  125 |  rc = rte_pci_write_config(dev->dev_handle, value, len, offset);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pci.c: In function ‘register_rte_driver’:
pci.c:375:2: error: call to ‘rte_pci_register’ declared with attribute error: Symbol is not public ABI
  375 |  rte_pci_register(&driver->driver); 

We may use the new added API to replace rte_pci_write_config and rte_pci_read_config, but SPDK
do require rte_pci_register().


> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Wednesday, October 6, 2021 12:26 PM
> To: Harris, James R <james.r.harris@intel.com>; David Marchand
> <david.marchand@redhat.com>; Liu, Changpeng <changpeng.liu@intel.com>
> Cc: dev@dpdk.org; ci@dpdk.org; Aaron Conole <aconole@redhat.com>; dpdklab
> <dpdklab@iol.unh.edu>; Zawadzki, Tomasz <tomasz.zawadzki@intel.com>;
> alexeymar@mellanox.com
> Subject: RE: [dpdk-dev] [PATCH v2 0/7] Removal of PCI bus ABIs
> 
> Thanks David for helping check this and including SPDK folks!
> 
> Hi Changpeng,
> 
> Although we have synced about this during last release's deprecation notice,
> I’d like to summarize two points for SPDK to change if this patchset applied.
> 
> 1. The pci bus header for drivers will only be exposed if meson option
> 'enable_driver_sdk' is added, so SPDK need this DPDK meson option to build.
> 
> 2. As some functions in pci bus is needed for apps and the rest for drivers,
> the header for driver is renamed to pci_driver.h (header for app is rte_bus_pci.h).
> So SPDK drivers will need pci_driver.h instead of rte_bus_pci.h starting from
> DPDK
> 21.11. David showed some tests he did below.
> 
> Could you help check above two updates are fine to SPDK?
> 
> Thanks,
> Chenbo
> 
> > -----Original Message-----
> > From: Harris, James R <james.r.harris@intel.com>
> > Sent: Monday, October 4, 2021 11:56 PM
> > To: David Marchand <david.marchand@redhat.com>; Xia, Chenbo
> > <chenbo.xia@intel.com>; Liu, Changpeng <changpeng.liu@intel.com>
> > Cc: dev@dpdk.org; ci@dpdk.org; Aaron Conole <aconole@redhat.com>;
> dpdklab
> > <dpdklab@iol.unh.edu>; Zawadzki, Tomasz <tomasz.zawadzki@intel.com>;
> > alexeymar@mellanox.com
> > Subject: Re: [dpdk-dev] [PATCH v2 0/7] Removal of PCI bus ABIs
> >
> > Adding Changpeng Liu from SPDK side.
> >
> > On 10/4/21, 6:48 AM, "David Marchand" <david.marchand@redhat.com>
> wrote:
> >
> >     On Thu, Sep 30, 2021 at 10:45 AM David Marchand
> >     <david.marchand@redhat.com> wrote:
> >     > On Wed, Sep 29, 2021 at 9:38 AM Xia, Chenbo <chenbo.xia@intel.com>
> > wrote:
> >     > > @David, could you help me understand what is the compile error in
> > Fedora 31?
> >     > > DPDK_compile_spdk failure is expected as the header name for SPDK
> > is changed,
> >     > > I am not sure if it's the same error...
> >     >
> >     > The error log is odd (no compilation "backtrace").
> >     > You'll need to test spdk manually I guess.
> >
> >     Tried your series with SPDK (w/o and w/ enable_driver_sdk).
> >     I think the same, and the error is likely due to the file rename.
> >
> >     $ make
> >       CC lib/env_dpdk/env.o
> >     In file included from env.c:39:0:
> >     env_internal.h:64:25: error: field ‘driver’ has incomplete type
> >       struct rte_pci_driver  driver;
> >                              ^
> >     env_internal.h:75:59: warning: ‘struct rte_pci_device’ declared inside
> >     parameter list [enabled by default]
> >      int pci_device_init(struct rte_pci_driver *driver, struct
> >     rte_pci_device *device);
> >                                                                ^
> >     env_internal.h:75:59: warning: its scope is only this definition or
> >     declaration, which is probably not what you want [enabled by default]
> >     env_internal.h:76:28: warning: ‘struct rte_pci_device’ declared inside
> >     parameter list [enabled by default]
> >      int pci_device_fini(struct rte_pci_device *device);
> >                                 ^
> >     env_internal.h:89:38: warning: ‘struct rte_pci_device’ declared inside
> >     parameter list [enabled by default]
> >      void vtophys_pci_device_added(struct rte_pci_device *pci_device);
> >                                           ^
> >     env_internal.h:96:40: warning: ‘struct rte_pci_device’ declared inside
> >     parameter list [enabled by default]
> >      void vtophys_pci_device_removed(struct rte_pci_device *pci_device);
> >                                             ^
> >     make[2]: *** [env.o] Error 1
> >     make[1]: *** [env_dpdk] Error 2
> >     make: *** [lib] Error 2
> >
> >
> >
> >     So basically, SPDK needs some updates since it has its own pci drivers.
> >     I copied some SPDK folks for info.
> >
> >     *Disclaimer* I only checked it links fine against my 21.11 dpdk env,
> >     and did not test the other cases:
> >
> >     diff --git a/dpdkbuild/Makefile b/dpdkbuild/Makefile
> >     index d51b1a6e5..0e666735d 100644
> >     --- a/dpdkbuild/Makefile
> >     +++ b/dpdkbuild/Makefile
> >     @@ -166,6 +166,7 @@ all: $(SPDK_ROOT_DIR)/dpdk/build-tmp
> >      $(SPDK_ROOT_DIR)/dpdk/build-tmp: $(SPDK_ROOT_DIR)/mk/cc.mk
> >     $(SPDK_ROOT_DIR)/include/spdk/config.h
> >             $(Q)rm -rf $(SPDK_ROOT_DIR)/dpdk/build
> > $(SPDK_ROOT_DIR)/dpdk/build-tmp
> >             $(Q)cd "$(SPDK_ROOT_DIR)/dpdk"; CC="$(SUB_CC)" meson
> >     --prefix="$(MESON_PREFIX)" --libdir lib -Dc_args="$(DPDK_CFLAGS)"
> >     -Dc_link_args="$(DPDK_LDFLAGS)" $(DPDK_OPTS)
> >     -Ddisable_drivers="$(shell echo $(DPDK_DISABLED_DRVERS) | sed -E "s/
> >     +/,/g")" build-tmp
> >     +       $(Q)! meson configure build-tmp | grep -qw enable_driver_sdk
> >     || meson configure build-tmp -Denable_driver_sdk=true
> >             $(Q)sed $(SED_INPLACE_FLAG) 's/#define RTE_EAL_PMD_PATH
> >     .*/#define RTE_EAL_PMD_PATH ""/g'
> >     $(SPDK_ROOT_DIR)/dpdk/build-tmp/rte_build_config.h
> >             $(Q) \
> >             # TODO Meson build adds libbsd dependency when it's available.
> >     This means any app will be \
> >     diff --git a/lib/env_dpdk/env.mk b/lib/env_dpdk/env.mk
> >     index cc7db8aab..e24c6942f 100644bits with an embedded dpdk
> >     --- a/lib/env_dpdk/env.mk
> >     +++ b/lib/env_dpdk/env.mk
> >     @@ -172,6 +172,12 @@ DPDK_PRIVATE_LINKER_ARGS += -lnuma
> >      endif
> >      endif
> >
> >     +ifneq (,$(wildcard $(DPDK_INC_DIR)/rte_build_config.h))
> >     +ifneq (,$(shell grep -e "define RTE_HAS_LIBARCHIVE 1"
> >     $(DPDK_INC_DIR)/rte_build_config.h))
> >     +DPDK_PRIVATE_LINKER_ARGS += -larchive
> >     +endif
> >     +endif
> >     +
> >      ifeq ($(OS),Linux)
> >      DPDK_PRIVATE_LINKER_ARGS += -ldl
> >      endif
> >     diff --git a/lib/env_dpdk/env_internal.h b/lib/env_dpdk/env_internal.h
> >     index 2303f432c..24b377545 100644
> >     --- a/lib/env_dpdk/env_internal.h
> >     +++ b/lib/env_dpdk/env_internal.h
> >     @@ -43,13 +43,18 @@
> >      #include <rte_eal.h>
> >      #include <rte_bus.h>
> >      #include <rte_pci.h>
> >     -#include <rte_bus_pci.h>
> >      #include <rte_dev.h>
> >
> >      #if RTE_VERSION < RTE_VERSION_NUM(19, 11, 0, 0)
> >      #error RTE_VERSION is too old! Minimum 19.11 is required.
> >      #endif
> >
> >     +#if RTE_VERSION < RTE_VERSION_NUM(21, 11, 0, 0)
> >     +#include <rte_bus_pci.h>
> >     +#else
> >     +#include <pci_driver.h>
> >     +#endif
> >     +
> >      /* x86-64 and ARM userspace virtual addresses use only the low 48
> > bits [0..47],
> >       * which is enough to cover 256 TB.
> >       */
> >
> >
> >
> >     --
> >     David Marchand
> >