[dpdk-dev] [PATCH] net/ena: get device info statically
Whenever the app is calling rte_eth_dev_info_get(), it shouldn't use the
admin command. It was causing problems, if it was called from the
secondary process - the main process was crashing, and the secondary app
wasn't getting any result, as the admin request couldn't be processed by
the process which was requesting the data.
To fix that, the data is being written to the adapter structure during
device initialization routine.
Signed-off-by: Michal Krawczyk
---
drivers/net/ena/ena_ethdev.c | 31 ---
drivers/net/ena/ena_ethdev.h | 8 +++-
2 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 8bb05caa2..08b1ab195 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1804,9 +1804,14 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
/* Set max MTU for this device */
adapter->max_mtu = get_feat_ctx.dev_attr.max_mtu;
- /* set device support for TSO */
- adapter->tso4_supported = get_feat_ctx.offload.tx &
- ENA_ADMIN_FEATURE_OFFLOAD_DESC_TSO_IPV4_MASK;
+ /* set device support for offloads */
+ adapter->offloads.tso4_supported = (get_feat_ctx.offload.tx &
+ ENA_ADMIN_FEATURE_OFFLOAD_DESC_TSO_IPV4_MASK) != 0;
+ adapter->offloads.tx_csum_supported = (get_feat_ctx.offload.tx &
+ ENA_ADMIN_FEATURE_OFFLOAD_DESC_TX_L4_IPV4_CSUM_PART_MASK) != 0;
+ adapter->offloads.tx_csum_supported =
+ (get_feat_ctx.offload.rx_supported &
+ ENA_ADMIN_FEATURE_OFFLOAD_DESC_RX_L4_IPV4_CSUM_MASK) != 0;
/* Copy MAC address and point DPDK to it */
eth_dev->data->mac_addrs = (struct ether_addr *)adapter->mac_addr;
@@ -1939,9 +1944,7 @@ static void ena_infos_get(struct rte_eth_dev *dev,
{
struct ena_adapter *adapter;
struct ena_com_dev *ena_dev;
- struct ena_com_dev_get_features_ctx feat;
uint64_t rx_feat = 0, tx_feat = 0;
- int rc = 0;
ena_assert_msg(dev->data != NULL, "Uninitialized device\n");
ena_assert_msg(dev->data->dev_private != NULL, "Uninitialized
device\n");
@@ -1960,26 +1963,16 @@ static void ena_infos_get(struct rte_eth_dev *dev,
ETH_LINK_SPEED_50G |
ETH_LINK_SPEED_100G;
- /* Get supported features from HW */
- rc = ena_com_get_dev_attr_feat(ena_dev, &feat);
- if (unlikely(rc)) {
- RTE_LOG(ERR, PMD,
- "Cannot get attribute for ena device rc= %d\n", rc);
- return;
- }
-
/* Set Tx & Rx features available for device */
- if (feat.offload.tx & ENA_ADMIN_FEATURE_OFFLOAD_DESC_TSO_IPV4_MASK)
+ if (adapter->offloads.tso4_supported)
tx_feat |= DEV_TX_OFFLOAD_TCP_TSO;
- if (feat.offload.tx &
- ENA_ADMIN_FEATURE_OFFLOAD_DESC_TX_L4_IPV4_CSUM_PART_MASK)
+ if (adapter->offloads.tx_csum_supported)
tx_feat |= DEV_TX_OFFLOAD_IPV4_CKSUM |
DEV_TX_OFFLOAD_UDP_CKSUM |
DEV_TX_OFFLOAD_TCP_CKSUM;
- if (feat.offload.rx_supported &
- ENA_ADMIN_FEATURE_OFFLOAD_DESC_RX_L4_IPV4_CSUM_MASK)
+ if (adapter->offloads.rx_csum_supported)
rx_feat |= DEV_RX_OFFLOAD_IPV4_CKSUM |
DEV_RX_OFFLOAD_UDP_CKSUM |
DEV_RX_OFFLOAD_TCP_CKSUM;
@@ -2171,7 +2164,7 @@ eth_ena_prep_pkts(void *tx_queue, struct rte_mbuf
**tx_pkts,
/* If IPv4 header has DF flag enabled and TSO support is
* disabled, partial chcecksum should not be calculated.
*/
- if (!tx_ring->adapter->tso4_supported)
+ if (!tx_ring->adapter->offloads.tso4_supported)
continue;
}
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index 309b92f65..221760c62 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -167,6 +167,12 @@ struct ena_stats_dev {
u64 dev_stop;
};
+struct ena_offloads {
+ bool tso4_supported;
+ bool tx_csum_supported;
+ bool rx_csum_supported;
+};
+
/* board specific private data structure */
struct ena_adapter {
/* OS defined structs */
@@ -188,7 +194,7 @@ struct ena_adapter {
u16 num_queues;
u16 max_mtu;
- u8 tso4_supported;
+ struct ena_offloads offloads;
int id_number;
char name[ENA_NAME_MAX_LEN];
--
2.14.1
Re: [dpdk-dev] [PATCH 3/5] app/testpmd: add missing transmit errors stats
14/02/2019 19:51, David Marchand: > What is the purpose of oerrors ? > > Since the drivers (via rte_eth_tx_burst return value) report the numbers of > packets successfully transmitted, the application can try to retransmit the > packets that did not make it and counts this. > If the driver counts such "missed" packets, then it does the job the > application will do anyway (wasting some cycles). > But what is more a problem is that the application does not know if the > packets in oerrors are its own retries or problems that the driver can not > detect (hw problems) but the hw can. > > So the best option is that oerrors does not report the packets the driver > refuses (and I can see some drivers that do not comply to this) but only > "external" errors from the driver pov. I can see the benefit of having driver errors in the stats, so it is generically stored for later analysis or print. It could be managed at ethdev level instead of the application doing the computation. What about splitting the Tx errors in 2 fields? oerrors / ofull ? Who said it's awful? sorry Bruce for anticipating ;)
Re: [dpdk-dev] [PATCH 3/5] app/testpmd: add missing transmit errors stats
On Fri, Feb 15, 2019 at 9:58 AM Thomas Monjalon wrote: > 14/02/2019 19:51, David Marchand: > > What is the purpose of oerrors ? > > > > Since the drivers (via rte_eth_tx_burst return value) report the numbers > of > > packets successfully transmitted, the application can try to retransmit > the > > packets that did not make it and counts this. > > If the driver counts such "missed" packets, then it does the job the > > application will do anyway (wasting some cycles). > > But what is more a problem is that the application does not know if the > > packets in oerrors are its own retries or problems that the driver can > not > > detect (hw problems) but the hw can. > > > > So the best option is that oerrors does not report the packets the driver > > refuses (and I can see some drivers that do not comply to this) but only > > "external" errors from the driver pov. > > I can see the benefit of having driver errors in the stats, > so it is generically stored for later analysis or print. > It could be managed at ethdev level instead of the application > doing the computation. > > What about splitting the Tx errors in 2 fields? oerrors / ofull ? > Who said it's awful? sorry Bruce for anticipating ;) > Summary, correct me if we are not aligned :-) - ofull (maybe ofifoerrors?) is actually a count of SW failed transmits - it would be handled in rte_eth_tx_burst() itself in a generic way - the drivers do not need to track such SW failed transmits - oerrors only counts packets HW failed transmits, dropped out of the driver tx_pkt_burst() knowledge - the application does not have to track SW failed transmits since the stats is in ethdev It sounds good to me, this means an ethdev abi breakage. I will drop my current patch anyway. Touching oerrors would be a separate effort. -- David Marchand
[dpdk-dev] [PATCH] test/compress: add max mbuf size test case
This patch adds new test case in which max. size of
chain mbufs has been used to compress random data dynamically.
Signed-off-by: Tomasz Jozwiak
---
test/test/test_compressdev.c | 157 +--
1 file changed, 136 insertions(+), 21 deletions(-)
diff --git a/test/test/test_compressdev.c b/test/test/test_compressdev.c
index e8476ed..f59b3d2 100644
--- a/test/test/test_compressdev.c
+++ b/test/test/test_compressdev.c
@@ -1,9 +1,10 @@
/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2018 Intel Corporation
+ * Copyright(c) 2018 - 2019 Intel Corporation
*/
#include
#include
#include
+#include
#include
#include
@@ -44,6 +45,11 @@
#define OUT_OF_SPACE_BUF 1
+#define MAX_MBUF_SEGMENT_SIZE 65535
+#define MAX_DATA_MBUF_SIZE (MAX_MBUF_SEGMENT_SIZE - RTE_PKTMBUF_HEADROOM)
+#define NUM_BIG_MBUFS 4
+#define BIG_DATA_TEST_SIZE (MAX_DATA_MBUF_SIZE * NUM_BIG_MBUFS / 2)
+
const char *
huffman_type_strings[] = {
[RTE_COMP_HUFFMAN_DEFAULT] = "PMD default",
@@ -72,6 +78,7 @@ struct priv_op_data {
struct comp_testsuite_params {
struct rte_mempool *large_mbuf_pool;
struct rte_mempool *small_mbuf_pool;
+ struct rte_mempool *big_mbuf_pool;
struct rte_mempool *op_pool;
struct rte_comp_xform *def_comp_xform;
struct rte_comp_xform *def_decomp_xform;
@@ -91,6 +98,7 @@ struct test_data_params {
enum varied_buff buff_type;
enum zlib_direction zlib_dir;
unsigned int out_of_space;
+ unsigned int big_data;
};
static struct comp_testsuite_params testsuite_params = { 0 };
@@ -104,11 +112,14 @@ testsuite_teardown(void)
RTE_LOG(ERR, USER1, "Large mbuf pool still has unfreed bufs\n");
if (rte_mempool_in_use_count(ts_params->small_mbuf_pool))
RTE_LOG(ERR, USER1, "Small mbuf pool still has unfreed bufs\n");
+ if (rte_mempool_in_use_count(ts_params->big_mbuf_pool))
+ RTE_LOG(ERR, USER1, "Big mbuf pool still has unfreed bufs\n");
if (rte_mempool_in_use_count(ts_params->op_pool))
RTE_LOG(ERR, USER1, "op pool still has unfreed ops\n");
rte_mempool_free(ts_params->large_mbuf_pool);
rte_mempool_free(ts_params->small_mbuf_pool);
+ rte_mempool_free(ts_params->big_mbuf_pool);
rte_mempool_free(ts_params->op_pool);
rte_free(ts_params->def_comp_xform);
rte_free(ts_params->def_decomp_xform);
@@ -161,6 +172,17 @@ testsuite_setup(void)
goto exit;
}
+ /* Create mempool with big buffers for SGL testing */
+ ts_params->big_mbuf_pool = rte_pktmbuf_pool_create("big_mbuf_pool",
+ NUM_BIG_MBUFS + 1,
+ CACHE_SIZE, 0,
+ MAX_MBUF_SEGMENT_SIZE,
+ rte_socket_id());
+ if (ts_params->big_mbuf_pool == NULL) {
+ RTE_LOG(ERR, USER1, "Big mbuf pool could not be created\n");
+ goto exit;
+ }
+
ts_params->op_pool = rte_comp_op_pool_create("op_pool", NUM_OPS,
0, sizeof(struct priv_op_data),
rte_socket_id());
@@ -597,10 +619,11 @@ prepare_sgl_bufs(const char *test_buf, struct rte_mbuf
*head_buf,
uint32_t total_data_size,
struct rte_mempool *small_mbuf_pool,
struct rte_mempool *large_mbuf_pool,
- uint8_t limit_segs_in_sgl)
+ uint8_t limit_segs_in_sgl,
+ uint16_t seg_size)
{
uint32_t remaining_data = total_data_size;
- uint16_t num_remaining_segs = DIV_CEIL(remaining_data, SMALL_SEG_SIZE);
+ uint16_t num_remaining_segs = DIV_CEIL(remaining_data, seg_size);
struct rte_mempool *pool;
struct rte_mbuf *next_seg;
uint32_t data_size;
@@ -616,10 +639,10 @@ prepare_sgl_bufs(const char *test_buf, struct rte_mbuf
*head_buf,
* Allocate data in the first segment (header) and
* copy data if test buffer is provided
*/
- if (remaining_data < SMALL_SEG_SIZE)
+ if (remaining_data < seg_size)
data_size = remaining_data;
else
- data_size = SMALL_SEG_SIZE;
+ data_size = seg_size;
buf_ptr = rte_pktmbuf_append(head_buf, data_size);
if (buf_ptr == NULL) {
RTE_LOG(ERR, USER1,
@@ -643,13 +666,13 @@ prepare_sgl_bufs(const char *test_buf, struct rte_mbuf
*head_buf,
if (i == (num_remaining_segs - 1)) {
/* last segment */
- if (remaining_data > SMALL_SEG_SIZE)
+ if (remaining_data > seg_size)
pool = large_mbuf_pool;
else
pool = small_mbuf_pool;
data_size = remaining_data;
} else {
- data_size = SMALL_S
[dpdk-dev] [PATCH] compress/qat: add dynamic sgl allocation
This patch adds dynamic SGL allocation instead of static one.
The number of element in SGL can be adjusted in each operation
depend of the request.
Signed-off-by: Tomasz Jozwiak
---
config/common_base | 1 -
doc/guides/compressdevs/qat_comp.rst | 1 -
doc/guides/cryptodevs/qat.rst| 5
drivers/compress/qat/qat_comp.c | 56
drivers/compress/qat/qat_comp.h | 13 -
drivers/compress/qat/qat_comp_pmd.c | 49 ++-
6 files changed, 99 insertions(+), 26 deletions(-)
diff --git a/config/common_base b/config/common_base
index 7c6da51..5df1752 100644
--- a/config/common_base
+++ b/config/common_base
@@ -549,7 +549,6 @@ CONFIG_RTE_LIBRTE_PMD_QAT_SYM=n
# Max. number of QuickAssist devices, which can be detected and attached
#
CONFIG_RTE_PMD_QAT_MAX_PCI_DEVICES=48
-CONFIG_RTE_PMD_QAT_COMP_SGL_MAX_SEGMENTS=16
CONFIG_RTE_PMD_QAT_COMP_IM_BUFFER_SIZE=65536
#
diff --git a/doc/guides/compressdevs/qat_comp.rst
b/doc/guides/compressdevs/qat_comp.rst
index 5631cb1..6f583a4 100644
--- a/doc/guides/compressdevs/qat_comp.rst
+++ b/doc/guides/compressdevs/qat_comp.rst
@@ -35,7 +35,6 @@ Limitations
* Compressdev level 0, no compression, is not supported.
* Queue pairs are not thread-safe (that is, within a single queue pair, RX and
TX from different lcores is not supported).
* No BSD support as BSD QAT kernel driver not available.
-* Number of segments in mbuf chains in the op must be <=
RTE_PMD_QAT_COMP_SGL_MAX_SEGMENTS from the config file.
* When using Deflate dynamic huffman encoding for compression, the input size
(op.src.length)
must be < CONFIG_RTE_PMD_QAT_COMP_IM_BUFFER_SIZE from the config file,
see :ref:`building_qat_config` for more details.
diff --git a/doc/guides/cryptodevs/qat.rst b/doc/guides/cryptodevs/qat.rst
index b079aa3..1f6b0d8 100644
--- a/doc/guides/cryptodevs/qat.rst
+++ b/doc/guides/cryptodevs/qat.rst
@@ -156,7 +156,6 @@ These are the build configuration options affecting QAT,
and their default value
CONFIG_RTE_LIBRTE_PMD_QAT=y
CONFIG_RTE_LIBRTE_PMD_QAT_SYM=n
CONFIG_RTE_PMD_QAT_MAX_PCI_DEVICES=48
- CONFIG_RTE_PMD_QAT_COMP_SGL_MAX_SEGMENTS=16
CONFIG_RTE_PMD_QAT_COMP_IM_BUFFER_SIZE=65536
CONFIG_RTE_LIBRTE_PMD_QAT must be enabled for any QAT PMD to be built.
@@ -174,10 +173,6 @@ Note, there are separate config items for max cryptodevs
CONFIG_RTE_CRYPTO_MAX_D
and max compressdevs CONFIG_RTE_COMPRESS_MAX_DEVS, if necessary these should be
adjusted to handle the total of QAT and other devices which the process will
use.
-QAT allocates internal structures to handle SGLs. For the compression service
-CONFIG_RTE_PMD_QAT_COMP_SGL_MAX_SEGMENTS can be changed if more segments are
needed.
-An extra (max_inflight_ops x 16) bytes per queue_pair will be used for every
increment.
-
QAT compression PMD needs intermediate buffers to support Deflate compression
with Dynamic Huffman encoding. CONFIG_RTE_PMD_QAT_COMP_IM_BUFFER_SIZE
specifies the size of a single buffer, the PMD will allocate a multiple of
these,
diff --git a/drivers/compress/qat/qat_comp.c b/drivers/compress/qat/qat_comp.c
index 32ca753..0c03bc3 100644
--- a/drivers/compress/qat/qat_comp.c
+++ b/drivers/compress/qat/qat_comp.c
@@ -1,5 +1,5 @@
/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2018 Intel Corporation
+ * Copyright(c) 2018-2019 Intel Corporation
*/
#include
@@ -55,22 +55,68 @@ qat_comp_build_request(void *in_op, uint8_t *out_msg,
ICP_QAT_FW_COMN_PTR_TYPE_SET(comp_req->comn_hdr.comn_req_flags,
QAT_COMN_PTR_TYPE_SGL);
+ if (unlikely(op->m_src->nb_segs > cookie->src_nb_elems)) {
+ /* we need to allocate more elements in SGL*/
+ void *tmp;
+
+ tmp = rte_realloc(cookie->qat_sgl_src_d,
+ sizeof(struct qat_sgl) +
+ sizeof(struct qat_flat_buf) *
+ op->m_src->nb_segs, 64);
+
+ if (unlikely(tmp == NULL)) {
+ QAT_DP_LOG(ERR, "QAT PMD can't allocate memory"
+ " for %d elements of SGL",
+ op->m_src->nb_segs);
+ op->status = RTE_COMP_OP_STATUS_INVALID_ARGS;
+ return -ENOMEM;
+ }
+ /* new SGL is valid now */
+ cookie->qat_sgl_src_d = (struct qat_sgl *)tmp;
+ cookie->src_nb_elems = op->m_src->nb_segs;
+ cookie->qat_sgl_src_phys_addr =
+ rte_malloc_virt2iova(cookie->qat_sgl_src_d);
+ }
+
ret = qat_sgl_fill_array(op->m_src,
op->s
[dpdk-dev] [PATCH] compress/qat: add fallback to fixed compression
This patch adds fallback to fixed compression
feature during dynamic compression, when the input data size
is greater than IM buffer size / 1.1. This feature doesn't
stop compression proccess when IM buffer can be too small
to handle produced data.
Signed-off-by: Tomasz Jozwiak
---
doc/guides/cryptodevs/qat.rst | 10 +-
drivers/compress/qat/qat_comp.c | 24
drivers/compress/qat/qat_comp.h | 3 +++
3 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/doc/guides/cryptodevs/qat.rst b/doc/guides/cryptodevs/qat.rst
index b079aa3..907c2a9 100644
--- a/doc/guides/cryptodevs/qat.rst
+++ b/doc/guides/cryptodevs/qat.rst
@@ -188,11 +188,11 @@ allocated while for GEN1 devices, 12 buffers are
allocated, plus 1472 bytes over
If the compressed output of a Deflate operation using Dynamic Huffman
Encoding is too big to fit in an intermediate buffer, then the
-operation will return RTE_COMP_OP_STATUS_ERROR and an error will be
-displayed. Options for the application in this case
-are to split the input data into smaller chunks and resubmit
-in multiple operations or to configure QAT with
-larger intermediate buffers.
+ operation will fall back to fixed compression rather than failing the
operation.
+ To avoid this less performant case, applications should configure
+ the intermediate buffer size to be larger than the expected input data
size
+ (compressed output size is usually unknown, so the only option is to
make
+ larger than the input size).
Device and driver naming
diff --git a/drivers/compress/qat/qat_comp.c b/drivers/compress/qat/qat_comp.c
index 32ca753..b9367d3 100644
--- a/drivers/compress/qat/qat_comp.c
+++ b/drivers/compress/qat/qat_comp.c
@@ -43,6 +43,30 @@ qat_comp_build_request(void *in_op, uint8_t *out_msg,
rte_mov128(out_msg, tmpl);
comp_req->comn_mid.opaque_data = (uint64_t)(uintptr_t)op;
+ if (likely(qat_xform->qat_comp_request_type ==
+ QAT_COMP_REQUEST_DYNAMIC_COMP_STATELESS)) {
+ if (unlikely(op->src.length > QAT_FALLBACK_THLD)) {
+
+ /* fallback to fixed compression */
+ comp_req->comn_hdr.service_cmd_id =
+ ICP_QAT_FW_COMP_CMD_STATIC;
+
+ ICP_QAT_FW_COMN_NEXT_ID_SET(&comp_req->comp_cd_ctrl,
+ ICP_QAT_FW_SLICE_DRAM_WR);
+
+ ICP_QAT_FW_COMN_NEXT_ID_SET(&comp_req->u2.xlt_cd_ctrl,
+ ICP_QAT_FW_SLICE_NULL);
+ ICP_QAT_FW_COMN_CURR_ID_SET(&comp_req->u2.xlt_cd_ctrl,
+ ICP_QAT_FW_SLICE_NULL);
+
+ QAT_DP_LOG(DEBUG, "QAT PMD: fallback to fixed "
+ "compression! IM buffer size can be too low "
+ "for produced data.\n Please use input "
+ "buffer length lower than %d bytes",
+ QAT_FALLBACK_THLD);
+ }
+ }
+
/* common for sgl and flat buffers */
comp_req->comp_pars.comp_len = op->src.length;
comp_req->comp_pars.out_buffer_sz = rte_pktmbuf_pkt_len(op->m_dst) -
diff --git a/drivers/compress/qat/qat_comp.h b/drivers/compress/qat/qat_comp.h
index 19f48df..12b37b1 100644
--- a/drivers/compress/qat/qat_comp.h
+++ b/drivers/compress/qat/qat_comp.h
@@ -21,6 +21,9 @@
#define ERR_CODE_QAT_COMP_WRONG_FW -99
+/* fallback to fixed compression threshold */
+#define QAT_FALLBACK_THLD ((uint32_t)(RTE_PMD_QAT_COMP_IM_BUFFER_SIZE / 1.1))
+
enum qat_comp_request_type {
QAT_COMP_REQUEST_FIXED_COMP_STATELESS,
QAT_COMP_REQUEST_DYNAMIC_COMP_STATELESS,
--
2.7.4
[dpdk-dev] [PATCH v3] service: fix parameter type
The type of value parameter to rte_service_attr_get
should be uint64_t *, since the attributes
are of type uint64_t.
Fixes: 4d55194d76a4 ("service: add attribute get function")
Reviewed-by: Gage Eads
Signed-off-by: Nikhil Rao
Acked-by: Harry van Haaren
---
lib/librte_eal/common/include/rte_service.h | 2 +-
lib/librte_eal/common/rte_service.c | 2 +-
test/test/test_service_cores.c | 2 +-
doc/guides/rel_notes/deprecation.rst| 4
doc/guides/rel_notes/release_19_05.rst | 5 -
lib/librte_eal/bsdapp/eal/Makefile | 2 +-
lib/librte_eal/linuxapp/eal/Makefile| 2 +-
lib/librte_eal/meson.build | 2 +-
8 files changed, 10 insertions(+), 11 deletions(-)
v2:
* Update release notes.
* Update library version in makefiles.
v3:
* Remove deprecation notice.
diff --git a/lib/librte_eal/common/include/rte_service.h
b/lib/librte_eal/common/include/rte_service.h
index 34b41af..670c89a 100644
--- a/lib/librte_eal/common/include/rte_service.h
+++ b/lib/librte_eal/common/include/rte_service.h
@@ -372,7 +372,7 @@ int32_t rte_service_run_iter_on_app_lcore(uint32_t id,
* -EINVAL Invalid id, attr_id or attr_value was NULL.
*/
int32_t rte_service_attr_get(uint32_t id, uint32_t attr_id,
- uint32_t *attr_value);
+ uint64_t *attr_value);
/**
* Reset all attribute values of a service.
diff --git a/lib/librte_eal/common/rte_service.c
b/lib/librte_eal/common/rte_service.c
index 03fde97..5f75e5a 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -734,7 +734,7 @@ int32_t rte_service_run_iter_on_app_lcore(uint32_t id,
}
int32_t
-rte_service_attr_get(uint32_t id, uint32_t attr_id, uint32_t *attr_value)
+rte_service_attr_get(uint32_t id, uint32_t attr_id, uint64_t *attr_value)
{
struct rte_service_spec_impl *s;
SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL);
diff --git a/test/test/test_service_cores.c b/test/test/test_service_cores.c
index ec31882..82bb2ce 100644
--- a/test/test/test_service_cores.c
+++ b/test/test/test_service_cores.c
@@ -259,7 +259,7 @@ static int32_t dummy_mt_safe_cb(void *args)
rte_service_set_stats_enable(id, 1);
uint32_t attr_id = UINT32_MAX;
- uint32_t attr_value = 0xdead;
+ uint64_t attr_value = 0xdead;
/* check error return values */
TEST_ASSERT_EQUAL(-EINVAL, rte_service_attr_get(id, attr_id,
&attr_value),
diff --git a/doc/guides/rel_notes/deprecation.rst
b/doc/guides/rel_notes/deprecation.rst
index 1b4fcb7..93ed31f 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -20,10 +20,6 @@ Deprecation Notices
* kvargs: The function ``rte_kvargs_process`` will get a new parameter
for returning key match count. It will ease handling of no-match case.
-* eal: The ``attr_value`` parameter of ``rte_service_attr_get()``
- will be changed from ``uint32_t *`` to ``uint64_t *``
- as the attributes are of type ``uint64_t``.
-
* eal: both declaring and identifying devices will be streamlined in v18.11.
New functions will appear to query a specific port from buses, classes of
device and device drivers. Device declaration will be made coherent with the
diff --git a/doc/guides/rel_notes/release_19_05.rst
b/doc/guides/rel_notes/release_19_05.rst
index 2b0f60d..b8ed3d3 100644
--- a/doc/guides/rel_notes/release_19_05.rst
+++ b/doc/guides/rel_notes/release_19_05.rst
@@ -94,6 +94,9 @@ API Changes
Also, make sure to start the actual text at the margin.
=
+eal: as shown in the 19.02 deprecation notice, the type of the ``attr_value``
+ parameter of the function ``rte_service_attr_get()`` has been changed
+ from ``uint32_t *`` to ``uint64_t *``.
ABI Changes
---
@@ -143,7 +146,7 @@ The libraries prepended with a plus sign were incremented
in this version.
librte_compressdev.so.1
librte_cryptodev.so.6
librte_distributor.so.1
- librte_eal.so.9
++librte_eal.so.10
librte_efd.so.1
librte_ethdev.so.11
librte_eventdev.so.6
diff --git a/lib/librte_eal/bsdapp/eal/Makefile
b/lib/librte_eal/bsdapp/eal/Makefile
index bfeddaa..4bc8555 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -22,7 +22,7 @@ LDLIBS += -lrte_kvargs
EXPORT_MAP := ../../rte_eal_version.map
-LIBABIVER := 9
+LIBABIVER := 10
# specific to bsdapp exec-env
SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) := eal.c
diff --git a/lib/librte_eal/linuxapp/eal/Makefile
b/lib/librte_eal/linuxapp/eal/Makefile
index 51deb57..db6aca3 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -10,7 +10,7 @@ ARCH_DIR ?= $(RTE_ARCH)
EXPORT_MAP := ../../rte_eal_version.map
VPATH += $(RTE_SDK)/lib/librte_eal/common/arch/$(ARCH_DIR)
-LIBABIVER := 9
+LIBABIVE
[dpdk-dev] [PATCH v3] power: fix to remove unused variable
Variable pfi_str is removed since it is unused.
Fixes: 450f0791312c ("power: add traffic pattern aware power control")
Cc: sta...@dpdk.org
Signed-off-by: Pallantla Poornima
Reviewed-by: Rami Rosen
---
v3: Updated fixes line.
v2: Removed unused variable as suggested.
---
lib/librte_power/rte_power_empty_poll.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/lib/librte_power/rte_power_empty_poll.c
b/lib/librte_power/rte_power_empty_poll.c
index e6145462f..15d4f0509 100644
--- a/lib/librte_power/rte_power_empty_poll.c
+++ b/lib/librte_power/rte_power_empty_poll.c
@@ -156,11 +156,8 @@ update_training_stats(struct priority_worker *poll_stats,
{
RTE_SET_USED(specific_freq);
- char pfi_str[32];
uint64_t p0_empty_deq;
- sprintf(pfi_str, "%02d", freq);
-
if (poll_stats->cur_freq == freq &&
poll_stats->thresh[freq].trained == false) {
if (poll_stats->thresh[freq].cur_train_iter == 0) {
--
2.17.2
Re: [dpdk-dev] rte_flow update support?
Hi Shahaf,
This is great news! I'll definitely stay tuned.
Is there any way to support replacement with the current system with
some patching? Eg the driver refuses to overwrite rules with kernel
message such as "FTE flow tag 196608 already exists with different flow
tag 327680". Would it be possible to ignore the message and overwrite?
Tom
On 2019-02-14 14:15, Shahaf Shuler wrote:
Hi Tom,
Thursday, February 14, 2019 1:31 PM, Tom Barbette:
Subject: rte_flow update support?
Hi all,
Are there plans to add support for modifying rules using the rte_flow API ?
The first problem with destroy+create is atomicity. During the process some
packets will get lost.
Then the second problem is performance. We measured Mellanox CX5 (mlx5
driver) to be able to "update" at best 2K rules/sec, but that drops to
200 rules/sec when updating TC rules ("transfer" rules, to switch packets
between VFs). Real support for update should boost those numbers.
Yes you are right, the current update rate of verbs and TC is not so good.
I saw the ibverbs API backing the mlx5 supports updating the action of a rule.
This would already solve a lot of use cases. Eg, changing destination queue(s)
of some rules. Given that mlx5 does not support changing global RSS queues
without restarting the device, this would also solve re-balancing issue by
using rte_flow.
Updating the action list will solve only part of issue, what you really want
(for OVS case) is to update the flow pattern as well (since TCP connection got
terminated and new one was created).
Then, beyond updating only the action of a rule, some researchers have
shown[1] that updating rules patterns data instead of creating and deleting
rules with similar patterns improve drastically the performance. Eg that could
be very interesting to accelerate the offloading of OVS's flow cache (5-
tuples), or similar setups.
Stay tuned, we are working on it.
There is a new engine for flow rules which will be very fast. Expected
performance will be ~300K updates per second and will include both transfer and
regular flow rules.
It is in plans for 19.XX releases.
Would recommend you to have a try on it once ready.
Thanks,
Tom
[1] Turboflow: information rich flow record generation on commodity
switches, J Sonchack et al.
Re: [dpdk-dev] [PATCH] app/testbbdev: fix sprintf with snprintf
> -Original Message- > From: Parthasarathy, JananeeX M > Sent: Friday 15 February 2019 11:08 > To: Yigit, Ferruh ; Poornima, PallantlaX > ; dev@dpdk.org > Cc: Pattan, Reshma ; Mokhtar, Amr > ; sta...@dpdk.org > Subject: RE: [dpdk-dev] [PATCH] app/testbbdev: fix sprintf with snprintf > > Hi > > >-Original Message- > >From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ferruh Yigit > >Sent: Friday, February 08, 2019 7:44 PM > >To: Poornima, PallantlaX ; > dev@dpdk.org > >Cc: Pattan, Reshma ; Mokhtar, Amr > >; sta...@dpdk.org > >Subject: Re: [dpdk-dev] [PATCH] app/testbbdev: fix sprintf with snprintf > > > >On 2/4/2019 7:15 AM, Pallantla Poornima wrote: > >> @@ -825,7 +825,7 @@ > >> > >> test_bbdev_driver_init(void) > >> > >> TEST_ASSERT_FAIL(rte_bbdev_release(NULL), > >> "Failed to uninitialize bbdev driver with NULL bbdev"); > >> - sprintf(name_tmp, "%s", "invalid_name"); > >> + snprintf(name_tmp, sizeof(name_tmp), "%s", "invalid_name"); > >> dev2 = rte_bbdev_get_named_dev(name_tmp); > >> TEST_ASSERT_FAIL(rte_bbdev_release(dev2), > >> "Failed to uninitialize bbdev driver with invalid name"); > > > >Can use strlcpy() instead of snprintf() here. > > Please confirm whether we can do this change (snprintf to strlcpy) as there > was Ack from other reviewer. I agree with Ferruh suggestion, as this is a simple string copying. Feel free to use strlcpy instead and retain my Ack. Thanks. > Thanks > M.P.Jananee
Re: [dpdk-dev] [PATCH v3] power: fix to remove unused variable
On 15/2/2019 10:28 AM, Pallantla Poornima wrote:
Variable pfi_str is removed since it is unused.
Fixes: 450f0791312c ("power: add traffic pattern aware power control")
Cc: sta...@dpdk.org
Signed-off-by: Pallantla Poornima
Reviewed-by: Rami Rosen
---
v3: Updated fixes line.
v2: Removed unused variable as suggested.
---
lib/librte_power/rte_power_empty_poll.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/lib/librte_power/rte_power_empty_poll.c
b/lib/librte_power/rte_power_empty_poll.c
index e6145462f..15d4f0509 100644
--- a/lib/librte_power/rte_power_empty_poll.c
+++ b/lib/librte_power/rte_power_empty_poll.c
@@ -156,11 +156,8 @@ update_training_stats(struct priority_worker *poll_stats,
{
RTE_SET_USED(specific_freq);
- char pfi_str[32];
uint64_t p0_empty_deq;
- sprintf(pfi_str, "%02d", freq);
-
if (poll_stats->cur_freq == freq &&
poll_stats->thresh[freq].trained == false) {
if (poll_stats->thresh[freq].cur_train_iter == 0) {
Acked-by: David Hunt
Re: [dpdk-dev] [PATCH v2 0/2] Minor changes to checkpatches
On Thu, Feb 14, 2019 at 02:35:45PM -0500, Michael Santana wrote: > Fixed a minor bug with variable assignment, as well as added an > option for checkpatches > > v1->v2: > Enable codespell by default. Disable via config file > > Michael Santana (2): > Enable codespell by default. Can be disabled from config file. > Fix variable assignment. > > devtools/checkpatches.sh | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > Series-Acked-by: Bruce Richardson
Re: [dpdk-dev] [PATCH 3/5] app/testpmd: add missing transmit errors stats
> On Feb 15, 2019, at 8:05 AM, Bruce Richardson > wrote: > > On Fri, Feb 15, 2019 at 10:33:47AM +0100, David Marchand wrote: >> On Fri, Feb 15, 2019 at 9:58 AM Thomas Monjalon >> <[1]tho...@monjalon.net> wrote: >> >> 14/02/2019 19:51, David Marchand: >>> What is the purpose of oerrors ? >>> >>> Since the drivers (via rte_eth_tx_burst return value) report the >> numbers of >>> packets successfully transmitted, the application can try to >> retransmit the >>> packets that did not make it and counts this. >>> If the driver counts such "missed" packets, then it does the job >> the >>> application will do anyway (wasting some cycles). >>> But what is more a problem is that the application does not know >> if the >>> packets in oerrors are its own retries or problems that the driver >> can not >>> detect (hw problems) but the hw can. >>> >>> So the best option is that oerrors does not report the packets the >> driver >>> refuses (and I can see some drivers that do not comply to this) >> but only >>> "external" errors from the driver pov. >> I can see the benefit of having driver errors in the stats, >> so it is generically stored for later analysis or print. >> It could be managed at ethdev level instead of the application >> doing the computation. >> What about splitting the Tx errors in 2 fields? oerrors / ofull ? >> Who said it's awful? sorry Bruce for anticipating ;) >> >> Summary, correct me if we are not aligned :-) >> - ofull (maybe ofifoerrors?) is actually a count of SW failed transmits >> - it would be handled in rte_eth_tx_burst() itself in a generic way >> - the drivers do not need to track such SW failed transmits >> - oerrors only counts packets HW failed transmits, dropped out of the >> driver tx_pkt_burst() knowledge >> - the application does not have to track SW failed transmits since the >> stats is in ethdev >> It sounds good to me, this means an ethdev abi breakage. > > Hang on, why do we need ethdev to track this at all, given that it's > trivial for apps to track this themselves. Would we not be better just to > add this tracking into testpmd and leave ethdev and drivers alone? Perhaps > I'm missing something? Adding the counters to ethdev stats is a good idea to me, the number of tx full failures is a great counter as it can tell you a lot about performance of the application. if the ofull counter is high then we have a lot of re-xmit attempts which can point to the problem quicker IMO. Adding it to the PMDs is the right place for this type of information as it is a very common needed counter. > Regards, Keith
Re: [dpdk-dev] [PATCH v3] power: fix to remove unused variable
Pallantla Poornima writes:
> Variable pfi_str is removed since it is unused.
>
> Fixes: 450f0791312c ("power: add traffic pattern aware power control")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Pallantla Poornima
> Reviewed-by: Rami Rosen
> ---
Whoops, missed the v3
Acked-by: Aaron Conole
Re: [dpdk-dev] [PATCH 3/5] app/testpmd: add missing transmit errors stats
On Fri, Feb 15, 2019 at 10:33:47AM +0100, David Marchand wrote: >On Fri, Feb 15, 2019 at 9:58 AM Thomas Monjalon ><[1]tho...@monjalon.net> wrote: > > 14/02/2019 19:51, David Marchand: > > What is the purpose of oerrors ? > > > > Since the drivers (via rte_eth_tx_burst return value) report the > numbers of > > packets successfully transmitted, the application can try to > retransmit the > > packets that did not make it and counts this. > > If the driver counts such "missed" packets, then it does the job > the > > application will do anyway (wasting some cycles). > > But what is more a problem is that the application does not know > if the > > packets in oerrors are its own retries or problems that the driver > can not > > detect (hw problems) but the hw can. > > > > So the best option is that oerrors does not report the packets the > driver > > refuses (and I can see some drivers that do not comply to this) > but only > > "external" errors from the driver pov. > I can see the benefit of having driver errors in the stats, > so it is generically stored for later analysis or print. > It could be managed at ethdev level instead of the application > doing the computation. > What about splitting the Tx errors in 2 fields? oerrors / ofull ? > Who said it's awful? sorry Bruce for anticipating ;) > >Summary, correct me if we are not aligned :-) >- ofull (maybe ofifoerrors?) is actually a count of SW failed transmits >- it would be handled in rte_eth_tx_burst() itself in a generic way >- the drivers do not need to track such SW failed transmits >- oerrors only counts packets HW failed transmits, dropped out of the >driver tx_pkt_burst() knowledge >- the application does not have to track SW failed transmits since the >stats is in ethdev >It sounds good to me, this means an ethdev abi breakage. Hang on, why do we need ethdev to track this at all, given that it's trivial for apps to track this themselves. Would we not be better just to add this tracking into testpmd and leave ethdev and drivers alone? Perhaps I'm missing something?
Re: [dpdk-dev] [PATCH v2] power: fix to remove unused variable
Pallantla Poornima writes:
> Variable pfi_str is removed since it is unused.
>
> Fixes: 450f079131 ("power: add traffic pattern aware power control")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Pallantla Poornima
> ---
Acked-by: Aaron Conole
Re: [dpdk-dev] [PATCH 3/5] app/testpmd: add missing transmit errors stats
On Fri, Feb 15, 2019 at 3:05 PM Bruce Richardson wrote: > On Fri, Feb 15, 2019 at 10:33:47AM +0100, David Marchand wrote: > >On Fri, Feb 15, 2019 at 9:58 AM Thomas Monjalon > ><[1]tho...@monjalon.net> wrote: > > > > 14/02/2019 19:51, David Marchand: > > > What is the purpose of oerrors ? > > > > > > Since the drivers (via rte_eth_tx_burst return value) report the > > numbers of > > > packets successfully transmitted, the application can try to > > retransmit the > > > packets that did not make it and counts this. > > > If the driver counts such "missed" packets, then it does the job > > the > > > application will do anyway (wasting some cycles). > > > But what is more a problem is that the application does not know > > if the > > > packets in oerrors are its own retries or problems that the driver > > can not > > > detect (hw problems) but the hw can. > > > > > > So the best option is that oerrors does not report the packets the > > driver > > > refuses (and I can see some drivers that do not comply to this) > > but only > > > "external" errors from the driver pov. > > I can see the benefit of having driver errors in the stats, > > so it is generically stored for later analysis or print. > > It could be managed at ethdev level instead of the application > > doing the computation. > > What about splitting the Tx errors in 2 fields? oerrors / ofull ? > > Who said it's awful? sorry Bruce for anticipating ;) > > > >Summary, correct me if we are not aligned :-) > >- ofull (maybe ofifoerrors?) is actually a count of SW failed > transmits > >- it would be handled in rte_eth_tx_burst() itself in a generic way > >- the drivers do not need to track such SW failed transmits > >- oerrors only counts packets HW failed transmits, dropped out of the > >driver tx_pkt_burst() knowledge > >- the application does not have to track SW failed transmits since the > >stats is in ethdev > >It sounds good to me, this means an ethdev abi breakage. > > Hang on, why do we need ethdev to track this at all, given that it's > trivial for apps to track this themselves. Would we not be better just to > add this tracking into testpmd and leave ethdev and drivers alone? Perhaps > I'm missing something? > This was my first intention but Thomas hopped in ;-) testpmd does it already via the fs->fwd_dropped stats and ovs has its tx_dropped stat. The problem is that all drivers have different approach about this. Some drivers only count real hw errors in oerrors. But others count the packets it can't send in oerrors (+ there are some cases that seem buggy to me where the driver will always refuse the mbufs for reason X and the application can retry indefinitely to send...). -- David Marchand
Re: [dpdk-dev] [PATCH 3/5] app/testpmd: add missing transmit errors stats
15/02/2019 16:04, David Marchand: > On Fri, Feb 15, 2019 at 3:05 PM Bruce Richardson > wrote: > > > On Fri, Feb 15, 2019 at 10:33:47AM +0100, David Marchand wrote: > > >On Fri, Feb 15, 2019 at 9:58 AM Thomas Monjalon > > ><[1]tho...@monjalon.net> wrote: > > > > > > 14/02/2019 19:51, David Marchand: > > > > What is the purpose of oerrors ? > > > > > > > > Since the drivers (via rte_eth_tx_burst return value) report the > > > numbers of > > > > packets successfully transmitted, the application can try to > > > retransmit the > > > > packets that did not make it and counts this. > > > > If the driver counts such "missed" packets, then it does the job > > > the > > > > application will do anyway (wasting some cycles). > > > > But what is more a problem is that the application does not know > > > if the > > > > packets in oerrors are its own retries or problems that the driver > > > can not > > > > detect (hw problems) but the hw can. > > > > > > > > So the best option is that oerrors does not report the packets the > > > driver > > > > refuses (and I can see some drivers that do not comply to this) > > > but only > > > > "external" errors from the driver pov. > > > I can see the benefit of having driver errors in the stats, > > > so it is generically stored for later analysis or print. > > > It could be managed at ethdev level instead of the application > > > doing the computation. > > > What about splitting the Tx errors in 2 fields? oerrors / ofull ? > > > Who said it's awful? sorry Bruce for anticipating ;) > > > > > >Summary, correct me if we are not aligned :-) > > >- ofull (maybe ofifoerrors?) is actually a count of SW failed > > transmits > > >- it would be handled in rte_eth_tx_burst() itself in a generic way > > >- the drivers do not need to track such SW failed transmits > > >- oerrors only counts packets HW failed transmits, dropped out of the > > >driver tx_pkt_burst() knowledge > > >- the application does not have to track SW failed transmits since the > > >stats is in ethdev > > >It sounds good to me, this means an ethdev abi breakage. > > > > Hang on, why do we need ethdev to track this at all, given that it's > > trivial for apps to track this themselves. Would we not be better just to > > add this tracking into testpmd and leave ethdev and drivers alone? Perhaps > > I'm missing something? > > > > This was my first intention but Thomas hopped in ;-) I was just opening the discussion :) > testpmd does it already via the fs->fwd_dropped stats and ovs has its > tx_dropped stat. > > The problem is that all drivers have different approach about this. > Some drivers only count real hw errors in oerrors. > But others count the packets it can't send in oerrors (+ there are some > cases that seem buggy to me where the driver will always refuse the mbufs > for reason X and the application can retry indefinitely to send...). We have 3 options: 1/ status quo = oerrors is inconsistent across drivers 2/ API break = oerrors stop being incremented for temporary unavailability (i.e. queue full, kind of ERETRY), report only packets which will be never sent, may be a small performance gain for some drivers 3/ API + ABI break = same as 2/ + report ERETRY errors in ofull (same as tx_burst() delta) Note that the option 2 is a light API break which does not require any deprecation notice because the original definition of oerrors is really vague: "failed transmitted packets" By changing the definition of errors to "packets lost", we can count HW errors + packets not matching requirements. As David suggests, the packets not matching requirements can be freed as it is done for packets successfully transmitted to the HW. We need also to update the definition of the return value of rte_eth_tx_burst(): "packets actually stored in transmit descriptors". We should also count the bad packets rejected by the driver. Then the number of bad packets would be the difference between the return value of rte_eth_tx_burst() and opackets counter. This solution is fixing some bugs and enforce a consistent behaviour. The option 3 is breaking the ABI and may degrade the performances. The only benefit is convenience or semantic: ofull would be the equivalent of imissed. The application can count the same by making the difference between the burst size and the return of rte_eth_tx_burst. My vote is for the option 2.
Re: [dpdk-dev] [PATCH] compress/qat: add fallback to fixed compression
> -Original Message- > From: Jozwiak, TomaszX > Sent: Friday, February 15, 2019 9:45 AM > To: dev@dpdk.org; Trahe, Fiona ; Jozwiak, TomaszX > > Subject: [PATCH] compress/qat: add fallback to fixed compression > > This patch adds fallback to fixed compression > feature during dynamic compression, when the input data size > is greater than IM buffer size / 1.1. This feature doesn't > stop compression proccess when IM buffer can be too small > to handle produced data. > > Signed-off-by: Tomasz Jozwiak Acked-by: Fiona Trahe
Re: [dpdk-dev] [PATCH 3/5] app/testpmd: add missing transmit errors stats
On Fri, Feb 15, 2019 at 5:19 PM Thomas Monjalon wrote: > 15/02/2019 16:04, David Marchand: > > On Fri, Feb 15, 2019 at 3:05 PM Bruce Richardson < > bruce.richard...@intel.com> > > wrote: > > > > > On Fri, Feb 15, 2019 at 10:33:47AM +0100, David Marchand wrote: > > > >On Fri, Feb 15, 2019 at 9:58 AM Thomas Monjalon > > > ><[1]tho...@monjalon.net> wrote: > > > > > > > > 14/02/2019 19:51, David Marchand: > > > > > What is the purpose of oerrors ? > > > > > > > > > > Since the drivers (via rte_eth_tx_burst return value) report > the > > > > numbers of > > > > > packets successfully transmitted, the application can try to > > > > retransmit the > > > > > packets that did not make it and counts this. > > > > > If the driver counts such "missed" packets, then it does the > job > > > > the > > > > > application will do anyway (wasting some cycles). > > > > > But what is more a problem is that the application does not > know > > > > if the > > > > > packets in oerrors are its own retries or problems that the > driver > > > > can not > > > > > detect (hw problems) but the hw can. > > > > > > > > > > So the best option is that oerrors does not report the > packets the > > > > driver > > > > > refuses (and I can see some drivers that do not comply to > this) > > > > but only > > > > > "external" errors from the driver pov. > > > > I can see the benefit of having driver errors in the stats, > > > > so it is generically stored for later analysis or print. > > > > It could be managed at ethdev level instead of the application > > > > doing the computation. > > > > What about splitting the Tx errors in 2 fields? oerrors / ofull > ? > > > > Who said it's awful? sorry Bruce for anticipating ;) > > > > > > > >Summary, correct me if we are not aligned :-) > > > >- ofull (maybe ofifoerrors?) is actually a count of SW failed > > > transmits > > > >- it would be handled in rte_eth_tx_burst() itself in a generic > way > > > >- the drivers do not need to track such SW failed transmits > > > >- oerrors only counts packets HW failed transmits, dropped out of > the > > > >driver tx_pkt_burst() knowledge > > > >- the application does not have to track SW failed transmits > since the > > > >stats is in ethdev > > > >It sounds good to me, this means an ethdev abi breakage. > > > > > > Hang on, why do we need ethdev to track this at all, given that it's > > > trivial for apps to track this themselves. Would we not be better just > to > > > add this tracking into testpmd and leave ethdev and drivers alone? > Perhaps > > > I'm missing something? > > > > > > > This was my first intention but Thomas hopped in ;-) > > I was just opening the discussion :) > > > testpmd does it already via the fs->fwd_dropped stats and ovs has its > > tx_dropped stat. > > > > The problem is that all drivers have different approach about this. > > Some drivers only count real hw errors in oerrors. > > But others count the packets it can't send in oerrors (+ there are some > > cases that seem buggy to me where the driver will always refuse the mbufs > > for reason X and the application can retry indefinitely to send...). > > We have 3 options: > 1/ status quo = oerrors is inconsistent across drivers > 2/ API break = oerrors stop being incremented for temporary > unavailability (i.e. queue full, kind of ERETRY), > report only packets which will be never sent, > may be a small performance gain for some drivers > 3/ API + ABI break = same as 2/ + > report ERETRY errors in ofull (same as tx_burst() delta) > > Note that the option 2 is a light API break which does not require > any deprecation notice because the original definition of oerrors > is really vague: "failed transmitted packets" > By changing the definition of errors to "packets lost", we can count > HW errors + packets not matching requirements. > As David suggests, the packets not matching requirements can be freed > as it is done for packets successfully transmitted to the HW. > We need also to update the definition of the return value of > rte_eth_tx_burst(): "packets actually stored in transmit descriptors". > We should also count the bad packets rejected by the driver. > Then the number of bad packets would be the difference between > the return value of rte_eth_tx_burst() and opackets counter. > This solution is fixing some bugs and enforce a consistent behaviour. > I am also for option 2 especially because of this. A driver that refuses a packet for reason X (which is a limitation, or an incorrect config or whatever that is not a transient condition) but gives it back to the application is a bad driver. > The option 3 is breaking the ABI and may degrade the performances. > The only benefit is convenience or semantic: ofull would be the > equivalent of imissed. > The application can
[dpdk-dev] [PATCH 1/1] net/ice: faster bit check
From: Jesse Brandeburg
Implement a slightly faster bit check, used for checking descriptors in the hot
path.
Signed-off-by: Jesse Brandeburg
Signed-off-by: Paul M Stillwell Jr
---
drivers/net/ice/ice_rxtx.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index c794ee8..7c82931 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -955,14 +955,13 @@
static inline uint64_t
ice_rxd_status_to_pkt_flags(uint64_t qword)
{
- uint64_t flags;
-
- /* Check if RSS_HASH */
- flags = (((qword >> ICE_RX_DESC_STATUS_FLTSTAT_S) &
- ICE_RX_DESC_FLTSTAT_RSS_HASH) ==
-ICE_RX_DESC_FLTSTAT_RSS_HASH) ? PKT_RX_RSS_HASH : 0;
-
- return flags;
+ static const uint64_t bitcheck =
+ (ICE_RX_DESC_FLTSTAT_RSS_HASH << ICE_RX_DESC_STATUS_FLTSTAT_S);
+ /* Check if RSS_HASH */
+ if ((qword & bitcheck) == bitcheck)
+ return PKT_RX_RSS_HASH;
+
+ return 0;
}
/* Rx L3/L4 checksum */
--
1.8.3.1
[dpdk-dev] [PATCH v2] net/ice: faster bit check
From: Jesse Brandeburg
Implement a slightly faster bit check, used for checking
descriptors in the hot path.
Signed-off-by: Jesse Brandeburg
Signed-off-by: Paul M Stillwell Jr
---
v2: fixed checkpatch issues
---
drivers/net/ice/ice_rxtx.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index c794ee8..7c82931 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -955,14 +955,13 @@
static inline uint64_t
ice_rxd_status_to_pkt_flags(uint64_t qword)
{
- uint64_t flags;
-
- /* Check if RSS_HASH */
- flags = (((qword >> ICE_RX_DESC_STATUS_FLTSTAT_S) &
- ICE_RX_DESC_FLTSTAT_RSS_HASH) ==
-ICE_RX_DESC_FLTSTAT_RSS_HASH) ? PKT_RX_RSS_HASH : 0;
-
- return flags;
+ static const uint64_t bitcheck =
+ (ICE_RX_DESC_FLTSTAT_RSS_HASH << ICE_RX_DESC_STATUS_FLTSTAT_S);
+ /* Check if RSS_HASH */
+ if ((qword & bitcheck) == bitcheck)
+ return PKT_RX_RSS_HASH;
+
+ return 0;
}
/* Rx L3/L4 checksum */
--
1.8.3.1
[dpdk-dev] [PATCH v3] net/ice: faster bit check
From: Jesse Brandeburg
Implement a slightly faster bit check, used for checking
descriptors in the hot path.
Signed-off-by: Jesse Brandeburg
Signed-off-by: Paul M Stillwell Jr
---
v3: really fix checkpatch issues
v2: fixed checkpatch issues
---
drivers/net/ice/ice_rxtx.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index c794ee8..c01fdaf 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -955,14 +955,13 @@
static inline uint64_t
ice_rxd_status_to_pkt_flags(uint64_t qword)
{
- uint64_t flags;
-
+ static const uint64_t bitcheck =
+ (ICE_RX_DESC_FLTSTAT_RSS_HASH << ICE_RX_DESC_STATUS_FLTSTAT_S);
/* Check if RSS_HASH */
- flags = (((qword >> ICE_RX_DESC_STATUS_FLTSTAT_S) &
- ICE_RX_DESC_FLTSTAT_RSS_HASH) ==
-ICE_RX_DESC_FLTSTAT_RSS_HASH) ? PKT_RX_RSS_HASH : 0;
+ if ((qword & bitcheck) == bitcheck)
+ return PKT_RX_RSS_HASH;
- return flags;
+ return 0;
}
/* Rx L3/L4 checksum */
--
1.8.3.1
Re: [dpdk-dev] [PATCH 3/5] app/testpmd: add missing transmit errors stats
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of David Marchand > Sent: Friday, February 15, 2019 5:32 PM > To: Thomas Monjalon > Cc: Richardson, Bruce ; dev@dpdk.org; Lu, Wenzhuo > ; Wu, Jingjing > ; Iremonger, Bernard ; > Maxime Coquelin ; Yigit, Ferruh > ; Andrew Rybchenko ; > Wiles, Keith > Subject: Re: [dpdk-dev] [PATCH 3/5] app/testpmd: add missing transmit errors > stats > > On Fri, Feb 15, 2019 at 5:19 PM Thomas Monjalon wrote: > > > 15/02/2019 16:04, David Marchand: > > > On Fri, Feb 15, 2019 at 3:05 PM Bruce Richardson < > > bruce.richard...@intel.com> > > > wrote: > > > > > > > On Fri, Feb 15, 2019 at 10:33:47AM +0100, David Marchand wrote: > > > > >On Fri, Feb 15, 2019 at 9:58 AM Thomas Monjalon > > > > ><[1]tho...@monjalon.net> wrote: > > > > > > > > > > 14/02/2019 19:51, David Marchand: > > > > > > What is the purpose of oerrors ? > > > > > > > > > > > > Since the drivers (via rte_eth_tx_burst return value) report > > the > > > > > numbers of > > > > > > packets successfully transmitted, the application can try to > > > > > retransmit the > > > > > > packets that did not make it and counts this. > > > > > > If the driver counts such "missed" packets, then it does the > > job > > > > > the > > > > > > application will do anyway (wasting some cycles). > > > > > > But what is more a problem is that the application does not > > know > > > > > if the > > > > > > packets in oerrors are its own retries or problems that the > > driver > > > > > can not > > > > > > detect (hw problems) but the hw can. > > > > > > > > > > > > So the best option is that oerrors does not report the > > packets the > > > > > driver > > > > > > refuses (and I can see some drivers that do not comply to > > this) > > > > > but only > > > > > > "external" errors from the driver pov. > > > > > I can see the benefit of having driver errors in the stats, > > > > > so it is generically stored for later analysis or print. > > > > > It could be managed at ethdev level instead of the application > > > > > doing the computation. > > > > > What about splitting the Tx errors in 2 fields? oerrors / ofull > > ? > > > > > Who said it's awful? sorry Bruce for anticipating ;) > > > > > > > > > >Summary, correct me if we are not aligned :-) > > > > >- ofull (maybe ofifoerrors?) is actually a count of SW failed > > > > transmits > > > > >- it would be handled in rte_eth_tx_burst() itself in a generic > > way > > > > >- the drivers do not need to track such SW failed transmits > > > > >- oerrors only counts packets HW failed transmits, dropped out of > > the > > > > >driver tx_pkt_burst() knowledge > > > > >- the application does not have to track SW failed transmits > > since the > > > > >stats is in ethdev > > > > >It sounds good to me, this means an ethdev abi breakage. > > > > > > > > Hang on, why do we need ethdev to track this at all, given that it's > > > > trivial for apps to track this themselves. Would we not be better just > > to > > > > add this tracking into testpmd and leave ethdev and drivers alone? > > Perhaps > > > > I'm missing something? > > > > > > > > > > This was my first intention but Thomas hopped in ;-) > > > > I was just opening the discussion :) > > > > > testpmd does it already via the fs->fwd_dropped stats and ovs has its > > > tx_dropped stat. > > > > > > The problem is that all drivers have different approach about this. > > > Some drivers only count real hw errors in oerrors. > > > But others count the packets it can't send in oerrors (+ there are some > > > cases that seem buggy to me where the driver will always refuse the mbufs > > > for reason X and the application can retry indefinitely to send...). > > > > We have 3 options: > > 1/ status quo = oerrors is inconsistent across drivers > > 2/ API break = oerrors stop being incremented for temporary > > unavailability (i.e. queue full, kind of ERETRY), > > report only packets which will be never sent, > > may be a small performance gain for some drivers > > 3/ API + ABI break = same as 2/ + > > report ERETRY errors in ofull (same as tx_burst() delta) > > > > Note that the option 2 is a light API break which does not require > > any deprecation notice because the original definition of oerrors > > is really vague: "failed transmitted packets" > > By changing the definition of errors to "packets lost", we can count > > HW errors + packets not matching requirements. > > As David suggests, the packets not matching requirements can be freed > > as it is done for packets successfully transmitted to the HW. > > We need also to update the definition of the return value of > > rte_eth_tx_burst(): "packets actually stored in transmit descriptors". > > We should also count the bad packets rejected by the driver. >
Re: [dpdk-dev] [PATCH 3/5] app/testpmd: add missing transmit errors stats
On Fri, Feb 15, 2019 at 7:15 PM Ananyev, Konstantin < konstantin.anan...@intel.com> wrote: > > -Original Message- > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of David Marchand > > I am also for option 2 especially because of this. > > A driver that refuses a packet for reason X (which is a limitation, or an > > incorrect config or whatever that is not a transient condition) but gives > > it back to the application is a bad driver. > > Why? What.s wrong to leave it to the upper layer to decide what to > do with the packets that can't be sent (by one reason or another)? > How does the upper layer know if this is a transient state or something that can't be resolved? -- David Marchand
Re: [dpdk-dev] [PATCH 3/5] app/testpmd: add missing transmit errors stats
>>> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of David Marchand >>> I am also for option 2 especially because of this. >>> A driver that refuses a packet for reason X (which is a limitation, or an >>> incorrect config or whatever that is not a transient condition) but gives >>> it back to the application is a bad driver. >>Why? What.s wrong to leave it to the upper layer to decide what to >>do with the packets that can't be sent (by one reason or another)? >How does the upper layer know if this is a transient state or something that >can't be resolved? Via rte_errno, for example.
Re: [dpdk-dev] [PATCH 3/5] app/testpmd: add missing transmit errors stats
15/02/2019 19:42, Ananyev, Konstantin: > >>> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of David Marchand > >>> I am also for option 2 especially because of this. > >>> A driver that refuses a packet for reason X (which is a limitation, or an > >>> incorrect config or whatever that is not a transient condition) but gives > >>> it back to the application is a bad driver. > > >>Why? What.s wrong to leave it to the upper layer to decide what to > >>do with the packets that can't be sent (by one reason or another)? > > >How does the upper layer know if this is a transient state or something that > >can't be resolved? > > Via rte_errno, for example. rte_errno is not a result per packet. I think it is better to "eat" the packet as it is done for those transmitted to the HW.
Re: [dpdk-dev] [PATCH 3/5] app/testpmd: add missing transmit errors stats
On Fri, 15 Feb 2019 20:38:59 +0100 Thomas Monjalon wrote: > 15/02/2019 19:42, Ananyev, Konstantin: > > >>> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of David Marchand > > >>> I am also for option 2 especially because of this. > > >>> A driver that refuses a packet for reason X (which is a limitation, or > > >>> an > > >>> incorrect config or whatever that is not a transient condition) but > > >>> gives > > >>> it back to the application is a bad driver. > > > > >>Why? What.s wrong to leave it to the upper layer to decide what to > > >>do with the packets that can't be sent (by one reason or another)? > > > > >How does the upper layer know if this is a transient state or something > > >that can't be resolved? > > > > Via rte_errno, for example. > > rte_errno is not a result per packet. > I think it is better to "eat" the packet > as it is done for those transmitted to the HW. > > First off rte_errno doesn't work for a burst API. IMHO (which matches /2) all drivers should only increment oerrors for something for a packet which it could not transmit because of hardware condition (link down etc) or mbuf which has parameters which can not be handled. In either case, the packet must be dropped by driver and oerrors incremented. The driver should also maintain internal stats (available by xstats) for any conditions like this. When no tx descriptors are available, the driver must not increment any counter and return partial success to the application. If application then wants to do back pressure or drop it should keep its own statistics. This is close to the original model in the Intel drivers, and matches what BSD and Linux do on the OS level for drivers. Like many driver assumptions the corner cases were not explicitly documented and new drivers probably don't follow the same pattern.
[dpdk-dev] [PATCH] app/testpmd: fix crash when doing port info of vdev
From: Stephen Hemminger
Noticed a SEGV in testpmd doing:
> show port info 1
on Hyper-V with failsafe/tap PMD.
A vdev may not have an associated device (i.e NULL) and therefore
testpmd should skip devargs in that case.
Fixes: cf72ed09181b ("app/testpmd: display devargs in port info output")
Signed-off-by: Stephen Hemminger
---
app/test-pmd/config.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index b9e5dd923b0f..38708db943d2 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -415,7 +415,8 @@ port_infos_display(portid_t port_id)
rte_eth_dev_get_name_by_port(port_id, name);
printf("\nDevice name: %s", name);
printf("\nDriver name: %s", dev_info.driver_name);
- if (dev_info.device->devargs && dev_info.device->devargs->args)
+ if (dev_info.device && dev_info.device->devargs &&
+ dev_info.device->devargs->args)
printf("\nDevargs: %s", dev_info.device->devargs->args);
printf("\nConnect to socket: %u", port->socket_id);
--
2.17.1
[dpdk-dev] [PATCH v2] mem: poison memory when freed
DPDK malloc library allows broken programs to work because
the semantics of zmalloc and malloc are the same.
This patch enables a more secure model which will catch
(and crash) programs that reuse memory already freed if
RTE_MALLOC_DEBUG is enabled.
Signed-off-by: Stephen Hemminger
Acked-by: Andrew Rybchenko
Reviewed-by: Anatoly Burakov
---
v2 -- fix #ifdef to get correct semantics, and add more comments
lib/librte_eal/common/malloc_elem.c | 18 +++---
lib/librte_eal/common/rte_malloc.c | 13 -
2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/lib/librte_eal/common/malloc_elem.c
b/lib/librte_eal/common/malloc_elem.c
index d54a528ed653..658c9b5b7993 100644
--- a/lib/librte_eal/common/malloc_elem.c
+++ b/lib/librte_eal/common/malloc_elem.c
@@ -23,6 +23,17 @@
#include "malloc_elem.h"
#include "malloc_heap.h"
+/*
+ * If debugging is enabled, freed memory is set to poison value
+ * to catch buggy programs. Otherwise, freed memory is set to zero
+ * to avoid having to zero in zmalloc
+ */
+#ifdef RTE_MALLOC_DEBUG
+#define MALLOC_POISON 0x6b
+#else
+#define MALLOC_POISON 0
+#endif
+
size_t
malloc_elem_find_max_iova_contig(struct malloc_elem *elem, size_t align)
{
@@ -494,7 +505,7 @@ malloc_elem_join_adjacent_free(struct malloc_elem *elem)
join_elem(elem, elem->next);
/* erase header, trailer and pad */
- memset(erase, 0, erase_len);
+ memset(erase, MALLOC_POISON, erase_len);
}
/*
@@ -518,7 +529,7 @@ malloc_elem_join_adjacent_free(struct malloc_elem *elem)
join_elem(new_elem, elem);
/* erase header, trailer and pad */
- memset(erase, 0, erase_len);
+ memset(erase, MALLOC_POISON, erase_len);
elem = new_elem;
}
@@ -549,7 +560,8 @@ malloc_elem_free(struct malloc_elem *elem)
/* decrease heap's count of allocated elements */
elem->heap->alloc_count--;
- memset(ptr, 0, data_len);
+ /* poison memory */
+ memset(ptr, MALLOC_POISON, data_len);
return elem;
}
diff --git a/lib/librte_eal/common/rte_malloc.c
b/lib/librte_eal/common/rte_malloc.c
index b39de3c99e58..9db8ded73bfc 100644
--- a/lib/librte_eal/common/rte_malloc.c
+++ b/lib/librte_eal/common/rte_malloc.c
@@ -74,7 +74,18 @@ rte_malloc(const char *type, size_t size, unsigned align)
void *
rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
{
- return rte_malloc_socket(type, size, align, socket);
+ void *ptr = rte_malloc_socket(type, size, align, socket);
+
+#ifdef RTE_MALLOC_DEBUG
+ /*
+* If DEBUG is enabled, then freed memory is marked with poison
+* value and set to zero on allocation.
+* If DEBUG is not enabled then memory is already zeroed.
+*/
+ if (ptr != NULL)
+ memset(ptr, 0, size);
+#endif
+ return ptr;
}
/*
--
2.17.1
