From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 538ABA0562; Tue, 31 Mar 2020 11:45:18 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 24D1D1BFC5; Tue, 31 Mar 2020 11:45:17 +0200 (CEST) Received: from mail-ed1-f65.google.com (mail-ed1-f65.google.com [209.85.208.65]) by dpdk.org (Postfix) with ESMTP id 9114B2BCE for ; Tue, 31 Mar 2020 11:45:15 +0200 (CEST) Received: by mail-ed1-f65.google.com with SMTP id o1so6085446edv.1 for ; Tue, 31 Mar 2020 02:45:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=UYpUOS5bqQgw08zAHgRcrt8V1HfsXN4yZkhKnlDGV4g=; b=ZSbJ5IB2bh+9CraQ/I43LIaPvIP5D5iqjPSxiy2E7X1W+2WnC07IGaMPyTdKox88xt ZTxNlcsYdS4Q8qVgxBbEPppAzLu4YkF8AYZSx8CRMo8FT8e15aefGSF29aJoHOp0w9xq uqUr8gA+COV5gmdfNRq4NSDC/XYtayx9mi6CGIP7r0uEGM8FzQICRs5QKPtnstKGi6s2 NPbFhRC/N3S29wO9MSpY2+2vw7fCAVFB5jgF0S2cDMHl42Sg3cKQr6Kamxv5fjxRZslT zUkAWDdlY5MF/foYuR57BuxHuVsBMOuFbNrFX5/44AQTwptmp1XVDiwB4AX8/fWUwkom 8CXg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=UYpUOS5bqQgw08zAHgRcrt8V1HfsXN4yZkhKnlDGV4g=; b=lXDwS8+hYEZA631zw9RCzjRQ9J9MGt9lcW06BCNAmj0N2ebrrfJZau7Bju+JWCWrLL PqzaqSDzSseCTZeq3qw151+yAgx1uhp27Y5Z1/Wl2IglSHuD2IXB24+MjJo9vbHZy0HN pqs/NuvZtHMsJTzi1umq5u34irQ4j/ezZK/MtS8VbSXYwe8U7iKUM1IJh6aHys0lIaht eBDP99yLKwm5cII6fHbxwdoTLByWesvBHSbSTTKWHdvftjroQ1VcYpNZuavwXnshD+E7 b4EQ6wXWXyeE09ZFuUpAdLjURgYvcdnm0i3UWL2wiM71q2cVg5dp52vivOb8bLixottH T63w== X-Gm-Message-State: ANhLgQ3pZkH432LILq6WTos7ZhbTHPS8CaNnxnXfiH/651rXbF4zeXNY 9ytfsqudFP6zQ46KiAIT21pHu9Mx23FzeXGaqZb7cg== X-Google-Smtp-Source: ADFU+vvbGYN/3ZSplgLRYKRY4pRoUgR8EDmJlNgAvR8/onf1YP1KqggcxH+4NufLzhFh4umKpFGWXHNz4lxTmb55Hho= X-Received: by 2002:aa7:ccd4:: with SMTP id y20mr14799690edt.119.1585647915286; Tue, 31 Mar 2020 02:45:15 -0700 (PDT) MIME-Version: 1.0 References: <20200327101823.12646-1-mk@semihalf.com> <20200327101823.12646-27-mk@semihalf.com> In-Reply-To: From: =?UTF-8?Q?Micha=C5=82_Krawczyk?= Date: Tue, 31 Mar 2020 11:45:03 +0200 Message-ID: To: Andrew Rybchenko Cc: dev@dpdk.org, Marcin Wojtas , Maciej Bielski , "Tzalik, Guy" , "Schmeilin, Evgeny" , "Chauskin, Igor" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH 26/29] net/ena: reuse 0 length Rx descriptor X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" pt., 27 mar 2020 o 12:30 Andrew Rybchenko napisa=C5=82(a): > > 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 > > Reviewed-by: Igor Chauskin > > Reviewed-by: Guy Tzalik > > --- > > 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 =3D mbuf->buf_iova + RTE_PKTMBUF_HEADROOM; > > + ebuf.len =3D mbuf->buf_len - RTE_PKTMBUF_HEADROOM; > > + > > + /* pass resource to device */ > > + rc =3D 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-pointer > I will fix that. I can see that the whole driver is using invalid style for checking rc. However, I'll fix the style of the conditional checks that are touched by this patch and fix the rest of them in the future, in the separate patch. > > + 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 =3D 0; i < count; i++) { > > struct rte_mbuf *mbuf =3D 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 =3D &rxq->rx_buffer_info[req_id]; > > > > - /* prepare physical address for DMA transaction */ > > - ebuf.paddr =3D mbuf->buf_iova + RTE_PKTMBUF_HEADROOM; > > - ebuf.len =3D mbuf->buf_len - RTE_PKTMBUF_HEADROOM; > > - /* pass resource to device */ > > - rc =3D 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 =3D ena_add_single_rx_desc(rxq->ena_com_io_sq, mbuf, req_id); > > + if (unlikely(rc)) > > same here > > > break; > > - } > > + > > rx_info->mbuf =3D mbuf; > > next_to_use =3D 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 =3D &rx_ring->rx_buffer_info[req_id]; > > RTE_ASSERT(rx_info->mbuf !=3D NULL); > > > > - /* Create an mbuf chain. */ > > - mbuf->next =3D rx_info->mbuf; > > - mbuf =3D mbuf->next; > > + if (unlikely(len =3D=3D 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 =3D 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 =3D rx_info->mbuf; > > + mbuf =3D mbuf->next; > > > > - ena_init_rx_mbuf(mbuf, len); > > - mbuf_head->pkt_len +=3D len; > > + ena_init_rx_mbuf(mbuf, len); > > + mbuf_head->pkt_len +=3D 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 th= e > > + * descriptor as used up by the device. > > + */ > > rx_info->mbuf =3D NULL; > > rx_ring->empty_rx_reqs[ntc] =3D req_id; > > ntc =3D ENA_IDX_NEXT_MASKED(ntc, rx_ring->size_mask); > > >