From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 05D97324D for ; Mon, 22 Jan 2018 13:25:17 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Jan 2018 04:25:16 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,396,1511856000"; d="scan'208";a="21599880" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.48]) ([10.237.220.48]) by FMSMGA003.fm.intel.com with ESMTP; 22 Jan 2018 04:25:15 -0800 To: Ajit Khaparde , dev@dpdk.org Cc: Somnath Kotur References: <20180122062046.81908-1-ajit.khaparde@broadcom.com> <20180122062046.81908-6-ajit.khaparde@broadcom.com> From: Ferruh Yigit Message-ID: <0bf4b23f-58b1-3b1e-ed6c-a4318d6c3caf@intel.com> Date: Mon, 22 Jan 2018 12:25:15 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <20180122062046.81908-6-ajit.khaparde@broadcom.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 5/5] net/bnxt: Support for rx/tx_queue_start/stop ops 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: Mon, 22 Jan 2018 12:25:18 -0000 On 1/22/2018 6:20 AM, Ajit Khaparde wrote: > Currently this is implemented entirely in the PMD as there is no explicit > support in the HW. Re-program the RSS Table without this queue on stop > and add it back to the table on start. > > Signed-off-by: Somnath Kotur > Signed-off-by: Ajit Khaparde <...> > @@ -1,4 +1,4 @@ > -/*- > +/* > * BSD LICENSE > * > * Copyright(c) Broadcom Limited. Unrelated but since I saw this, do you plan switching to the SPDX tags? <...> > @@ -321,8 +334,7 @@ static int bnxt_init_chip(struct bnxt *bp) > !RTE_ETH_DEV_SRIOV(bp->eth_dev).active) && > bp->eth_dev->data->dev_conf.intr_conf.rxq != 0) { > intr_vector = bp->eth_dev->data->nb_rx_queues; > - PMD_DRV_LOG(INFO, "%s(): intr_vector = %d\n", __func__, > - intr_vector); > + PMD_DRV_LOG(INFO, "intr_vector = %d\n", intr_vector); With new logging macro "__func__" is duplicate, why not fix them all in the patch 2/5 that introduces new macro? <...> > @@ -2969,6 +2981,49 @@ bnxt_set_eeprom_op(struct rte_eth_dev *dev, > return 0; > } > > +int bnxt_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id) > +{ > + struct bnxt *bp = (struct bnxt *)dev->data->dev_private; > + struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf; > + struct bnxt_rx_queue *rxq = bp->rx_queues[rx_queue_id]; > + struct bnxt_vnic_info *vnic = NULL; > + > + dev->data->rx_queue_state[rx_queue_id] = RTE_ETH_QUEUE_STATE_STARTED; > + rxq->rx_deferred_start = false; > + PMD_DRV_LOG(INFO, "Rx queue started %d\n", rx_queue_id); > + if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG) { > + vnic = rxq->vnic; > + if (vnic->fw_grp_ids[rx_queue_id] != INVALID_HW_RING_ID) > + return 0; > + PMD_DRV_LOG(DEBUG, "vnic = %p fw_grp_id = %d\n", > + vnic, bp->grp_info[rx_queue_id + 1].fw_grp_id); > + vnic->fw_grp_ids[rx_queue_id] = > + bp->grp_info[rx_queue_id + 1].fw_grp_id; > + return bnxt_vnic_rss_configure(bp, vnic); > + } > + > + return 0; > +} > + > +int bnxt_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rx_queue_id) > +{ > + struct bnxt *bp = (struct bnxt *)dev->data->dev_private; > + struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf; > + struct bnxt_rx_queue *rxq = bp->rx_queues[rx_queue_id]; > + struct bnxt_vnic_info *vnic = NULL; > + > + dev->data->rx_queue_state[rx_queue_id] = RTE_ETH_QUEUE_STATE_STOPPED; > + rxq->rx_deferred_start = true; > + PMD_DRV_LOG(DEBUG, "Rx queue stopped\n"); > + > + if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG) { > + vnic = rxq->vnic; > + vnic->fw_grp_ids[rx_queue_id] = INVALID_HW_RING_ID; > + return bnxt_vnic_rss_configure(bp, vnic); > + } > + return 0; > +} There is already a source file "bnxt_rxq.c", which seems for Rxq related functions, why not add new functions there? <...> > @@ -364,3 +369,30 @@ uint16_t bnxt_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, > > return nb_tx_pkts; > } > + > +int bnxt_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id) > +{ > + struct bnxt *bp = (struct bnxt *)dev->data->dev_private; > + struct bnxt_tx_queue *txq = bp->tx_queues[tx_queue_id]; > + > + dev->data->tx_queue_state[tx_queue_id] = RTE_ETH_QUEUE_STATE_STARTED; > + txq->tx_deferred_start = false; > + PMD_DRV_LOG(DEBUG, "Tx queue started\n"); > + > + return 0; > +} > + > +int bnxt_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id) > +{ > + struct bnxt *bp = (struct bnxt *)dev->data->dev_private; > + struct bnxt_tx_queue *txq = bp->tx_queues[tx_queue_id]; > + > + /* Handle TX completions */ > + bnxt_handle_tx_cp(txq); > + > + dev->data->tx_queue_state[tx_queue_id] = RTE_ETH_QUEUE_STATE_STOPPED; > + txq->tx_deferred_start = true; > + PMD_DRV_LOG(DEBUG, "Tx queue stopped\n"); > + > + return 0; > +} Similar question for these functions, they seem implemented in txr (Tx Ring ?) source file, bnxt_txq.c seems better fit, what do you think? <...>