DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Roger Melton (rmelton)" <rmelton@cisco.com>
To: "Chauskin, Igor" <igorch@amazon.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"mw@semihalf.com" <mw@semihalf.com>,
	"mk@semihalf.com" <mk@semihalf.com>,
	"Tzalik, Guy" <gtzalik@amazon.com>
Subject: Re: [dpdk-dev] DPDK ENA PMD spurious ierrors
Date: Fri, 18 Dec 2020 18:37:29 +0000	[thread overview]
Message-ID: <5DC8CBD9-63F5-49ED-93F9-866137B7DAC4@cisco.com> (raw)
In-Reply-To: <93d7b6f154bb4e97a4dd3ffde839ca93@EX13D12EUA003.ant.amazon.com>

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@amazon.com>
Date: Friday, December 18, 2020 at 12:29 PM
To: "Roger Melton (rmelton)" <rmelton@cisco.com>, "dev@dpdk.org" <dev@dpdk.org>, "mw@semihalf.com" <mw@semihalf.com>, "mk@semihalf.com" <mk@semihalf.com>, "Tzalik, Guy" <gtzalik@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@cisco.com>
Sent: Thursday, December 17, 2020 01:57
To: dev@dpdk.org; mw@semihalf.com; mk@semihalf.com; Tzalik, Guy <gtzalik@amazon.com>; Chauskin, Igor <igorch@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

  reply	other threads:[~2020-12-18 18:37 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-16 23:57 Roger Melton (rmelton)
2020-12-18 17:28 ` Chauskin, Igor
2020-12-18 18:37   ` Roger Melton (rmelton) [this message]
2020-12-18 21:28     ` Chauskin, Igor
2020-12-19  1:11       ` Roger Melton (rmelton)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5DC8CBD9-63F5-49ED-93F9-866137B7DAC4@cisco.com \
    --to=rmelton@cisco.com \
    --cc=dev@dpdk.org \
    --cc=gtzalik@amazon.com \
    --cc=igorch@amazon.com \
    --cc=mk@semihalf.com \
    --cc=mw@semihalf.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).