Re: [PATCH] lib: Initial implementation of PQoS lib with CAT and CDP support
From: Thomas Monjalon <hidden>
Date: 2016-02-16 16:20:41
Hi, 2016-01-29 13:17, Wojciech Andralojc:
25 files changed, 2111 insertions(+)
The patch is too big to be easily reviewed.
quoted hunk ↗ jump to hunk
+PQoS API +M: Kantecki, Tomasz [off-list ref] +K: RTE_LIBRTE_PQOS +F: lib/librte_pqos/ Drivers -------diff --git a/config/common_linuxapp b/config/common_linuxapp index 74bc515..553e640 100644 --- a/config/common_linuxapp +++ b/config/common_linuxapp@@ -99,6 +99,7 @@ CONFIG_RTE_NEXT_ABI=y CONFIG_RTE_LIBRTE_EAL=y CONFIG_RTE_MAX_LCORE=128 CONFIG_RTE_MAX_NUMA_NODES=8 +CONFIG_RTE_MAX_PHY_PKGS=8
The physical packages management should be a separate patch.
quoted hunk ↗ jump to hunk
CONFIG_RTE_MAX_MEMSEG=256 CONFIG_RTE_MAX_MEMZONE=2560 CONFIG_RTE_MAX_TAILQ=32@@ -514,6 +515,13 @@ CONFIG_RTE_LIBRTE_VHOST_NUMA=n CONFIG_RTE_LIBRTE_VHOST_DEBUG=n # +# Compile librte_pqos +# IA only +# LIBRTE_PQOS is for librte_pqos only
I think this line is not needed.
+CONFIG_RTE_LIBRTE_PQOS=n +CONFIG_RTE_LIBRTE_PQOS_DEBUG=n
quoted hunk ↗ jump to hunk
--- a/lib/librte_eal/common/eal_private.h +++ b/lib/librte_eal/common/eal_private.h@@ -346,4 +346,13 @@ int rte_eal_hugepage_init(void); */ int rte_eal_hugepage_attach(void); +#ifdef RTE_LIBRTE_PQOS +/** + * Get cpu physical_package_id. + * + * This function is private to the EAL. + */ +unsigned eal_cpu_phy_pkg_id(unsigned lcore_id); +#endif
No need to restrict this function to LIBRTE_PQOS. Maybe it should be a public API.
quoted hunk ↗ jump to hunk
--- a/lib/librte_eal/common/include/rte_log.h +++ b/lib/librte_eal/common/include/rte_log.h@@ -79,6 +79,9 @@ extern struct rte_logs rte_logs; #define RTE_LOGTYPE_PIPELINE 0x00008000 /**< Log related to pipeline. */ #define RTE_LOGTYPE_MBUF 0x00010000 /**< Log related to mbuf. */ #define RTE_LOGTYPE_CRYPTODEV 0x00020000 /**< Log related to cryptodev. */ +#ifdef RTE_LIBRTE_PQOS +#define RTE_LOGTYPE_PQOS 0x00040000 /**< Log related to pqos. */ +#endif
The ifdef is useless and forbidden in a header.
quoted hunk ↗ jump to hunk
--- a/lib/librte_eal/linuxapp/eal/Makefile +++ b/lib/librte_eal/linuxapp/eal/Makefile@@ -45,6 +45,9 @@ CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include CFLAGS += -I$(RTE_SDK)/lib/librte_ring CFLAGS += -I$(RTE_SDK)/lib/librte_mempool CFLAGS += -I$(RTE_SDK)/lib/librte_ivshmem +ifeq ($(CONFIG_RTE_LIBRTE_PQOS),y) +CFLAGS += -I$(RTE_SDK)/lib/librte_pqos +endif
Please make EAL independent of librte_pqos.
quoted hunk ↗ jump to hunk
+++ b/lib/librte_pqos/rte_pqos_version.map@@ -0,0 +1,16 @@ +DPDK_2.3 {
It will be DPDK_16.04. I won't comment the library itself in this version. Please rework EAL and patch splitting first. Thanks