DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] DPDK ENA PMD spurious ierrors
@ 2020-12-16 23:57 Roger Melton (rmelton)
  2020-12-18 17:28 ` Chauskin, Igor
  0 siblings, 1 reply; 5+ messages in thread
From: Roger Melton (rmelton) @ 2020-12-16 23:57 UTC (permalink / raw)
  To: dev, mw, mk, gtzalik, igorch

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] DPDK ENA PMD spurious ierrors
  2020-12-16 23:57 [dpdk-dev] DPDK ENA PMD spurious ierrors Roger Melton (rmelton)
@ 2020-12-18 17:28 ` Chauskin, Igor
  2020-12-18 18:37   ` Roger Melton (rmelton)
  0 siblings, 1 reply; 5+ messages in thread
From: Chauskin, Igor @ 2020-12-18 17:28 UTC (permalink / raw)
  To: Roger Melton (rmelton), dev, mw, mk, Tzalik, Guy

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] DPDK ENA PMD spurious ierrors
  2020-12-18 17:28 ` Chauskin, Igor
@ 2020-12-18 18:37   ` Roger Melton (rmelton)
  2020-12-18 21:28     ` Chauskin, Igor
  0 siblings, 1 reply; 5+ messages in thread
From: Roger Melton (rmelton) @ 2020-12-18 18:37 UTC (permalink / raw)
  To: Chauskin, Igor, dev, mw, mk, Tzalik, Guy

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] DPDK ENA PMD spurious ierrors
  2020-12-18 18:37   ` Roger Melton (rmelton)
@ 2020-12-18 21:28     ` Chauskin, Igor
  2020-12-19  1:11       ` Roger Melton (rmelton)
  0 siblings, 1 reply; 5+ messages in thread
From: Chauskin, Igor @ 2020-12-18 21:28 UTC (permalink / raw)
  To: Roger Melton (rmelton), dev, mw, mk, Tzalik, Guy

Hi Roger,

 

We plan to address both issues.

 

Thanks,

Igor

 

From: Roger Melton (rmelton) <rmelton@cisco.com> 
Sent: Friday, December 18, 2020 20:37
To: Chauskin, Igor <igorch@amazon.com>; dev@dpdk.org; mw@semihalf.com; mk@semihalf.com; Tzalik, Guy <gtzalik@amazon.com>
Subject: RE: [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.

 

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] DPDK ENA PMD spurious ierrors
  2020-12-18 21:28     ` Chauskin, Igor
@ 2020-12-19  1:11       ` Roger Melton (rmelton)
  0 siblings, 0 replies; 5+ messages in thread
From: Roger Melton (rmelton) @ 2020-12-19  1:11 UTC (permalink / raw)
  To: Chauskin, Igor, dev, mw, mk, Tzalik, Guy

Hi Igor,

Thanks for the help.  I’ll be happy to test patches when you are ready.

Regards,
Roger


From: "Chauskin, Igor" <igorch@amazon.com>
Date: Friday, December 18, 2020 at 4: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,

We plan to address both issues.

Thanks,
Igor

From: Roger Melton (rmelton) <rmelton@cisco.com>
Sent: Friday, December 18, 2020 20:37
To: Chauskin, Igor <igorch@amazon.com>; dev@dpdk.org; mw@semihalf.com; mk@semihalf.com; Tzalik, Guy <gtzalik@amazon.com>
Subject: RE: [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.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-12-22 20:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-16 23:57 [dpdk-dev] DPDK ENA PMD spurious ierrors Roger Melton (rmelton)
2020-12-18 17:28 ` Chauskin, Igor
2020-12-18 18:37   ` Roger Melton (rmelton)
2020-12-18 21:28     ` Chauskin, Igor
2020-12-19  1:11       ` Roger Melton (rmelton)

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git