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

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

[David Marchand]

Hello Chenbo,

On Wed, Sep 29, 2021 at 9:38 AM Xia, Chenbo <chenbo.xia@intel.com> wrote:
>
> Gentle ping for comments..

Sorry, I'll try to look at it.

>
> @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.


-- 
David Marchand

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

[David Marchand]

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

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

[Harris, James R]

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

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

[Xia, Chenbo]

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
> 

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

[Liu, Changpeng]

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
> >

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

[David Marchand]

Hello,

On Fri, Oct 8, 2021 at 8:15 AM Liu, Changpeng <changpeng.liu@intel.com> wrote:
>
> 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);

I should have got this warning... but compilation passed fine for me.
Happy you tested it.

>
> 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().

Since SPDK has a PCI driver, you'll need to compile code that calls
those PCI driver internal API with ALLOW_INTERNAL_API defined.
You can probably add a #define ALLOW_INTERNAL_API first thing (it's
important to have it defined before including any dpdk header) in
pci.c

Another option, is to add it to lib/env_dpdk/env.mk:ENV_CFLAGS =
$(DPDK_INC) -DALLOW_EXPERIMENTAL_API.

Can someone from SPDK take over this and sync with Chenbo?


Thanks.

-- 
David Marchand

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

[Liu, Changpeng]

Thanks, I have worked with Chenbo to address this issue before.  After enable the `ALLOW_INTERNAL_API` option, it works now with SPDK.

Another issue raised by Jim Harris is that for distro packaged DPDK, since this option isn't enabled by default, this will not allow SPDK
to use the distro packaged DPDK after this release.

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, October 8, 2021 3:08 PM
> To: Liu, Changpeng <changpeng.liu@intel.com>
> Cc: Xia, Chenbo <chenbo.xia@intel.com>; Harris, James R
> <james.r.harris@intel.com>; 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
> 
> Hello,
> 
> On Fri, Oct 8, 2021 at 8:15 AM Liu, Changpeng <changpeng.liu@intel.com> wrote:
> >
> > 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);
> 
> I should have got this warning... but compilation passed fine for me.
> Happy you tested it.
> 
> >
> > 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().
> 
> Since SPDK has a PCI driver, you'll need to compile code that calls
> those PCI driver internal API with ALLOW_INTERNAL_API defined.
> You can probably add a #define ALLOW_INTERNAL_API first thing (it's
> important to have it defined before including any dpdk header) in
> pci.c
> 
> Another option, is to add it to lib/env_dpdk/env.mk:ENV_CFLAGS =
> $(DPDK_INC) -DALLOW_EXPERIMENTAL_API.
> 
> Can someone from SPDK take over this and sync with Chenbo?
> 
> 
> Thanks.
> 
> --
> David Marchand

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

[Xia, Chenbo]

Hi David & Changpeng,

> -----Original Message-----
> From: Liu, Changpeng <changpeng.liu@intel.com>
> Sent: Friday, October 8, 2021 3:45 PM
> To: David Marchand <david.marchand@redhat.com>; Harris, James R
> <james.r.harris@intel.com>
> Cc: Xia, Chenbo <chenbo.xia@intel.com>; 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, I have worked with Chenbo to address this issue before.  After enable
> the `ALLOW_INTERNAL_API` option, it works now with SPDK.
> 
> Another issue raised by Jim Harris is that for distro packaged DPDK, since
> this option isn't enabled by default, this will not allow SPDK
> to use the distro packaged DPDK after this release.

I think for this problem, we have two options: enable driver sdk by default or
let OSV configure the option when building distros. I'm fine with either option.

@David, What do you think?

Thanks,
Chenbo

> 
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Friday, October 8, 2021 3:08 PM
> > To: Liu, Changpeng <changpeng.liu@intel.com>
> > Cc: Xia, Chenbo <chenbo.xia@intel.com>; Harris, James R
> > <james.r.harris@intel.com>; 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
> >
> > Hello,
> >
> > On Fri, Oct 8, 2021 at 8:15 AM Liu, Changpeng <changpeng.liu@intel.com>
> wrote:
> > >
> > > 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);
> >
> > I should have got this warning... but compilation passed fine for me.
> > Happy you tested it.
> >
> > >
> > > 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().
> >
> > Since SPDK has a PCI driver, you'll need to compile code that calls
> > those PCI driver internal API with ALLOW_INTERNAL_API defined.
> > You can probably add a #define ALLOW_INTERNAL_API first thing (it's
> > important to have it defined before including any dpdk header) in
> > pci.c
> >
> > Another option, is to add it to lib/env_dpdk/env.mk:ENV_CFLAGS =
> > $(DPDK_INC) -DALLOW_EXPERIMENTAL_API.
> >
> > Can someone from SPDK take over this and sync with Chenbo?
> >
> >
> > Thanks.
> >
> > --
> > David Marchand