From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id A30A21B57B for ; 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 , 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 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 > --- > 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: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 939F7A00E6 for ; 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 ; 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 , 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 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > --- > 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 */ >