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 at igel.co.jp>, with corrections related to data->name.
>
> Signed-off-by: John W. Linville <linville at tuxdriver.com>
> Cc: Tetsuya Mukawa <mukawa at 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
