[dpdk-dev] DPDK ENA PMD spurious ierrors

[Roger Melton (rmelton)]

Hi Igor,

Thanks for the reply.  By work on preparing it do you mean that you plan to:


  1.  check rxmode.offloads in ena_rx_mbuf_prepare() before setting PKT_RX_[L3,L4]_CKSUM_BAD bits in ol_flags, and
  2.  increment oerrors instead of ierrors in the TX path?

FWIW, Since we do not enable hardware checksum offloads and since our application validates L3/L4 ingress checksums, our workaround in DPDK 18.11.10 is to eliminate incrementing ierrors.  There’s no point in wasting cycles.

Regards,
Roger



From: "Chauskin, Igor" <igorch at amazon.com>
Date: Friday, December 18, 2020 at 12:29 PM
To: "Roger Melton (rmelton)" <rmelton at cisco.com>, "dev at dpdk.org" <dev at dpdk.org>, "mw at semihalf.com" <mw at semihalf.com>, "mk at semihalf.com" <mk at semihalf.com>, "Tzalik, Guy" <gtzalik at amazon.com>
Subject: RE: DPDK ENA PMD spurious ierrors

Hi Roger,

Thanks for reporting this. Your suggestion seems like a valid workaround, we’ll work on preparing it.

Regards,
Igor

From: Roger Melton (rmelton) <rmelton at cisco.com>
Sent: Thursday, December 17, 2020 01:57
To: dev at dpdk.org; mw at semihalf.com; mk at semihalf.com; Tzalik, Guy <gtzalik at amazon.com>; Chauskin, Igor <igorch at amazon.com>
Subject: [EXTERNAL] DPDK ENA PMD spurious ierrors


CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.

We are seeing issues with the DPDK 18.11 ENA PMD incrementing rx_errors stat on good packets (checksum later validated by software). We’ve tried several versions from DPDK 18.11 stable, including 18.11.9 and 18.11.10.  Looking through ENA PMD commits, I see there have been a number of rx stats improvements.  Some but not all have been back ported into DPDK 18.11 stable, some of those presumably because they depend on updates to the base HW/HAL layer.  For example,  in the latest ENA PMD driver, PKT_RX_L4_CKSUM_BAD can only be set if the base layer has set l4_csum_checked in the RX context, a feature that is not available in the DPDK 18.11 ENA base driver.

Is there a way to avoid incorrectly updating ierrors in DPDK 18.11 that does not require upgrading the base HW/HAL?  For example, if an application does not enable IPV4, UDP or TCP RX checksum offloads, would l3_csum_err or l4_csum_err ever be valid?  If not, then would it be valid to pass ena_rx_mbuf_prepare() a pointer to the adapter and check for l3/l4 checksum errors only if any of DEV_RX_OFFLOAD_*CKSUM are set in adapter->rte_eth_dev_data->dev_conf.rxmode.offloads?  From DPDK 18.11.10, if RX checksum offloads are not enabled, skip the highlighted code:

static inline void ena_rx_mbuf_prepare(struct rte_mbuf *mbuf,
                                                                       struct ena_com_rx_ctx *ena_rx_ctx)
{
                uint64_t ol_flags = 0;
                uint32_t packet_type = 0;

                if (ena_rx_ctx->l4_proto == ENA_ETH_IO_L4_PROTO_TCP)
                                packet_type |= RTE_PTYPE_L4_TCP;
                else if (ena_rx_ctx->l4_proto == ENA_ETH_IO_L4_PROTO_UDP)
                                packet_type |= RTE_PTYPE_L4_UDP;

                if (ena_rx_ctx->l3_proto == ENA_ETH_IO_L3_PROTO_IPV4)
                                packet_type |= RTE_PTYPE_L3_IPV4;
                else if (ena_rx_ctx->l3_proto == ENA_ETH_IO_L3_PROTO_IPV6)
                                packet_type |= RTE_PTYPE_L3_IPV6;

                if (unlikely(ena_rx_ctx->l4_csum_err))
                                ol_flags |= PKT_RX_L4_CKSUM_BAD;
                if (unlikely(ena_rx_ctx->l3_csum_err))
                                ol_flags |= PKT_RX_IP_CKSUM_BAD;

                mbuf->ol_flags = ol_flags;
                mbuf->packet_type = packet_type;
}

While reviewing the code, I also noticed that at the top of the tree, ierrors are incremented in the transmit path:

static int ena_check_and_linearize_mbuf(struct ena_ring *tx_ring,
                                                                                struct rte_mbuf *mbuf)
{
                struct ena_com_dev *ena_dev;
                int num_segments, header_len, rc;

--- snip ---
                rc = rte_pktmbuf_linearize(mbuf);
                if (unlikely(rc)) {
                                PMD_DRV_LOG(WARNING, "Mbuf linearize failed\n");
                                rte_atomic64_inc(&tx_ring->adapter->drv_stats->ierrors);
                                ++tx_ring->tx_stats.linearize_failed;
                                return rc;
                }

                return rc;
}

This was introduced by 7830e905b7 net/ena: expose extended stats.

Shouldn’t oerrors be incremented in this case?

Thanks in advance for your help.

Regards,
Roger Melton