DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Andrew Boyer <aboyer@pensando.io>, dev@dpdk.org
Cc: Alfredo Cardigliano <cardigliano@ntop.org>
Subject: Re: [dpdk-dev] [PATCH 3/6] net/ionic: fully implement remove-on-close
Date: Tue, 15 Dec 2020 13:42:11 +0000
Message-ID: <b426f616-58f0-b18b-5951-eed50e665d5d@intel.com> (raw)
In-Reply-To: <20201210142231.63209-4-aboyer@pensando.io>

On 12/10/2020 2:22 PM, Andrew Boyer wrote:
> This is now required behavior for a PMD.
> Remove the UNMAINTAINED flag.
> 
> Signed-off-by: Andrew Boyer <aboyer@pensando.io>
> ---
>   MAINTAINERS                      |  2 +-
>   drivers/net/ionic/ionic_ethdev.c | 41 ++++++++++++++++----------------
>   drivers/net/ionic/ionic_lif.c    | 15 ++++++++++++
>   drivers/net/ionic/ionic_lif.h    |  1 +
>   4 files changed, 38 insertions(+), 21 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7bc0010f2..fc1b09923 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -840,7 +840,7 @@ F: doc/guides/nics/pfe.rst
>   F: drivers/net/pfe/
>   F: doc/guides/nics/features/pfe.ini
>   
> -Pensando ionic - UNMAINTAINED
> +Pensando ionic
>   M: Andrew Boyer <aboyer@pensando.io>
>   F: drivers/net/ionic/
>   F: doc/guides/nics/ionic.rst
> diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
> index 5a360ac08..629d7068b 100644
> --- a/drivers/net/ionic/ionic_ethdev.c
> +++ b/drivers/net/ionic/ionic_ethdev.c
> @@ -958,6 +958,8 @@ ionic_dev_stop(struct rte_eth_dev *eth_dev)
>   	return err;
>   }
>   
> +static void ionic_unconfigure_intr(struct ionic_adapter *adapter);
> +
>   /*
>    * Reset and stop device.
>    */
> @@ -965,6 +967,8 @@ static int
>   ionic_dev_close(struct rte_eth_dev *eth_dev)
>   {
>   	struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
> +	struct ionic_adapter *adapter = lif->adapter;
> +	uint32_t i;
>   	int err;
>   
>   	IONIC_PRINT_CALL();
> @@ -977,12 +981,21 @@ ionic_dev_close(struct rte_eth_dev *eth_dev)
>   		return -1;
>   	}
>   
> -	err = eth_ionic_dev_uninit(eth_dev);
> -	if (err) {
> -		IONIC_PRINT(ERR, "Cannot destroy LIF: %d", err);
> -		return -1;
> +	ionic_lif_free_queues(lif);
> +
> +	IONIC_PRINT(NOTICE, "Removing device %s", eth_dev->device->name);
> +	ionic_unconfigure_intr(adapter);
> +
> +	for (i = 0; i < adapter->nlifs; i++) {
> +		lif = adapter->lifs[i];
> +		rte_eth_dev_destroy(lif->eth_dev, eth_ionic_dev_uninit);

Should 'ionic_lif_stop()' & 'ionic_lif_free_queues()' be called per 'lif' here?

>   	}
>   
> +	ionic_port_reset(adapter);
> +	ionic_reset(adapter);
> +
> +	rte_free(adapter);
> +

In ionic, an ethdev is created per 'lif' during probe and when one of the ethdev 
closed, all 'lif' destroyed and 'adapter' freed, so all ethdev should be 
unusable at this stage.
1) First better to document this behavior in the commit log, and as overall can 
you please prefer more descriptive commit logs.

2) What happens to the ethdev statuses, 'lif' destroyed and ethdev are not 
usable but ethdev status not updated or freed?
What happens if the application tries to access to ethdev of another 'lif' after 
'ionic_dev_close()'?

>   	return 0;
>   }
>   
> @@ -1280,10 +1293,7 @@ static int
>   eth_ionic_pci_remove(struct rte_pci_device *pci_dev __rte_unused)
>   {
>   	char name[RTE_ETH_NAME_MAX_LEN];
> -	struct ionic_adapter *adapter = NULL;
>   	struct rte_eth_dev *eth_dev;
> -	struct ionic_lif *lif;
> -	uint32_t i;
>   
>   	/* Adapter lookup is using (the first) eth_dev name */
>   	snprintf(name, sizeof(name), "net_%s_lif_0",
> @@ -1291,19 +1301,10 @@ eth_ionic_pci_remove(struct rte_pci_device *pci_dev __rte_unused)
>   

Should remove '__rte_unused'

>   	eth_dev = rte_eth_dev_allocated(name);
>   	if (eth_dev) {
> -		lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
> -		adapter = lif->adapter;
> -	}
> -
> -	if (adapter) {
> -		ionic_unconfigure_intr(adapter);
> -
> -		for (i = 0; i < adapter->nlifs; i++) {
> -			lif = adapter->lifs[i];
> -			rte_eth_dev_destroy(lif->eth_dev, eth_ionic_dev_uninit);
> -		}
> -
> -		rte_free(adapter);
> +		ionic_dev_close(eth_dev);
> +	} else {
> +		IONIC_PRINT(WARNING, "Cannot find device %s",
> +			pci_dev->device.name);

Not sure if this warning is correct, if there is no allocated ethdev this is not 
an error, this means there is nothing to remove and function can continue with 
'return 0'


>   	}
>   
>   	return 0;
> diff --git a/drivers/net/ionic/ionic_lif.c b/drivers/net/ionic/ionic_lif.c
> index 875c7e585..e213597ee 100644
> --- a/drivers/net/ionic/ionic_lif.c
> +++ b/drivers/net/ionic/ionic_lif.c
> @@ -927,6 +927,21 @@ ionic_lif_free(struct ionic_lif *lif)
>   	}
>   }
>   
> +void
> +ionic_lif_free_queues(struct ionic_lif *lif)
> +{
> +	uint32_t i;
> +
> +	for (i = 0; i < lif->ntxqcqs; i++) {
> +		ionic_dev_tx_queue_release(lif->eth_dev->data->tx_queues[i]);
> +		lif->eth_dev->data->tx_queues[i] = NULL;
> +	}
> +	for (i = 0; i < lif->nrxqcqs; i++) {
> +		ionic_dev_rx_queue_release(lif->eth_dev->data->rx_queues[i]);
> +		lif->eth_dev->data->rx_queues[i] = NULL;
> +	}
> +}
> +
>   int
>   ionic_lif_rss_config(struct ionic_lif *lif,
>   		const uint16_t types, const uint8_t *key, const uint32_t *indir)
> diff --git a/drivers/net/ionic/ionic_lif.h b/drivers/net/ionic/ionic_lif.h
> index b80931c61..bf010716e 100644
> --- a/drivers/net/ionic/ionic_lif.h
> +++ b/drivers/net/ionic/ionic_lif.h
> @@ -122,6 +122,7 @@ int ionic_lifs_size(struct ionic_adapter *ionic);
>   
>   int ionic_lif_alloc(struct ionic_lif *lif);
>   void ionic_lif_free(struct ionic_lif *lif);
> +void ionic_lif_free_queues(struct ionic_lif *lif);
>   
>   int ionic_lif_init(struct ionic_lif *lif);
>   void ionic_lif_deinit(struct ionic_lif *lif);
> 


  reply	other threads:[~2020-12-15 13:42 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-10 14:22 [dpdk-dev] [PATCH 0/6] net/ionic: fixes for stop and start Andrew Boyer
2020-12-10 14:22 ` [dpdk-dev] [PATCH 1/6] net/ionic: preserve RSS state unless RETA size changes Andrew Boyer
2020-12-10 14:22 ` [dpdk-dev] [PATCH 2/6] net/ionic: preserve RX mode across LIF stop/start Andrew Boyer
2020-12-10 14:22 ` [dpdk-dev] [PATCH 3/6] net/ionic: fully implement remove-on-close Andrew Boyer
2020-12-15 13:42   ` Ferruh Yigit [this message]
2020-12-16 19:52     ` Andrew Boyer
2020-12-10 14:22 ` [dpdk-dev] [PATCH 4/6] net/ionic: improve link state handling Andrew Boyer
2020-12-10 14:22 ` [dpdk-dev] [PATCH 5/6] net/ionic: improve queue " Andrew Boyer
2020-12-10 14:22 ` [dpdk-dev] [PATCH 6/6] net/ionic: stop queues when LIF is stopped Andrew Boyer
2020-12-15 14:01 ` [dpdk-dev] [PATCH 0/6] net/ionic: fixes for stop and start Ferruh Yigit
2020-12-16 21:12 ` [dpdk-dev] [PATCH v2 0/7] " Andrew Boyer
2021-01-11 17:08   ` Ferruh Yigit
2020-12-16 21:12 ` [dpdk-dev] [PATCH v2 1/7] net/ionic: preserve RSS state unless RETA size changes Andrew Boyer
2020-12-16 21:12 ` [dpdk-dev] [PATCH v2 2/7] net/ionic: preserve Rx mode across LIF stop/start Andrew Boyer
2020-12-16 21:12 ` [dpdk-dev] [PATCH v2 3/7] net/ionic: remove multi-LIF support Andrew Boyer
2020-12-16 21:12 ` [dpdk-dev] [PATCH v2 4/7] net/ionic: fully implement remove-on-close Andrew Boyer
2020-12-16 21:12 ` [dpdk-dev] [PATCH v2 5/7] net/ionic: improve link state handling Andrew Boyer
2020-12-16 21:12 ` [dpdk-dev] [PATCH v2 6/7] net/ionic: improve queue " Andrew Boyer
2020-12-16 21:12 ` [dpdk-dev] [PATCH v2 7/7] net/ionic: stop queues when LIF is stopped Andrew Boyer

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=b426f616-58f0-b18b-5951-eed50e665d5d@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=aboyer@pensando.io \
    --cc=cardigliano@ntop.org \
    --cc=dev@dpdk.org \
    /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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git