Thread (59 messages) 59 messages, 10 authors, 2022-07-11

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

From: Liu, Changpeng <hidden>
Date: 2021-10-08 06:15:36

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 <redacted>
Sent: Wednesday, October 6, 2021 12:26 PM
To: Harris, James R <redacted>; David Marchand
[off-list ref]; Liu, Changpeng [off-list ref]
Cc: dev@dpdk.org; ci@dpdk.org; Aaron Conole <aconole@redhat.com>; dpdklab
[off-list ref]; Zawadzki, Tomasz [off-list ref];
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
quoted
-----Original Message-----
From: Harris, James R <redacted>
Sent: Monday, October 4, 2021 11:56 PM
To: David Marchand <redacted>; Xia, Chenbo
[off-list ref]; Liu, Changpeng [off-list ref]
Cc: dev@dpdk.org; ci@dpdk.org; Aaron Conole <aconole@redhat.com>;
dpdklab
quoted
[off-list ref]; Zawadzki, Tomasz [off-list ref];
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" [off-list ref]
wrote:
quoted
    On Thu, Sep 30, 2021 at 10:45 AM David Marchand
    [off-list ref] wrote:
    > On Wed, Sep 29, 2021 at 9:38 AM Xia, Chenbo [off-list ref]
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
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help