DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Kamil Rytarowski <krytarowski@caviumnetworks.com>, dev@dpdk.org
Cc: Maciej Czekaj <maciej.czekaj@caviumnetworks.com>,
	zyta.szpak@semihalf.com, slawomir.rosek@semihalf.com,
	rad@semihalf.com, jerin.jacob@caviumnetworks.com,
	Kamil Rytarowski <kamil.rytarowski@caviumnetworks.com>
Subject: Re: [dpdk-dev] [PATCH 01/13] net/thunderx: cleanup the driver before adding new features
Date: Tue, 20 Sep 2016 14:48:27 +0100	[thread overview]
Message-ID: <568fafa1-53f3-980b-9ca4-e086980651b2@intel.com> (raw)
In-Reply-To: <1472230448-17490-2-git-send-email-krytarowski@caviumnetworks.com>

On 8/26/2016 5:53 PM, Kamil Rytarowski wrote:
> From: Kamil Rytarowski <kamil.rytarowski@caviumnetworks.com>
> 
> Refactored features:
>  - enable nicvf_qset_rbdr_precharge to handle handle secondary queue sets
double "handle"

>  - rte_free already handles NULL pointer
>  - check mempool flags to predict being contiguous in memory
>  - allow to use mempool with multiple memory chunks

I am not able to find the implementation for this.

>  - simplify local construct of accessing nb_rx_queus
s/nb_rx_queus/nb_rx_queues

> 
> Signed-off-by: Maciej Czekaj <maciej.czekaj@caviumnetworks.com>
> Signed-off-by: Kamil Rytarowski <kamil.rytarowski@caviumnetworks.com>
> Signed-off-by: Zyta Szpak <zyta.szpak@semihalf.com>
> Signed-off-by: Slawomir Rosek <slawomir.rosek@semihalf.com>
> Signed-off-by: Radoslaw Biernacki <rad@semihalf.com>
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> ---
>  drivers/net/thunderx/base/nicvf_hw.c | 10 +++++-----
>  drivers/net/thunderx/base/nicvf_hw.h |  6 +++---
>  drivers/net/thunderx/nicvf_ethdev.c  | 32 ++++++++++++--------------------
>  3 files changed, 20 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/net/thunderx/base/nicvf_hw.c b/drivers/net/thunderx/base/nicvf_hw.c
> index 4bdd183..1f08ef2 100644
> --- a/drivers/net/thunderx/base/nicvf_hw.c
> +++ b/drivers/net/thunderx/base/nicvf_hw.c
> @@ -141,7 +141,7 @@ nicvf_base_init(struct nicvf *nic)
>  		return NICVF_ERR_BASE_INIT;
>  
>  	if (nicvf_hw_version(nic) == PCI_SUB_DEVICE_ID_CN88XX_PASS2_NICVF)
> -		nic->hwcap |= NICVF_CAP_TUNNEL_PARSING;
> +		nic->hwcap |= NICVF_CAP_TUNNEL_PARSING | NICVF_CAP_CQE_RX2;

is this new flag NICVF_CAP_CQE_RX2 flag described in commit log?

>  
>  	if (nicvf_hw_version(nic) == PCI_SUB_DEVICE_ID_CN81XX_NICVF)
>  		nic->hwcap |= NICVF_CAP_TUNNEL_PARSING | NICVF_CAP_CQE_RX2;
> @@ -497,9 +497,9 @@ nicvf_qsize_rbdr_roundup(uint32_t val)
>  }
>  
>  int
> -nicvf_qset_rbdr_precharge(struct nicvf *nic, uint16_t ridx,
> -			  rbdr_pool_get_handler handler,
> -			  void *opaque, uint32_t max_buffs)
> +nicvf_qset_rbdr_precharge(void *dev, struct nicvf *nic,
> +			  uint16_t ridx, rbdr_pool_get_handler handler,
> +			  uint32_t max_buffs)
>  {
>  	struct rbdr_entry_t *desc, *desc0;
>  	struct nicvf_rbdr *rbdr = nic->rbdr;
> @@ -514,7 +514,7 @@ nicvf_qset_rbdr_precharge(struct nicvf *nic, uint16_t ridx,
>  		if (count >= max_buffs)
>  			break;
>  		desc0 = desc + count;
> -		phy = handler(opaque);
> +		phy = handler(dev, nic);
>  		if (phy) {
>  			desc0->full_addr = phy;
>  			count++;
> diff --git a/drivers/net/thunderx/base/nicvf_hw.h b/drivers/net/thunderx/base/nicvf_hw.h
> index a6cda82..2b8738b 100644
> --- a/drivers/net/thunderx/base/nicvf_hw.h
> +++ b/drivers/net/thunderx/base/nicvf_hw.h
> @@ -85,7 +85,7 @@ enum nicvf_err_e {
>  	NICVF_ERR_RSS_GET_SZ,    /* -8171 */
>  };
>  
> -typedef nicvf_phys_addr_t (*rbdr_pool_get_handler)(void *opaque);
> +typedef nicvf_phys_addr_t (*rbdr_pool_get_handler)(void *dev, void *opaque);
>  
>  struct nicvf_hw_rx_qstats {
>  	uint64_t q_rx_bytes;
> @@ -194,8 +194,8 @@ int nicvf_qset_reclaim(struct nicvf *nic);
>  
>  int nicvf_qset_rbdr_config(struct nicvf *nic, uint16_t qidx);
>  int nicvf_qset_rbdr_reclaim(struct nicvf *nic, uint16_t qidx);
> -int nicvf_qset_rbdr_precharge(struct nicvf *nic, uint16_t ridx,
> -			      rbdr_pool_get_handler handler, void *opaque,
> +int nicvf_qset_rbdr_precharge(void *dev, struct nicvf *nic,
> +			      uint16_t ridx, rbdr_pool_get_handler handler,
>  			      uint32_t max_buffs);
>  int nicvf_qset_rbdr_active(struct nicvf *nic, uint16_t qidx);
>  
> diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
> index 4402f6a..48f2cd2 100644
> --- a/drivers/net/thunderx/nicvf_ethdev.c
> +++ b/drivers/net/thunderx/nicvf_ethdev.c
> @@ -691,7 +691,7 @@ nicvf_configure_cpi(struct rte_eth_dev *dev)
>  	int ret;
>  
>  	/* Count started rx queues */
> -	for (qidx = qcnt = 0; qidx < nic->eth_dev->data->nb_rx_queues; qidx++)
> +	for (qidx = qcnt = 0; qidx < dev->data->nb_rx_queues; qidx++)
>  		if (dev->data->rx_queue_state[qidx] ==
>  		    RTE_ETH_QUEUE_STATE_STARTED)
>  			qcnt++;
> @@ -1023,12 +1023,9 @@ nicvf_stop_rx_queue(struct rte_eth_dev *dev, uint16_t qidx)
>  static void
>  nicvf_dev_rx_queue_release(void *rx_queue)
>  {
> -	struct nicvf_rxq *rxq = rx_queue;
> -
>  	PMD_INIT_FUNC_TRACE();
>  
> -	if (rxq)
> -		rte_free(rxq);
> +	rte_free(rx_queue);
>  }
>  
>  static int
> @@ -1087,9 +1084,9 @@ nicvf_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t qidx,
>  		PMD_DRV_LOG(WARNING, "socket_id expected %d, configured %d",
>  		socket_id, nic->node);
>  
> -	/* Mempool memory should be contiguous */
> -	if (mp->nb_mem_chunks != 1) {
> -		PMD_INIT_LOG(ERR, "Non contiguous mempool, check huge page sz");
> +	/* Mempool memory must be contiguous */
> +	if (mp->flags & MEMPOOL_F_NO_PHYS_CONTIG) {

If you need continuous memory, this check is not enough.
Not having this flag doesn't guaranties that memory is continuous, this
flag can be set but still can have multiple mem_chunks. And there is no
guarantee that mem_chunks are continuous.

> +		PMD_INIT_LOG(ERR, "Mempool memory must be contiguous");
>  		return -EINVAL;
>  	}
>  
> @@ -1212,15 +1209,16 @@ nicvf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>  }
>  
>  static nicvf_phys_addr_t
> -rbdr_rte_mempool_get(void *opaque)
> +rbdr_rte_mempool_get(void *dev, void *opaque)
>  {
>  	uint16_t qidx;
>  	uintptr_t mbuf;
>  	struct nicvf_rxq *rxq;
> -	struct nicvf *nic = nicvf_pmd_priv((struct rte_eth_dev *)opaque);
> +	struct rte_eth_dev *eth_dev = (struct rte_eth_dev *)dev;
> +	struct nicvf *nic __rte_unused = (struct nicvf *)opaque;
>  
> -	for (qidx = 0; qidx < nic->eth_dev->data->nb_rx_queues; qidx++) {
> -		rxq = nic->eth_dev->data->rx_queues[qidx];
> +	for (qidx = 0; qidx < eth_dev->data->nb_rx_queues; qidx++) {
> +		rxq = eth_dev->data->rx_queues[qidx];
>  		/* Maintain equal buffer count across all pools */
>  		if (rxq->precharge_cnt >= rxq->qlen_mask)
>  			continue;
> @@ -1354,8 +1352,8 @@ nicvf_dev_start(struct rte_eth_dev *dev)
>  	}
>  
>  	/* Fill rte_mempool buffers in RBDR pool and precharge it */
> -	ret = nicvf_qset_rbdr_precharge(nic, 0, rbdr_rte_mempool_get,
> -					dev, total_rxq_desc);
> +	ret = nicvf_qset_rbdr_precharge(dev, nic, 0, rbdr_rte_mempool_get,
> +					total_rxq_desc);
>  	if (ret) {
>  		PMD_INIT_LOG(ERR, "Failed to fill rbdr %d", ret);
>  		goto qset_rbdr_reclaim;
> @@ -1721,12 +1719,6 @@ nicvf_eth_dev_init(struct rte_eth_dev *eth_dev)
>  		goto malloc_fail;
>  	}
>  
> -	ret = nicvf_mbox_get_rss_size(nic);
> -	if (ret) {
> -		PMD_INIT_LOG(ERR, "Failed to get rss table size");
> -		goto malloc_fail;
> -	}
> -

Is removing mbox_get_rss_size() mentioned in commit log?

>  	PMD_INIT_LOG(INFO, "Port %d (%x:%x) mac=%02x:%02x:%02x:%02x:%02x:%02x",
>  		eth_dev->data->port_id, nic->vendor_id, nic->device_id,
>  		nic->mac_addr[0], nic->mac_addr[1], nic->mac_addr[2],
> 

  reply	other threads:[~2016-09-20 13:48 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-26 16:53 [dpdk-dev] [PATCH 00/13] Add support for secondary queue set in nicvf thunderx driver Kamil Rytarowski
2016-08-26 16:53 ` [dpdk-dev] [PATCH 01/13] net/thunderx: cleanup the driver before adding new features Kamil Rytarowski
2016-09-20 13:48   ` Ferruh Yigit [this message]
2016-09-29 14:21     ` Maciej Czekaj
2016-08-26 16:53 ` [dpdk-dev] [PATCH 02/13] net/thunderx: correct transmit checksum handling Kamil Rytarowski
2016-08-26 16:53 ` [dpdk-dev] [PATCH 03/13] net/thunderx/base: add family of functions to store qsets Kamil Rytarowski
2016-08-26 16:53 ` [dpdk-dev] [PATCH 04/13] net/thunderx/base: add secondary queue set support Kamil Rytarowski
2016-09-20 13:48   ` Ferruh Yigit
2016-09-29 14:22     ` Maciej Czekaj
2016-08-26 16:54 ` [dpdk-dev] [PATCH 05/13] net/thunderx: add family of functions to store DPDK qsets Kamil Rytarowski
2016-08-26 16:54 ` [dpdk-dev] [PATCH 06/13] net/thunderx: add secondary queue set in interrupt functions Kamil Rytarowski
2016-08-26 16:54 ` [dpdk-dev] [PATCH 07/13] net/thunderx: fix multiprocess support in stats Kamil Rytarowski
2016-09-20 13:48   ` Ferruh Yigit
2016-09-29 14:35     ` Maciej Czekaj
2016-08-26 16:54 ` [dpdk-dev] [PATCH 08/13] net/thunderx: add helper utils for secondary qset support Kamil Rytarowski
2016-08-26 16:54 ` [dpdk-dev] [PATCH 09/13] net/thunderx: add secondary qset support in dev stop/close Kamil Rytarowski
2016-08-26 16:54 ` [dpdk-dev] [PATCH 10/13] net/thunderx: add secondary qset support in device start Kamil Rytarowski
2016-08-26 16:54 ` [dpdk-dev] [PATCH 11/13] net/thunderx: add secondary qset support in device configure Kamil Rytarowski
2016-08-26 16:54 ` [dpdk-dev] [PATCH 12/13] net/thunderx: add final bits for secondary queue support Kamil Rytarowski
2016-09-20 13:49   ` Ferruh Yigit
2016-09-29 14:37     ` Maciej Czekaj
2016-08-26 16:54 ` [dpdk-dev] [PATCH 13/13] net/thunderx: document secondary queue set support Kamil Rytarowski
2016-09-26 20:17   ` Mcnamara, John
2016-09-29 14:38     ` Maciej Czekaj
2016-09-12 10:59 ` [dpdk-dev] [PATCH 00/13] Add support for secondary queue set in nicvf thunderx driver Kamil Rytarowski
2016-09-19 12:23   ` Kamil Rytarowski
2016-09-30 12:05 ` [dpdk-dev] [PATCH v2 00/15] " Kamil Rytarowski
2016-09-30 12:05   ` [dpdk-dev] [PATCH v2 01/15] net/thunderx: cleanup the driver before adding new features Kamil Rytarowski
2016-09-30 12:05   ` [dpdk-dev] [PATCH v2 02/15] net/thunderx: correct transmit checksum handling Kamil Rytarowski
2016-09-30 12:05   ` [dpdk-dev] [PATCH v2 03/15] net/thunderx/base: add family of functions to store qsets Kamil Rytarowski
2016-09-30 12:05   ` [dpdk-dev] [PATCH v2 04/15] net/thunderx/base: add secondary queue set support Kamil Rytarowski
2016-09-30 12:05   ` [dpdk-dev] [PATCH v2 05/15] net/thunderx: add family of functions to store DPDK qsets Kamil Rytarowski
2016-09-30 12:05   ` [dpdk-dev] [PATCH v2 06/15] net/thunderx: add secondary queue set in interrupt functions Kamil Rytarowski
2016-09-30 12:05   ` [dpdk-dev] [PATCH v2 07/15] net/thunderx: remove problematic private_data->eth_dev link Kamil Rytarowski
2016-09-30 12:05   ` [dpdk-dev] [PATCH v2 08/15] net/thunderx: add helper utils for secondary qset support Kamil Rytarowski
2016-09-30 12:05   ` [dpdk-dev] [PATCH v2 09/15] net/thunderx: add secondary qset support in dev stop/close Kamil Rytarowski
2016-09-30 12:05   ` [dpdk-dev] [PATCH v2 10/15] net/thunderx: add secondary qset support in device start Kamil Rytarowski
2016-09-30 12:05   ` [dpdk-dev] [PATCH v2 11/15] net/thunderx: add secondary qset support in device configure Kamil Rytarowski
2016-09-30 12:05   ` [dpdk-dev] [PATCH v2 12/15] net/thunderx: add final bits for secondary queue support Kamil Rytarowski
2016-09-30 12:05   ` [dpdk-dev] [PATCH v2 13/15] net/thunderx: document secondary queue set support Kamil Rytarowski
2016-09-30 15:12     ` Mcnamara, John
2016-09-30 12:05   ` [dpdk-dev] [PATCH v2 14/15] ethdev: Support VFs on the different PCI domains Kamil Rytarowski
2016-10-10 10:19     ` Ferruh Yigit
2016-10-10 13:01       ` Kamil Rytarowski
2016-10-10 13:27         ` Ferruh Yigit
2016-10-11 13:52           ` Kamil Rytarowski
2016-09-30 12:05   ` [dpdk-dev] [PATCH v2 15/15] net/thunderx: Bump driver version to 2.0 Kamil Rytarowski
2016-10-12 15:59   ` [dpdk-dev] [PATCH v2 00/15] Add support for secondary queue set in nicvf thunderx driver Bruce Richardson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=568fafa1-53f3-980b-9ca4-e086980651b2@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=dev@dpdk.org \
    --cc=jerin.jacob@caviumnetworks.com \
    --cc=kamil.rytarowski@caviumnetworks.com \
    --cc=krytarowski@caviumnetworks.com \
    --cc=maciej.czekaj@caviumnetworks.com \
    --cc=rad@semihalf.com \
    --cc=slawomir.rosek@semihalf.com \
    --cc=zyta.szpak@semihalf.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).