All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Varghese, Vipin" <vipin.varghese@intel.com>
To: Ophir Munk <ophirmu@mellanox.com>, "dev@dpdk.org" <dev@dpdk.org>,
	"Pascal Mazon" <pascal.mazon@6wind.com>,
	"Wiles, Keith" <keith.wiles@intel.com>
Cc: Thomas Monjalon <thomas@monjalon.net>,
	Olga Shern <olgas@mellanox.com>,
	Shahaf Shuler <shahafs@mellanox.com>,
	"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [PATCH v4] net/tap: fix device removal when no queues	exist
Date: Wed, 23 May 2018 04:50:18 +0000	[thread overview]
Message-ID: <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D1F8841@BGSMSX101.gar.corp.intel.com> (raw)
In-Reply-To: <1526889273-14691-1-git-send-email-ophirmu@mellanox.com>

Hi Ophir,

One suggestion shared inline to email

<Snipped>

> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c index
> c006d07..52ef799 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -929,6 +929,15 @@ tap_dev_close(struct rte_eth_dev *dev)
>  		ioctl(internals->ioctl_sock, SIOCSIFFLAGS,
>  				&internals->remote_initial_flags);
>  	}
> +
> +	if (internals->ka_fd != -1) {
> +		close(internals->ka_fd);

Do we need to notify the user which fd is been closed via LOG DEBUG?

> +		internals->ka_fd = -1;
> +	}
> +	/*
> +	 * Since TUN device has no more opened file descriptors
> +	 * it will be removed from kernel
> +	 */
>  }
> 
>  static void
> @@ -1549,6 +1558,7 @@ eth_dev_tap_create(struct rte_vdev_device *vdev,
> char *tap_name,
>  	dev->intr_handle = &pmd->intr_handle;
> 
>  	/* Presetup the fds to -1 as being not valid */
> +	pmd->ka_fd = -1;
>  	for (i = 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
>  		pmd->rxq[i].fd = -1;
>  		pmd->txq[i].fd = -1;
> @@ -1561,13 +1571,17 @@ eth_dev_tap_create(struct rte_vdev_device *vdev,
> char *tap_name,
>  			rte_memcpy(&pmd->eth_addr, mac_addr,
> sizeof(*mac_addr));
>  	}
> 
> -	/* Immediately create the netdevice (this will create the 1st queue). */
> -	/* rx queue */
> -	if (tap_setup_queue(dev, pmd, 0, 1) == -1)
> -		goto error_exit;
> -	/* tx queue */
> -	if (tap_setup_queue(dev, pmd, 0, 0) == -1)
> +	/*
> +	 * Allocate a TUN device keep-alive file descriptor that will only be
> +	 * closed when the TUN device itself is closed or removed.
> +	 * This keep-alive file descriptor will guarantee that the TUN device
> +	 * exists even when all of its queues are closed
> +	 */
> +	pmd->ka_fd = tun_alloc(pmd);
> +	if (pmd->ka_fd == -1) {
> +		TAP_LOG(ERR, "Unable to create %s interface", tuntap_name);
>  		goto error_exit;
> +	}
> 
>  	ifr.ifr_mtu = dev->data->mtu;
>  	if (tap_ioctl(pmd, SIOCSIFMTU, &ifr, 1, LOCAL_AND_REMOTE) < 0) @@
> -1961,9 +1975,12 @@ rte_pmd_tap_remove(struct rte_vdev_device *dev)
> 
>  	close(internals->ioctl_sock);
>  	rte_free(eth_dev->data->dev_private);
> -
>  	rte_eth_dev_release_port(eth_dev);
> 
> +	if (internals->ka_fd != -1) {
> +		close(internals->ka_fd);
> +		internals->ka_fd = -1;
> +	}
>  	return 0;
>  }
> 
> diff --git a/drivers/net/tap/rte_eth_tap.h b/drivers/net/tap/rte_eth_tap.h index
> babe42d..575dce4 100644
> --- a/drivers/net/tap/rte_eth_tap.h
> +++ b/drivers/net/tap/rte_eth_tap.h
> @@ -81,6 +81,7 @@ struct pmd_internals {
>  	struct rx_queue rxq[RTE_PMD_TAP_MAX_QUEUES]; /* List of RX
> queues */
>  	struct tx_queue txq[RTE_PMD_TAP_MAX_QUEUES]; /* List of TX queues
> */
>  	struct rte_intr_handle intr_handle;          /* LSC interrupt handle. */
> +	int ka_fd;                        /* keep-alive file descriptor */
>  };
> 
>  /* tap_intr.c */
> --
> 2.7.4

  parent reply	other threads:[~2018-05-23  4:50 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-16 16:10 [PATCH v1] net/tap: keep device alive when no queues exist Ophir Munk
2018-05-17  9:47 ` [PATCH v2] net/tap: fix device removal " Ophir Munk
2018-05-17 12:59   ` Wiles, Keith
2018-05-18  8:38     ` Ophir Munk
2018-05-18  8:27   ` [PATCH v3] " Ophir Munk
2018-05-21  7:54     ` [PATCH v4] " Ophir Munk
2018-05-21 12:52       ` Wiles, Keith
2018-05-21 15:12         ` [dpdk-stable] " Ferruh Yigit
2018-05-23  4:50       ` Varghese, Vipin [this message]
2018-05-23  4:53         ` Wiles, Keith
2018-05-23  5:22           ` Varghese, Vipin

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=4C9E0AB70F954A408CC4ADDBF0F8FA7D4D1F8841@BGSMSX101.gar.corp.intel.com \
    --to=vipin.varghese@intel.com \
    --cc=dev@dpdk.org \
    --cc=keith.wiles@intel.com \
    --cc=olgas@mellanox.com \
    --cc=ophirmu@mellanox.com \
    --cc=pascal.mazon@6wind.com \
    --cc=shahafs@mellanox.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.