From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <maxime.coquelin@redhat.com>
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
by dpdk.org (Postfix) with ESMTP id A30A21B57B
for <dev@dpdk.org>; Fri, 22 Mar 2019 09:59:19 +0100 (CET)
Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com
[10.5.11.12])
(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
(No client certificate requested)
by mx1.redhat.com (Postfix) with ESMTPS id CC97330BC643;
Fri, 22 Mar 2019 08:59:18 +0000 (UTC)
Received: from [10.36.112.59] (ovpn-112-59.ams2.redhat.com [10.36.112.59])
by smtp.corp.redhat.com (Postfix) with ESMTPS id EB3C260BE2;
Fri, 22 Mar 2019 08:59:17 +0000 (UTC)
To: Wenzhuo Lu <wenzhuo.lu@intel.com>, dev@dpdk.org
References: <1551340136-83843-1-git-send-email-wenzhuo.lu@intel.com>
<1553149569-105555-1-git-send-email-wenzhuo.lu@intel.com>
<1553149569-105555-3-git-send-email-wenzhuo.lu@intel.com>
From: Maxime Coquelin <maxime.coquelin@redhat.com>
Message-ID: <98500237-4a47-704f-f0f0-99220701a897@redhat.com>
Date: Fri, 22 Mar 2019 09:59:16 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
Thunderbird/60.5.1
MIME-Version: 1.0
In-Reply-To: <1553149569-105555-3-git-send-email-wenzhuo.lu@intel.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16
(mx1.redhat.com [10.5.110.46]); Fri, 22 Mar 2019 08:59:18 +0000 (UTC)
Subject: Re: [dpdk-dev] [PATCH v4 2/8] net/ice: add pointer for queue buffer
release
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 22 Mar 2019 08:59:20 -0000
On 3/21/19 7:26 AM, Wenzhuo Lu wrote:
> Add function pointers of buffer releasing for RX and
> TX queues, for vector functions will be added for RX
> and TX.
>
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
> drivers/net/ice/ice_rxtx.c | 24 +++++++++++++++---------
> drivers/net/ice/ice_rxtx.h | 5 +++++
> 2 files changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
> index c794ee8..d540ed1 100644
> --- a/drivers/net/ice/ice_rxtx.c
> +++ b/drivers/net/ice/ice_rxtx.c
> @@ -366,7 +366,7 @@
> PMD_DRV_LOG(ERR, "Failed to switch RX queue %u on",
> rx_queue_id);
>
> - ice_rx_queue_release_mbufs(rxq);
> + rxq->rx_rel_mbufs(rxq);
> ice_reset_rx_queue(rxq);
> return -EINVAL;
> }
> @@ -393,7 +393,7 @@
> rx_queue_id);
> return -EINVAL;
> }
> - ice_rx_queue_release_mbufs(rxq);
> + rxq->rx_rel_mbufs(rxq);
> ice_reset_rx_queue(rxq);
> dev->data->rx_queue_state[rx_queue_id] =
> RTE_ETH_QUEUE_STATE_STOPPED;
> @@ -555,7 +555,7 @@
> return -EINVAL;
> }
>
> - ice_tx_queue_release_mbufs(txq);
> + txq->tx_rel_mbufs(txq);
> ice_reset_tx_queue(txq);
> dev->data->tx_queue_state[tx_queue_id] = RTE_ETH_QUEUE_STATE_STOPPED;
>
> @@ -669,6 +669,7 @@
> ice_reset_rx_queue(rxq);
> rxq->q_set = TRUE;
> dev->data->rx_queues[queue_idx] = rxq;
> + rxq->rx_rel_mbufs = ice_rx_queue_release_mbufs;
I think it could be cleaner to have ice_rx_queue_release_mbufs() to call
the callback. So you would rename current ice_rx_queue_release_mbufs to
something else.
I would make the code more consistent IMHO, and would avoid to patch all
call sites.
>
> use_def_burst_func = ice_check_rx_burst_bulk_alloc_preconditions(rxq);
>
> @@ -701,7 +702,7 @@
> return;
> }
>
> - ice_rx_queue_release_mbufs(q);
> + q->rx_rel_mbufs(q);
> rte_free(q->sw_ring);
> rte_free(q);
> }
> @@ -866,6 +867,7 @@
> ice_reset_tx_queue(txq);
> txq->q_set = TRUE;
> dev->data->tx_queues[queue_idx] = txq;
> + txq->tx_rel_mbufs = ice_tx_queue_release_mbufs;
>
> return 0;
> }
> @@ -880,7 +882,7 @@
> return;
> }
>
> - ice_tx_queue_release_mbufs(q);
> + q->tx_rel_mbufs(q);
> rte_free(q->sw_ring);
> rte_free(q);
> }
> @@ -1552,18 +1554,22 @@
> void
> ice_clear_queues(struct rte_eth_dev *dev)
> {
> + struct ice_rx_queue *rxq;
> + struct ice_tx_queue *txq;
> uint16_t i;
>
> PMD_INIT_FUNC_TRACE();
>
> for (i = 0; i < dev->data->nb_tx_queues; i++) {
> - ice_tx_queue_release_mbufs(dev->data->tx_queues[i]);
> - ice_reset_tx_queue(dev->data->tx_queues[i]);
> + txq = dev->data->tx_queues[i];
> + txq->tx_rel_mbufs(txq);
> + ice_reset_tx_queue(txq);
> }
>
> for (i = 0; i < dev->data->nb_rx_queues; i++) {
> - ice_rx_queue_release_mbufs(dev->data->rx_queues[i]);
> - ice_reset_rx_queue(dev->data->rx_queues[i]);
> + rxq = dev->data->rx_queues[i];
> + rxq->rx_rel_mbufs(rxq);
> + ice_reset_rx_queue(rxq);
> }
> }
>
> diff --git a/drivers/net/ice/ice_rxtx.h b/drivers/net/ice/ice_rxtx.h
> index ec0e52e..78b4928 100644
> --- a/drivers/net/ice/ice_rxtx.h
> +++ b/drivers/net/ice/ice_rxtx.h
> @@ -27,6 +27,9 @@
>
> #define ICE_SUPPORT_CHAIN_NUM 5
>
> +typedef void (*ice_rx_release_mbufs_t)(struct ice_rx_queue *rxq);
> +typedef void (*ice_tx_release_mbufs_t)(struct ice_tx_queue *txq);
> +
> struct ice_rx_entry {
> struct rte_mbuf *mbuf;
> };
> @@ -61,6 +64,7 @@ struct ice_rx_queue {
> uint16_t max_pkt_len; /* Maximum packet length */
> bool q_set; /* indicate if rx queue has been configured */
> bool rx_deferred_start; /* don't start this queue in dev start */
> + ice_rx_release_mbufs_t rx_rel_mbufs;
> };
>
> struct ice_tx_entry {
> @@ -100,6 +104,7 @@ struct ice_tx_queue {
> uint16_t tx_next_rs;
> bool tx_deferred_start; /* don't start this queue in dev start */
> bool q_set; /* indicate if tx queue has been configured */
> + ice_tx_release_mbufs_t tx_rel_mbufs;
> };
>
> /* Offload features */
>
From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by dpdk.space (Postfix) with ESMTP id 939F7A00E6
for <public@inbox.dpdk.org>; Fri, 22 Mar 2019 09:59:22 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id B76C81B584;
Fri, 22 Mar 2019 09:59:20 +0100 (CET)
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
by dpdk.org (Postfix) with ESMTP id A30A21B57B
for <dev@dpdk.org>; Fri, 22 Mar 2019 09:59:19 +0100 (CET)
Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com
[10.5.11.12])
(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
(No client certificate requested)
by mx1.redhat.com (Postfix) with ESMTPS id CC97330BC643;
Fri, 22 Mar 2019 08:59:18 +0000 (UTC)
Received: from [10.36.112.59] (ovpn-112-59.ams2.redhat.com [10.36.112.59])
by smtp.corp.redhat.com (Postfix) with ESMTPS id EB3C260BE2;
Fri, 22 Mar 2019 08:59:17 +0000 (UTC)
To: Wenzhuo Lu <wenzhuo.lu@intel.com>, dev@dpdk.org
References: <1551340136-83843-1-git-send-email-wenzhuo.lu@intel.com>
<1553149569-105555-1-git-send-email-wenzhuo.lu@intel.com>
<1553149569-105555-3-git-send-email-wenzhuo.lu@intel.com>
From: Maxime Coquelin <maxime.coquelin@redhat.com>
Message-ID: <98500237-4a47-704f-f0f0-99220701a897@redhat.com>
Date: Fri, 22 Mar 2019 09:59:16 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
Thunderbird/60.5.1
MIME-Version: 1.0
In-Reply-To: <1553149569-105555-3-git-send-email-wenzhuo.lu@intel.com>
Content-Type: text/plain; charset="UTF-8"; format="flowed"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16
(mx1.redhat.com [10.5.110.46]); Fri, 22 Mar 2019 08:59:18 +0000 (UTC)
Subject: Re: [dpdk-dev] [PATCH v4 2/8] net/ice: add pointer for queue buffer
release
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Message-ID: <20190322085916.Cud0SiAOOqLQA3iijH-FmAV1OvTZ2rHElyDMLYh9EfA@z>
On 3/21/19 7:26 AM, Wenzhuo Lu wrote:
> Add function pointers of buffer releasing for RX and
> TX queues, for vector functions will be added for RX
> and TX.
>
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
> drivers/net/ice/ice_rxtx.c | 24 +++++++++++++++---------
> drivers/net/ice/ice_rxtx.h | 5 +++++
> 2 files changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
> index c794ee8..d540ed1 100644
> --- a/drivers/net/ice/ice_rxtx.c
> +++ b/drivers/net/ice/ice_rxtx.c
> @@ -366,7 +366,7 @@
> PMD_DRV_LOG(ERR, "Failed to switch RX queue %u on",
> rx_queue_id);
>
> - ice_rx_queue_release_mbufs(rxq);
> + rxq->rx_rel_mbufs(rxq);
> ice_reset_rx_queue(rxq);
> return -EINVAL;
> }
> @@ -393,7 +393,7 @@
> rx_queue_id);
> return -EINVAL;
> }
> - ice_rx_queue_release_mbufs(rxq);
> + rxq->rx_rel_mbufs(rxq);
> ice_reset_rx_queue(rxq);
> dev->data->rx_queue_state[rx_queue_id] =
> RTE_ETH_QUEUE_STATE_STOPPED;
> @@ -555,7 +555,7 @@
> return -EINVAL;
> }
>
> - ice_tx_queue_release_mbufs(txq);
> + txq->tx_rel_mbufs(txq);
> ice_reset_tx_queue(txq);
> dev->data->tx_queue_state[tx_queue_id] = RTE_ETH_QUEUE_STATE_STOPPED;
>
> @@ -669,6 +669,7 @@
> ice_reset_rx_queue(rxq);
> rxq->q_set = TRUE;
> dev->data->rx_queues[queue_idx] = rxq;
> + rxq->rx_rel_mbufs = ice_rx_queue_release_mbufs;
I think it could be cleaner to have ice_rx_queue_release_mbufs() to call
the callback. So you would rename current ice_rx_queue_release_mbufs to
something else.
I would make the code more consistent IMHO, and would avoid to patch all
call sites.
>
> use_def_burst_func = ice_check_rx_burst_bulk_alloc_preconditions(rxq);
>
> @@ -701,7 +702,7 @@
> return;
> }
>
> - ice_rx_queue_release_mbufs(q);
> + q->rx_rel_mbufs(q);
> rte_free(q->sw_ring);
> rte_free(q);
> }
> @@ -866,6 +867,7 @@
> ice_reset_tx_queue(txq);
> txq->q_set = TRUE;
> dev->data->tx_queues[queue_idx] = txq;
> + txq->tx_rel_mbufs = ice_tx_queue_release_mbufs;
>
> return 0;
> }
> @@ -880,7 +882,7 @@
> return;
> }
>
> - ice_tx_queue_release_mbufs(q);
> + q->tx_rel_mbufs(q);
> rte_free(q->sw_ring);
> rte_free(q);
> }
> @@ -1552,18 +1554,22 @@
> void
> ice_clear_queues(struct rte_eth_dev *dev)
> {
> + struct ice_rx_queue *rxq;
> + struct ice_tx_queue *txq;
> uint16_t i;
>
> PMD_INIT_FUNC_TRACE();
>
> for (i = 0; i < dev->data->nb_tx_queues; i++) {
> - ice_tx_queue_release_mbufs(dev->data->tx_queues[i]);
> - ice_reset_tx_queue(dev->data->tx_queues[i]);
> + txq = dev->data->tx_queues[i];
> + txq->tx_rel_mbufs(txq);
> + ice_reset_tx_queue(txq);
> }
>
> for (i = 0; i < dev->data->nb_rx_queues; i++) {
> - ice_rx_queue_release_mbufs(dev->data->rx_queues[i]);
> - ice_reset_rx_queue(dev->data->rx_queues[i]);
> + rxq = dev->data->rx_queues[i];
> + rxq->rx_rel_mbufs(rxq);
> + ice_reset_rx_queue(rxq);
> }
> }
>
> diff --git a/drivers/net/ice/ice_rxtx.h b/drivers/net/ice/ice_rxtx.h
> index ec0e52e..78b4928 100644
> --- a/drivers/net/ice/ice_rxtx.h
> +++ b/drivers/net/ice/ice_rxtx.h
> @@ -27,6 +27,9 @@
>
> #define ICE_SUPPORT_CHAIN_NUM 5
>
> +typedef void (*ice_rx_release_mbufs_t)(struct ice_rx_queue *rxq);
> +typedef void (*ice_tx_release_mbufs_t)(struct ice_tx_queue *txq);
> +
> struct ice_rx_entry {
> struct rte_mbuf *mbuf;
> };
> @@ -61,6 +64,7 @@ struct ice_rx_queue {
> uint16_t max_pkt_len; /* Maximum packet length */
> bool q_set; /* indicate if rx queue has been configured */
> bool rx_deferred_start; /* don't start this queue in dev start */
> + ice_rx_release_mbufs_t rx_rel_mbufs;
> };
>
> struct ice_tx_entry {
> @@ -100,6 +104,7 @@ struct ice_tx_queue {
> uint16_t tx_next_rs;
> bool tx_deferred_start; /* don't start this queue in dev start */
> bool q_set; /* indicate if tx queue has been configured */
> + ice_tx_release_mbufs_t tx_rel_mbufs;
> };
>
> /* Offload features */
>