DPDK patches and discussions
 help / color / mirror / Atom feed
From: Tetsuya Mukawa <mukawa@igel.co.jp>
To: "John W. Linville" <linville@tuxdriver.com>, dev@dpdk.org
Subject: Re: [dpdk-dev] [RFC] af_packet: support port hotplug
Date: Thu, 12 Mar 2015 11:45:12 +0900	[thread overview]
Message-ID: <5500FDB8.2000604@igel.co.jp> (raw)
In-Reply-To: <1426012584-3614-1-git-send-email-linville@tuxdriver.com>

Hi John.

On 2015/03/11 3:36, John W. Linville wrote:
> This patch adds finalization code to free resources allocated by
> the PMD.  This is based on earlier patches for other PMDs by Tetsuya
> Mukawa <mukawa@igel.co.jp>, with corrections related to data->name.
>
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
> Cc: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---
>  lib/librte_pmd_af_packet/rte_eth_af_packet.c | 56 ++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
>
> diff --git a/lib/librte_pmd_af_packet/rte_eth_af_packet.c b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
> index 80e9bdf7f819..57998ab19c96 100644
> --- a/lib/librte_pmd_af_packet/rte_eth_af_packet.c
> +++ b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
> @@ -399,6 +399,13 @@ static struct eth_dev_ops ops = {
>  	.stats_reset = eth_stats_reset,
>  };
>  
> +static struct eth_driver rte_af_packet_pmd = {
> +	.pci_drv = {
> +		.name = "rte_af_packet_pmd",
> +		.drv_flags = RTE_PCI_DRV_DETACHABLE,
> +	},
> +};
> +
>  /*
>   * Opens an AF_PACKET socket
>   */
> @@ -653,6 +660,10 @@ rte_pmd_init_internals(const char *name,
>  	if (*eth_dev == NULL)
>  		goto error;
>  
> +	/* check length of device name */
> +	if ((strlen((*eth_dev)->data->name) + 1) > sizeof(data->name))
> +		goto error;
> +
>  	/*
>  	 * now put it all together
>  	 * - store queue data in internals,
> @@ -669,12 +680,14 @@ rte_pmd_init_internals(const char *name,
>  	data->nb_tx_queues = (uint16_t)nb_queues;
>  	data->dev_link = pmd_link;
>  	data->mac_addrs = &(*internals)->eth_addr;
> +	strncpy(data->name, (*eth_dev)->data->name, strlen((*eth_dev)->data->name));

Above line is over 80 characters.
(I am not sure we need to keep it, but may be good.)

>  
>  	pci_dev->numa_node = numa_node;
>  
>  	(*eth_dev)->data = data;
>  	(*eth_dev)->dev_ops = &ops;
>  	(*eth_dev)->pci_dev = pci_dev;
> +	(*eth_dev)->driver = &rte_af_packet_pmd;
>  
>  	return 0;
>  
> @@ -835,10 +848,53 @@ rte_pmd_af_packet_devinit(const char *name, const char *params)
>  	return 0;
>  }
>  
> +static int
> +rte_pmd_af_packet_devuninit(const char *name)
> +{
> +	struct rte_eth_dev *eth_dev = NULL;
> +	struct pmd_internals *internals;
> +	struct tpacket_req req;
> +
> +	unsigned q;
> +
> +	RTE_LOG(INFO, PMD, "Closing AF_PACKET ethdev on numa socket %u\n",
> +			rte_socket_id());
> +
> +	if (name == NULL)
> +		return -1;
> +
> +	/* retrieve ethdev entry */
> +	eth_dev = rte_eth_dev_allocated(name);
> +	if (eth_dev == NULL)
> +		return -1;
> +
> +	internals = eth_dev->data->dev_private;
> +	req = internals->req;
> +
> +	for (q = 0; q < internals->nb_queues; q++) {
> +		munmap(internals->rx_queue[q].map,
> +		       2 * req.tp_block_size * req.tp_block_nr);
> +		if (internals->rx_queue[q].rd)
> +			rte_free(internals->rx_queue[q].rd);
> +		if (internals->tx_queue[q].rd)
> +			rte_free(internals->tx_queue[q].rd);
> +	}
> +
> +	rte_free(internals);
> +	rte_free(eth_dev->data);
> +	rte_free(eth_dev->pci_dev);
> +
> +	rte_eth_dev_release_port(eth_dev);
> +
> +
> +	return 0;
> +}
> +
>  static struct rte_driver pmd_af_packet_drv = {
>  	.name = "eth_af_packet",
>  	.type = PMD_VDEV,
>  	.init = rte_pmd_af_packet_devinit,
> +	.uninit = rte_pmd_af_packet_devuninit,
>  };
>  
>  PMD_REGISTER_DRIVER(pmd_af_packet_drv);

Except above, I guess it's good.
I've checked followings.

- Code review
- No missing symbol
- No compile error with icc
- No compile error with gcc-4.6
- No compile error with gcc-4.7
- No compile error with gcc-4.8
- No compile error with gcc-4.9
- checkpatch.pl

Thanks,
Tetsuya

  reply	other threads:[~2015-03-12  2:45 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-10 18:36 John W. Linville
2015-03-12  2:45 ` Tetsuya Mukawa [this message]
2015-03-12 17:05 ` Iremonger, Bernard
2015-03-13  0:10   ` Tetsuya Mukawa
2015-03-13 10:14     ` Iremonger, Bernard
2015-03-16  8:38       ` Tetsuya Mukawa
2015-03-16  8:56         ` Tetsuya Mukawa
2015-03-16 14:47           ` Iremonger, Bernard
2015-03-17  3:42             ` Tetsuya Mukawa
2015-03-19 11:44               ` Iremonger, Bernard
2015-06-08  9:21               ` Iremonger, Bernard

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=5500FDB8.2000604@igel.co.jp \
    --to=mukawa@igel.co.jp \
    --cc=dev@dpdk.org \
    --cc=linville@tuxdriver.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).