On 3/27/20 1:18 PM, Michal Krawczyk wrote:
> Some ENA devices can pass to the driver descriptor with length 0. To
> avoid extra allocation, the descriptor can be reused by simply putting
> it back to the device.
>
> Signed-off-by: Michal Krawczyk <m...@semihalf.com>
> Reviewed-by: Igor Chauskin <igo...@amazon.com>
> Reviewed-by: Guy Tzalik <gtza...@amazon.com>
> ---
> drivers/net/ena/ena_ethdev.c | 74 ++++++++++++++++++++++++++++--------
> 1 file changed, 59 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index 54bd2760e5..9b95772e9e 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -191,6 +191,8 @@ static struct rte_mbuf *ena_rx_mbuf(struct ena_ring
> *rx_ring,
> uint8_t offset);
> static uint16_t eth_ena_recv_pkts(void *rx_queue,
> struct rte_mbuf **rx_pkts, uint16_t nb_pkts);
> +static int ena_add_single_rx_desc(struct ena_com_io_sq *io_sq,
> + struct rte_mbuf *mbuf, uint16_t id);
> static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count);
> static void ena_init_rings(struct ena_adapter *adapter,
> bool disable_meta_caching);
> @@ -1403,6 +1405,24 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
> return 0;
> }
>
> +static int ena_add_single_rx_desc(struct ena_com_io_sq *io_sq,
> + struct rte_mbuf *mbuf, uint16_t id)
> +{
> + struct ena_com_buf ebuf;
> + int rc;
> +
> + /* prepare physical address for DMA transaction */
> + ebuf.paddr = mbuf->buf_iova + RTE_PKTMBUF_HEADROOM;
> + ebuf.len = mbuf->buf_len - RTE_PKTMBUF_HEADROOM;
> +
> + /* pass resource to device */
> + rc = ena_com_add_single_rx_desc(io_sq, &ebuf, id);
> + if (unlikely(rc))
DPDK style says to compare to 0 [1] and [2].
[1] https://doc.dpdk.org/guides/contributing
/coding_style.html#function-calls
[2] https://doc.dpdk.org/guides/contributing/coding_style.html#null-pointers
> + PMD_DRV_LOG(WARNING, "failed adding rx desc\n");
> +
> + return rc;
> +}
> +
> static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
> {
> unsigned int i;
> @@ -1430,7 +1450,6 @@ static int ena_populate_rx_queue(struct ena_ring *rxq,
> unsigned int count)
>
> for (i = 0; i < count; i++) {
> struct rte_mbuf *mbuf = mbufs[i];
> - struct ena_com_buf ebuf;
> struct ena_rx_buffer *rx_info;
>
> if (likely((i + 4) < count))
> @@ -1443,16 +1462,10 @@ static int ena_populate_rx_queue(struct ena_ring
> *rxq, unsigned int count)
>
> rx_info = &rxq->rx_buffer_info[req_id];
>
> - /* prepare physical address for DMA transaction */
> - ebuf.paddr = mbuf->buf_iova + RTE_PKTMBUF_HEADROOM;
> - ebuf.len = mbuf->buf_len - RTE_PKTMBUF_HEADROOM;
> - /* pass resource to device */
> - rc = ena_com_add_single_rx_desc(rxq->ena_com_io_sq,
> - &ebuf, req_id);
> - if (unlikely(rc)) {
> - PMD_DRV_LOG(WARNING, "failed adding rx desc\n");
> + rc = ena_add_single_rx_desc(rxq->ena_com_io_sq, mbuf, req_id);
> + if (unlikely(rc))
same here
> break;
> - }
> +
> rx_info->mbuf = mbuf;
> next_to_use = ENA_IDX_NEXT_MASKED(next_to_use, rxq->size_mask);
> }
> @@ -2105,13 +2118,44 @@ static struct rte_mbuf *ena_rx_mbuf(struct ena_ring
> *rx_ring,
> rx_info = &rx_ring->rx_buffer_info[req_id];
> RTE_ASSERT(rx_info->mbuf != NULL);
>
> - /* Create an mbuf chain. */
> - mbuf->next = rx_info->mbuf;
> - mbuf = mbuf->next;
> + if (unlikely(len == 0)) {
> + /*
> + * Some devices can pass descriptor with the length 0.
> + * To avoid confusion, the PMD is simply putting the
> + * descriptor back, as it was never used. We'll avoid
> + * mbuf allocation that way.
> + */
> + int rc = ena_add_single_rx_desc(rx_ring->ena_com_io_sq,
> + rx_info->mbuf, req_id);
> + if (unlikely(rc)) {
same here
> + /* Free the mbuf in case of an error. */
> + rte_mbuf_raw_free(rx_info->mbuf);
> + } else {
> + /*
> + * If there was no error, just exit the loop as
> + * 0 length descriptor is always the last one.
> + */
> + break;
> + }
> + } else {
> + /* Create an mbuf chain. */
> + mbuf->next = rx_info->mbuf;
> + mbuf = mbuf->next;
>
> - ena_init_rx_mbuf(mbuf, len);
> - mbuf_head->pkt_len += len;
> + ena_init_rx_mbuf(mbuf, len);
> + mbuf_head->pkt_len += len;
> + }
>
> + /*
> + * Mark the descriptor as depleted and perform necessary
> + * cleanup.
> + * This code will execute in two cases:
> + * 1. Descriptor len was greater than 0 - normal situation.
> + * 2. Descriptor len was 0 and we failed to add the descriptor
> + * to the device. In that situation, we should try to add
> + * the mbuf again in the populate routine and mark the
> + * descriptor as used up by the device.
> + */
> rx_info->mbuf = NULL;
> rx_ring->empty_rx_reqs[ntc] = req_id;
> ntc = ENA_IDX_NEXT_MASKED(ntc, rx_ring->size_mask);
>
