Re: [dpdk-dev] Suggestions on how to customize the metadata fields of each packet
Thanks for your quick answer, I have read so many documents and web pages on this issue that probably I confounded the utility of the headroom. It is good to know that this 128 bytes space is available to my disposal. The fact of being lost once the NIC transmits the frame it is not a problem at all for my application. However, in case that this space is not enough, I have seen in the rte_mbuf struct a (void *) pointer called userdata which is in theory used for extra user-defined metadata. If I wanted to attach an additional metadata struct, I guess that I just have to assign the pointer to this struct to the userdata field. However, what happens if I want that the content of this struct travels with the packet through a software ring in order to be processed by another thread? Should I reserve more space in the ring to allocate such extra metadata? Thanks again, PD: I have copied the message to users mailing list 2018-02-23 4:13 GMT+01:00 : > Hi, > > First, I think your question should be sent to the user mailing list, not > the dev mailing list. > > > I have seen that each packet has a headroom memory space (128 bytes > long) > > > where RSS hashing and other metadata provided by the NIC is stored. > > If I’m not mistaken, the headroom is not where metadata provided by the > NIC are stored. Those metadata are stored in the rte_mbuf struct, which > is also 128 bytes long. > > The headroom area is located AFTER the end of rte_mbuf (at offset 128). > By default the headroom area is also 128 byte long, so the actual packet > data is stored at offset 256. > > You can store whatever you want in this headroom area. However those > information are lost as soon as the packet leaves DPDK (the NIC will start > sending at offset 256). > > -BL. > -- Victor
Re: [dpdk-dev] [PATCH 2/2] mk: clean up static link with DPAA libraries
23/02/2018 07:25, Hemant Agrawal: > Hi Thomas, > On 2/23/2018 4:23 AM, Thomas Monjalon wrote: > > The bus and mempool dependencies should be declared after the PMD > > libraries needing them. > > > > Moreover there is no need to disable the PMDs at the Makefile level, > > in case the dependencies are not met. > > Such dependencies should be handled at configuration time. > > > > The other side effect of this clean-up is to take into account > > the mempool option CONFIG_RTE_LIBRTE_DPAA2_MEMPOOL. > > > > Signed-off-by: Thomas Monjalon > > --- > > mk/rte.app.mk | 20 > > 1 file changed, 4 insertions(+), 16 deletions(-) > > This patch looks good, however I think we (NXP) need to also set some > dependency check in makefile to avoid dpaaX PMD compilations, if bus is > not available. I am working on a patch for the same. Why do you think such check is necessary? If the PMD is enable but not the bus, it is a configuration error. A good configuration system would resolve it automatically, but currently we do the configuration manually, so the user must fix its configuration file.
Re: [dpdk-dev] Suggestions on how to customize the metadata fields of each packet
Hi Victor, > > Thanks for your quick answer, > > I have read so many documents and web pages on this issue that probably I > confounded the utility of the headroom. It is good to know that this 128 > bytes space is available to my disposal. The fact of being lost once the > NIC transmits the frame it is not a problem at all for my application. > However, in case that this space is not enough, I have seen in the rte_mbuf > struct a (void *) pointer called userdata which is in theory used for extra > user-defined metadata. If I wanted to attach an additional metadata struct, > I guess that I just have to assign the pointer to this struct to the > userdata field. However, what happens if I want that the content of this > struct travels with the packet through a software ring in order to be > processed by another thread? Should I reserve more space in the ring to > allocate such extra metadata? > > Thanks again, In theory headroom inside mbuf should be left for packet's data. To do things properly you'll need to create your mbuf mempools with priv_size >= your_extra_metadata_size. Konstantin > > PD: I have copied the message to users mailing list > > 2018-02-23 4:13 GMT+01:00 : > > > Hi, > > > > First, I think your question should be sent to the user mailing list, not > > the dev mailing list. > > > > > I have seen that each packet has a headroom memory space (128 bytes > > long) > > > > > where RSS hashing and other metadata provided by the NIC is stored. > > > > If I’m not mistaken, the headroom is not where metadata provided by the > > NIC are stored. Those metadata are stored in the rte_mbuf struct, which > > is also 128 bytes long. > > > > The headroom area is located AFTER the end of rte_mbuf (at offset 128). > > By default the headroom area is also 128 byte long, so the actual packet > > data is stored at offset 256. > > > > You can store whatever you want in this headroom area. However those > > information are lost as soon as the packet leaves DPDK (the NIC will start > > sending at offset 256). > > > > -BL. > > > > > > -- > Victor
[dpdk-dev] [PATCH v2 1/2] app/testpmd: fix DPAA shared library dependency
The dynamic link is broken for ARM platform because the dependencies
of the DPAA PMD are not declared.
Fixes: 83c82e15e1c0 ("app/testpmd: support loopback config for DPAA")
Cc: sta...@dpdk.org
Reported-by: Marco Varlese
Signed-off-by: Thomas Monjalon
Signed-off-by: Hemant Agrawal
---
app/test-pmd/Makefile | 4 +++-
app/test-pmd/cmdline.c | 4 ++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile
index ed588ab..60ae9b9 100644
--- a/app/test-pmd/Makefile
+++ b/app/test-pmd/Makefile
@@ -44,8 +44,10 @@ ifeq ($(CONFIG_RTE_LIBRTE_PMD_BOND),y)
LDLIBS += -lrte_pmd_bond
endif
-ifeq ($(CONFIG_RTE_LIBRTE_DPAA_PMD),y)
+ifeq ($(CONFIG_RTE_LIBRTE_DPAA_BUS)$(CONFIG_RTE_LIBRTE_DPAA_PMD),yy)
LDLIBS += -lrte_pmd_dpaa
+LDLIBS += -lrte_bus_dpaa
+LDLIBS += -lrte_mempool_dpaa
endif
ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index d1dc1de..40b31ad 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -60,7 +60,7 @@
#include
#include
#endif
-#ifdef RTE_LIBRTE_DPAA_PMD
+#if defined RTE_LIBRTE_DPAA_BUS && defined RTE_LIBRTE_DPAA_PMD
#include
#endif
#ifdef RTE_LIBRTE_IXGBE_PMD
@@ -12861,7 +12861,7 @@ cmd_set_tx_loopback_parsed(
if (ret == -ENOTSUP)
ret = rte_pmd_bnxt_set_tx_loopback(res->port_id, is_on);
#endif
-#ifdef RTE_LIBRTE_DPAA_PMD
+#if defined RTE_LIBRTE_DPAA_BUS && defined RTE_LIBRTE_DPAA_PMD
if (ret == -ENOTSUP)
ret = rte_pmd_dpaa_set_tx_loopback(res->port_id, is_on);
#endif
--
2.7.4
[dpdk-dev] [PATCH v2 2/2] mk: fix the build dependency structure for dpaaX
This patch fixes the build dependency of various
dpaaX components, when the dpaa or fslmc bus is disabled,
or VFIO is disabled.
Fixes: 1ee9569576f6 ("config: enable dpaaX drivers for generic ARMv8")
Cc: sta...@dpdk.org
Reported-by: Yongseok Koh
Suggested-by: Thomas Monjalon
Signed-off-by: Hemant Agrawal
---
drivers/bus/Makefile | 2 ++
drivers/bus/fslmc/Makefile | 4
drivers/crypto/Makefile| 4
drivers/event/Makefile | 4
drivers/mempool/Makefile | 4
drivers/net/Makefile | 4
mk/rte.app.mk | 29 ++---
7 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index 7ef2593..c251b65 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -4,7 +4,9 @@
include $(RTE_SDK)/mk/rte.vars.mk
DIRS-$(CONFIG_RTE_LIBRTE_DPAA_BUS) += dpaa
+ifeq ($(CONFIG_RTE_EAL_VFIO),y)
DIRS-$(CONFIG_RTE_LIBRTE_FSLMC_BUS) += fslmc
+endif
DIRS-$(CONFIG_RTE_LIBRTE_PCI_BUS) += pci
DIRS-$(CONFIG_RTE_LIBRTE_VDEV_BUS) += vdev
diff --git a/drivers/bus/fslmc/Makefile b/drivers/bus/fslmc/Makefile
index de237f0..952b4c0 100644
--- a/drivers/bus/fslmc/Makefile
+++ b/drivers/bus/fslmc/Makefile
@@ -9,10 +9,6 @@ include $(RTE_SDK)/mk/rte.vars.mk
#
LIB = librte_bus_fslmc.a
-ifeq ($(CONFIG_RTE_LIBRTE_DPAA2_PMD),y)
-CONFIG_RTE_LIBRTE_FSLMC_BUS = $(CONFIG_RTE_LIBRTE_DPAA2_PMD)
-endif
-
CFLAGS += -DALLOW_EXPERIMENTAL_API
ifeq ($(CONFIG_RTE_LIBRTE_DPAA2_DEBUG_INIT),y)
CFLAGS += -O0 -g
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index 628bd14..26e503e 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -14,7 +14,11 @@ DIRS-$(CONFIG_RTE_LIBRTE_PMD_KASUMI) += kasumi
DIRS-$(CONFIG_RTE_LIBRTE_PMD_ZUC) += zuc
DIRS-$(CONFIG_RTE_LIBRTE_PMD_MRVL_CRYPTO) += mrvl
DIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL_CRYPTO) += null
+ifeq ($(CONFIG_RTE_EAL_VFIO)$(CONFIG_RTE_LIBRTE_FSLMC_BUS),yy)
DIRS-$(CONFIG_RTE_LIBRTE_PMD_DPAA2_SEC) += dpaa2_sec
+endif
+ifeq ($(CONFIG_RTE_LIBRTE_DPAA_BUS),y)
DIRS-$(CONFIG_RTE_LIBRTE_PMD_DPAA_SEC) += dpaa_sec
+endif
include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/drivers/event/Makefile b/drivers/event/Makefile
index c3d89a1..f301d8d 100644
--- a/drivers/event/Makefile
+++ b/drivers/event/Makefile
@@ -7,8 +7,12 @@ include $(RTE_SDK)/mk/rte.vars.mk
DIRS-$(CONFIG_RTE_LIBRTE_PMD_SKELETON_EVENTDEV) += skeleton
DIRS-$(CONFIG_RTE_LIBRTE_PMD_SW_EVENTDEV) += sw
DIRS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX_SSOVF) += octeontx
+ifeq ($(CONFIG_RTE_LIBRTE_DPAA_BUS),y)
DIRS-$(CONFIG_RTE_LIBRTE_PMD_DPAA_EVENTDEV) += dpaa
+endif
+ifeq ($(CONFIG_RTE_EAL_VFIO)$(CONFIG_RTE_LIBRTE_FSLMC_BUS),yy)
DIRS-$(CONFIG_RTE_LIBRTE_PMD_DPAA2_EVENTDEV) += dpaa2
+endif
DIRS-$(CONFIG_RTE_LIBRTE_PMD_OPDL_EVENTDEV) += opdl
include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/drivers/mempool/Makefile b/drivers/mempool/Makefile
index aae2cb1..fc8b73b 100644
--- a/drivers/mempool/Makefile
+++ b/drivers/mempool/Makefile
@@ -3,8 +3,12 @@
include $(RTE_SDK)/mk/rte.vars.mk
+ifeq ($(CONFIG_RTE_LIBRTE_DPAA_BUS),y)
DIRS-$(CONFIG_RTE_LIBRTE_DPAA_MEMPOOL) += dpaa
+endif
+ifeq ($(CONFIG_RTE_EAL_VFIO)$(CONFIG_RTE_LIBRTE_FSLMC_BUS),yy)
DIRS-$(CONFIG_RTE_LIBRTE_DPAA2_MEMPOOL) += dpaa2
+endif
DIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_RING) += ring
DIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_STACK) += stack
DIRS-$(CONFIG_RTE_LIBRTE_OCTEONTX_MEMPOOL) += octeontx
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index e112732..39eb550 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -15,8 +15,12 @@ DIRS-$(CONFIG_RTE_LIBRTE_AVP_PMD) += avp
DIRS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD) += bnx2x
DIRS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += bonding
DIRS-$(CONFIG_RTE_LIBRTE_CXGBE_PMD) += cxgbe
+ifeq ($(CONFIG_RTE_LIBRTE_DPAA_BUS),y)
DIRS-$(CONFIG_RTE_LIBRTE_DPAA_PMD) += dpaa
+endif
+ifeq ($(CONFIG_RTE_EAL_VFIO)$(CONFIG_RTE_LIBRTE_FSLMC_BUS),yy)
DIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += dpaa2
+endif
DIRS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000
DIRS-$(CONFIG_RTE_LIBRTE_ENA_PMD) += ena
DIRS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += enic
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 3eb41d1..94525dc 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -113,11 +113,21 @@ endif
_LDLIBS-$(CONFIG_RTE_LIBRTE_PCI_BUS)+= -lrte_bus_pci
_LDLIBS-$(CONFIG_RTE_LIBRTE_VDEV_BUS) += -lrte_bus_vdev
+_LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA_BUS) += -lrte_bus_dpaa
+ifeq ($(CONFIG_RTE_EAL_VFIO),y)
+_LDLIBS-$(CONFIG_RTE_LIBRTE_FSLMC_BUS) += -lrte_bus_fslmc
+endif
ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
# plugins (link only if static libraries)
_LDLIBS-$(CONFIG_RTE_DRIVER_MEMPOOL_STACK) += -lrte_mempool_stack
+ifeq ($(CONFIG_RTE_LIBRTE_DPAA_BUS),y)
+_LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA_MEMPOOL) += -lrte_mempool_dpaa
+endif
+ifeq ($(CONFIG_RTE_EAL_VFIO)$(CONFIG_RTE_LIBRTE_FSLMC_BUS),yy)
+_LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA2_MEMPOOL) += -lrte_mempool_dpaa2
+endif
_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) +=
Re: [dpdk-dev] [PATCH 2/2] mk: clean up static link with DPAA libraries
On 2/23/2018 2:55 PM, Thomas Monjalon wrote: 23/02/2018 07:25, Hemant Agrawal: Hi Thomas, On 2/23/2018 4:23 AM, Thomas Monjalon wrote: The bus and mempool dependencies should be declared after the PMD libraries needing them. Moreover there is no need to disable the PMDs at the Makefile level, in case the dependencies are not met. Such dependencies should be handled at configuration time. The other side effect of this clean-up is to take into account the mempool option CONFIG_RTE_LIBRTE_DPAA2_MEMPOOL. Signed-off-by: Thomas Monjalon --- mk/rte.app.mk | 20 1 file changed, 4 insertions(+), 16 deletions(-) This patch looks good, however I think we (NXP) need to also set some dependency check in makefile to avoid dpaaX PMD compilations, if bus is not available. I am working on a patch for the same. Why do you think such check is necessary? If the PMD is enable but not the bus, it is a configuration error. A good configuration system would resolve it automatically, but currently we do the configuration manually, so the user must fix its configuration file. I agree with your comment. That is the ideal approach. However, just to make it convenient and to support cases like VFIO disable, I have added few checks in my v2.
[dpdk-dev] Suggestions on how to customize the metadata fields of each packet
Hi all, Victor, I suggest taking a closer look at section 7.1. here: http://dpdk.org/doc/guides/prog_guide/mbuf_lib.html The approach chosen by DPDK is to store everything, metadata and packet data, in contiguous memory. That way, network packets will always have 1 to 1 relationship with DPDK mbufs, no extra pointer needed. Every task that you need to perform, from allocating, freeing, to transferring mbufs to another lcore via software rings, are handled by DPDK. You don't have to worry about them. You can save your metadata either directly in the userdata field of struct rte_mbuf or in the headroom area. I agree with Konstantin that in theory we should think of the userdata field as space exclusively for metadata and reserve the headroom area for packet header manipulation purposes only. However in practice I tend to think that using headroom for metadata is more useful since you don't really need to worry about any special configuration when creating mbuf pool. The headroom is gonna be there by default and you can always adjust its size after initialization. Please let me know if I missed something. -BL > -Original Message- > From: konstantin.anan...@intel.com [mailto:konstantin.anan...@intel.com] > Sent: Friday, February 23, 2018 4:27 PM > To: Victor Huertas ; long...@viettel.com.vn > Cc: dev@dpdk.org; us...@dpdk.org > Subject: RE: [dpdk-dev] Suggestions on how to customize the metadata fields > of each packet > > Hi Victor, > > > > > Thanks for your quick answer, > > > > I have read so many documents and web pages on this issue that > > probably I confounded the utility of the headroom. It is good to know > > that this 128 bytes space is available to my disposal. The fact of > > being lost once the NIC transmits the frame it is not a problem at all for > > my > application. > > However, in case that this space is not enough, I have seen in the > > rte_mbuf struct a (void *) pointer called userdata which is in theory > > used for extra user-defined metadata. If I wanted to attach an > > additional metadata struct, I guess that I just have to assign the > > pointer to this struct to the userdata field. However, what happens if > > I want that the content of this struct travels with the packet through > > a software ring in order to be processed by another thread? Should I > > reserve more space in the ring to allocate such extra metadata? > > > > Thanks again, > > > In theory headroom inside mbuf should be left for packet's data. > To do things properly you'll need to create your mbuf mempools with > priv_size >= your_extra_metadata_size. > > Konstantin > > > > > PD: I have copied the message to users mailing list > > > > 2018-02-23 4:13 GMT+01:00 : > > > > > Hi, > > > > > > First, I think your question should be sent to the user mailing > > > list, not the dev mailing list. > > > > > > > I have seen that each packet has a headroom memory space (128 > > > > bytes > > > long) > > > > > > > where RSS hashing and other metadata provided by the NIC is stored. > > > > > > If I’m not mistaken, the headroom is not where metadata provided by > > > the NIC are stored. Those metadata are stored in the rte_mbuf > > > struct, which is also 128 bytes long. > > > > > > The headroom area is located AFTER the end of rte_mbuf (at offset 128). > > > By default the headroom area is also 128 byte long, so the actual > > > packet data is stored at offset 256. > > > > > > You can store whatever you want in this headroom area. However those > > > information are lost as soon as the packet leaves DPDK (the NIC will > > > start sending at offset 256). > > > > > > -BL. > > > > > > > > > > > -- > > Victor
[dpdk-dev] [PATCH 0/1] net/ixgbe: Add API to update SBP bit
From: Shweta Choudaha -- 2.11.0
[dpdk-dev] [PATCH 1/1] net/ixgbe: Add API to update SBP bit
From: Shweta Choudaha
Add ixgbe API to enable SBP bit in FCTRL register
to allow receiving packets that may otherwise be
considered length errors by ixgbe and dropped
Signed-off-by: Shweta Choudaha
Reviewed-by: Chas Williams
Reviewed-by: Luca Boccassi
---
drivers/net/ixgbe/rte_pmd_ixgbe.c | 28
drivers/net/ixgbe/rte_pmd_ixgbe.h | 13 +
drivers/net/ixgbe/rte_pmd_ixgbe_version.map | 6 ++
3 files changed, 47 insertions(+)
diff --git a/drivers/net/ixgbe/rte_pmd_ixgbe.c
b/drivers/net/ixgbe/rte_pmd_ixgbe.c
index d8ca8ca31..3b6f68f9e 100644
--- a/drivers/net/ixgbe/rte_pmd_ixgbe.c
+++ b/drivers/net/ixgbe/rte_pmd_ixgbe.c
@@ -880,6 +880,34 @@ rte_pmd_ixgbe_set_tc_bw_alloc(uint16_t port,
return 0;
}
+int __rte_experimental
+rte_pmd_ixgbe_upd_fctrl_sbp(uint16_t port, int enable)
+{
+ struct ixgbe_hw *hw;
+ struct rte_eth_dev *dev;
+ uint32_t fctrl;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+ dev = &rte_eth_devices[port];
+ if (!is_ixgbe_supported(dev))
+ return -ENOTSUP;
+
+ hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+ if (!hw)
+ return -ENOTSUP;
+
+ fctrl = IXGBE_READ_REG(hw, IXGBE_FCTRL);
+
+ /* If 'enable' set the SBP bit else clear it */
+ if (enable)
+ fctrl |= IXGBE_FCTRL_SBP;
+ else
+ fctrl &= ~(IXGBE_FCTRL_SBP);
+
+ IXGBE_WRITE_REG(hw, IXGBE_FCTRL, fctrl);
+ return 0;
+}
+
#ifdef RTE_LIBRTE_IXGBE_BYPASS
int
rte_pmd_ixgbe_bypass_init(uint16_t port_id)
diff --git a/drivers/net/ixgbe/rte_pmd_ixgbe.h
b/drivers/net/ixgbe/rte_pmd_ixgbe.h
index 11a9f334b..a3026bd98 100644
--- a/drivers/net/ixgbe/rte_pmd_ixgbe.h
+++ b/drivers/net/ixgbe/rte_pmd_ixgbe.h
@@ -637,4 +637,17 @@ enum {
((x) > RTE_PMD_IXGBE_BYPASS_TMT_OFF && \
(x) < RTE_PMD_IXGBE_BYPASS_TMT_NUM))
+/**
+ * @param port
+ * The port identifier of the Ethernet device.
+ * @param enable
+ *0 to disable and nonzero to enable 'SBP' bit in FCTRL register
+ *to receive all packets
+ * @return
+ * - (0) if successful.
+ * - (-ENODEV) if *port* invalid.
+ * - (-ENOTSUP) if hardware doesn't support this feature.
+ */
+int __rte_experimental
+rte_pmd_ixgbe_upd_fctrl_sbp(uint16_t port, int enable);
#endif /* _PMD_IXGBE_H_ */
diff --git a/drivers/net/ixgbe/rte_pmd_ixgbe_version.map
b/drivers/net/ixgbe/rte_pmd_ixgbe_version.map
index bf776742c..a360d383b 100644
--- a/drivers/net/ixgbe/rte_pmd_ixgbe_version.map
+++ b/drivers/net/ixgbe/rte_pmd_ixgbe_version.map
@@ -52,3 +52,9 @@ DPDK_17.08 {
rte_pmd_ixgbe_bypass_wd_timeout_show;
rte_pmd_ixgbe_bypass_wd_timeout_store;
} DPDK_17.05;
+
+EXPERIMENTAL {
+ global:
+
+ rte_pmd_ixgbe_upd_fctrl_sbp;
+} DPDK_17.08;
--
2.11.0
Re: [dpdk-dev] [RFC v2, 2/2] eventdev: add crypto adapter API header
Hi Folks, On 2/20/2018 7:29 PM, Jerin Jacob wrote: -Original Message- Date: Mon, 19 Feb 2018 10:55:58 + From: "Gujjar, Abhinandan S" To: Jerin Jacob CC: "dev@dpdk.org" , "Vangati, Narender" , "Rao, Nikhil" , "Eads, Gage" , "hemant.agra...@nxp.com" , "akhil.go...@nxp.com" , "narayanaprasad.athr...@cavium.com" , "nidadavolu.mur...@cavium.com" , "nithin.dabilpu...@cavium.com" Subject: RE: [RFC v2, 2/2] eventdev: add crypto adapter API header Hi Jerin, Hi Abhinandan, Thanks for the review. Please find few comments inline. -Original Message- From: Jerin Jacob [mailto:jerin.ja...@caviumnetworks.com] Sent: Saturday, February 17, 2018 1:04 AM To: Gujjar, Abhinandan S Cc: dev@dpdk.org; Vangati, Narender ; Rao, Nikhil ; Eads, Gage ; hemant.agra...@nxp.com; akhil.go...@nxp.com; narayanaprasad.athr...@cavium.com; nidadavolu.mur...@cavium.com; nithin.dabilpu...@cavium.com Subject: Re: [RFC v2, 2/2] eventdev: add crypto adapter API header -Original Message- Date: Mon, 15 Jan 2018 16:23:50 +0530 From: Abhinandan Gujjar To: jerin.ja...@caviumnetworks.com CC: dev@dpdk.org, narender.vang...@intel.com, Abhinandan Gujjar , Nikhil Rao , Gage Eads Subject: [RFC v2, 2/2] eventdev: add crypto adapter API header X-Mailer: git-send-email 1.9.1 + +/** + * This adapter adds support to enqueue crypto completions to event device. + * The packet flow from cryptodev to the event device can be +accomplished + * using both SW and HW based transfer mechanisms. + * The adapter uses a EAL service core function for SW based packet +transfer + * and uses the eventdev PMD functions to configure HW based packet +transfer + * between the cryptodev and the event device. + * + * In the case of SW based transfers, application can choose to +submit a I think, we can remove "In the case of SW based transfers" as it should be applicable for HW case too Ok. In that case, adapter will detect the presence of HW connection between cryptodev & eventdev and will not dequeue crypto completions. I would say presence of "specific capability" instead of HW. + * crypto operation directly to cryptodev or send it to the + cryptodev + * adapter via eventdev, the cryptodev adapter then submits the + crypto + * operation to the crypto device. The first mode is known as the The first mode (DEQ) is very clear. In the second mode(ENQ_DEQ), - How does "worker" submits the crypto work through crypto-adapter? If I understand it correctly, "workers" always deals with only cryptodev's rte_cryptodev_enqueue_burst() API and "service" function in crypto adapter would be responsible for dequeue() from cryptodev and enqueue to eventdev? I understand the need for OP_NEW vs OP_FWD mode difference in both modes. Other than that, What makes ENQ_DEQ different? Could you share the flow for ENQ_DEQ mode with APIs. /* Application changes for ENQ_DEQ mode: - /* In ENQ_DEQ mode, to enqueue to adapter app * has to fill out following details. */ struct rte_event_crypto_request *req; struct rte_crypto_op *op = rte_crypto_op_alloc(); /* fill request info */ req = (void *)((char *)op + op.private_data_offset); req->cdev_id = 1; req->queue_pair_id = 1; /* fill response info */ ... /* send event to crypto adapter */ ev->event_ptr = op; ev->queue_id = dst_event_qid; ev->priority = dst_priority; ev->sched_type = dst_sched_type; ev->event_type = RTE_EVENT_TYPE_CRYPTODEV; ev->sub_event_type = sub_event_type; ev->flow_id = dst_flow_id; ret = rte_event_enqueue_burst(event_dev_id, event_port_id, ev, 1); Adapter in ENQ_DEQ mode, submitting crypto ops to cryptodev: - n = rte_event_dequeue_burst(event_dev_id, event_port_id, ev, BATCH_SIZE, time_out); struct rte_crypto_op *op = ev->event_ptr; struct rte_event_crypto_request *req = (void *)op + op.private_data_offset; cdev_id = req->cdev_id; qp_id = req->queue_pair_id ret = rte_cryptodev_enqueue_burst(cdev_id, qp_id, op, 1); This mode wont work for the HW implementations that I know. As in HW implementations, The Adapter is embedded in HW. The DEQ mode works. But, This would call for to have two separate application logic for DEQ and ENQ_DEQ mode. I think, it is unavoidable as SW scheme has better performance with ENQ_DEQ MODE. If you think, there is no option other than introducing a capability in adapter then please create capability in Rx adapter to inform the adapter capability to the application. Do we think, it possible to have scheme with ENQ_DEQ mode, Where application still enqueue to cryptodev like DEQ mode but using cryptodev. ie. Adapter patches the cryptodev dev->enqueue_burst() to "eventdev enqueue burst" followed by "e
[dpdk-dev] [PATCH] net/qede: fix alloc from socket 0
In case osal_dma_alloc_coherent() is called from a management thread,
core_id turn out to be LCORE_ID_ANY, and the resulting socket for alloc
will be socket 0.
This is not desirable when using a NIC from socket 1 which might very
likely be configured to use memory from that socket only.
In that case, allocation will fail.
To address this, use master lcore instead when called from mgmt thread.
The associated socket should have memory available.
Fixes: ec94dbc57362 ("qede: add base driver")
Cc: sta...@dpdk.org
Signed-off-by: Pascal Mazon
---
drivers/net/qede/base/bcm_osal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/qede/base/bcm_osal.c b/drivers/net/qede/base/bcm_osal.c
index fe42f3256400..0760cdcb9523 100644
--- a/drivers/net/qede/base/bcm_osal.c
+++ b/drivers/net/qede/base/bcm_osal.c
@@ -133,7 +133,7 @@ void *osal_dma_alloc_coherent(struct ecore_dev *p_dev,
snprintf(mz_name, sizeof(mz_name) - 1, "%lx",
(unsigned long)rte_get_timer_cycles());
if (core_id == (unsigned int)LCORE_ID_ANY)
- core_id = 0;
+ core_id = rte_get_master_lcore();
socket_id = rte_lcore_to_socket_id(core_id);
mz = rte_memzone_reserve_aligned(mz_name, size,
socket_id, 0, RTE_CACHE_LINE_SIZE);
--
2.16.1.72.g5be1f00a9
Re: [dpdk-dev] [PATCH 0/1] net/ixgbe: Add API to update SBP bit
Hi, Not sure why cover letter wasn’ t appended. In some cases we require all packets (including the ones that ixgbe considers as Rx length errors) to be received on backplane ixgbe port. The proposed API sets the requisite SBP bit for ixgbe to receive Rx length error packets. If this is required more generically for all adapters, the other approach can be to allocate a bit in ‘struct rte_eth_rxmode’ to disable rx_length_checking on the port. Please let me know if that is preferred or if more info is needed Thanks, Shweta On Fri, Feb 23, 2018 at 11:59 AM, Shweta Choudaha wrote: > From: Shweta Choudaha > > -- > 2.11.0 > >
Re: [dpdk-dev] Suggestions on how to customize the metadata fields of each packet
Thanks a lot for your suggestions, Taking them into account and having a look a this example on userdata field usage (http://dpdk.org/doc/api/examples_2bbdev_app_2main_8c-example.html#a19), I have though the following plan. I think that the most elegant way to do it is to use "userdata" for metadata, leaving the headroom as it is for further and future header manipulation or encapsulation. Therefore, allow me to expose it and tell me if it is a good practice or not: 1. I create two independent mempools: "packet_pool" with N mbufs of capacity to store all captured packets and a second pool called "metadata_pool" with the same N positions of sizeof(struct additional-metadata). 2. On thread#1, I setup an rx queue on the eth port0 assigning it the "packet_pool". 3. The thread#1 captures a burst of 32 packets and store them in a struct *rte_mbuf packets[32]. At the same time I pick up 32 objects from the "metadata_pool" and store them in struct *additional-metadata custom_metadata[32]. The content of every struct vector item should be empty, but just in case I initialize every struct field to the default values. 4. For every packet vector position (32 in total) I perform the following assignment: packest[i].userdata = custom_metadata[i]; 5. I modify one field of the metadata for each packet this way: packets[i].userdata->field1=X 6. I send through a software ring (which I previously created) all the 32 elements of "packets" vector. I do NOT implement any parallel software ring to put the custom_metadata 32 objects as I assume that such userdata assignment prevails through the previous software ring. 7. Thread#2 reads 32 packets from the mentioned software ring and prints the content of packets[i].userdata->field1 to check that the content of metadata is maintained through the software ring. 8. Thread#2 sends the 32 packets through a tx queue in port 1. 9. Thread#2 frees the 32 packets ret_mbufs structs AND also frees the content in packets[i].userdata. 10. Go to point 1 and repeat in a loop all the time. Is this a valid procedure or do you think that there could be a better one? Thanks for your attention Regards, 2018-02-23 11:07 GMT+01:00 : > Hi all, > > Victor, I suggest taking a closer look at section 7.1. here: > http://dpdk.org/doc/guides/prog_guide/mbuf_lib.html > > The approach chosen by DPDK is to store everything, metadata and packet > data, in contiguous memory. That way, network packets will always have 1 to > 1 relationship with DPDK mbufs, no extra pointer needed. Every task that > you need to perform, from allocating, freeing, to transferring mbufs to > another lcore via software rings, are handled by DPDK. You don't have to > worry about them. You can save your metadata either directly in the > userdata field of struct rte_mbuf or in the headroom area. > > I agree with Konstantin that in theory we should think of the userdata > field as space exclusively for metadata and reserve the headroom area for > packet header manipulation purposes only. However in practice I tend to > think that using headroom for metadata is more useful since you don't > really need to worry about any special configuration when creating mbuf > pool. The headroom is gonna be there by default and you can always adjust > its size after initialization. Please let me know if I missed something. > > -BL > > > -Original Message- > > From: konstantin.anan...@intel.com [mailto:konstantin.anan...@intel.com] > > Sent: Friday, February 23, 2018 4:27 PM > > To: Victor Huertas ; long...@viettel.com.vn > > Cc: dev@dpdk.org; us...@dpdk.org > > Subject: RE: [dpdk-dev] Suggestions on how to customize the metadata > fields > > of each packet > > > > Hi Victor, > > > > > > > > Thanks for your quick answer, > > > > > > I have read so many documents and web pages on this issue that > > > probably I confounded the utility of the headroom. It is good to know > > > that this 128 bytes space is available to my disposal. The fact of > > > being lost once the NIC transmits the frame it is not a problem at all > for my > > application. > > > However, in case that this space is not enough, I have seen in the > > > rte_mbuf struct a (void *) pointer called userdata which is in theory > > > used for extra user-defined metadata. If I wanted to attach an > > > additional metadata struct, I guess that I just have to assign the > > > pointer to this struct to the userdata field. However, what happens if > > > I want that the content of this struct travels with the packet through > > > a software ring in order to be processed by another thread? Should I > > > reserve more space in the ring to allocate such extra metadata? > > > > > > Thanks again, > > > > > > In theory headroom inside mbuf should be left for packet's data. > > To do things properly you'll need to create your mbuf mempools with > > priv_size >= your_extra_metadata_size. > > > > Konstantin > > >
Re: [dpdk-dev] [PATCH v1] net/mlx4: fix RSS actions with no parameters
On Wed, Feb 21, 2018 at 01:38:38PM +, Ophir Munk wrote:
> When creating an RSS flow with missing actions parameters, for example:
> flow create 0 ingress pattern / end actions rss / end
>
> testpmd aborts with segmentation fault.
> In the corrupted code mlx4_flow_prepare() accesses RSS action->conf pointer
> without verifying its validity.
> In case of missing RSS actions parameters this pointer is NULL and must not
> be accessed.
Problem is that testpmd is far from perfect and shouldn't feed PMDs with
invalid pointers in the first place. The configuration structure is not
documented as optional with actions that take one.
> The fix is to return an error in such cases "missing rss actions".
>
> Fixes: 078b8b452e6b ("net/mlx4: add RSS flow rule action support")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Ophir Munk
I suggest to fix this at once for all present and future PMDs in testpmd
directly. It may be added as a workaround in mlx4 but not as a fix since the
cause is not in that PMD.
> ---
> drivers/net/mlx4/mlx4_flow.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c
> index 2d55bfe..7a127a8 100644
> --- a/drivers/net/mlx4/mlx4_flow.c
> +++ b/drivers/net/mlx4/mlx4_flow.c
> @@ -735,6 +735,10 @@ mlx4_flow_prepare(struct priv *priv,
> if (flow->rss)
> break;
> rss = action->conf;
> + if (!rss) {
> + msg = "missing rss actions";
> + goto exit_action_not_supported;
> + }
This message may be understood as a lack of RSS action, while it is in fact
present. This error can be more accurately described as:
"RSS action configuration wasn't provided"
Note the same issue exists with the QUEUE action handled just prior to this
one and probably affects other PMDs as well. You really should consider
fixing testpmd instead.
--
Adrien Mazarguil
6WIND
Re: [dpdk-dev] [RFC PATCH] use strlcpy for string copies
On Tue, Feb 20, 2018 at 05:07:27PM +, Bruce Richardson wrote:
> Following on from the number of patches needing to be done for strncpy
> issues highlighted by coverity...
>
> The strncpy function is error prone for doing "safe" string copies, so
> we generally try to use "snprintf" instead in the code. The function
> "strlcpy" is a better alternative, though, since it better conveys the
> intention of the programmer, and doesn't suffer from the non-null
> terminating behaviour of it's n'ed brethern.
>
> The downside of this function is that it is not available by default
> on linux, though standard in the BSD's. It is available on most
> distros by installing "libbsd" package.
>
> This RFC therefore provides the following in rte_string_fns.h to ensure
> that strlcpy is available there:
> * for BSD, include string.h as normal
> * if RTE_USE_LIBBSD is set, include
> * if not set, fallback to snprintf for strlcpy
>
> Using make build system, the RTE_USE_LIBBSD is a hard-coded value to "n",
> but when using meson, it's automatically set based on what is available
> on the platform.
>
> Instances of snprintf using "%s" alone as a string format are replaced
> via coccinelle script with the new strlcpy function. Instances of
> strncpy should be replaced too, but requires manual checking as to
> whether the NULL termination is manually done afterward or not.
>
> Signed-off-by: Bruce Richardson
OK with the RFC, a few comments below regarding mlx4, mlx5 and the
definition itself though.
> diff --git a/drivers/net/mlx4/mlx4_ethdev.c b/drivers/net/mlx4/mlx4_ethdev.c
> index 3bc692731..c92dd6d43 100644
> --- a/drivers/net/mlx4/mlx4_ethdev.c
> +++ b/drivers/net/mlx4/mlx4_ethdev.c
> @@ -120,7 +120,7 @@ mlx4_get_ifname(const struct priv *priv, char
> (*ifname)[IF_NAMESIZE])
> goto try_dev_id;
> dev_port_prev = dev_port;
> if (dev_port == (priv->port - 1u))
> - snprintf(match, sizeof(match), "%s", name);
> + strlcpy(match, name, sizeof(match));
> }
> closedir(dir);
> if (match[0] == '\0') {
Missing #include
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index 666507691..894a045ec 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -163,7 +163,7 @@ priv_get_ifname(const struct priv *priv, char
> (*ifname)[IF_NAMESIZE])
> goto try_dev_id;
> dev_port_prev = dev_port;
> if (dev_port == (priv->port - 1u))
> - snprintf(match, sizeof(match), "%s", name);
> + strlcpy(match, name, sizeof(match));
> }
> closedir(dir);
> if (match[0] == '\0')
Ditto (note I didn't check missing occurrences in other components).
> diff --git a/lib/librte_eal/common/include/rte_string_fns.h
> b/lib/librte_eal/common/include/rte_string_fns.h
> index e97047a47..ff4c98b2a 100644
> --- a/lib/librte_eal/common/include/rte_string_fns.h
> +++ b/lib/librte_eal/common/include/rte_string_fns.h
> @@ -45,6 +45,20 @@ int
> rte_strsplit(char *string, int stringlen,
> char **tokens, int maxtokens, char delim);
>
> +/* pull in a strlcpy function */
> +#ifdef RTE_EXEC_ENV_BSDAPP
> +#include
> +
> +#else /* non-BSD platforms */
> +#ifdef RTE_USE_LIBBSD
> +#include
> +
> +#else /* no BSD header files, create own */
> +#define strlcpy(dst, src, size) snprintf(dst, size, "%s", src)
Missing #include for that.
What also bothers me is that on some platforms, applications get a true
function definition and a macro on others. I suggest a static inline
or even a proper versioned definition in eal_common_string_fns.c instead.
--
Adrien Mazarguil
6WIND
Re: [dpdk-dev] [RFC PATCH] use strlcpy for string copies
On Tue, Feb 20, 2018 at 6:07 PM, Bruce Richardson
wrote:
> Following on from the number of patches needing to be done for strncpy
> issues highlighted by coverity...
>
> The strncpy function is error prone for doing "safe" string copies, so
> we generally try to use "snprintf" instead in the code. The function
> "strlcpy" is a better alternative, though, since it better conveys the
> intention of the programmer, and doesn't suffer from the non-null
> terminating behaviour of it's n'ed brethern.
>
> The downside of this function is that it is not available by default
> on linux, though standard in the BSD's. It is available on most
> distros by installing "libbsd" package.
>
> This RFC therefore provides the following in rte_string_fns.h to ensure
> that strlcpy is available there:
> * for BSD, include string.h as normal
> * if RTE_USE_LIBBSD is set, include
> * if not set, fallback to snprintf for strlcpy
>
> Using make build system, the RTE_USE_LIBBSD is a hard-coded value to "n",
> but when using meson, it's automatically set based on what is available
> on the platform.
>
> Instances of snprintf using "%s" alone as a string format are replaced
> via coccinelle script with the new strlcpy function. Instances of
> strncpy should be replaced too, but requires manual checking as to
> whether the NULL termination is manually done afterward or not.
>
> Signed-off-by: Bruce Richardson
> ---
> app/pdump/main.c | 11 +--
> app/test-pmd/parameters.c | 5 ++---
> config/common_base | 1 +
> config/meson.build | 7 +++
> devtools/cocci/strlcpy.cocci | 8
> drivers/net/bonding/rte_eth_bond_pmd.c | 2 +-
> drivers/net/failsafe/failsafe_args.c | 5 +++--
> drivers/net/mlx4/mlx4_ethdev.c | 2 +-
> drivers/net/mlx5/mlx5_ethdev.c | 2 +-
> drivers/net/mrvl/mrvl_qos.c| 2 +-
> drivers/net/tap/rte_eth_tap.c | 5 +++--
> .../ip_pipeline/pipeline/pipeline_flow_classification_be.c | 4 ++--
> examples/ip_pipeline/pipeline/pipeline_passthrough_be.c| 4 ++--
> examples/load_balancer/config.c| 2 +-
> lib/librte_cmdline/cmdline_parse.c | 6 +++---
> lib/librte_cmdline/cmdline_parse_etheraddr.c | 2 +-
> lib/librte_cmdline/cmdline_parse_ipaddr.c | 2 +-
> lib/librte_cmdline/cmdline_parse_portlist.c| 2 +-
> lib/librte_cmdline/cmdline_parse_string.c | 4 ++--
> lib/librte_eal/common/eal_common_bus.c | 3 ++-
> lib/librte_eal/common/include/rte_string_fns.h | 14
> ++
> lib/librte_pdump/rte_pdump.c | 9 +
> test/test/test_cmdline_cirbuf.c| 2 +-
> test/test/test_eal_flags.c | 10 ++
> test/test/test_malloc.c| 2 +-
> 25 files changed, 75 insertions(+), 41 deletions(-)
> create mode 100644 devtools/cocci/strlcpy.cocci
>
> diff --git a/app/pdump/main.c b/app/pdump/main.c
> index f6865bdbd..d29de0321 100644
> --- a/app/pdump/main.c
> +++ b/app/pdump/main.c
> @@ -24,6 +24,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #define CMD_LINE_OPT_PDUMP "pdump"
> @@ -408,17 +409,15 @@ launch_args_parse(int argc, char **argv, char *prgname)
> if (!strncmp(long_option[option_index].name,
> CMD_LINE_OPT_SER_SOCK_PATH,
> sizeof(CMD_LINE_OPT_SER_SOCK_PATH))) {
> - snprintf(server_socket_path,
> - sizeof(server_socket_path), "%s",
> - optarg);
> + strlcpy(server_socket_path, optarg,
> + sizeof(server_socket_path));
> }
>
> if (!strncmp(long_option[option_index].name,
> CMD_LINE_OPT_CLI_SOCK_PATH,
> sizeof(CMD_LINE_OPT_CLI_SOCK_PATH))) {
> - snprintf(client_socket_path,
> - sizeof(client_socket_path), "%s",
> - optarg);
> + strlcpy(client_socket_path, optarg,
> + sizeof(client_socket_path));
> }
>
> break;
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 97d22b860..2192bdcdf 100644
> --- a/app/t
Re: [dpdk-dev] [PATCH 08/10] event/octeontx: add option to use fpavf as chunk pool
Hi Pavan,
> -Original Message-
> From: Pavan Nikhilesh [mailto:pbhagavat...@caviumnetworks.com]
> Sent: Friday, February 16, 2018 3:37 PM
> To: jerin.ja...@caviumnetworks.com;
> santosh.shu...@caviumnetworks.com; Carrillo, Erik G
>
> Cc: dev@dpdk.org; Pavan Nikhilesh
> Subject: [dpdk-dev] [PATCH 08/10] event/octeontx: add option to use fpavf
> as chunk pool
>
> Add compile-time configurable option to force TIMvf to use Octeontx FPAvf
> pool manager as its chunk pool.
> When FPAvf is used as pool manager the TIMvf automatically frees the
> chunks to FPAvf through gpool-id.
>
> Signed-off-by: Pavan Nikhilesh
> ---
<...snipped...>
> @@ -241,7 +243,16 @@ timvf_add_entry_brst(struct timvf_ring *timr, const
> uint16_t rel_bkt,
> bkt->first_chunk = (uint64_t) chunk;
> }
> } else {
> +#ifndef RTE_PMD_OCTEONTX_TIMVF_USE_FPAVF
> chunk = timr_clr_bkt(timr, bkt);
> +#else
> + if (unlikely(rte_mempool_get(timr-
> >meta.chunk_pool,
> + (void **)&chunk))) {
> + timr_bkt_set_rem(bkt, 0);
> + tim[index]->state =
> RTE_EVENT_TIMER_ERROR;
> + return -ENOMEM;
You return a negative errno value here, but in this case the caller was
expecting the number that succeeded.
Regards,
Gabriel
> + }
> +#endif
> bkt->first_chunk = (uint64_t) chunk;
> }
> *(uint64_t *)(chunk + nb_chunk_slots) = 0; @@ -355,7
> +366,18 @@ timvf_add_entry_sp(struct timvf_ring *timr, const uint32_t
<...snipped...>
[dpdk-dev] [PATCH 0/3] logging enhancments
The current dynamic logging has some awkward user interface choices. It uses integers for log levels which requires user to know the mapping between numeric and symbolic values. A bigger problem was the choice of regular expressions and option format for dynamic logging. Dynamic log names are seperated with a period and the wildcard character for regular expressions is a period. It is just a happy accident the expressions like: "pmd.net.virtio.*" work as expected. This patch set adds a more usable solution with filename style matching. Also, the choice of comma as seperator for log-level option was not consistent with other options. For other options, comma is used to seperate list of equal values as in: -l 1,2,3 Since new match required a backwards compatiable option the colon is now used to seperate name and value. So: --log-level='pmd.net.virtio.*,7' still works as expected. But the prefered syntax is: --log-level='pmd.net.virtio.*:info' If this is accepted, I think we should mark the old regex style matching as deprecated and remove it in 18.11?? Also, the dynamic log level pattern stuff is not adaquately documented right now. There are only a couple of vague references in the current documentation (which this updates). Stephen Hemminger (3): eal: allow symbolic log levels log: add ability to match dynamic log based on shell pattern doc: update log level matching in documentation doc/guides/contributing/coding_style.rst | 2 +- doc/guides/nics/qede.rst | 2 +- lib/librte_eal/common/eal_common_log.c | 22 +++- lib/librte_eal/common/eal_common_options.c | 86 +++--- lib/librte_eal/common/include/rte_log.h| 16 +- 5 files changed, 103 insertions(+), 25 deletions(-) -- 2.16.1
[dpdk-dev] [PATCH 1/3] eal: allow symbolic log levels
Much easeier to remember names than numbers. Allows
--log-level=pmd.net.ixgbe.*,debug
The option argument allow shortened form so
--log-level=pmd.net.ixgbe.*,i
also works.
Signed-off-by: Stephen Hemminger
---
lib/librte_eal/common/eal_common_options.c | 68 ++
1 file changed, 51 insertions(+), 17 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_options.c
b/lib/librte_eal/common/eal_common_options.c
index 9f2f8d25afd9..f2339c3907e4 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -911,10 +911,52 @@ eal_parse_syslog(const char *facility, struct
internal_config *conf)
}
static int
-eal_parse_log_level(const char *arg)
+eal_parse_log_priority(const char *level)
{
- char *end, *str, *type, *level;
+ static const char *levels[] = {
+ [RTE_LOG_EMERG] = "emergency",
+ [RTE_LOG_ALERT] = "alert",
+ [RTE_LOG_CRIT]= "critical",
+ [RTE_LOG_ERR] = "error",
+ [RTE_LOG_WARNING] = "warning",
+ [RTE_LOG_NOTICE] = "notice",
+ [RTE_LOG_INFO]= "info",
+ [RTE_LOG_DEBUG] = "debug",
+ };
+ size_t len = strlen(level);
unsigned long tmp;
+ char *end;
+ unsigned int i;
+
+ if (len == 0)
+ return -1;
+
+ /* look for named values, skip 0 which is not a valid level */
+ for (i = 1; i < sizeof(levels) / sizeof(levels[0]); i++) {
+ if (strncmp(levels[i], level, len) == 0)
+ return i;
+ }
+
+ /* not a string, maybe it is numeric */
+ errno = 0;
+ tmp = strtoul(level, &end, 0);
+
+ /* check for errors */
+ if (errno != 0 || end == NULL || *end != '\0')
+ return -1;
+
+ /* log_level is a uint32_t */
+ if (tmp >= UINT32_MAX)
+ return -1;
+
+ return tmp;
+}
+
+static int
+eal_parse_log_level(const char *arg)
+{
+ char *str, *type, *level;
+ int priority;
str = strdup(arg);
if (str == NULL)
@@ -928,23 +970,15 @@ eal_parse_log_level(const char *arg)
level = strsep(&str, ",");
}
- errno = 0;
- tmp = strtoul(level, &end, 0);
-
- /* check for errors */
- if ((errno != 0) || (level[0] == '\0') ||
- end == NULL || (*end != '\0'))
+ priority = eal_parse_log_priority(level);
+ if (priority < 0)
goto fail;
-
- /* log_level is a uint32_t */
- if (tmp >= UINT32_MAX)
- goto fail;
-
+
if (type == NULL) {
- rte_log_set_global_level(tmp);
- } else if (rte_log_set_level_regexp(type, tmp) < 0) {
- printf("cannot set log level %s,%lu\n",
- type, tmp);
+ rte_log_set_global_level(priority);
+ } else if (rte_log_set_level_regexp(type, priority) < 0) {
+ fprintf(stderr, "cannot set log level %s,%d\n",
+ type, priority);
goto fail;
}
--
2.16.1
[dpdk-dev] [PATCH 2/3] log: add ability to match dynamic log based on shell pattern
Regular expressions are not the best way to match a hierarchal
pattern like dynamic log levels. And the seperator for dynamic
log levels is period which is the regex wildcard character.
A better solution is to use filename matching 'globbing' so
that log levels match like file paths. For compatiablity,
use colon to seperate pattern match style arguments. For
exmaple:
--log-level 'pmd.net.virtio.*:debug'
Signed-off-by: Stephen Hemminger
---
lib/librte_eal/common/eal_common_log.c | 22 +-
lib/librte_eal/common/eal_common_options.c | 36 --
lib/librte_eal/common/include/rte_log.h| 16 +++--
3 files changed, 59 insertions(+), 15 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_log.c
b/lib/librte_eal/common/eal_common_log.c
index 37b2e20e539b..2601c7cd4e4f 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -89,7 +90,26 @@ rte_log_set_level(uint32_t type, uint32_t level)
return 0;
}
-/* set level */
+int
+rte_log_set_level_match(const char *pattern, uint32_t level)
+{
+ size_t i;
+
+ if (level > RTE_LOG_DEBUG)
+ return -1;
+
+ for (i = 0; i < rte_logs.dynamic_types_len; i++) {
+ if (rte_logs.dynamic_types[i].name == NULL)
+ continue;
+
+ if (fnmatch(pattern, rte_logs.dynamic_types[i].name, 0))
+ rte_logs.dynamic_types[i].loglevel = level;
+ }
+
+ return 0;
+}
+
+/* set level by regular expression (using pattern match is preferred) */
int
rte_log_set_level_regexp(const char *pattern, uint32_t level)
{
diff --git a/lib/librte_eal/common/eal_common_options.c
b/lib/librte_eal/common/eal_common_options.c
index f2339c3907e4..e8d832cc694a 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -955,31 +955,43 @@ eal_parse_log_priority(const char *level)
static int
eal_parse_log_level(const char *arg)
{
- char *str, *type, *level;
+ char *str, *level;
+ const char *regex = NULL;
+ const char *pattern = NULL;
int priority;
str = strdup(arg);
if (str == NULL)
return -1;
- if (strchr(str, ',') == NULL) {
- type = NULL;
- level = str;
+ if ((level = strchr(str, ','))) {
+ regex = str;
+ *level++ = '\0';
+ } else if ((level = strchr(str, ':'))) {
+ pattern = str;
+ *level++ = '\0';
} else {
- type = strsep(&str, ",");
- level = strsep(&str, ",");
+ level = str;
}
priority = eal_parse_log_priority(level);
if (priority < 0)
goto fail;
- if (type == NULL) {
+ if (regex) {
+ if (rte_log_set_level_regexp(regex, priority) < 0) {
+ fprintf(stderr, "cannot set log level %s,%d\n",
+ pattern, priority);
+ goto fail;
+ }
+ } else if (pattern) {
+ if (rte_log_set_level_match(pattern, priority) < 0) {
+ fprintf(stderr, "cannot set log level %s:%d\n",
+ pattern, priority);
+ goto fail;
+ }
+ } else {
rte_log_set_global_level(priority);
- } else if (rte_log_set_level_regexp(type, priority) < 0) {
- fprintf(stderr, "cannot set log level %s,%d\n",
- type, priority);
- goto fail;
}
free(str);
@@ -1336,7 +1348,7 @@ eal_common_usage(void)
" --"OPT_PROC_TYPE" Type of this process
(primary|secondary|auto)\n"
" --"OPT_SYSLOG"Set syslog facility\n"
" --"OPT_LOG_LEVEL"= Set global log level\n"
- " --"OPT_LOG_LEVEL"=,\n"
+ " --"OPT_LOG_LEVEL"=:\n"
" Set specific log level\n"
" -v Display version information on startup\n"
" -h, --help This help\n"
diff --git a/lib/librte_eal/common/include/rte_log.h
b/lib/librte_eal/common/include/rte_log.h
index 9029c7856d31..6d0ff9fe4623 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -132,13 +132,25 @@ int rte_log_get_level(uint32_t logtype);
* Set the log level for a given type.
*
* @param pattern
- * The regexp identifying the log type.
+ * The match pattern identifying the log type.
* @param level
* The level to be set.
* @return
* 0 on success, a negative value if level is invalid.
*/
-int rte_log_set_level_regexp(const char *pattern, uint32_t level);
+int rte_log_set_lev
[dpdk-dev] [PATCH 3/3] doc: update log level matching in documentation
The use of dynamic log in documentation should use matching not
regex notation.
Signed-off-by: Stephen Hemminger
---
doc/guides/contributing/coding_style.rst | 2 +-
doc/guides/nics/qede.rst | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/doc/guides/contributing/coding_style.rst
b/doc/guides/contributing/coding_style.rst
index b0f0adb887b4..365a4b17a983 100644
--- a/doc/guides/contributing/coding_style.rst
+++ b/doc/guides/contributing/coding_style.rst
@@ -615,7 +615,7 @@ In the DPDK environment, use the logging interface provided:
rte_log_set_level(my_logtype2, RTE_LOG_NOTICE);
/* enable all PMD logs (whose identifier string starts with "pmd") */
- rte_log_set_level_regexp("pmd.*", RTE_LOG_DEBUG);
+ rte_log_set_level_match("pmd.*", RTE_LOG_DEBUG);
/* log in debug level */
rte_log_set_global_level(RTE_LOG_DEBUG);
diff --git a/doc/guides/nics/qede.rst b/doc/guides/nics/qede.rst
index 63ce9b4c60c6..42dd70db39df 100644
--- a/doc/guides/nics/qede.rst
+++ b/doc/guides/nics/qede.rst
@@ -193,7 +193,7 @@ This section provides instructions to configure SR-IOV with
Linux OS.
#. Running testpmd
- (Supply ``--log-level="pmd.net.qede.driver",7`` to view informational
messages):
+ (Supply ``--log-level="pmd.net.qede.driver:7`` to view informational
messages):
Refer to the document
:ref:`compiling and testing a PMD for a NIC ` to run
--
2.16.1
[dpdk-dev] [PATCH v2 0/3] logging enhancements
The current dynamic logging has some awkward user interface choices. It uses integers for log levels which requires user to know the mapping between numeric and symbolic values. A bigger problem was the choice of regular expressions and option format for dynamic logging. Dynamic log names are seperated with a period and the wildcard character for regular expressions is a period. It is just a happy accident the expressions like: "pmd.net.virtio.*" work as expected. This patch set adds a more usable solution with filename style matching. Also, the choice of comma as seperator for log-level option was not consistent with other options. For other options, comma is used to seperate list of equal values as in: -l 1,2,3 Since new match required a backwards compatiable option the colon is now used to seperate name and value. So: --log-level='pmd.net.virtio.*,7' still works as expected. But the prefered syntax is: --log-level='pmd.net.virtio.*:info' If this is accepted, I think we should mark the old regex style matching as deprecated and remove it in 18.11?? Also, the dynamic log level pattern stuff is not adaquately documented right now. There are only a couple of vague references in the current documentation (which this updates). v2 - whitespace checkpatch revisions Stephen Hemminger (3): eal: allow symbolic log levels log: add ability to match dynamic log based on shell pattern doc: update log level matching in documentation doc/guides/contributing/coding_style.rst | 2 +- doc/guides/nics/qede.rst | 2 +- lib/librte_eal/common/eal_common_log.c | 22 +++- lib/librte_eal/common/eal_common_options.c | 86 +++--- lib/librte_eal/common/include/rte_log.h| 16 +- 5 files changed, 103 insertions(+), 25 deletions(-) -- 2.16.1
[dpdk-dev] [PATCH v2 1/3] eal: allow symbolic log levels
Much easeier to remember names than numbers. Allows
--log-level=pmd.net.ixgbe.*,debug
Signed-off-by: Stephen Hemminger
---
lib/librte_eal/common/eal_common_options.c | 66 ++
1 file changed, 50 insertions(+), 16 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_options.c
b/lib/librte_eal/common/eal_common_options.c
index 9f2f8d25afd9..19069638ea05 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -911,10 +911,52 @@ eal_parse_syslog(const char *facility, struct
internal_config *conf)
}
static int
-eal_parse_log_level(const char *arg)
+eal_parse_log_priority(const char *level)
{
- char *end, *str, *type, *level;
+ static const char *levels[] = {
+ [RTE_LOG_EMERG] = "emergency",
+ [RTE_LOG_ALERT] = "alert",
+ [RTE_LOG_CRIT]= "critical",
+ [RTE_LOG_ERR] = "error",
+ [RTE_LOG_WARNING] = "warning",
+ [RTE_LOG_NOTICE] = "notice",
+ [RTE_LOG_INFO]= "info",
+ [RTE_LOG_DEBUG] = "debug",
+ };
+ size_t len = strlen(level);
unsigned long tmp;
+ char *end;
+ unsigned int i;
+
+ if (len == 0)
+ return -1;
+
+ /* look for named values, skip 0 which is not a valid level */
+ for (i = 1; i < sizeof(levels) / sizeof(levels[0]); i++) {
+ if (strncmp(levels[i], level, len) == 0)
+ return i;
+ }
+
+ /* not a string, maybe it is numeric */
+ errno = 0;
+ tmp = strtoul(level, &end, 0);
+
+ /* check for errors */
+ if (errno != 0 || end == NULL || *end != '\0')
+ return -1;
+
+ /* log_level is a uint32_t */
+ if (tmp >= UINT32_MAX)
+ return -1;
+
+ return tmp;
+}
+
+static int
+eal_parse_log_level(const char *arg)
+{
+ char *str, *type, *level;
+ int priority;
str = strdup(arg);
if (str == NULL)
@@ -928,23 +970,15 @@ eal_parse_log_level(const char *arg)
level = strsep(&str, ",");
}
- errno = 0;
- tmp = strtoul(level, &end, 0);
-
- /* check for errors */
- if ((errno != 0) || (level[0] == '\0') ||
- end == NULL || (*end != '\0'))
- goto fail;
-
- /* log_level is a uint32_t */
- if (tmp >= UINT32_MAX)
+ priority = eal_parse_log_priority(level);
+ if (priority < 0)
goto fail;
if (type == NULL) {
- rte_log_set_global_level(tmp);
- } else if (rte_log_set_level_regexp(type, tmp) < 0) {
- printf("cannot set log level %s,%lu\n",
- type, tmp);
+ rte_log_set_global_level(priority);
+ } else if (rte_log_set_level_regexp(type, priority) < 0) {
+ fprintf(stderr, "cannot set log level %s,%d\n",
+ type, priority);
goto fail;
}
--
2.16.1
[dpdk-dev] [PATCH v2 2/3] log: add ability to match dynamic log based on shell pattern
Regular expressions are not the best way to match a hierarchical
pattern like dynamic log levels. And the separator for dynamic
log levels is period which is the regex wildcard character.
A better solution is to use filename matching 'globbing' so
that log levels match like file paths. For compatibility,
use colon to separate pattern match style arguments. For
example:
--log-level 'pmd.net.virtio.*:debug'
Signed-off-by: Stephen Hemminger
---
lib/librte_eal/common/eal_common_log.c | 22 +-
lib/librte_eal/common/eal_common_options.c | 36 --
lib/librte_eal/common/include/rte_log.h| 16 +++--
3 files changed, 59 insertions(+), 15 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_log.c
b/lib/librte_eal/common/eal_common_log.c
index 37b2e20e539b..2601c7cd4e4f 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -89,7 +90,26 @@ rte_log_set_level(uint32_t type, uint32_t level)
return 0;
}
-/* set level */
+int
+rte_log_set_level_match(const char *pattern, uint32_t level)
+{
+ size_t i;
+
+ if (level > RTE_LOG_DEBUG)
+ return -1;
+
+ for (i = 0; i < rte_logs.dynamic_types_len; i++) {
+ if (rte_logs.dynamic_types[i].name == NULL)
+ continue;
+
+ if (fnmatch(pattern, rte_logs.dynamic_types[i].name, 0))
+ rte_logs.dynamic_types[i].loglevel = level;
+ }
+
+ return 0;
+}
+
+/* set level by regular expression (using pattern match is preferred) */
int
rte_log_set_level_regexp(const char *pattern, uint32_t level)
{
diff --git a/lib/librte_eal/common/eal_common_options.c
b/lib/librte_eal/common/eal_common_options.c
index 19069638ea05..45a75f75ae94 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -955,31 +955,43 @@ eal_parse_log_priority(const char *level)
static int
eal_parse_log_level(const char *arg)
{
- char *str, *type, *level;
+ char *str, *level;
+ const char *regex = NULL;
+ const char *pattern = NULL;
int priority;
str = strdup(arg);
if (str == NULL)
return -1;
- if (strchr(str, ',') == NULL) {
- type = NULL;
- level = str;
+ if ((level = strchr(str, ','))) {
+ regex = str;
+ *level++ = '\0';
+ } else if ((level = strchr(str, ':'))) {
+ pattern = str;
+ *level++ = '\0';
} else {
- type = strsep(&str, ",");
- level = strsep(&str, ",");
+ level = str;
}
priority = eal_parse_log_priority(level);
if (priority < 0)
goto fail;
- if (type == NULL) {
+ if (regex) {
+ if (rte_log_set_level_regexp(regex, priority) < 0) {
+ fprintf(stderr, "cannot set log level %s,%d\n",
+ pattern, priority);
+ goto fail;
+ }
+ } else if (pattern) {
+ if (rte_log_set_level_match(pattern, priority) < 0) {
+ fprintf(stderr, "cannot set log level %s:%d\n",
+ pattern, priority);
+ goto fail;
+ }
+ } else {
rte_log_set_global_level(priority);
- } else if (rte_log_set_level_regexp(type, priority) < 0) {
- fprintf(stderr, "cannot set log level %s,%d\n",
- type, priority);
- goto fail;
}
free(str);
@@ -1336,7 +1348,7 @@ eal_common_usage(void)
" --"OPT_PROC_TYPE" Type of this process
(primary|secondary|auto)\n"
" --"OPT_SYSLOG"Set syslog facility\n"
" --"OPT_LOG_LEVEL"= Set global log level\n"
- " --"OPT_LOG_LEVEL"=,\n"
+ " --"OPT_LOG_LEVEL"=:\n"
" Set specific log level\n"
" -v Display version information on startup\n"
" -h, --help This help\n"
diff --git a/lib/librte_eal/common/include/rte_log.h
b/lib/librte_eal/common/include/rte_log.h
index 9029c7856d31..6d0ff9fe4623 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -132,13 +132,25 @@ int rte_log_get_level(uint32_t logtype);
* Set the log level for a given type.
*
* @param pattern
- * The regexp identifying the log type.
+ * The match pattern identifying the log type.
* @param level
* The level to be set.
* @return
* 0 on success, a negative value if level is invalid.
*/
-int rte_log_set_level_regexp(const char *pattern, uint32_t level);
+int rte_log_set_level_ma
[dpdk-dev] [PATCH v2 3/3] doc: update log level matching in documentation
The use of dynamic log in documentation should use matching not
regex notation.
Signed-off-by: Stephen Hemminger
---
doc/guides/contributing/coding_style.rst | 2 +-
doc/guides/nics/qede.rst | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/doc/guides/contributing/coding_style.rst
b/doc/guides/contributing/coding_style.rst
index b0f0adb887b4..365a4b17a983 100644
--- a/doc/guides/contributing/coding_style.rst
+++ b/doc/guides/contributing/coding_style.rst
@@ -615,7 +615,7 @@ In the DPDK environment, use the logging interface provided:
rte_log_set_level(my_logtype2, RTE_LOG_NOTICE);
/* enable all PMD logs (whose identifier string starts with "pmd") */
- rte_log_set_level_regexp("pmd.*", RTE_LOG_DEBUG);
+ rte_log_set_level_match("pmd.*", RTE_LOG_DEBUG);
/* log in debug level */
rte_log_set_global_level(RTE_LOG_DEBUG);
diff --git a/doc/guides/nics/qede.rst b/doc/guides/nics/qede.rst
index 63ce9b4c60c6..42dd70db39df 100644
--- a/doc/guides/nics/qede.rst
+++ b/doc/guides/nics/qede.rst
@@ -193,7 +193,7 @@ This section provides instructions to configure SR-IOV with
Linux OS.
#. Running testpmd
- (Supply ``--log-level="pmd.net.qede.driver",7`` to view informational
messages):
+ (Supply ``--log-level="pmd.net.qede.driver:7`` to view informational
messages):
Refer to the document
:ref:`compiling and testing a PMD for a NIC ` to run
--
2.16.1
Re: [dpdk-dev] [PATCH] compressdev: implement API
Hi Fiona,
Thanks for starting this discussion. In the current API the user must
make 12 API calls just to get information to compress. Maybe there is a
way to simplify. At least for some use cases (stateless). I think a call
sometime next week would be good to help clarify coalesce some of the
complexity.
I added specific comments inline.
Thanks,
Ahmed
On 2/21/2018 2:12 PM, Trahe, Fiona wrote:
> We've been struggling with the idea of session in compressdev.
>
> Is it really a session?
> - It's not in the same sense as cryptodev where it's used to hold a key, and
> maps to a Security Association.
> - It's a set of immutable data that is needed with the op and stream to
> perform the operation.
> - It inherited from cryptodev the ability to be set up for multiple driver
> types and used across any
> devices of those types. For stateful ops this facility can't be used.
> For stateless we don't think it's important, and think it's unlikely to
> be used.
> - Drivers use it to prepare private data, set up resources, do pre-work, so
> there's
> less work to be done on the data path. Initially we didn't have a stream,
> we do now,
> this may be a better alternative place for that work.
> So we've been toying with the idea of getting rid of the session.
[Ahmed] In our proprietary API the stream and session are one. A session
holds many properties like the op-type, instead of having this
information in the op itself. This way we lower the per op setup cost.
This also allows rapid reuse of stateful infrastructure, once a stream
is closed on a stateful session, the next op (stream) on this session
reuses the stateful storage. Obviously if a stream is in "pause mode" on
a session, all following ops that may be unrelated to this
stream/session must also wait until this current stream is closed or
aborted before the infrastructure can be reused.
>
> We also struggle with the idea of setting up a stream for stateless ops.
> - Well, really I just think the name is misleading, i.e. there's no problem
> with setting
> up some private PMD data to use with stateless operations, just calling
> it a
> stream doesn't seem right.
[Ahmed] I agree. The op has all the necessary information to process it
in the current API? Both the stream and the op are one time use. We
can't attach multiple similar ops to a single stream/session and rely on
their properties to simplify op setup, so why the hassle.
>
> So putting above thoughts together I want to propose:
> - Removal of the session and all associated APIs.
> - Passing in one of three data types in the rte_comp_op
>
> union {
> struct rte_comp_xform *xform;
> /**< Immutable compress/decompress params */
> void *pmd_stateless_data;
> /**< Stateless private PMD data derived from an rte_comp_xform
> * rte_comp_stateless_data_init() must be called on a device
> * before sending any STATELESS operations. If the PMD returns a
> non-NULL
> * value the handle must be attached to subsequent STATELESS
> operations.
> * If a PMD returns NULL, then the xform should be passed directly to
> each op
> */
> void *stream;
> /* Private PMD data derived initially from an rte_comp_xform, which
> holds state
> * and history data and evolves as operations are processed.
> * rte_comp_stream_create() must be called on a device for all
> STATEFUL
> * data streams and the resulting stream attached
> * to the one or more operations associated with the data stream.
> * All operations in a stream must be sent to the same device.
> */
> }
[Ahmed] I like this setup, but I am not sure in what cases the xform
immutable would be used. I understand the other two.
> Notes:
> 1. Internally if a PMD wants to use the exact same data structure for both it
> can do,
> just on the API I think it's better if they're named differently with
> different comments.
> 2. I'm not clear of the constraints if any, which attach to the
> pmd_stateless_data
> For our PMD it would only hold immutable data as the session did, and so
> could be attached to many ops in parallel.
> Is this true for all PMDs or are there constraints which should be
> called out?
> Is it limited to a specific device, qp, or to be used on one op at a
> time?
> 3. Am open to other naming suggestions, just trying to capture the essence
> of these data structs better than our current API does.
>
> We would put some more helper fns and structure around the above code if
> people
> are in agreement, just want to see if the concept flies before going further?
>
> Fiona
>
>
>
[dpdk-dev] [PATCH v2] app/testpmd: add option to avoid lock all memory
In some cases, we don't want to lock all memory.
Add an option --no-mlockall to avoid lock all memory.
Cc: wenzhuo...@intel.com
Cc: jingjing...@intel.com
Signed-off-by: Jianfeng Tan
---
app/test-pmd/parameters.c | 5 -
app/test-pmd/testpmd.c| 32 ++--
app/test-pmd/testpmd.h| 1 +
doc/guides/testpmd_app_ug/run_app.rst | 4
4 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 97d22b8..5060a6c 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -186,6 +186,7 @@ usage(char* progname)
printf(" --flow-isolate-all: "
"requests flow API isolated mode on all ports at initialization
time.\n");
printf(" --tx-offloads=0x: hexadecimal bitmask of TX queue
offloads\n");
+ printf(" --no-mlockall: do not lock all memory\n");
}
#ifdef RTE_LIBRTE_CMDLINE
@@ -621,6 +622,7 @@ launch_args_parse(int argc, char** argv)
{ "print-event",1, 0, 0 },
{ "mask-event", 1, 0, 0 },
{ "tx-offloads",1, 0, 0 },
+ { "no-mlockall",0, 0, 0 },
{ 0, 0, 0, 0 },
};
@@ -1102,7 +1104,8 @@ launch_args_parse(int argc, char** argv)
rte_exit(EXIT_FAILURE,
"invalid mask-event
argument\n");
}
-
+ if (!strcmp(lgopts[opt_idx].name, "no-mlockall"))
+ no_mlockall = 1;
break;
case 'h':
usage(argv[0]);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 4c0e258..59bdc85 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -294,6 +294,10 @@ uint32_t event_print_mask = (UINT32_C(1) <<
RTE_ETH_EVENT_UNKNOWN) |
(UINT32_C(1) << RTE_ETH_EVENT_INTR_RESET) |
(UINT32_C(1) << RTE_ETH_EVENT_MACSEC) |
(UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV);
+/*
+ * Decide if all memory are locked for performance.
+ */
+int no_mlockall = 0;
/*
* NIC bypass mode configuration options.
@@ -2489,7 +2493,20 @@ main(int argc, char** argv)
rte_panic("Cannot register log type");
rte_log_set_level(testpmd_logtype, RTE_LOG_DEBUG);
- if (mlockall(MCL_CURRENT | MCL_FUTURE)) {
+ /* Bitrate/latency stats disabled by default */
+#ifdef RTE_LIBRTE_BITRATE
+ bitrate_enabled = 0;
+#endif
+#ifdef RTE_LIBRTE_LATENCY_STATS
+ latencystats_enabled = 0;
+#endif
+
+ argc -= diag;
+ argv += diag;
+ if (argc > 1)
+ launch_args_parse(argc, argv);
+
+ if (!no_mlockall && mlockall(MCL_CURRENT | MCL_FUTURE)) {
TESTPMD_LOG(NOTICE, "mlockall() failed with error \"%s\"\n",
strerror(errno));
}
@@ -2511,19 +2528,6 @@ main(int argc, char** argv)
rte_panic("Empty set of forwarding logical cores - check the "
"core mask supplied in the command parameters\n");
- /* Bitrate/latency stats disabled by default */
-#ifdef RTE_LIBRTE_BITRATE
- bitrate_enabled = 0;
-#endif
-#ifdef RTE_LIBRTE_LATENCY_STATS
- latencystats_enabled = 0;
-#endif
-
- argc -= diag;
- argv += diag;
- if (argc > 1)
- launch_args_parse(argc, argv);
-
if (tx_first && interactive)
rte_exit(EXIT_FAILURE, "--tx-first cannot be used on "
"interactive mode.\n");
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 153abea..028b873 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -319,6 +319,7 @@ extern volatile int test_done; /* stop packet forwarding
when set to 1. */
extern uint8_t lsc_interrupt; /**< disabled by "--no-lsc-interrupt" parameter
*/
extern uint8_t rmv_interrupt; /**< disabled by "--no-rmv-interrupt" parameter
*/
extern uint32_t event_print_mask;
+extern int no_mlockall; /**< set by "--no-mlockall" parameter */
/**< set by "--print-event " and "--mask-event parameters */
#ifdef RTE_LIBRTE_IXGBE_BYPASS
diff --git a/doc/guides/testpmd_app_ug/run_app.rst
b/doc/guides/testpmd_app_ug/run_app.rst
index 1fd5395..bdc6262 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -479,3 +479,7 @@ The commandline options are:
Set the hexadecimal bitmask of TX queue offloads.
The default value is 0.
+
+* ``--no-mlockall``
+
+Disable locking all memory.
--
2.7.4
[dpdk-dev] [PATCH] doc: add driver limitation for vhost dequeue zero copy
In vhost-switch example, when binding nic to vfio-pci, dequeue zero copy cannot work in VM2NIC mode due to no iommu dma mapping is setup for guest memory currently. Signed-off-by: Junjie Chen --- doc/guides/sample_app_ug/vhost.rst | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/guides/sample_app_ug/vhost.rst b/doc/guides/sample_app_ug/vhost.rst index a4bdc6a..1591a31 100644 --- a/doc/guides/sample_app_ug/vhost.rst +++ b/doc/guides/sample_app_ug/vhost.rst @@ -147,7 +147,10 @@ retries on an RX burst, it takes effect only when rx retry is enabled. The default value is 15. **--dequeue-zero-copy** -Dequeue zero copy will be enabled when this option is given. +Dequeue zero copy will be enabled when this option is given, it is worth to +note that if NIC is binded to vfio-pci driver, dequeue zero copy cannot work +at VM2NIC mode (vm2vm=0) due to currently we don't setup iommu dma mapping for +guest memory. **--vlan-strip 0|1** VLAN strip option is removed, because different NICs have different behaviors -- 2.0.1
