On 1/10/2024 3:25 AM, Mingjin Ye wrote:
Implemented a Tx wrapper to perform a thorough check on mbufs,
categorizing and counting invalid cases by types for diagnostic
purposes. The count of invalid cases is accessible through xstats_get.
Also, the devarg option "mbuf_check" was introduced to configure the
diagnostic parameters to enable the appropriate diagnostic features.
supported cases: mbuf, size, segment, offload.
1. mbuf: check for corrupted mbuf.
2. size: check min/max packet length according to hw spec.
3. segment: check number of mbuf segments not exceed hw limitation.
4. offload: check any unsupported offload flag.
parameter format: "mbuf_check=<case>" or "mbuf_check=[<case1>,<case2>]"
eg: dpdk-testpmd -a 0000:81:01.0,mbuf_check=[mbuf,size] -- -i
Signed-off-by: Mingjin Ye <mingjinx...@intel.com>
---
<snip>
diff --git a/doc/guides/nics/intel_vf.rst b/doc/guides/nics/intel_vf.rst
index ce96c2e1f8..f62bb4233c 100644
--- a/doc/guides/nics/intel_vf.rst
+++ b/doc/guides/nics/intel_vf.rst
@@ -111,6 +111,17 @@ For more detail on SR-IOV, please refer to the following
documents:
by setting the ``devargs`` parameter like ``-a
18:01.0,no-poll-on-link-down=1``
when IAVF is backed by an Intel\ |reg| E810 device or an Intel\ |reg| 700
Series Ethernet device.
+ When IAVF is backed by an Intel\ |reg| E810 device or an Intel\ |reg| 700 series Ethernet devices.
This looks like a duplicate line of the one above it, so probably needs
to be removed?
+ Set the ``devargs`` parameter ``mbuf_check`` to enable TX diagnostics. For
example,
+ ``-a 18:01.0,mbuf_check=<case>`` or ``-a
18:01.0,mbuf_check=[<case1>,<case2>...]``. Also,
+ ``xstats_get`` can be used to get the error counts, which are collected in
``tx_mbuf_error_packets``
+ xstats. For example, ``testpmd> show port xstats all``. Supported cases:
+
+ * mbuf: Check for corrupted mbuf.
+ * size: Check min/max packet length according to hw spec.
+ * segment: Check number of mbuf segments not exceed hw limitation.
+ * offload: Check any unsupported offload flag.
+
<snip>
+
+ int ret = 0;
+ uint64_t *mc_flags = args;
+ char *str2 = strdup(value);
+ if (str2 == NULL)
+ return -1;
+
+ str_len = strlen(str2);
+ if (str2[0] == '[' && str2[str_len - 1] == ']') {
+ if (str_len < 3) {
+ ret = -1;
+ goto mdd_end;
+ }
+ valid_len = str_len - 2;
+ memmove(str2, str2 + 1, valid_len);
+ memset(str2 + valid_len, '\0', 2);
+ }
I would suggest adding a comment mentioning that we're removing outer
square brackets from the value. Even better would be to factor it out
into a separate function, so that code for this function is not polluted
with implementation details of brackets removal.
+ cur = strtok_r(str2, ",", &tmp);
+ while (cur != NULL) {
+ if (!strcmp(cur, "mbuf"))
+ *mc_flags |= IAVF_MBUF_CHECK_F_TX_MBUF;
+ else if (!strcmp(cur, "size"))
+ *mc_flags |= IAVF_MBUF_CHECK_F_TX_SIZE;
+ else if (!strcmp(cur, "segment"))
+ *mc_flags |= IAVF_MBUF_CHECK_F_TX_SEGMENT;
+ else if (!strcmp(cur, "offload"))
+ *mc_flags |= IAVF_MBUF_CHECK_F_TX_OFFLOAD;
+ else
+ PMD_DRV_LOG(ERR, "Unsupported mdd check type: %s", cur);
I suspect 'mdd' is an artifact of earlier versions? What does it mean,
and can the error message be made more meaningful?
+ cur = strtok_r(NULL, ",", &tmp);
+ }
+
+mdd_end:
+ free(str2);
+ return ret;
+}
+
static int iavf_parse_devargs(struct rte_eth_dev *dev)
{
struct iavf_adapter *ad =
@@ -2344,6 +2411,11 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev)
goto bail;
}
<snip>
+ }
+ }
+
+ if (pkt_error) {
+ txq->mbuf_errors++;
+ good_pkts = idx;
+ if (good_pkts == 0)
+ return 0;
+ }
+
+ return iavf_tx_pkt_burst_ops[tx_burst_type](tx_queue,
+ tx_pkts,
good_pkts);
The indentation here is a bit odd.
The above suggestions are not blocking, so overall
Acked-by: Anatoly Burakov <anatoly.bura...@intel.com>
--
Thanks,
Anatoly
