From f87f3bafba4ab3aab31400b5860df4ac05a8bd9a Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Mon, 20 Aug 2018 11:28:54 -0400 Subject: [PATCH 1/5] msg/dpdk: use ceph::crypto::MD5 instead of cryptopp Signed-off-by: Casey Bodley --- src/msg/async/dpdk/TCP.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/msg/async/dpdk/TCP.h b/src/msg/async/dpdk/TCP.h index 9445dbce536..8005e783c2a 100644 --- a/src/msg/async/dpdk/TCP.h +++ b/src/msg/async/dpdk/TCP.h @@ -32,14 +32,12 @@ #include #include -#define CRYPTOPP_ENABLE_NAMESPACE_WEAK 1 -#include - #include "msg/async/dpdk/EventDPDK.h" #include "include/utime.h" #include "common/Throttle.h" #include "common/ceph_time.h" +#include "common/ceph_crypto.h" #include "msg/async/Event.h" #include "IPChecksum.h" #include "IP.h" @@ -1441,7 +1439,9 @@ tcp_sequence tcp::tcb::get_isn() { hash[1] = _foreign_ip.ip; hash[2] = (_local_port << 16) + _foreign_port; hash[3] = _isn_secret.key[15]; - CryptoPP::Weak::MD5::Transform(hash, _isn_secret.key); + ceph::crypto::MD5 md5; + md5.Update((const unsigned char*)_isn_secret.key, sizeof(_isn_secret.key)); + md5.Final((unsigned char*)hash); auto seq = hash[0]; auto m = duration_cast(clock_type::now().time_since_epoch()); seq += m.count() / 4; From c976f580e7f0a4f548e850cf7b04a74019242066 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Mon, 20 Aug 2018 22:11:02 +0800 Subject: [PATCH 2/5] cmake: fix the build WITH_DPDK=ON * add Findcryptopp.cmake back cryptopp support was dropped in #20015, but it's required by src/msg/async/dpdk/TCP.h, which `#include ` so, to fix the FTBFS of WITH_DPDK=ON, we need to bring Findcryptopp.cmake back. it was also removed in #20015. * pass "-march=core2" when building sources which include dpdk headers. i was wrong that the headers shipped by distro are generic. the headers use the sse instructions for speedup memcpy, see /usr/include/x86_64-linux-gnu/dpdk/rte_memcpy.h . * also, we need to include the arch specific include directory for building with dpdk. Signed-off-by: Kefu Chai --- cmake/modules/Finddpdk.cmake | 54 ++++++++++++++++++++++++++++-------- src/msg/CMakeLists.txt | 3 ++ 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/cmake/modules/Finddpdk.cmake b/cmake/modules/Finddpdk.cmake index 2a40ca8d3ca..964ae5dfb0f 100644 --- a/cmake/modules/Finddpdk.cmake +++ b/cmake/modules/Finddpdk.cmake @@ -8,14 +8,27 @@ find_package(PkgConfig QUIET) if(PKG_CONFIG_FOUND) - pkg_check_modules(dpdk_pc QUIET libdpdk) + pkg_check_modules(dpdk QUIET libdpdk) endif() -find_path(dpdk_INCLUDE_DIR rte_config.h - HINTS - ENV DPDK_DIR - ${dpdk_pc_INCLUDE_DIRS} - PATH_SUFFIXES dpdk include) +if(NOT dpdk_INCLUDE_DIRS) + find_path(dpdk_config_INCLUDE_DIR rte_config.h + HINTS + ENV DPDK_DIR + PATH_SUFFIXES + dpdk + include) + find_path(dpdk_common_INCLUDE_DIR rte_common.h + HINTS + ENC DPDK_DIR + PATH_SUFFIXES + dpdk + include) + set(dpdk_INCLUDE_DIRS "${dpdk_config_INCLUDE_DIR}") + if(NOT dpdk_config_INCLUDE_DIR EQUAL dpdk_common_INCLUDE_DIR) + list(APPEND dpdk_INCLUDE_DIRS "${dpdk_common_INCLUDE_DIR}") + endif() +endif() set(components bus_pci @@ -44,14 +57,14 @@ foreach(c ${components}) find_library(DPDK_rte_${c}_LIBRARY rte_${c} HINTS ENV DPDK_DIR - ${dpdk_pc_LIBRARY_DIRS} + ${dpdk_LIBRARY_DIRS} PATH_SUFFIXES lib) if(DPDK_rte_${c}_LIBRARY) set(dpdk_lib dpdk::${c}) if (NOT TARGET ${dpdk_lib}) add_library(${dpdk_lib} UNKNOWN IMPORTED) set_target_properties(${dpdk_lib} PROPERTIES - INTERFACE_INCLUDE_DIRECTORIES "${dpdk_INCLUDE_DIR}" + INTERFACE_INCLUDE_DIRECTORIES "${dpdk_INCLUDE_DIRS}" IMPORTED_LOCATION "${DPDK_rte_${c}_LIBRARY}") if(c STREQUAL pmd_mlx5) find_package(verbs QUIET) @@ -64,18 +77,37 @@ foreach(c ${components}) endif() endforeach() -mark_as_advanced(dpdk_INCLUDE_DIR ${dpdk_LIBRARIES}) +mark_as_advanced(dpdk_INCLUDE_DIRS ${dpdk_LIBRARIES}) include(FindPackageHandleStandardArgs) find_package_handle_standard_args(dpdk DEFAULT_MSG - dpdk_INCLUDE_DIR + dpdk_INCLUDE_DIRS dpdk_LIBRARIES) if(dpdk_FOUND) + if(NOT TARGET dpdk::cflags) + if(CMAKE_SYSTEM_PROCESSOR MATCHES "amd64|x86_64|AMD64") + set(rte_cflags "-march=core2") + elseif(CMAKE_SYSTEM_PROCESSOR MATCHES "arm|ARM") + set(rte_cflags "-march=armv7-a") + elseif(CMAKE_SYSTEM_PROCESSOR MATCHES "aarch64|AARCH64") + set(rte_cflags "-march=armv8-a+crc") + endif() + add_library(dpdk::cflags INTERFACE IMPORTED) + if (rte_cflags) + set_target_properties(dpdk::cflags PROPERTIES + INTERFACE_COMPILE_OPTIONS "${rte_cflags}") + endif() + endif() + if(NOT TARGET dpdk::dpdk) add_library(dpdk::dpdk INTERFACE IMPORTED) find_package(Threads QUIET) + list(APPEND dpdk_LIBRARIES + Threads::Threads + dpdk::cflags) set_target_properties(dpdk::dpdk PROPERTIES - INTERFACE_LINK_LIBRARIES ${dpdk_LIBRARIES}) + INTERFACE_LINK_LIBRARIES "${dpdk_LIBRARIES}" + INTERFACE_INCLUDE_DIRECTORIES "${dpdk_INCLUDE_DIRS}") endif() endif() diff --git a/src/msg/CMakeLists.txt b/src/msg/CMakeLists.txt index 5c34d4af3e6..28c9ad78400 100644 --- a/src/msg/CMakeLists.txt +++ b/src/msg/CMakeLists.txt @@ -65,4 +65,7 @@ if(WITH_DPDK) ${async_dpdk_srcs}) target_link_libraries(common_async_dpdk PRIVATE dpdk::dpdk) + # Stack.cc includes DPDKStack.h, which includes rte_config.h indirectly + target_include_directories(common-msg-objs PRIVATE + $) endif(WITH_DPDK) From d080e216e0e41e25dfbf1f38f56d1dba84761726 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Tue, 21 Aug 2018 12:12:46 +0800 Subject: [PATCH 3/5] msg/async: do not include dpdk headers in public header before this change, async/Stack.cc includes DPDKStack.h, which in turn includes dpdk headers. this practically renders the dpdk::dpdk an PUBLIC library of common_async_dpdk. as dpdk needs to be compiled with -march=core2, we should constrain the scope of this compiler option. hence this change. Signed-off-by: Kefu Chai --- src/msg/async/dpdk/DPDKStack.cc | 13 +++++++++++++ src/msg/async/dpdk/DPDKStack.h | 12 ++---------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/msg/async/dpdk/DPDKStack.cc b/src/msg/async/dpdk/DPDKStack.cc index fabfb024129..b5550f3537c 100644 --- a/src/msg/async/dpdk/DPDKStack.cc +++ b/src/msg/async/dpdk/DPDKStack.cc @@ -41,6 +41,7 @@ #include #include "common/ceph_argparse.h" +#include "dpdk_rte.h" #include "DPDKStack.h" #include "DPDK.h" #include "IP.h" @@ -197,6 +198,11 @@ DPDKWorker::Impl::Impl(CephContext *cct, unsigned i, EventCenter *c, std::shared _inet.set_netmask_address(ipv4_address(std::get<2>(tuples[0]))); } +DPDKWorker::Impl::~Impl() +{ + _dev->unset_local_queue(id); +} + int DPDKWorker::listen(entity_addr_t &sa, const SocketOptions &opt, ServerSocket *sock) { @@ -265,3 +271,10 @@ void DPDKStack::spawn_worker(unsigned i, std::function &&func) } }); } + +void DPDKStack::join_worker(unsigned i) +{ + dpdk::eal::execute_on_master([&]() { + rte_eal_wait_lcore(i+1); + }); +} diff --git a/src/msg/async/dpdk/DPDKStack.h b/src/msg/async/dpdk/DPDKStack.h index 3ccf2a22e59..cc374af9384 100644 --- a/src/msg/async/dpdk/DPDKStack.h +++ b/src/msg/async/dpdk/DPDKStack.h @@ -21,8 +21,6 @@ #include "common/Tub.h" #include "msg/async/Stack.h" -#include "dpdk_rte.h" -#include "DPDK.h" #include "net.h" #include "const.h" #include "IP.h" @@ -215,9 +213,7 @@ class DPDKWorker : public Worker { std::shared_ptr _dev; ipv4 _inet; Impl(CephContext *cct, unsigned i, EventCenter *c, std::shared_ptr dev); - ~Impl() { - _dev->unset_local_queue(id); - } + ~Impl(); }; std::unique_ptr _impl; @@ -251,11 +247,7 @@ class DPDKStack : public NetworkStack { virtual bool support_local_listen_table() const override { return true; } virtual void spawn_worker(unsigned i, std::function &&func) override; - virtual void join_worker(unsigned i) override { - dpdk::eal::execute_on_master([&]() { - rte_eal_wait_lcore(i+1); - }); - } + virtual void join_worker(unsigned i) override; }; #endif From 1a6877dc048321226cdb764778c6ed3971f36566 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Wed, 29 Aug 2018 15:38:25 +0800 Subject: [PATCH 4/5] cmake/modules/BuildDPDK.cmake: link whole-archive we build dpdk as static library, linking against them without specifying --whole-archive will get the constructor symbols droppped by the linker as they are not referenced directly by the excecutable. so we need to do so to make sure we call all the constructors. Signed-off-by: Kefu Chai --- cmake/modules/BuildDPDK.cmake | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/cmake/modules/BuildDPDK.cmake b/cmake/modules/BuildDPDK.cmake index 04eb3bc3e56..0fd115ca169 100644 --- a/cmake/modules/BuildDPDK.cmake +++ b/cmake/modules/BuildDPDK.cmake @@ -35,14 +35,7 @@ function(do_build_dpdk dpdk_dir) message(FATAL_ERROR "not able to build DPDK support: " "unknown arch \"${CMAKE_SYSTEM_PROCESSOR}\"") endif() - - if(NOT TARGET dpdk::cflags) - add_library(dpdk::cflags INTERFACE IMPORTED) - if (rte_cflags) - set_target_properties(dpdk::cflags PROPERTIES - INTERFACE_COMPILE_OPTIONS "${rte_cflags}") - endif() - endif() + set(dpdk_rte_CFLAGS "${rte_cflags}" CACHE INTERNAL "") if(CMAKE_SYSTEM_NAME MATCHES "Linux") set(execenv "linuxapp") elseif(CMAKE_SYSTEM_NAME MATCHES "FreeBSD") @@ -100,6 +93,15 @@ macro(build_dpdk) # create the directory so cmake won't complain when looking at the imported # target file(MAKE_DIRECTORY ${DPDK_INCLUDE_DIR}) + + if(NOT TARGET dpdk::cflags) + add_library(dpdk::cflags INTERFACE IMPORTED) + if (dpdk_rte_CFLAGS) + set_target_properties(dpdk::cflags PROPERTIES + INTERFACE_COMPILE_OPTIONS "${dpdk_rte_CFLAGS}") + endif() + endif() + foreach(c bus_pci eal @@ -117,8 +119,19 @@ macro(build_dpdk) INTERFACE_LINK_LIBRARIES dpdk::cflags IMPORTED_LOCATION "${dpdk_${c}_LIBRARY}") list(APPEND DPDK_LIBRARIES dpdk::${c}) + list(APPEND DPDK_ARCHIVES "${dpdk_${c}_LIBRARY}") endforeach() + add_library(dpdk::dpdk INTERFACE IMPORTED) + add_dependencies(dpdk::dpdk + ${DPDK_LIBRARIES}) + # workaround for https://gitlab.kitware.com/cmake/cmake/issues/16947 set_target_properties(dpdk::dpdk PROPERTIES - INTERFACE_LINK_LIBRARIES "${DPDK_LIBRARIES}") + INTERFACE_INCLUDE_DIRECTORIES ${DPDK_INCLUDE_DIR} + INTERFACE_LINK_LIBRARIES + "-Wl,--whole-archive $ -Wl,--no-whole-archive") + if(dpdk_rte_CFLAGS) + set_target_properties(dpdk::dpdk PROPERTIES + INTERFACE_COMPILE_OPTIONS "${dpdk_rte_CFLAGS}") + endif() endmacro() From 2c823a18cb3f2abd059d1293b6b6a6bbfe4752de Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Wed, 5 Sep 2018 15:16:26 +0800 Subject: [PATCH 5/5] cmake: pass DPDK_DIR explicitly so we don't rely on build_dpdk() to be a macro to set variables in its parent scope. Signed-off-by: Kefu Chai --- CMakeLists.txt | 2 +- cmake/modules/BuildDPDK.cmake | 11 +++++------ cmake/modules/BuildSPDK.cmake | 5 +++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8142f17d5a1..7945bd76d4e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -391,7 +391,7 @@ if(WITH_DPDK) find_package(dpdk) if(NOT DPDK_FOUND AND NOT TARGET dpdk-ext) include(BuildDPDK) - build_dpdk() + build_dpdk(${CMAKE_BINARY_DIR}/src/dpdk) endif() set(HAVE_DPDK TRUE) endif() diff --git a/cmake/modules/BuildDPDK.cmake b/cmake/modules/BuildDPDK.cmake index 0fd115ca169..8159d7d47cb 100644 --- a/cmake/modules/BuildDPDK.cmake +++ b/cmake/modules/BuildDPDK.cmake @@ -86,10 +86,9 @@ function(do_build_dpdk dpdk_dir) ExternalProject_Add_StepTargets(dpdk-ext configure patch-config build) endfunction() -macro(build_dpdk) - set(DPDK_DIR ${CMAKE_BINARY_DIR}/src/dpdk) - do_build_dpdk(${DPDK_DIR}) - set(DPDK_INCLUDE_DIR ${DPDK_DIR}/include) +function(build_dpdk dpdk_dir) + do_build_dpdk(${dpdk_dir}) + set(DPDK_INCLUDE_DIR ${dpdk_dir}/include) # create the directory so cmake won't complain when looking at the imported # target file(MAKE_DIRECTORY ${DPDK_INCLUDE_DIR}) @@ -113,7 +112,7 @@ macro(build_dpdk) add_library(dpdk::${c} STATIC IMPORTED) add_dependencies(dpdk::${c} dpdk-ext) set(dpdk_${c}_LIBRARY - "${DPDK_DIR}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}rte_${c}${CMAKE_STATIC_LIBRARY_SUFFIX}") + "${dpdk_dir}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}rte_${c}${CMAKE_STATIC_LIBRARY_SUFFIX}") set_target_properties(dpdk::${c} PROPERTIES INTERFACE_INCLUDE_DIRECTORIES ${DPDK_INCLUDE_DIR} INTERFACE_LINK_LIBRARIES dpdk::cflags @@ -134,4 +133,4 @@ macro(build_dpdk) set_target_properties(dpdk::dpdk PROPERTIES INTERFACE_COMPILE_OPTIONS "${dpdk_rte_CFLAGS}") endif() -endmacro() +endfunction() diff --git a/cmake/modules/BuildSPDK.cmake b/cmake/modules/BuildSPDK.cmake index 43adff82d24..46242dc930a 100644 --- a/cmake/modules/BuildSPDK.cmake +++ b/cmake/modules/BuildSPDK.cmake @@ -1,7 +1,8 @@ macro(build_spdk) + set(DPDK_DIR ${CMAKE_BINARY_DIR}/src/dpdk) if(NOT TARGET dpdk-ext) include(BuildDPDK) - build_dpdk() + build_dpdk(${DPDK_DIR}) endif() find_package(CUnit REQUIRED) if(LINUX) @@ -29,7 +30,7 @@ macro(build_spdk) list(APPEND SPDK_LIBRARIES spdk::${c}) endforeach() set_target_properties(spdk::env_dpdk PROPERTIES - INTERFACE_LINK_LIBRARIES "${DPDK_LIBRARIES};rt") + INTERFACE_LINK_LIBRARIES "dpdk::dpdk;rt") set_target_properties(spdk::lvol PROPERTIES INTERFACE_LINK_LIBRARIES spdk::util) set_target_properties(spdk::util PROPERTIES