[dpdk-dev] [PATCH v2 0/4] Ena PMD fixes

[dpdk-dev] [PATCH v2 0/4] Ena PMD fixes

[Marcin Wojtas]

Hi,

I sent second version of a patchset with various fixes for ena PMD.
All remarks after v1 review have been taken into account. Details
can be found in the changelog below.

We are looking forward to any comments or remarks.

Best regards,
Marcin

Changelog:
v1 -> v2
  * 1/4:
    - Part of the changes related to the allocation of wrong amount of Rx 
      descriptors from patch 2 were moved to patch 1
  * 2/4:
    - Remove additional variable desc_to_refill
    - Update desc_to_use after reading all descriptors - significant diff
      reduction
    - Update information why next_to_use must be assigned before call of the
      ena_rx_populate()
    - Update commit message to reflect recent changes
  * 4/4:
    - Check for the type of the packet before doing further investigation in TSO

Michal Krawczyk (4):
  net/ena: fix incorrect Rx descriptors allocation
  net/ena: fix delayed cleanup of Rx descriptors
  net/ena: cleanup if refilling of rx descriptors fails
  net/ena: calculate partial checksum if DF bit is disabled

 drivers/net/ena/ena_ethdev.c | 52 ++++++++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 19 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v2 1/4] net/ena: fix incorrect Rx descriptors allocation

[Marcin Wojtas]

From: Michal Krawczyk <mk@semihalf.com>

When application tried to allocate 1024 descriptors, device was not
initializing properly.

This patch solves it by avoiding allocation of all descriptors in the
ring in one attempt. At least one descriptor must remain unused in the
HW ring.

Fixes: 1173fca25af9 ("ena: add polling-mode driver")

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index b5e6db6..90cf2ad 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -919,7 +919,7 @@ static int ena_start(struct rte_eth_dev *dev)
 
 static int ena_queue_restart(struct ena_ring *ring)
 {
-	int rc;
+	int rc, bufs_num;
 
 	ena_assert_msg(ring->configured == 1,
 		       "Trying to restart unconfigured queue\n");
@@ -930,8 +930,9 @@ static int ena_queue_restart(struct ena_ring *ring)
 	if (ring->type == ENA_RING_TYPE_TX)
 		return 0;
 
-	rc = ena_populate_rx_queue(ring, ring->ring_size);
-	if ((unsigned int)rc != ring->ring_size) {
+	bufs_num = ring->ring_size - 1;
+	rc = ena_populate_rx_queue(ring, bufs_num);
+	if (rc != bufs_num) {
 		PMD_INIT_LOG(ERR, "Failed to populate rx ring !");
 		return (-1);
 	}
@@ -1143,7 +1144,7 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 		return 0;
 
 	in_use = rxq->next_to_use - rxq->next_to_clean;
-	ena_assert_msg(((in_use + count) <= ring_size), "bad ring state");
+	ena_assert_msg(((in_use + count) < ring_size), "bad ring state");
 
 	count = RTE_MIN(count,
 			(uint16_t)(ring_size - (next_to_use & ring_mask)));
@@ -1574,6 +1575,7 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		recv_idx++;
 	}
 
+	desc_in_use += 1;
 	/* Burst refill to save doorbells, memory barriers, const interval */
 	if (ring_size - desc_in_use > ENA_RING_DESCS_RATIO(ring_size))
 		ena_populate_rx_queue(rx_ring, ring_size - desc_in_use);
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 2/4] net/ena: fix delayed cleanup of Rx descriptors

[Marcin Wojtas]

From: Michal Krawczyk <mk@semihalf.com>

On RX path, after receiving bunch of packets, variable tracking
available descriptors in HW queue was not updated.

To fix this issue, variable tracking used descriptors must be updated
after receiving packets - it must be reduced by the amount of received
descriptors in current batch.

Additionally, variable next_to_clean in rx_ring must be updated before
entering ena_populate_rx_queue() to keep it up to date with the current
ring state.

Fixes: 1daff5260ff8 ("net/ena: use unmasked head and tail")

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 90cf2ad..b4c713f 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1575,13 +1575,13 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		recv_idx++;
 	}
 
-	desc_in_use += 1;
+	rx_ring->next_to_clean = next_to_clean;
+
+	desc_in_use = desc_in_use - completed + 1;
 	/* Burst refill to save doorbells, memory barriers, const interval */
 	if (ring_size - desc_in_use > ENA_RING_DESCS_RATIO(ring_size))
 		ena_populate_rx_queue(rx_ring, ring_size - desc_in_use);
 
-	rx_ring->next_to_clean = next_to_clean;
-
 	return recv_idx;
 }
 
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 3/4] net/ena: cleanup if refilling of rx descriptors fails

[Marcin Wojtas]

From: Michal Krawczyk <mk@semihalf.com>

If wrong number of descriptors for refilling was passed to the Rx
repopulate function, there was memory leak which caused memory pool to
run out of resources in longer go.

In case of fail when refilling Rx descriptors, all additional mbufs
have to be released.

Fixes: 1173fca25af9 ("ena: add polling-mode driver")

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index b4c713f..e6e889b 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1172,6 +1172,8 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 		rc = ena_com_add_single_rx_desc(rxq->ena_com_io_sq,
 						&ebuf, next_to_use_masked);
 		if (unlikely(rc)) {
+			rte_mempool_put_bulk(rxq->mb_pool, (void **)(&mbuf),
+					     count - i);
 			RTE_LOG(WARNING, PMD, "failed adding rx desc\n");
 			break;
 		}
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 4/4] net/ena: calculate partial checksum if DF bit is disabled

[Marcin Wojtas]

From: Michal Krawczyk <mk@semihalf.com>

When TSO is disabled we still have to calculate partial checksum if DF bit
if turned off. This is caused by firmware bug.

First of all, we must make sure that we are dealing with IPV4 packet.
If not, we will just skip further checking of this packet and move to
the next one.

If application will not set m2_len field, we assume we that it was Ethernet
frame because we have to look inside the packet to check for the DF flag.
To make it work properly, PMD is assuming that before sending
packet application called function rte_eth_tx_prepare().

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index e6e889b..3ba9901 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1599,14 +1599,33 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	uint64_t ol_flags;
 	uint16_t frag_field;
 
-	/* ENA needs partial checksum for TSO packets only, skip early */
-	if (!tx_ring->adapter->tso4_supported)
-		return nb_pkts;
-
 	for (i = 0; i != nb_pkts; i++) {
 		m = tx_pkts[i];
 		ol_flags = m->ol_flags;
 
+		if (!(ol_flags & PKT_TX_IPV4))
+			continue;
+
+		/* If there was not L2 header length specified, assume it is
+		 * length of the ethernet header.
+		 */
+		if (unlikely(m->l2_len == 0))
+			m->l2_len = sizeof(struct ether_hdr);
+
+		ip_hdr = rte_pktmbuf_mtod_offset(m, struct ipv4_hdr *,
+						 m->l2_len);
+		frag_field = rte_be_to_cpu_16(ip_hdr->fragment_offset);
+
+		if ((frag_field & IPV4_HDR_DF_FLAG) != 0) {
+			m->packet_type |= RTE_PTYPE_L4_NONFRAG;
+
+			/* If IPv4 header has DF flag enabled and TSO support is
+			 * disabled, partial chcecksum should not be calculated.
+			 */
+			if (!tx_ring->adapter->tso4_supported)
+				continue;
+		}
+
 		if ((ol_flags & ENA_TX_OFFLOAD_NOTSUP_MASK) != 0 ||
 				(ol_flags & PKT_TX_L4_MASK) ==
 				PKT_TX_SCTP_CKSUM) {
@@ -1622,15 +1641,6 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		}
 #endif
 
-		if (!(m->ol_flags & PKT_TX_IPV4))
-			continue;
-
-		ip_hdr = rte_pktmbuf_mtod_offset(m, struct ipv4_hdr *,
-						 m->l2_len);
-		frag_field = rte_be_to_cpu_16(ip_hdr->fragment_offset);
-		if (frag_field & IPV4_HDR_DF_FLAG)
-			continue;
-
 		/* In case we are supposed to TSO and have DF not set (DF=0)
 		 * hardware must be provided with partial checksum, otherwise
 		 * it will take care of necessary calculations.
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 0/4] Ena PMD fixes

[Jakub Palider]

On Mon, Apr 10, 2017 at 4:28 PM, Marcin Wojtas <mw@semihalf.com> wrote:

> Hi,
>
> I sent second version of a patchset with various fixes for ena PMD.
> All remarks after v1 review have been taken into account. Details
> can be found in the changelog below.
>
> We are looking forward to any comments or remarks.
>
> Best regards,
> Marcin
>
> Changelog:
> v1 -> v2
>   * 1/4:
>     - Part of the changes related to the allocation of wrong amount of Rx
>       descriptors from patch 2 were moved to patch 1
>   * 2/4:
>     - Remove additional variable desc_to_refill
>     - Update desc_to_use after reading all descriptors - significant diff
>       reduction
>     - Update information why next_to_use must be assigned before call of
> the
>       ena_rx_populate()
>     - Update commit message to reflect recent changes
>   * 4/4:
>     - Check for the type of the packet before doing further investigation
> in TSO
>
> Michal Krawczyk (4):
>   net/ena: fix incorrect Rx descriptors allocation
>   net/ena: fix delayed cleanup of Rx descriptors
>   net/ena: cleanup if refilling of rx descriptors fails
>   net/ena: calculate partial checksum if DF bit is disabled
>
>  drivers/net/ena/ena_ethdev.c | 52 ++++++++++++++++++++++++++++--
> --------------
>  1 file changed, 33 insertions(+), 19 deletions(-)
>
> --
> 1.8.3.1
>
>
​Reviewed-by: Jakub Palider <jpalider@gmail.com>
​

Re: [dpdk-dev] [PATCH v2 0/4] Ena PMD fixes

[Ferruh Yigit]

On 4/10/2017 11:20 PM, Jakub Palider wrote:
> On Mon, Apr 10, 2017 at 4:28 PM, Marcin Wojtas <mw@semihalf.com> wrote:

<...>
>> Michal Krawczyk (4):
>>   net/ena: fix incorrect Rx descriptors allocation
>>   net/ena: fix delayed cleanup of Rx descriptors
>>   net/ena: cleanup if refilling of rx descriptors fails
>>   net/ena: calculate partial checksum if DF bit is disabled

> ​Reviewed-by: Jakub Palider <jpalider@gmail.com>

Acked-by: Jan Medala <jan.medala@outlook.com>

Series applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH v2 0/4] Ena PMD fixes

[Jan M?dala]

Acked-by: Jan Medala <jan.medala@outlook.com>

Jan