[PATCH 1/5] nic/tap: fix tap docs for device name

[PATCH 1/5] nic/tap: fix tap docs for device name

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 doc/guides/nics/tap.rst | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
index 622b9e7..2ab60ff 100644
--- a/doc/guides/nics/tap.rst
+++ b/doc/guides/nics/tap.rst
@@ -45,18 +45,18 @@ device.
 These TAP interfaces can be used with Wireshark or tcpdump or Pktgen-DPDK
 along with being able to be used as a network connection to the DPDK
 application. The method enable one or more interfaces is to use the
-``--vdev=net_tap`` option on the DPDK application command line. Each
-``--vdev=net_tap`` option give will create an interface named dtap0, dtap1,
+``--vdev=net_tap0`` option on the DPDK application command line. Each
+``--vdev=net_tap1`` option give will create an interface named dtap0, dtap1,
 and so on.
 
 The interfaced name can be changed by adding the ``iface=foo0``, for example::
 
-   --vdev=net_tap,iface=foo0 --vdev=net_tap,iface=foo1, ...
+   --vdev=net_tap0,iface=foo0 --vdev=net_tap1,iface=foo1, ...
 
 Also the speed of the interface can be changed from 10G to whatever number
 needed, but the interface does not enforce that speed, for example::
 
-   --vdev=net_tap,iface=foo0,speed=25000
+   --vdev=net_tap0,iface=foo0,speed=25000
 
 After the DPDK application is started you can send and receive packets on the
 interface using the standard rx_burst/tx_burst APIs in DPDK. From the host
@@ -97,7 +97,7 @@ following::
 
     sudo ./app/app/x86_64-native-linuxapp-gcc/app/pktgen -l 1-5 -n 4        \
      --proc-type auto --log-level 8 --socket-mem 512,512 --file-prefix pg   \
-     --vdev=net_tap --vdev=net_tap -b 05:00.0 -b 05:00.1                    \
+     --vdev=net_tap0 --vdev=net_tap1 -b 05:00.0 -b 05:00.1                  \
      -b 04:00.0 -b 04:00.1 -b 04:00.2 -b 04:00.3                            \
      -b 81:00.0 -b 81:00.1 -b 81:00.2 -b 81:00.3                            \
      -b 82:00.0 -b 83:00.0 -- -T -P -m [2:3].0 -m [4:5].1                   \
@@ -131,6 +131,6 @@ time with ``start all``. The command ``str`` is an alias for ``start all`` and
 
 While running you should see the 64 byte counters increasing to verify the
 traffic is being looped back. You can use ``set all size XXX`` to change the
-size of the packets after you stop the traffic. Use the pktgen ``help``
+size of the packets after you stop the traffic. Use pktgen ``help``
 command to see a list of all commands. You can also use the ``-f`` option to
-load commands at startup.
+load commands at startup in command line or Lua script in pktgen.
-- 
2.8.0.GIT

[PATCH 2/5] net/tap: fix multi-queue support

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 93 ++++++++++++++++++++++---------------------
 1 file changed, 48 insertions(+), 45 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 3f179c3..9ed7a87 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -115,10 +115,9 @@ struct pmd_internals {
  * supplied name.
  */
 static int
-tun_alloc(char *name)
+tun_alloc(char *name, uint16_t qid)
 {
 	struct ifreq ifr;
-	unsigned int features;
 	int fd;
 
 	memset(&ifr, 0, sizeof(struct ifreq));
@@ -133,55 +132,57 @@ tun_alloc(char *name)
 		goto error;
 	}
 
-	/* Grab the TUN features to verify we can work */
-	if (ioctl(fd, TUNGETFEATURES, &features) < 0) {
-		RTE_LOG(ERR, PMD, "Unable to get TUN/TAP features\n");
-		goto error;
-	}
-	RTE_LOG(DEBUG, PMD, "TUN/TAP Features %08x\n", features);
+	/* This can only be done once per interface */
+	if (qid == 0) {
+		unsigned int features;
+
+		/* Grab the TUN features to verify we can work */
+		if (ioctl(fd, TUNGETFEATURES, &features) < 0) {
+			RTE_LOG(ERR, PMD, "Unable to get TUN/TAP features\n");
+			goto error;
+		}
+		RTE_LOG(DEBUG, PMD, "TUN/TAP Features %08x\n", features);
 
 #ifdef IFF_MULTI_QUEUE
-	if (!(features & IFF_MULTI_QUEUE) && (RTE_PMD_TAP_MAX_QUEUES > 1)) {
-		RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n");
-		goto error;
-	} else if ((features & IFF_ONE_QUEUE) &&
-			(RTE_PMD_TAP_MAX_QUEUES == 1)) {
-		ifr.ifr_flags |= IFF_ONE_QUEUE;
-		RTE_LOG(DEBUG, PMD, "Single queue only support\n");
-	} else {
-		ifr.ifr_flags |= IFF_MULTI_QUEUE;
-		RTE_LOG(DEBUG, PMD, "Multi-queue support for %d queues\n",
-			RTE_PMD_TAP_MAX_QUEUES);
-	}
+		if (!(features & IFF_MULTI_QUEUE) && (RTE_PMD_TAP_MAX_QUEUES > 1)) {
+			RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n");
+			goto error;
+		} else if ((features & IFF_ONE_QUEUE) &&
+				(RTE_PMD_TAP_MAX_QUEUES == 1)) {
+			ifr.ifr_flags |= IFF_ONE_QUEUE;
+			RTE_LOG(DEBUG, PMD, "Single queue only support\n");
+		} else {
+			ifr.ifr_flags |= IFF_MULTI_QUEUE;
+			RTE_LOG(DEBUG, PMD, "Multi-queue support for %d queues\n",
+				RTE_PMD_TAP_MAX_QUEUES);
+		}
 #else
-	if (RTE_PMD_TAP_MAX_QUEUES > 1) {
-		RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n");
-		goto error;
-	} else {
-		ifr.ifr_flags |= IFF_ONE_QUEUE;
-		RTE_LOG(DEBUG, PMD, "Single queue only support\n");
-	}
+		if (RTE_PMD_TAP_MAX_QUEUES > 1) {
+			RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n");
+			goto error;
+		} else {
+			ifr.ifr_flags |= IFF_ONE_QUEUE;
+			RTE_LOG(DEBUG, PMD, "Single queue only support\n");
+		}
 #endif
 
-	/* Set the TUN/TAP configuration and get the name if needed */
-	if (ioctl(fd, TUNSETIFF, (void *)&ifr) < 0) {
-		RTE_LOG(ERR, PMD, "Unable to set TUNSETIFF for %s\n",
-			ifr.ifr_name);
-		perror("TUNSETIFF");
-		goto error;
-	}
+		/* Set the TUN/TAP configuration and get the name if needed */
+		if (ioctl(fd, TUNSETIFF, (void *)&ifr) < 0) {
+			RTE_LOG(ERR, PMD, "Unable to set TUNSETIFF for %s\n",
+				ifr.ifr_name);
+			perror("TUNSETIFF");
+			goto error;
+		}
 
-	/* Always set the file descriptor to non-blocking */
-	if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
-		RTE_LOG(ERR, PMD, "Unable to set to nonblocking\n");
-		perror("F_SETFL, NONBLOCK");
-		goto error;
+		/* Always set the file descriptor to non-blocking */
+		if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
+			RTE_LOG(WARNING, PMD, "Unable to set %s to nonblocking\n",
+				ifr.ifr_name);
+			perror("F_SETFL, NONBLOCK");
+			goto error;
+		}
 	}
 
-	/* If the name is different that new name as default */
-	if (name && strcmp(name, ifr.ifr_name))
-		snprintf(name, RTE_ETH_NAME_MAX_LEN - 1, "%s", ifr.ifr_name);
-
 	return fd;
 
 error:
@@ -512,7 +513,7 @@ tap_setup_queue(struct rte_eth_dev *dev,
 		if (fd < 0) {
 			RTE_LOG(INFO, PMD, "Add queue to TAP %s for qid %d\n",
 				pmd->name, qid);
-			fd = tun_alloc(pmd->name);
+			fd = tun_alloc(pmd->name, qid);
 			if (fd < 0) {
 				RTE_LOG(ERR, PMD, "tun_alloc(%s) failed\n", pmd->name);
 				return -1;
@@ -711,7 +712,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	snprintf(dev->data->name, sizeof(dev->data->name), "%s", name);
 
 	/* Create the first Tap device */
-	fd = tun_alloc(tap_name);
+	fd = tun_alloc(tap_name, 0);
 	if (fd < 0) {
 		RTE_LOG(ERR, PMD, "tun_alloc() failed\n");
 		goto error_exit;
@@ -739,6 +740,8 @@ eth_dev_tap_create(const char *name, char *tap_name)
 error_exit:
 	RTE_PMD_DEBUG_TRACE("Unable to initialize %s\n", name);
 
+	if (fd > 0)
+		close(fd);
 	rte_free(data);
 	rte_free(pmd);
 
-- 
2.8.0.GIT

Re: [PATCH 2/5] net/tap: fix multi-queue support

[Pascal Mazon]

On Thu,  2 Feb 2017 16:33:27 -0600
Keith Wiles <keith.wiles@intel.com> wrote:

> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
>  drivers/net/tap/rte_eth_tap.c | 93
> ++++++++++++++++++++++--------------------- 1 file changed, 48 insertions(+),
> 45 deletions(-)
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 3f179c3..9ed7a87 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -115,10 +115,9 @@ struct pmd_internals {
>   * supplied name.
>   */
>  static int
> -tun_alloc(char *name)
> +tun_alloc(char *name, uint16_t qid)
>  {
>  	struct ifreq ifr;
> -	unsigned int features;
>  	int fd;
>  
>  	memset(&ifr, 0, sizeof(struct ifreq));
> @@ -133,55 +132,57 @@ tun_alloc(char *name)
>  		goto error;
>  	}
>  
> -	/* Grab the TUN features to verify we can work */
> -	if (ioctl(fd, TUNGETFEATURES, &features) < 0) {
> -		RTE_LOG(ERR, PMD, "Unable to get TUN/TAP features\n");
> -		goto error;
> -	}
> -	RTE_LOG(DEBUG, PMD, "TUN/TAP Features %08x\n", features);
> +	/* This can only be done once per interface */
> +	if (qid == 0) {
> +		unsigned int features;
> +
> +		/* Grab the TUN features to verify we can work */
> +		if (ioctl(fd, TUNGETFEATURES, &features) < 0) {
> +			RTE_LOG(ERR, PMD, "Unable to get TUN/TAP
> features\n");
> +			goto error;
> +		}
> +		RTE_LOG(DEBUG, PMD, "TUN/TAP Features %08x\n", features);
>  
>  #ifdef IFF_MULTI_QUEUE
> -	if (!(features & IFF_MULTI_QUEUE) && (RTE_PMD_TAP_MAX_QUEUES > 1)) {
> -		RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n");
> -		goto error;
> -	} else if ((features & IFF_ONE_QUEUE) &&
> -			(RTE_PMD_TAP_MAX_QUEUES == 1)) {
> -		ifr.ifr_flags |= IFF_ONE_QUEUE;
> -		RTE_LOG(DEBUG, PMD, "Single queue only support\n");
> -	} else {
> -		ifr.ifr_flags |= IFF_MULTI_QUEUE;
> -		RTE_LOG(DEBUG, PMD, "Multi-queue support for %d queues\n",
> -			RTE_PMD_TAP_MAX_QUEUES);
> -	}
> +		if (!(features & IFF_MULTI_QUEUE) && (RTE_PMD_TAP_MAX_QUEUES
> > 1)) {
> +			RTE_LOG(DEBUG, PMD, "TUN/TAP device only one
> queue\n");
> +			goto error;
> +		} else if ((features & IFF_ONE_QUEUE) &&
> +				(RTE_PMD_TAP_MAX_QUEUES == 1)) {
> +			ifr.ifr_flags |= IFF_ONE_QUEUE;
> +			RTE_LOG(DEBUG, PMD, "Single queue only support\n");
> +		} else {
> +			ifr.ifr_flags |= IFF_MULTI_QUEUE;
> +			RTE_LOG(DEBUG, PMD, "Multi-queue support for %d
> queues\n",
> +				RTE_PMD_TAP_MAX_QUEUES);
> +		}
>  #else
> -	if (RTE_PMD_TAP_MAX_QUEUES > 1) {
> -		RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n");
> -		goto error;
> -	} else {
> -		ifr.ifr_flags |= IFF_ONE_QUEUE;
> -		RTE_LOG(DEBUG, PMD, "Single queue only support\n");
> -	}
> +		if (RTE_PMD_TAP_MAX_QUEUES > 1) {
> +			RTE_LOG(DEBUG, PMD, "TUN/TAP device only one
> queue\n");
> +			goto error;
> +		} else {
> +			ifr.ifr_flags |= IFF_ONE_QUEUE;
> +			RTE_LOG(DEBUG, PMD, "Single queue only support\n");
> +		}
>  #endif
>  
> -	/* Set the TUN/TAP configuration and get the name if needed */
> -	if (ioctl(fd, TUNSETIFF, (void *)&ifr) < 0) {
> -		RTE_LOG(ERR, PMD, "Unable to set TUNSETIFF for %s\n",
> -			ifr.ifr_name);
> -		perror("TUNSETIFF");
> -		goto error;
> -	}
> +		/* Set the TUN/TAP configuration and get the name if needed
> */
> +		if (ioctl(fd, TUNSETIFF, (void *)&ifr) < 0) {
> +			RTE_LOG(ERR, PMD, "Unable to set TUNSETIFF for %s\n",
> +				ifr.ifr_name);
> +			perror("TUNSETIFF");
> +			goto error;
> +		}
>  
> -	/* Always set the file descriptor to non-blocking */
> -	if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
> -		RTE_LOG(ERR, PMD, "Unable to set to nonblocking\n");
> -		perror("F_SETFL, NONBLOCK");
> -		goto error;
> +		/* Always set the file descriptor to non-blocking */
> +		if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
> +			RTE_LOG(WARNING, PMD, "Unable to set %s to
> nonblocking\n",
> +				ifr.ifr_name);
> +			perror("F_SETFL, NONBLOCK");
> +			goto error;
> +		}
>  	}
>  
> -	/* If the name is different that new name as default */
> -	if (name && strcmp(name, ifr.ifr_name))
> -		snprintf(name, RTE_ETH_NAME_MAX_LEN - 1, "%s", ifr.ifr_name);
> -
>  	return fd;
>  
>  error:
> @@ -512,7 +513,7 @@ tap_setup_queue(struct rte_eth_dev *dev,
>  		if (fd < 0) {
>  			RTE_LOG(INFO, PMD, "Add queue to TAP %s for qid
> %d\n", pmd->name, qid);
> -			fd = tun_alloc(pmd->name);
> +			fd = tun_alloc(pmd->name, qid);
>  			if (fd < 0) {
>  				RTE_LOG(ERR, PMD, "tun_alloc(%s) failed\n",
> pmd->name); return -1;
> @@ -711,7 +712,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
>  	snprintf(dev->data->name, sizeof(dev->data->name), "%s", name);
>  
>  	/* Create the first Tap device */
> -	fd = tun_alloc(tap_name);
> +	fd = tun_alloc(tap_name, 0);
>  	if (fd < 0) {
>  		RTE_LOG(ERR, PMD, "tun_alloc() failed\n");
>  		goto error_exit;
> @@ -739,6 +740,8 @@ eth_dev_tap_create(const char *name, char *tap_name)
>  error_exit:
>  	RTE_PMD_DEBUG_TRACE("Unable to initialize %s\n", name);
>  
> +	if (fd > 0)
> +		close(fd);
>  	rte_free(data);
>  	rte_free(pmd);
>  

The patch looks good to me.

Can you maybe detail in the commit log that now queue 0 is created while
probing, while other queues are handled through tap_setup_queue() only?

Pascal

Re: [PATCH 2/5] net/tap: fix multi-queue support

[Wiles, Keith]

> On Feb 3, 2017, at 3:37 AM, Pascal Mazon <pascal.mazon@6wind.com> wrote:
> 
> On Thu,  2 Feb 2017 16:33:27 -0600
> Keith Wiles <keith.wiles@intel.com> wrote:
> 
>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>> ---
>> drivers/net/tap/rte_eth_tap.c | 93
>> ++++++++++++++++++++++--------------------- 1 file changed, 48 insertions(+),
>> 45 deletions(-)
>> 
>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>> index 3f179c3..9ed7a87 100644
>> --- a/drivers/net/tap/rte_eth_tap.c
>> +++ b/drivers/net/tap/rte_eth_tap.c
>> @@ -115,10 +115,9 @@ struct pmd_internals {
>>  * supplied name.
>>  */
>> static int
>> -tun_alloc(char *name)
>> +tun_alloc(char *name, uint16_t qid)
>> {
>> 	struct ifreq ifr;
>> -	unsigned int features;
>> 	int fd;
>> 
>> 	memset(&ifr, 0, sizeof(struct ifreq));
>> @@ -133,55 +132,57 @@ tun_alloc(char *name)
>> 		goto error;
>> 	}
>> 
>> -	/* Grab the TUN features to verify we can work */
>> -	if (ioctl(fd, TUNGETFEATURES, &features) < 0) {
>> -		RTE_LOG(ERR, PMD, "Unable to get TUN/TAP features\n");
>> -		goto error;
>> -	}
>> -	RTE_LOG(DEBUG, PMD, "TUN/TAP Features %08x\n", features);
>> +	/* This can only be done once per interface */
>> +	if (qid == 0) {
>> +		unsigned int features;
>> +
>> +		/* Grab the TUN features to verify we can work */
>> +		if (ioctl(fd, TUNGETFEATURES, &features) < 0) {
>> +			RTE_LOG(ERR, PMD, "Unable to get TUN/TAP
>> features\n");
>> +			goto error;
>> +		}
>> +		RTE_LOG(DEBUG, PMD, "TUN/TAP Features %08x\n", features);
>> 
>> #ifdef IFF_MULTI_QUEUE
>> -	if (!(features & IFF_MULTI_QUEUE) && (RTE_PMD_TAP_MAX_QUEUES > 1)) {
>> -		RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n");
>> -		goto error;
>> -	} else if ((features & IFF_ONE_QUEUE) &&
>> -			(RTE_PMD_TAP_MAX_QUEUES == 1)) {
>> -		ifr.ifr_flags |= IFF_ONE_QUEUE;
>> -		RTE_LOG(DEBUG, PMD, "Single queue only support\n");
>> -	} else {
>> -		ifr.ifr_flags |= IFF_MULTI_QUEUE;
>> -		RTE_LOG(DEBUG, PMD, "Multi-queue support for %d queues\n",
>> -			RTE_PMD_TAP_MAX_QUEUES);
>> -	}
>> +		if (!(features & IFF_MULTI_QUEUE) && (RTE_PMD_TAP_MAX_QUEUES
>>> 1)) {
>> +			RTE_LOG(DEBUG, PMD, "TUN/TAP device only one
>> queue\n");
>> +			goto error;
>> +		} else if ((features & IFF_ONE_QUEUE) &&
>> +				(RTE_PMD_TAP_MAX_QUEUES == 1)) {
>> +			ifr.ifr_flags |= IFF_ONE_QUEUE;
>> +			RTE_LOG(DEBUG, PMD, "Single queue only support\n");
>> +		} else {
>> +			ifr.ifr_flags |= IFF_MULTI_QUEUE;
>> +			RTE_LOG(DEBUG, PMD, "Multi-queue support for %d
>> queues\n",
>> +				RTE_PMD_TAP_MAX_QUEUES);
>> +		}
>> #else
>> -	if (RTE_PMD_TAP_MAX_QUEUES > 1) {
>> -		RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n");
>> -		goto error;
>> -	} else {
>> -		ifr.ifr_flags |= IFF_ONE_QUEUE;
>> -		RTE_LOG(DEBUG, PMD, "Single queue only support\n");
>> -	}
>> +		if (RTE_PMD_TAP_MAX_QUEUES > 1) {
>> +			RTE_LOG(DEBUG, PMD, "TUN/TAP device only one
>> queue\n");
>> +			goto error;
>> +		} else {
>> +			ifr.ifr_flags |= IFF_ONE_QUEUE;
>> +			RTE_LOG(DEBUG, PMD, "Single queue only support\n");
>> +		}
>> #endif
>> 
>> -	/* Set the TUN/TAP configuration and get the name if needed */
>> -	if (ioctl(fd, TUNSETIFF, (void *)&ifr) < 0) {
>> -		RTE_LOG(ERR, PMD, "Unable to set TUNSETIFF for %s\n",
>> -			ifr.ifr_name);
>> -		perror("TUNSETIFF");
>> -		goto error;
>> -	}
>> +		/* Set the TUN/TAP configuration and get the name if needed
>> */
>> +		if (ioctl(fd, TUNSETIFF, (void *)&ifr) < 0) {
>> +			RTE_LOG(ERR, PMD, "Unable to set TUNSETIFF for %s\n",
>> +				ifr.ifr_name);
>> +			perror("TUNSETIFF");
>> +			goto error;
>> +		}
>> 
>> -	/* Always set the file descriptor to non-blocking */
>> -	if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
>> -		RTE_LOG(ERR, PMD, "Unable to set to nonblocking\n");
>> -		perror("F_SETFL, NONBLOCK");
>> -		goto error;
>> +		/* Always set the file descriptor to non-blocking */
>> +		if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
>> +			RTE_LOG(WARNING, PMD, "Unable to set %s to
>> nonblocking\n",
>> +				ifr.ifr_name);
>> +			perror("F_SETFL, NONBLOCK");
>> +			goto error;
>> +		}
>> 	}
>> 
>> -	/* If the name is different that new name as default */
>> -	if (name && strcmp(name, ifr.ifr_name))
>> -		snprintf(name, RTE_ETH_NAME_MAX_LEN - 1, "%s", ifr.ifr_name);
>> -
>> 	return fd;
>> 
>> error:
>> @@ -512,7 +513,7 @@ tap_setup_queue(struct rte_eth_dev *dev,
>> 		if (fd < 0) {
>> 			RTE_LOG(INFO, PMD, "Add queue to TAP %s for qid
>> %d\n", pmd->name, qid);
>> -			fd = tun_alloc(pmd->name);
>> +			fd = tun_alloc(pmd->name, qid);
>> 			if (fd < 0) {
>> 				RTE_LOG(ERR, PMD, "tun_alloc(%s) failed\n",
>> pmd->name); return -1;
>> @@ -711,7 +712,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
>> 	snprintf(dev->data->name, sizeof(dev->data->name), "%s", name);
>> 
>> 	/* Create the first Tap device */
>> -	fd = tun_alloc(tap_name);
>> +	fd = tun_alloc(tap_name, 0);
>> 	if (fd < 0) {
>> 		RTE_LOG(ERR, PMD, "tun_alloc() failed\n");
>> 		goto error_exit;
>> @@ -739,6 +740,8 @@ eth_dev_tap_create(const char *name, char *tap_name)
>> error_exit:
>> 	RTE_PMD_DEBUG_TRACE("Unable to initialize %s\n", name);
>> 
>> +	if (fd > 0)
>> +		close(fd);
>> 	rte_free(data);
>> 	rte_free(pmd);
>> 
> 
> The patch looks good to me.
> 
> Can you maybe detail in the commit log that now queue 0 is created while
> probing, while other queues are handled through tap_setup_queue() only?

In my changes before your patch set I had removed creating queue 0 at probe and only created the interface/queue at queue setup time. It would have required a more changes compared to your patches and the original code. :-(

> 
> Pascal

Regards,
Keith

[PATCH 3/5] net/tap: remove redundant fds array

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 9ed7a87..6673182 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -103,7 +103,6 @@ struct pmd_internals {
 	struct ether_addr eth_addr;	/* Mac address of the device port */
 
 	int if_index;			/* IF_INDEX for the port */
-	int fds[RTE_PMD_TAP_MAX_QUEUES]; /* List of all file descriptors */
 
 	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 */
@@ -350,8 +349,8 @@ tap_dev_stop(struct rte_eth_dev *dev)
 	struct pmd_internals *internals = dev->data->dev_private;
 
 	for (i = 0; i < internals->nb_queues; i++)
-		if (internals->fds[i] != -1)
-			close(internals->fds[i]);
+		if (internals->rxq[i].fd != -1)
+			close(internals->rxq[i].fd);
 	tap_link_set_down(dev);
 }
 
@@ -583,7 +582,6 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
 	if (fd == -1)
 		return -1;
 
-	internals->fds[rx_queue_id] = fd;
 	RTE_LOG(INFO, PMD, "RX TAP device name %s, qid %d on fd %d\n",
 		internals->name, rx_queue_id, internals->rxq[rx_queue_id].fd);
 
@@ -720,7 +718,6 @@ eth_dev_tap_create(const char *name, char *tap_name)
 
 	/* Presetup the fds to -1 as being not working */
 	for (i = 1; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
-		pmd->fds[i] = -1;
 		pmd->rxq[i].fd = -1;
 		pmd->txq[i].fd = -1;
 	}
@@ -728,7 +725,6 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	/* Take the TUN/TAP fd and place in the first location */
 	pmd->rxq[0].fd = fd;
 	pmd->txq[0].fd = fd;
-	pmd->fds[0] = fd;
 
 	if (pmd_mac_address(fd, dev, &pmd->eth_addr) < 0) {
 		RTE_LOG(ERR, PMD, "Unable to get MAC address\n");
@@ -851,8 +847,8 @@ rte_pmd_tap_remove(const char *name)
 
 	internals = eth_dev->data->dev_private;
 	for (i = 0; i < internals->nb_queues; i++)
-		if (internals->fds[i] != -1)
-			close(internals->fds[i]);
+		if (internals->rxq[i].fd != -1)
+			close(internals->rxq[i].fd);
 
 	rte_free(eth_dev->data->dev_private);
 	rte_free(eth_dev->data);
-- 
2.8.0.GIT

Re: [PATCH 3/5] net/tap: remove redundant fds array

[Pascal Mazon]

On Thu,  2 Feb 2017 16:33:28 -0600
Keith Wiles <keith.wiles@intel.com> wrote:

> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
>  drivers/net/tap/rte_eth_tap.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 9ed7a87..6673182 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -103,7 +103,6 @@ struct pmd_internals {
>  	struct ether_addr eth_addr;	/* Mac address of the device port
> */ 
>  	int if_index;			/* IF_INDEX for the port */
> -	int fds[RTE_PMD_TAP_MAX_QUEUES]; /* List of all file descriptors */
>  
>  	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 */ @@ -350,8 +349,8 @@ tap_dev_stop(struct rte_eth_dev *dev)
>  	struct pmd_internals *internals = dev->data->dev_private;
>  
>  	for (i = 0; i < internals->nb_queues; i++)
> -		if (internals->fds[i] != -1)
> -			close(internals->fds[i]);
> +		if (internals->rxq[i].fd != -1)
> +			close(internals->rxq[i].fd);
>  	tap_link_set_down(dev);
>  }
>  
> @@ -583,7 +582,6 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
>  	if (fd == -1)
>  		return -1;
>  
> -	internals->fds[rx_queue_id] = fd;
>  	RTE_LOG(INFO, PMD, "RX TAP device name %s, qid %d on fd %d\n",
>  		internals->name, rx_queue_id,
> internals->rxq[rx_queue_id].fd); 
> @@ -720,7 +718,6 @@ eth_dev_tap_create(const char *name, char *tap_name)
>  
>  	/* Presetup the fds to -1 as being not working */
>  	for (i = 1; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
> -		pmd->fds[i] = -1;
>  		pmd->rxq[i].fd = -1;
>  		pmd->txq[i].fd = -1;
>  	}
> @@ -728,7 +725,6 @@ eth_dev_tap_create(const char *name, char *tap_name)
>  	/* Take the TUN/TAP fd and place in the first location */
>  	pmd->rxq[0].fd = fd;
>  	pmd->txq[0].fd = fd;
> -	pmd->fds[0] = fd;
>  
>  	if (pmd_mac_address(fd, dev, &pmd->eth_addr) < 0) {
>  		RTE_LOG(ERR, PMD, "Unable to get MAC address\n");
> @@ -851,8 +847,8 @@ rte_pmd_tap_remove(const char *name)
>  
>  	internals = eth_dev->data->dev_private;
>  	for (i = 0; i < internals->nb_queues; i++)
> -		if (internals->fds[i] != -1)
> -			close(internals->fds[i]);
> +		if (internals->rxq[i].fd != -1)
> +			close(internals->rxq[i].fd);
>  
>  	rte_free(eth_dev->data->dev_private);
>  	rte_free(eth_dev->data);

Acked-by: Pascal Mazon <pascal.mazon@6wind.com>

[PATCH 4/5] net/tap: fix up log message to correct channel

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 39 +++++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 6673182..4f7eacf 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -167,7 +167,7 @@ tun_alloc(char *name, uint16_t qid)
 
 		/* Set the TUN/TAP configuration and get the name if needed */
 		if (ioctl(fd, TUNSETIFF, (void *)&ifr) < 0) {
-			RTE_LOG(ERR, PMD, "Unable to set TUNSETIFF for %s\n",
+			RTE_LOG(WARNING, PMD, "Unable to set TUNSETIFF for %s\n",
 				ifr.ifr_name);
 			perror("TUNSETIFF");
 			goto error;
@@ -206,7 +206,7 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 		/* allocate the next mbuf */
 		mbuf = rte_pktmbuf_alloc(rxq->mp);
 		if (unlikely(!mbuf)) {
-			RTE_LOG(WARNING, PMD, "Unable to allocate mbuf\n");
+			RTE_LOG(WARNING, PMD, "TAP unable to allocate mbuf\n");
 			break;
 		}
 
@@ -559,8 +559,8 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
 	int fd;
 
 	if ((rx_queue_id >= internals->nb_queues) || !mp) {
-		RTE_LOG(ERR, PMD, "nb_queues %d mp %p\n",
-			internals->nb_queues, mp);
+		RTE_LOG(WARNING, PMD, "nb_queues %d too small or mempool NULL\n",
+			internals->nb_queues);
 		return -1;
 	}
 
@@ -572,7 +572,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
 				RTE_PKTMBUF_HEADROOM);
 
 	if (buf_size < ETH_FRAME_LEN) {
-		RTE_LOG(ERR, PMD,
+		RTE_LOG(WARNING, PMD,
 			"%s: %d bytes will not fit in mbuf (%d bytes)\n",
 			dev->data->name, ETH_FRAME_LEN, buf_size);
 		return -ENOMEM;
@@ -582,7 +582,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
 	if (fd == -1)
 		return -1;
 
-	RTE_LOG(INFO, PMD, "RX TAP device name %s, qid %d on fd %d\n",
+	RTE_LOG(DEBUG, PMD, "  RX TAP device name %s, qid %d on fd %d\n",
 		internals->name, rx_queue_id, internals->rxq[rx_queue_id].fd);
 
 	return 0;
@@ -605,7 +605,7 @@ tap_tx_queue_setup(struct rte_eth_dev *dev,
 	if (ret == -1)
 		return -1;
 
-	RTE_LOG(INFO, PMD, "TX TAP device name %s, qid %d on fd %d\n",
+	RTE_LOG(DEBUG, PMD, "  TX TAP device name %s, qid %d on fd %d\n",
 		internals->name, tx_queue_id, internals->txq[tx_queue_id].fd);
 
 	return 0;
@@ -643,7 +643,7 @@ pmd_mac_address(int fd, struct rte_eth_dev *dev, struct ether_addr *addr)
 	memset(&ifr, 0, sizeof(ifr));
 
 	if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
-		RTE_LOG(ERR, PMD, "ioctl failed (SIOCGIFHWADDR) (%s)\n",
+		RTE_LOG(WARNING, PMD, "ioctl failed (SIOCGIFHWADDR) (%s)\n",
 			ifr.ifr_name);
 		return -1;
 	}
@@ -662,26 +662,25 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	struct rte_eth_dev_data *data = NULL;
 	int i, fd = -1;
 
-	RTE_LOG(INFO, PMD,
-		"%s: Create TAP Ethernet device with %d queues on numa %u\n",
-		 name, RTE_PMD_TAP_MAX_QUEUES, rte_socket_id());
+	RTE_LOG(DEBUG, PMD, "  TAP device with %d queues on numa %u\n",
+		 RTE_PMD_TAP_MAX_QUEUES, rte_socket_id());
 
 	data = rte_zmalloc_socket(tap_name, sizeof(*data), 0, numa_node);
 	if (!data) {
-		RTE_LOG(ERR, PMD, "Failed to allocate data\n");
+		RTE_LOG(ERR, PMD, "TAP Failed to allocate data\n");
 		goto error_exit;
 	}
 
 	pmd = rte_zmalloc_socket(tap_name, sizeof(*pmd), 0, numa_node);
 	if (!pmd) {
-		RTE_LOG(ERR, PMD, "Unable to allocate internal struct\n");
+		RTE_LOG(ERR, PMD, "TAP Unable to allocate internal struct\n");
 		goto error_exit;
 	}
 
 	/* Use the name and not the tap_name */
 	dev = rte_eth_dev_allocate(tap_name);
 	if (!dev) {
-		RTE_LOG(ERR, PMD, "Unable to allocate device struct\n");
+		RTE_LOG(ERR, PMD, "TAP Unable to allocate device struct\n");
 		goto error_exit;
 	}
 
@@ -712,7 +711,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	/* Create the first Tap device */
 	fd = tun_alloc(tap_name, 0);
 	if (fd < 0) {
-		RTE_LOG(ERR, PMD, "tun_alloc() failed\n");
+		RTE_LOG(ERR, PMD, "TAP tun_alloc() failed for %s\n", tap_name);
 		goto error_exit;
 	}
 
@@ -734,7 +733,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	return 0;
 
 error_exit:
-	RTE_PMD_DEBUG_TRACE("Unable to initialize %s\n", name);
+	RTE_LOG(DEBUG, PMD, "TAP Unable to initialize %s\n", name);
 
 	if (fd > 0)
 		close(fd);
@@ -787,7 +786,7 @@ rte_pmd_tap_probe(const char *name, const char *params)
 		 DEFAULT_TAP_NAME, tap_unit++);
 
 	if (params && (params[0] != '\0')) {
-		RTE_LOG(INFO, PMD, "paramaters (%s)\n", params);
+		RTE_LOG(DEBUG, PMD, "paramaters (%s)\n", params);
 
 		kvlist = rte_kvargs_parse(params, valid_arguments);
 		if (kvlist) {
@@ -812,14 +811,14 @@ rte_pmd_tap_probe(const char *name, const char *params)
 	}
 	pmd_link.link_speed = speed;
 
-	RTE_LOG(INFO, PMD, "Initializing pmd_tap for %s as %s\n",
+	RTE_LOG(NOTICE, PMD, "Initializing pmd_tap for %s as %s\n",
 		name, tap_name);
 
 	ret = eth_dev_tap_create(name, tap_name);
 
 leave:
 	if (ret == -1) {
-		RTE_LOG(INFO, PMD, "Failed to create pmd for %s as %s\n",
+		RTE_LOG(ERR, PMD, "Failed to create pmd for %s as %s\n",
 			name, tap_name);
 		tap_unit--;		/* Restore the unit number */
 	}
@@ -837,7 +836,7 @@ rte_pmd_tap_remove(const char *name)
 	struct pmd_internals *internals;
 	int i;
 
-	RTE_LOG(INFO, PMD, "Closing TUN/TAP Ethernet device on numa %u\n",
+	RTE_LOG(DEBUG, PMD, "Closing TUN/TAP Ethernet device on numa %u\n",
 		rte_socket_id());
 
 	/* find the ethdev entry */
-- 
2.8.0.GIT

Re: [PATCH 4/5] net/tap: fix up log message to correct channel

[Pascal Mazon]

On Thu,  2 Feb 2017 16:33:29 -0600
Keith Wiles <keith.wiles@intel.com> wrote:

> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
>  drivers/net/tap/rte_eth_tap.c | 39 +++++++++++++++++++--------------------
>  1 file changed, 19 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 6673182..4f7eacf 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -167,7 +167,7 @@ tun_alloc(char *name, uint16_t qid)
>  
>  		/* Set the TUN/TAP configuration and get the name if needed
> */ if (ioctl(fd, TUNSETIFF, (void *)&ifr) < 0) {
> -			RTE_LOG(ERR, PMD, "Unable to set TUNSETIFF for %s\n",
> +			RTE_LOG(WARNING, PMD, "Unable to set TUNSETIFF for
> %s\n", ifr.ifr_name);
>  			perror("TUNSETIFF");
>  			goto error;
> @@ -206,7 +206,7 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs,
> uint16_t nb_pkts) /* allocate the next mbuf */
>  		mbuf = rte_pktmbuf_alloc(rxq->mp);
>  		if (unlikely(!mbuf)) {
> -			RTE_LOG(WARNING, PMD, "Unable to allocate mbuf\n");
> +			RTE_LOG(WARNING, PMD, "TAP unable to allocate
> mbuf\n"); break;
>  		}
>  
> @@ -559,8 +559,8 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
>  	int fd;
>  
>  	if ((rx_queue_id >= internals->nb_queues) || !mp) {
> -		RTE_LOG(ERR, PMD, "nb_queues %d mp %p\n",
> -			internals->nb_queues, mp);
> +		RTE_LOG(WARNING, PMD, "nb_queues %d too small or mempool
> NULL\n",
> +			internals->nb_queues);
>  		return -1;
>  	}
>  
> @@ -572,7 +572,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
>  				RTE_PKTMBUF_HEADROOM);
>  
>  	if (buf_size < ETH_FRAME_LEN) {
> -		RTE_LOG(ERR, PMD,
> +		RTE_LOG(WARNING, PMD,
>  			"%s: %d bytes will not fit in mbuf (%d bytes)\n",
>  			dev->data->name, ETH_FRAME_LEN, buf_size);
>  		return -ENOMEM;
> @@ -582,7 +582,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
>  	if (fd == -1)
>  		return -1;
>  
> -	RTE_LOG(INFO, PMD, "RX TAP device name %s, qid %d on fd %d\n",
> +	RTE_LOG(DEBUG, PMD, "  RX TAP device name %s, qid %d on fd %d\n",
>  		internals->name, rx_queue_id,
> internals->rxq[rx_queue_id].fd); 
>  	return 0;
> @@ -605,7 +605,7 @@ tap_tx_queue_setup(struct rte_eth_dev *dev,
>  	if (ret == -1)
>  		return -1;
>  
> -	RTE_LOG(INFO, PMD, "TX TAP device name %s, qid %d on fd %d\n",
> +	RTE_LOG(DEBUG, PMD, "  TX TAP device name %s, qid %d on fd %d\n",
>  		internals->name, tx_queue_id,
> internals->txq[tx_queue_id].fd); 
>  	return 0;
> @@ -643,7 +643,7 @@ pmd_mac_address(int fd, struct rte_eth_dev *dev, struct
> ether_addr *addr) memset(&ifr, 0, sizeof(ifr));
>  
>  	if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
> -		RTE_LOG(ERR, PMD, "ioctl failed (SIOCGIFHWADDR) (%s)\n",
> +		RTE_LOG(WARNING, PMD, "ioctl failed (SIOCGIFHWADDR) (%s)\n",
>  			ifr.ifr_name);
>  		return -1;
>  	}
> @@ -662,26 +662,25 @@ eth_dev_tap_create(const char *name, char *tap_name)
>  	struct rte_eth_dev_data *data = NULL;
>  	int i, fd = -1;
>  
> -	RTE_LOG(INFO, PMD,
> -		"%s: Create TAP Ethernet device with %d queues on numa %u\n",
> -		 name, RTE_PMD_TAP_MAX_QUEUES, rte_socket_id());
> +	RTE_LOG(DEBUG, PMD, "  TAP device with %d queues on numa %u\n",
> +		 RTE_PMD_TAP_MAX_QUEUES, rte_socket_id());
>  
>  	data = rte_zmalloc_socket(tap_name, sizeof(*data), 0, numa_node);
>  	if (!data) {
> -		RTE_LOG(ERR, PMD, "Failed to allocate data\n");
> +		RTE_LOG(ERR, PMD, "TAP Failed to allocate data\n");
>  		goto error_exit;
>  	}
>  
>  	pmd = rte_zmalloc_socket(tap_name, sizeof(*pmd), 0, numa_node);
>  	if (!pmd) {
> -		RTE_LOG(ERR, PMD, "Unable to allocate internal struct\n");
> +		RTE_LOG(ERR, PMD, "TAP Unable to allocate internal
> struct\n"); goto error_exit;
>  	}
>  
>  	/* Use the name and not the tap_name */
>  	dev = rte_eth_dev_allocate(tap_name);
>  	if (!dev) {
> -		RTE_LOG(ERR, PMD, "Unable to allocate device struct\n");
> +		RTE_LOG(ERR, PMD, "TAP Unable to allocate device struct\n");
>  		goto error_exit;
>  	}
>  
> @@ -712,7 +711,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
>  	/* Create the first Tap device */
>  	fd = tun_alloc(tap_name, 0);
>  	if (fd < 0) {
> -		RTE_LOG(ERR, PMD, "tun_alloc() failed\n");
> +		RTE_LOG(ERR, PMD, "TAP tun_alloc() failed for %s\n",
> tap_name); goto error_exit;
>  	}
>  
> @@ -734,7 +733,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
>  	return 0;
>  
>  error_exit:
> -	RTE_PMD_DEBUG_TRACE("Unable to initialize %s\n", name);
> +	RTE_LOG(DEBUG, PMD, "TAP Unable to initialize %s\n", name);
>  
>  	if (fd > 0)
>  		close(fd);
> @@ -787,7 +786,7 @@ rte_pmd_tap_probe(const char *name, const char *params)
>  		 DEFAULT_TAP_NAME, tap_unit++);
>  
>  	if (params && (params[0] != '\0')) {
> -		RTE_LOG(INFO, PMD, "paramaters (%s)\n", params);
> +		RTE_LOG(DEBUG, PMD, "paramaters (%s)\n", params);
>  
>  		kvlist = rte_kvargs_parse(params, valid_arguments);
>  		if (kvlist) {
> @@ -812,14 +811,14 @@ rte_pmd_tap_probe(const char *name, const char *params)
>  	}
>  	pmd_link.link_speed = speed;
>  
> -	RTE_LOG(INFO, PMD, "Initializing pmd_tap for %s as %s\n",
> +	RTE_LOG(NOTICE, PMD, "Initializing pmd_tap for %s as %s\n",
>  		name, tap_name);
>  
>  	ret = eth_dev_tap_create(name, tap_name);
>  
>  leave:
>  	if (ret == -1) {
> -		RTE_LOG(INFO, PMD, "Failed to create pmd for %s as %s\n",
> +		RTE_LOG(ERR, PMD, "Failed to create pmd for %s as %s\n",
>  			name, tap_name);
>  		tap_unit--;		/* Restore the unit number */
>  	}
> @@ -837,7 +836,7 @@ rte_pmd_tap_remove(const char *name)
>  	struct pmd_internals *internals;
>  	int i;
>  
> -	RTE_LOG(INFO, PMD, "Closing TUN/TAP Ethernet device on numa %u\n",
> +	RTE_LOG(DEBUG, PMD, "Closing TUN/TAP Ethernet device on numa %u\n",
>  		rte_socket_id());
>  
>  	/* find the ethdev entry */

Acked-by: Pascal Mazon <pascal.mazon@6wind.com>

[PATCH 5/5] net/tap: remove unused variable and minor cleanup

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 4f7eacf..238824e 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -390,9 +390,7 @@ tap_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *tap_stats)
 		tap_stats->q_ibytes[i] = pmd->rxq[i].stats.ibytes;
 		rx_total += tap_stats->q_ipackets[i];
 		rx_bytes_total += tap_stats->q_ibytes[i];
-	}
 
-	for (i = 0; i < imax; i++) {
 		tap_stats->q_opackets[i] = pmd->txq[i].stats.opackets;
 		tap_stats->q_errors[i] = pmd->txq[i].stats.errs;
 		tap_stats->q_obytes[i] = pmd->txq[i].stats.obytes;
@@ -417,9 +415,7 @@ tap_stats_reset(struct rte_eth_dev *dev)
 	for (i = 0; i < pmd->nb_queues; i++) {
 		pmd->rxq[i].stats.ipackets = 0;
 		pmd->rxq[i].stats.ibytes = 0;
-	}
 
-	for (i = 0; i < pmd->nb_queues; i++) {
 		pmd->txq[i].stats.opackets = 0;
 		pmd->txq[i].stats.errs = 0;
 		pmd->txq[i].stats.obytes = 0;
@@ -633,11 +629,11 @@ static const struct eth_dev_ops ops = {
 };
 
 static int
-pmd_mac_address(int fd, struct rte_eth_dev *dev, struct ether_addr *addr)
+pmd_mac_address(int fd, struct ether_addr *addr)
 {
 	struct ifreq ifr;
 
-	if ((fd <= 0) || !dev || !addr)
+	if ((fd <= 0) || !addr)
 		return -1;
 
 	memset(&ifr, 0, sizeof(ifr));
@@ -725,7 +721,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	pmd->rxq[0].fd = fd;
 	pmd->txq[0].fd = fd;
 
-	if (pmd_mac_address(fd, dev, &pmd->eth_addr) < 0) {
+	if (pmd_mac_address(fd, &pmd->eth_addr) < 0) {
 		RTE_LOG(ERR, PMD, "Unable to get MAC address\n");
 		goto error_exit;
 	}
-- 
2.8.0.GIT

Re: [PATCH 5/5] net/tap: remove unused variable and minor cleanup

[Pascal Mazon]

On Thu,  2 Feb 2017 16:33:30 -0600
Keith Wiles <keith.wiles@intel.com> wrote:

> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
>  drivers/net/tap/rte_eth_tap.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 4f7eacf..238824e 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -390,9 +390,7 @@ tap_stats_get(struct rte_eth_dev *dev, struct
> rte_eth_stats *tap_stats) tap_stats->q_ibytes[i] = pmd->rxq[i].stats.ibytes;
>  		rx_total += tap_stats->q_ipackets[i];
>  		rx_bytes_total += tap_stats->q_ibytes[i];
> -	}
>  
> -	for (i = 0; i < imax; i++) {
>  		tap_stats->q_opackets[i] = pmd->txq[i].stats.opackets;
>  		tap_stats->q_errors[i] = pmd->txq[i].stats.errs;
>  		tap_stats->q_obytes[i] = pmd->txq[i].stats.obytes;
> @@ -417,9 +415,7 @@ tap_stats_reset(struct rte_eth_dev *dev)
>  	for (i = 0; i < pmd->nb_queues; i++) {
>  		pmd->rxq[i].stats.ipackets = 0;
>  		pmd->rxq[i].stats.ibytes = 0;
> -	}
>  
> -	for (i = 0; i < pmd->nb_queues; i++) {
>  		pmd->txq[i].stats.opackets = 0;
>  		pmd->txq[i].stats.errs = 0;
>  		pmd->txq[i].stats.obytes = 0;
> @@ -633,11 +629,11 @@ static const struct eth_dev_ops ops = {
>  };
>  
>  static int
> -pmd_mac_address(int fd, struct rte_eth_dev *dev, struct ether_addr *addr)
> +pmd_mac_address(int fd, struct ether_addr *addr)
>  {
>  	struct ifreq ifr;
>  
> -	if ((fd <= 0) || !dev || !addr)
> +	if ((fd <= 0) || !addr)
>  		return -1;
>  
>  	memset(&ifr, 0, sizeof(ifr));
> @@ -725,7 +721,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
>  	pmd->rxq[0].fd = fd;
>  	pmd->txq[0].fd = fd;
>  
> -	if (pmd_mac_address(fd, dev, &pmd->eth_addr) < 0) {
> +	if (pmd_mac_address(fd, &pmd->eth_addr) < 0) {
>  		RTE_LOG(ERR, PMD, "Unable to get MAC address\n");
>  		goto error_exit;
>  	}

Acked-by: Pascal Mazon <pascal.mazon@6wind.com>

Re: [PATCH 1/5] nic/tap: fix tap docs for device name

[Pascal Mazon]

On Thu,  2 Feb 2017 16:33:26 -0600
Keith Wiles <keith.wiles@intel.com> wrote:

> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
>  doc/guides/nics/tap.rst | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
> index 622b9e7..2ab60ff 100644
> --- a/doc/guides/nics/tap.rst
> +++ b/doc/guides/nics/tap.rst
> @@ -45,18 +45,18 @@ device.
>  These TAP interfaces can be used with Wireshark or tcpdump or Pktgen-DPDK
>  along with being able to be used as a network connection to the DPDK
>  application. The method enable one or more interfaces is to use the
> -``--vdev=net_tap`` option on the DPDK application command line. Each
> -``--vdev=net_tap`` option give will create an interface named dtap0, dtap1,
> +``--vdev=net_tap0`` option on the DPDK application command line. Each
> +``--vdev=net_tap1`` option give will create an interface named dtap0, dtap1,
>  and so on.
>  
>  The interfaced name can be changed by adding the ``iface=foo0``, for
> example:: 
> -   --vdev=net_tap,iface=foo0 --vdev=net_tap,iface=foo1, ...
> +   --vdev=net_tap0,iface=foo0 --vdev=net_tap1,iface=foo1, ...
>  
>  Also the speed of the interface can be changed from 10G to whatever number
>  needed, but the interface does not enforce that speed, for example::
>  
> -   --vdev=net_tap,iface=foo0,speed=25000
> +   --vdev=net_tap0,iface=foo0,speed=25000
>  
>  After the DPDK application is started you can send and receive packets on the
>  interface using the standard rx_burst/tx_burst APIs in DPDK. From the host
> @@ -97,7 +97,7 @@ following::
>  
>      sudo ./app/app/x86_64-native-linuxapp-gcc/app/pktgen -l 1-5 -n 4        \
>       --proc-type auto --log-level 8 --socket-mem 512,512 --file-prefix pg   \
> -     --vdev=net_tap --vdev=net_tap -b 05:00.0 -b 05:00.1                    \
> +     --vdev=net_tap0 --vdev=net_tap1 -b 05:00.0 -b 05:00.1                  \
>       -b 04:00.0 -b 04:00.1 -b 04:00.2 -b 04:00.3                            \
>       -b 81:00.0 -b 81:00.1 -b 81:00.2 -b 81:00.3                            \
>       -b 82:00.0 -b 83:00.0 -- -T -P -m [2:3].0 -m [4:5].1                   \
> @@ -131,6 +131,6 @@ time with ``start all``. The command ``str`` is an alias
> for ``start all`` and 
>  While running you should see the 64 byte counters increasing to verify the
>  traffic is being looped back. You can use ``set all size XXX`` to change the
> -size of the packets after you stop the traffic. Use the pktgen ``help``
> +size of the packets after you stop the traffic. Use pktgen ``help``
>  command to see a list of all commands. You can also use the ``-f`` option to
> -load commands at startup.
> +load commands at startup in command line or Lua script in pktgen.

To be consistent. commit title should be "net/tap"

The patch looks ok to me, but while in the doc, could you fix the "The
interfaced name" into "The interface name"?

Regards,
Pascal

Re: [PATCH 1/5] nic/tap: fix tap docs for device name

[Ferruh Yigit]

On 2/3/2017 9:32 AM, Pascal Mazon wrote:
> On Thu,  2 Feb 2017 16:33:26 -0600
> Keith Wiles <keith.wiles@intel.com> wrote:
> 
>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>> ---
<...>
> 
> To be consistent. commit title should be "net/tap"
> 
> The patch looks ok to me, but while in the doc, could you fix the "The
> interfaced name" into "The interface name"?

Also checkpatch gives a few long line warnings.

Apart from that, series LGTM too.

> 
> Regards,
> Pascal
> 

[PATCH v2 1/6] net/tap: fix tap docs for device name

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 doc/guides/nics/tap.rst | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
index 622b9e7..c4f207b 100644
--- a/doc/guides/nics/tap.rst
+++ b/doc/guides/nics/tap.rst
@@ -45,18 +45,18 @@ device.
 These TAP interfaces can be used with Wireshark or tcpdump or Pktgen-DPDK
 along with being able to be used as a network connection to the DPDK
 application. The method enable one or more interfaces is to use the
-``--vdev=net_tap`` option on the DPDK application command line. Each
-``--vdev=net_tap`` option give will create an interface named dtap0, dtap1,
+``--vdev=net_tap0`` option on the DPDK application command line. Each
+``--vdev=net_tap1`` option give will create an interface named dtap0, dtap1,
 and so on.
 
-The interfaced name can be changed by adding the ``iface=foo0``, for example::
+The interface name can be changed by adding the ``iface=foo0``, for example::
 
-   --vdev=net_tap,iface=foo0 --vdev=net_tap,iface=foo1, ...
+   --vdev=net_tap0,iface=foo0 --vdev=net_tap1,iface=foo1, ...
 
 Also the speed of the interface can be changed from 10G to whatever number
 needed, but the interface does not enforce that speed, for example::
 
-   --vdev=net_tap,iface=foo0,speed=25000
+   --vdev=net_tap0,iface=foo0,speed=25000
 
 After the DPDK application is started you can send and receive packets on the
 interface using the standard rx_burst/tx_burst APIs in DPDK. From the host
@@ -97,7 +97,7 @@ following::
 
     sudo ./app/app/x86_64-native-linuxapp-gcc/app/pktgen -l 1-5 -n 4        \
      --proc-type auto --log-level 8 --socket-mem 512,512 --file-prefix pg   \
-     --vdev=net_tap --vdev=net_tap -b 05:00.0 -b 05:00.1                    \
+     --vdev=net_tap0 --vdev=net_tap1 -b 05:00.0 -b 05:00.1                  \
      -b 04:00.0 -b 04:00.1 -b 04:00.2 -b 04:00.3                            \
      -b 81:00.0 -b 81:00.1 -b 81:00.2 -b 81:00.3                            \
      -b 82:00.0 -b 83:00.0 -- -T -P -m [2:3].0 -m [4:5].1                   \
@@ -131,6 +131,6 @@ time with ``start all``. The command ``str`` is an alias for ``start all`` and
 
 While running you should see the 64 byte counters increasing to verify the
 traffic is being looped back. You can use ``set all size XXX`` to change the
-size of the packets after you stop the traffic. Use the pktgen ``help``
+size of the packets after you stop the traffic. Use pktgen ``help``
 command to see a list of all commands. You can also use the ``-f`` option to
-load commands at startup.
+load commands at startup in command line or Lua script in pktgen.
-- 
2.8.0.GIT

[PATCH v2 2/6] net/tap: remove redundant fds array

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index c7b04bb..6d93eb7 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -103,7 +103,6 @@ struct pmd_internals {
 	struct ether_addr eth_addr;	/* Mac address of the device port */
 
 	int if_index;			/* IF_INDEX for the port */
-	int fds[RTE_PMD_TAP_MAX_QUEUES]; /* List of all file descriptors */
 
 	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 */
@@ -349,8 +348,8 @@ tap_dev_stop(struct rte_eth_dev *dev)
 	struct pmd_internals *internals = dev->data->dev_private;
 
 	for (i = 0; i < internals->nb_queues; i++)
-		if (internals->fds[i] != -1)
-			close(internals->fds[i]);
+		if (internals->rxq[i].fd != -1)
+			close(internals->rxq[i].fd);
 	tap_link_set_down(dev);
 }
 
@@ -583,7 +582,6 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
 	if (fd == -1)
 		return -1;
 
-	internals->fds[rx_queue_id] = fd;
 	RTE_LOG(INFO, PMD, "RX TAP device name %s, qid %d on fd %d\n",
 		internals->name, rx_queue_id, internals->rxq[rx_queue_id].fd);
 
@@ -720,7 +718,6 @@ eth_dev_tap_create(const char *name, char *tap_name)
 
 	/* Presetup the fds to -1 as being not working */
 	for (i = 1; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
-		pmd->fds[i] = -1;
 		pmd->rxq[i].fd = -1;
 		pmd->txq[i].fd = -1;
 	}
@@ -728,7 +725,6 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	/* Take the TUN/TAP fd and place in the first location */
 	pmd->rxq[0].fd = fd;
 	pmd->txq[0].fd = fd;
-	pmd->fds[0] = fd;
 
 	if (pmd_mac_address(fd, dev, &pmd->eth_addr) < 0) {
 		RTE_LOG(ERR, PMD, "Unable to get MAC address\n");
@@ -849,8 +845,8 @@ rte_pmd_tap_remove(const char *name)
 
 	internals = eth_dev->data->dev_private;
 	for (i = 0; i < internals->nb_queues; i++)
-		if (internals->fds[i] != -1)
-			close(internals->fds[i]);
+		if (internals->rxq[i].fd != -1)
+			close(internals->rxq[i].fd);
 
 	rte_free(eth_dev->data->dev_private);
 	rte_free(eth_dev->data);
-- 
2.8.0.GIT

[PATCH v2 3/6] net/tap: remove unused variable and minor cleanup

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 6d93eb7..61659bc 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -389,9 +389,7 @@ tap_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *tap_stats)
 		tap_stats->q_ibytes[i] = pmd->rxq[i].stats.ibytes;
 		rx_total += tap_stats->q_ipackets[i];
 		rx_bytes_total += tap_stats->q_ibytes[i];
-	}
 
-	for (i = 0; i < imax; i++) {
 		tap_stats->q_opackets[i] = pmd->txq[i].stats.opackets;
 		tap_stats->q_errors[i] = pmd->txq[i].stats.errs;
 		tap_stats->q_obytes[i] = pmd->txq[i].stats.obytes;
@@ -416,9 +414,7 @@ tap_stats_reset(struct rte_eth_dev *dev)
 	for (i = 0; i < pmd->nb_queues; i++) {
 		pmd->rxq[i].stats.ipackets = 0;
 		pmd->rxq[i].stats.ibytes = 0;
-	}
 
-	for (i = 0; i < pmd->nb_queues; i++) {
 		pmd->txq[i].stats.opackets = 0;
 		pmd->txq[i].stats.errs = 0;
 		pmd->txq[i].stats.obytes = 0;
@@ -633,11 +629,11 @@ static const struct eth_dev_ops ops = {
 };
 
 static int
-pmd_mac_address(int fd, struct rte_eth_dev *dev, struct ether_addr *addr)
+pmd_mac_address(int fd, struct ether_addr *addr)
 {
 	struct ifreq ifr;
 
-	if ((fd <= 0) || !dev || !addr)
+	if ((fd <= 0) || !addr)
 		return -1;
 
 	memset(&ifr, 0, sizeof(ifr));
@@ -726,7 +722,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	pmd->rxq[0].fd = fd;
 	pmd->txq[0].fd = fd;
 
-	if (pmd_mac_address(fd, dev, &pmd->eth_addr) < 0) {
+	if (pmd_mac_address(fd, &pmd->eth_addr) < 0) {
 		RTE_LOG(ERR, PMD, "Unable to get MAC address\n");
 		goto error_exit;
 	}
-- 
2.8.0.GIT

[PATCH v2 4/6] net/tap: fix multi-queue support

[Keith Wiles]

At the same time remove the code which created the first device queue
at probe time. Now all queues are created during queue setup calls.

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 104 ++++++++++++++----------------------------
 1 file changed, 34 insertions(+), 70 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 61659bc..7c923a2 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -57,7 +57,11 @@
 #define ETH_TAP_IFACE_ARG       "iface"
 #define ETH_TAP_SPEED_ARG       "speed"
 
+#ifdef IFF_MULTI_QUEUE
 #define RTE_PMD_TAP_MAX_QUEUES	16
+#else
+#define RTE_PMD_TAP_MAX_QUEUES	1
+#endif
 
 static struct rte_vdev_driver pmd_tap_drv;
 
@@ -114,17 +118,20 @@ struct pmd_internals {
  * supplied name.
  */
 static int
-tun_alloc(char *name)
+tun_alloc(struct pmd_internals *pmd, uint16_t qid)
 {
 	struct ifreq ifr;
+#ifdef IFF_MULTI_QUEUE
 	unsigned int features;
+#endif
 	int fd;
 
 	memset(&ifr, 0, sizeof(struct ifreq));
 
 	ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
-	if (name && name[0])
-		strncpy(ifr.ifr_name, name, IFNAMSIZ);
+	strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
+
+	RTE_LOG(DEBUG, PMD, "ifr_name '%s'\n", ifr.ifr_name);
 
 	fd = open(TUN_TAP_DEV_PATH, O_RDWR);
 	if (fd < 0) {
@@ -132,37 +139,26 @@ tun_alloc(char *name)
 		goto error;
 	}
 
-	/* Grab the TUN features to verify we can work */
+#ifdef IFF_MULTI_QUEUE
+	/* Grab the TUN features to verify we can work multi-queue */
 	if (ioctl(fd, TUNGETFEATURES, &features) < 0) {
-		RTE_LOG(ERR, PMD, "Unable to get TUN/TAP features\n");
+		RTE_LOG(ERR, PMD, "TAP unable to get TUN/TAP features\n");
 		goto error;
 	}
-	RTE_LOG(DEBUG, PMD, "TUN/TAP Features %08x\n", features);
+	RTE_LOG(DEBUG, PMD, "  TAP Features %08x\n", features);
 
-#ifdef IFF_MULTI_QUEUE
-	if (!(features & IFF_MULTI_QUEUE) && (RTE_PMD_TAP_MAX_QUEUES > 1)) {
-		RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n");
-		goto error;
-	} else if ((features & IFF_ONE_QUEUE) &&
-			(RTE_PMD_TAP_MAX_QUEUES == 1)) {
-		ifr.ifr_flags |= IFF_ONE_QUEUE;
-		RTE_LOG(DEBUG, PMD, "Single queue only support\n");
-	} else {
-		ifr.ifr_flags |= IFF_MULTI_QUEUE;
-		RTE_LOG(DEBUG, PMD, "Multi-queue support for %d queues\n",
+	if (features & IFF_MULTI_QUEUE) {
+		RTE_LOG(DEBUG, PMD, "  Multi-queue support for %d queues\n",
 			RTE_PMD_TAP_MAX_QUEUES);
-	}
-#else
-	if (RTE_PMD_TAP_MAX_QUEUES > 1) {
-		RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n");
-		goto error;
-	} else {
+		ifr.ifr_flags |= IFF_MULTI_QUEUE;
+	} else
+#endif
+	{
 		ifr.ifr_flags |= IFF_ONE_QUEUE;
 		RTE_LOG(DEBUG, PMD, "Single queue only support\n");
 	}
-#endif
 
-	/* Set the TUN/TAP configuration and get the name if needed */
+	/* Set the TUN/TAP configuration and set the name if needed */
 	if (ioctl(fd, TUNSETIFF, (void *)&ifr) < 0) {
 		RTE_LOG(ERR, PMD, "Unable to set TUNSETIFF for %s\n",
 			ifr.ifr_name);
@@ -177,9 +173,15 @@ tun_alloc(char *name)
 		goto error;
 	}
 
-	/* If the name is different that new name as default */
-	if (name && strcmp(name, ifr.ifr_name))
-		snprintf(name, RTE_ETH_NAME_MAX_LEN - 1, "%s", ifr.ifr_name);
+	if (qid == 0) {
+		if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
+			RTE_LOG(ERR, PMD, "ioctl failed (SIOCGIFHWADDR) (%s)\n",
+				ifr.ifr_name);
+			goto error;
+		}
+
+		rte_memcpy(&pmd->eth_addr, ifr.ifr_hwaddr.sa_data, ETH_ALEN);
+	}
 
 	return fd;
 
@@ -507,7 +509,7 @@ tap_setup_queue(struct rte_eth_dev *dev,
 		if (fd < 0) {
 			RTE_LOG(INFO, PMD, "Add queue to TAP %s for qid %d\n",
 				pmd->name, qid);
-			fd = tun_alloc(pmd->name);
+			fd = tun_alloc(pmd, qid);
 			if (fd < 0) {
 				RTE_LOG(ERR, PMD, "tun_alloc(%s) failed\n",
 					pmd->name);
@@ -629,34 +631,13 @@ static const struct eth_dev_ops ops = {
 };
 
 static int
-pmd_mac_address(int fd, struct ether_addr *addr)
-{
-	struct ifreq ifr;
-
-	if ((fd <= 0) || !addr)
-		return -1;
-
-	memset(&ifr, 0, sizeof(ifr));
-
-	if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
-		RTE_LOG(ERR, PMD, "ioctl failed (SIOCGIFHWADDR) (%s)\n",
-			ifr.ifr_name);
-		return -1;
-	}
-
-	rte_memcpy(addr, ifr.ifr_hwaddr.sa_data, ETH_ALEN);
-
-	return 0;
-}
-
-static int
 eth_dev_tap_create(const char *name, char *tap_name)
 {
 	int numa_node = rte_socket_id();
 	struct rte_eth_dev *dev = NULL;
 	struct pmd_internals *pmd = NULL;
 	struct rte_eth_dev_data *data = NULL;
-	int i, fd = -1;
+	int i;
 
 	RTE_LOG(INFO, PMD,
 		"%s: Create TAP Ethernet device with %d queues on numa %u\n",
@@ -674,7 +655,6 @@ eth_dev_tap_create(const char *name, char *tap_name)
 		goto error_exit;
 	}
 
-	/* Use the name and not the tap_name */
 	dev = rte_eth_dev_allocate(tap_name);
 	if (!dev) {
 		RTE_LOG(ERR, PMD, "Unable to allocate device struct\n");
@@ -705,28 +685,12 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	dev->tx_pkt_burst = pmd_tx_burst;
 	snprintf(dev->data->name, sizeof(dev->data->name), "%s", name);
 
-	/* Create the first Tap device */
-	fd = tun_alloc(tap_name);
-	if (fd < 0) {
-		RTE_LOG(ERR, PMD, "tun_alloc() failed\n");
-		goto error_exit;
-	}
-
-	/* Presetup the fds to -1 as being not working */
-	for (i = 1; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
+	/* Presetup the fds to -1 as being not valid */
+	for (i = 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
 		pmd->rxq[i].fd = -1;
 		pmd->txq[i].fd = -1;
 	}
 
-	/* Take the TUN/TAP fd and place in the first location */
-	pmd->rxq[0].fd = fd;
-	pmd->txq[0].fd = fd;
-
-	if (pmd_mac_address(fd, &pmd->eth_addr) < 0) {
-		RTE_LOG(ERR, PMD, "Unable to get MAC address\n");
-		goto error_exit;
-	}
-
 	return 0;
 
 error_exit:
-- 
2.8.0.GIT

Re: [PATCH v2 4/6] net/tap: fix multi-queue support

[Pascal Mazon]

On Sun,  5 Feb 2017 10:05:07 -0600
Keith Wiles <keith.wiles@intel.com> wrote:

> At the same time remove the code which created the first device queue
> at probe time. Now all queues are created during queue setup calls.
> 
> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
>  drivers/net/tap/rte_eth_tap.c | 104
> ++++++++++++++---------------------------- 1 file changed, 34 insertions(+),
> 70 deletions(-)
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 61659bc..7c923a2 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -57,7 +57,11 @@
>  #define ETH_TAP_IFACE_ARG       "iface"
>  #define ETH_TAP_SPEED_ARG       "speed"
>  
> +#ifdef IFF_MULTI_QUEUE
>  #define RTE_PMD_TAP_MAX_QUEUES	16
> +#else
> +#define RTE_PMD_TAP_MAX_QUEUES	1
> +#endif
>  
>  static struct rte_vdev_driver pmd_tap_drv;
>  
> @@ -114,17 +118,20 @@ struct pmd_internals {
>   * supplied name.
>   */
>  static int
> -tun_alloc(char *name)
> +tun_alloc(struct pmd_internals *pmd, uint16_t qid)
>  {
>  	struct ifreq ifr;
> +#ifdef IFF_MULTI_QUEUE
>  	unsigned int features;
> +#endif
>  	int fd;
>  
>  	memset(&ifr, 0, sizeof(struct ifreq));
>  
>  	ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
> -	if (name && name[0])
> -		strncpy(ifr.ifr_name, name, IFNAMSIZ);
> +	strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
> +
> +	RTE_LOG(DEBUG, PMD, "ifr_name '%s'\n", ifr.ifr_name);
>  
>  	fd = open(TUN_TAP_DEV_PATH, O_RDWR);
>  	if (fd < 0) {
> @@ -132,37 +139,26 @@ tun_alloc(char *name)
>  		goto error;
>  	}
>  
> -	/* Grab the TUN features to verify we can work */
> +#ifdef IFF_MULTI_QUEUE
> +	/* Grab the TUN features to verify we can work multi-queue */
>  	if (ioctl(fd, TUNGETFEATURES, &features) < 0) {
> -		RTE_LOG(ERR, PMD, "Unable to get TUN/TAP features\n");
> +		RTE_LOG(ERR, PMD, "TAP unable to get TUN/TAP features\n");
>  		goto error;
>  	}
> -	RTE_LOG(DEBUG, PMD, "TUN/TAP Features %08x\n", features);
> +	RTE_LOG(DEBUG, PMD, "  TAP Features %08x\n", features);
>  
> -#ifdef IFF_MULTI_QUEUE
> -	if (!(features & IFF_MULTI_QUEUE) && (RTE_PMD_TAP_MAX_QUEUES > 1)) {
> -		RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n");
> -		goto error;
> -	} else if ((features & IFF_ONE_QUEUE) &&
> -			(RTE_PMD_TAP_MAX_QUEUES == 1)) {
> -		ifr.ifr_flags |= IFF_ONE_QUEUE;
> -		RTE_LOG(DEBUG, PMD, "Single queue only support\n");
> -	} else {
> -		ifr.ifr_flags |= IFF_MULTI_QUEUE;
> -		RTE_LOG(DEBUG, PMD, "Multi-queue support for %d queues\n",
> +	if (features & IFF_MULTI_QUEUE) {
> +		RTE_LOG(DEBUG, PMD, "  Multi-queue support for %d queues\n",
>  			RTE_PMD_TAP_MAX_QUEUES);
> -	}
> -#else
> -	if (RTE_PMD_TAP_MAX_QUEUES > 1) {
> -		RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n");
> -		goto error;
> -	} else {
> +		ifr.ifr_flags |= IFF_MULTI_QUEUE;
> +	} else
> +#endif
> +	{
>  		ifr.ifr_flags |= IFF_ONE_QUEUE;
>  		RTE_LOG(DEBUG, PMD, "Single queue only support\n");
>  	}
> -#endif
>  
> -	/* Set the TUN/TAP configuration and get the name if needed */
> +	/* Set the TUN/TAP configuration and set the name if needed */
>  	if (ioctl(fd, TUNSETIFF, (void *)&ifr) < 0) {
>  		RTE_LOG(ERR, PMD, "Unable to set TUNSETIFF for %s\n",
>  			ifr.ifr_name);
> @@ -177,9 +173,15 @@ tun_alloc(char *name)
>  		goto error;
>  	}
>  
> -	/* If the name is different that new name as default */
> -	if (name && strcmp(name, ifr.ifr_name))
> -		snprintf(name, RTE_ETH_NAME_MAX_LEN - 1, "%s", ifr.ifr_name);
> +	if (qid == 0) {
> +		if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
> +			RTE_LOG(ERR, PMD, "ioctl failed (SIOCGIFHWADDR)
> (%s)\n",
> +				ifr.ifr_name);
> +			goto error;
> +		}
> +
> +		rte_memcpy(&pmd->eth_addr, ifr.ifr_hwaddr.sa_data, ETH_ALEN);
> +	}
>  
>  	return fd;
>  
> @@ -507,7 +509,7 @@ tap_setup_queue(struct rte_eth_dev *dev,
>  		if (fd < 0) {
>  			RTE_LOG(INFO, PMD, "Add queue to TAP %s for qid
> %d\n", pmd->name, qid);
> -			fd = tun_alloc(pmd->name);
> +			fd = tun_alloc(pmd, qid);
>  			if (fd < 0) {
>  				RTE_LOG(ERR, PMD, "tun_alloc(%s) failed\n",
>  					pmd->name);
> @@ -629,34 +631,13 @@ static const struct eth_dev_ops ops = {
>  };
>  
>  static int
> -pmd_mac_address(int fd, struct ether_addr *addr)
> -{
> -	struct ifreq ifr;
> -
> -	if ((fd <= 0) || !addr)
> -		return -1;
> -
> -	memset(&ifr, 0, sizeof(ifr));
> -
> -	if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
> -		RTE_LOG(ERR, PMD, "ioctl failed (SIOCGIFHWADDR) (%s)\n",
> -			ifr.ifr_name);
> -		return -1;
> -	}
> -
> -	rte_memcpy(addr, ifr.ifr_hwaddr.sa_data, ETH_ALEN);
> -
> -	return 0;
> -}
> -
> -static int
>  eth_dev_tap_create(const char *name, char *tap_name)
>  {
>  	int numa_node = rte_socket_id();
>  	struct rte_eth_dev *dev = NULL;
>  	struct pmd_internals *pmd = NULL;
>  	struct rte_eth_dev_data *data = NULL;
> -	int i, fd = -1;
> +	int i;
>  
>  	RTE_LOG(INFO, PMD,
>  		"%s: Create TAP Ethernet device with %d queues on numa %u\n",
> @@ -674,7 +655,6 @@ eth_dev_tap_create(const char *name, char *tap_name)
>  		goto error_exit;
>  	}
>  
> -	/* Use the name and not the tap_name */
>  	dev = rte_eth_dev_allocate(tap_name);
>  	if (!dev) {
>  		RTE_LOG(ERR, PMD, "Unable to allocate device struct\n");
> @@ -705,28 +685,12 @@ eth_dev_tap_create(const char *name, char *tap_name)
>  	dev->tx_pkt_burst = pmd_tx_burst;
>  	snprintf(dev->data->name, sizeof(dev->data->name), "%s", name);
>  
> -	/* Create the first Tap device */
> -	fd = tun_alloc(tap_name);
> -	if (fd < 0) {
> -		RTE_LOG(ERR, PMD, "tun_alloc() failed\n");
> -		goto error_exit;
> -	}
> -
> -	/* Presetup the fds to -1 as being not working */
> -	for (i = 1; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
> +	/* Presetup the fds to -1 as being not valid */
> +	for (i = 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
>  		pmd->rxq[i].fd = -1;
>  		pmd->txq[i].fd = -1;
>  	}
>  
> -	/* Take the TUN/TAP fd and place in the first location */
> -	pmd->rxq[0].fd = fd;
> -	pmd->txq[0].fd = fd;
> -
> -	if (pmd_mac_address(fd, &pmd->eth_addr) < 0) {
> -		RTE_LOG(ERR, PMD, "Unable to get MAC address\n");
> -		goto error_exit;
> -	}
> -
>  	return 0;
>  
>  error_exit:

I like that you use #ifdef IFF_MULTI_QUEUE, it's a good thing.

Something bothers me, though.
I don't think you should close all fds in the dev_stop() function, but
rather in the dev_close() function.
After a dev_close(), we don't expect the user to re-use the device, so
we can safely remove the queues. However, after a dev_stop(), the user
can definitely do a dev_start(), which in your case would fail because
it will try to set the link up on an inexisting device.

dev_stop() should act as dev_start(), in symmetry, and dev_close()
should close the fd/queues.

The rest of the code looks OK to me.

Best regards,
Pascal

Re: [PATCH v2 4/6] net/tap: fix multi-queue support

[Wiles, Keith]

> On Feb 6, 2017, at 9:45 AM, Pascal Mazon <pascal.mazon@6wind.com> wrote:
> 
> On Sun,  5 Feb 2017 10:05:07 -0600
> Keith Wiles <keith.wiles@intel.com> wrote:
> 
>> At the same time remove the code which created the first device queue
>> at probe time. Now all queues are created during queue setup calls.
>> 
>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>> ---
>> drivers/net/tap/rte_eth_tap.c | 104
>> ++++++++++++++---------------------------- 1 file changed, 34 insertions(+),
>> 70 deletions(-)
>> 
>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>> index 61659bc..7c923a2 100644
>> --- a/drivers/net/tap/rte_eth_tap.c
>> +++ b/drivers/net/tap/rte_eth_tap.c
>> @@ -57,7 +57,11 @@
>> #define ETH_TAP_IFACE_ARG       "iface"
>> #define ETH_TAP_SPEED_ARG       "speed"
>> 
>> +#ifdef IFF_MULTI_QUEUE
>> #define RTE_PMD_TAP_MAX_QUEUES	16
>> +#else
>> +#define RTE_PMD_TAP_MAX_QUEUES	1
>> +#endif
>> 
>> static struct rte_vdev_driver pmd_tap_drv;
>> 
>> @@ -114,17 +118,20 @@ struct pmd_internals {
>>  * supplied name.
>>  */
>> static int
>> -tun_alloc(char *name)
>> +tun_alloc(struct pmd_internals *pmd, uint16_t qid)
>> {
>> 	struct ifreq ifr;
>> +#ifdef IFF_MULTI_QUEUE
>> 	unsigned int features;
>> +#endif
>> 	int fd;
>> 
>> 	memset(&ifr, 0, sizeof(struct ifreq));
>> 
>> 	ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
>> -	if (name && name[0])
>> -		strncpy(ifr.ifr_name, name, IFNAMSIZ);
>> +	strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
>> +
>> +	RTE_LOG(DEBUG, PMD, "ifr_name '%s'\n", ifr.ifr_name);
>> 
>> 	fd = open(TUN_TAP_DEV_PATH, O_RDWR);
>> 	if (fd < 0) {
>> @@ -132,37 +139,26 @@ tun_alloc(char *name)
>> 		goto error;
>> 	}
>> 
>> -	/* Grab the TUN features to verify we can work */
>> +#ifdef IFF_MULTI_QUEUE
>> +	/* Grab the TUN features to verify we can work multi-queue */
>> 	if (ioctl(fd, TUNGETFEATURES, &features) < 0) {
>> -		RTE_LOG(ERR, PMD, "Unable to get TUN/TAP features\n");
>> +		RTE_LOG(ERR, PMD, "TAP unable to get TUN/TAP features\n");
>> 		goto error;
>> 	}
>> -	RTE_LOG(DEBUG, PMD, "TUN/TAP Features %08x\n", features);
>> +	RTE_LOG(DEBUG, PMD, "  TAP Features %08x\n", features);
>> 
>> -#ifdef IFF_MULTI_QUEUE
>> -	if (!(features & IFF_MULTI_QUEUE) && (RTE_PMD_TAP_MAX_QUEUES > 1)) {
>> -		RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n");
>> -		goto error;
>> -	} else if ((features & IFF_ONE_QUEUE) &&
>> -			(RTE_PMD_TAP_MAX_QUEUES == 1)) {
>> -		ifr.ifr_flags |= IFF_ONE_QUEUE;
>> -		RTE_LOG(DEBUG, PMD, "Single queue only support\n");
>> -	} else {
>> -		ifr.ifr_flags |= IFF_MULTI_QUEUE;
>> -		RTE_LOG(DEBUG, PMD, "Multi-queue support for %d queues\n",
>> +	if (features & IFF_MULTI_QUEUE) {
>> +		RTE_LOG(DEBUG, PMD, "  Multi-queue support for %d queues\n",
>> 			RTE_PMD_TAP_MAX_QUEUES);
>> -	}
>> -#else
>> -	if (RTE_PMD_TAP_MAX_QUEUES > 1) {
>> -		RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n");
>> -		goto error;
>> -	} else {
>> +		ifr.ifr_flags |= IFF_MULTI_QUEUE;
>> +	} else
>> +#endif
>> +	{
>> 		ifr.ifr_flags |= IFF_ONE_QUEUE;
>> 		RTE_LOG(DEBUG, PMD, "Single queue only support\n");
>> 	}
>> -#endif
>> 
>> -	/* Set the TUN/TAP configuration and get the name if needed */
>> +	/* Set the TUN/TAP configuration and set the name if needed */
>> 	if (ioctl(fd, TUNSETIFF, (void *)&ifr) < 0) {
>> 		RTE_LOG(ERR, PMD, "Unable to set TUNSETIFF for %s\n",
>> 			ifr.ifr_name);
>> @@ -177,9 +173,15 @@ tun_alloc(char *name)
>> 		goto error;
>> 	}
>> 
>> -	/* If the name is different that new name as default */
>> -	if (name && strcmp(name, ifr.ifr_name))
>> -		snprintf(name, RTE_ETH_NAME_MAX_LEN - 1, "%s", ifr.ifr_name);
>> +	if (qid == 0) {
>> +		if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
>> +			RTE_LOG(ERR, PMD, "ioctl failed (SIOCGIFHWADDR)
>> (%s)\n",
>> +				ifr.ifr_name);
>> +			goto error;
>> +		}
>> +
>> +		rte_memcpy(&pmd->eth_addr, ifr.ifr_hwaddr.sa_data, ETH_ALEN);
>> +	}
>> 
>> 	return fd;
>> 
>> @@ -507,7 +509,7 @@ tap_setup_queue(struct rte_eth_dev *dev,
>> 		if (fd < 0) {
>> 			RTE_LOG(INFO, PMD, "Add queue to TAP %s for qid
>> %d\n", pmd->name, qid);
>> -			fd = tun_alloc(pmd->name);
>> +			fd = tun_alloc(pmd, qid);
>> 			if (fd < 0) {
>> 				RTE_LOG(ERR, PMD, "tun_alloc(%s) failed\n",
>> 					pmd->name);
>> @@ -629,34 +631,13 @@ static const struct eth_dev_ops ops = {
>> };
>> 
>> static int
>> -pmd_mac_address(int fd, struct ether_addr *addr)
>> -{
>> -	struct ifreq ifr;
>> -
>> -	if ((fd <= 0) || !addr)
>> -		return -1;
>> -
>> -	memset(&ifr, 0, sizeof(ifr));
>> -
>> -	if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
>> -		RTE_LOG(ERR, PMD, "ioctl failed (SIOCGIFHWADDR) (%s)\n",
>> -			ifr.ifr_name);
>> -		return -1;
>> -	}
>> -
>> -	rte_memcpy(addr, ifr.ifr_hwaddr.sa_data, ETH_ALEN);
>> -
>> -	return 0;
>> -}
>> -
>> -static int
>> eth_dev_tap_create(const char *name, char *tap_name)
>> {
>> 	int numa_node = rte_socket_id();
>> 	struct rte_eth_dev *dev = NULL;
>> 	struct pmd_internals *pmd = NULL;
>> 	struct rte_eth_dev_data *data = NULL;
>> -	int i, fd = -1;
>> +	int i;
>> 
>> 	RTE_LOG(INFO, PMD,
>> 		"%s: Create TAP Ethernet device with %d queues on numa %u\n",
>> @@ -674,7 +655,6 @@ eth_dev_tap_create(const char *name, char *tap_name)
>> 		goto error_exit;
>> 	}
>> 
>> -	/* Use the name and not the tap_name */
>> 	dev = rte_eth_dev_allocate(tap_name);
>> 	if (!dev) {
>> 		RTE_LOG(ERR, PMD, "Unable to allocate device struct\n");
>> @@ -705,28 +685,12 @@ eth_dev_tap_create(const char *name, char *tap_name)
>> 	dev->tx_pkt_burst = pmd_tx_burst;
>> 	snprintf(dev->data->name, sizeof(dev->data->name), "%s", name);
>> 
>> -	/* Create the first Tap device */
>> -	fd = tun_alloc(tap_name);
>> -	if (fd < 0) {
>> -		RTE_LOG(ERR, PMD, "tun_alloc() failed\n");
>> -		goto error_exit;
>> -	}
>> -
>> -	/* Presetup the fds to -1 as being not working */
>> -	for (i = 1; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
>> +	/* Presetup the fds to -1 as being not valid */
>> +	for (i = 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
>> 		pmd->rxq[i].fd = -1;
>> 		pmd->txq[i].fd = -1;
>> 	}
>> 
>> -	/* Take the TUN/TAP fd and place in the first location */
>> -	pmd->rxq[0].fd = fd;
>> -	pmd->txq[0].fd = fd;
>> -
>> -	if (pmd_mac_address(fd, &pmd->eth_addr) < 0) {
>> -		RTE_LOG(ERR, PMD, "Unable to get MAC address\n");
>> -		goto error_exit;
>> -	}
>> -
>> 	return 0;
>> 
>> error_exit:
> 
> I like that you use #ifdef IFF_MULTI_QUEUE, it's a good thing.
> 
> Something bothers me, though.
> I don't think you should close all fds in the dev_stop() function, but
> rather in the dev_close() function.
> After a dev_close(), we don't expect the user to re-use the device, so
> we can safely remove the queues. However, after a dev_stop(), the user
> can definitely do a dev_start(), which in your case would fail because
> it will try to set the link up on an inexisting device.
> 
> dev_stop() should act as dev_start(), in symmetry, and dev_close()
> should close the fd/queues.

That is a good point should I create v3 or do a patch for that after this release?

> 
> The rest of the code looks OK to me.
> 
> Best regards,
> Pascal

Regards,
Keith

[PATCH v2 5/6] net/tap: cleanup log messages

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 54 ++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 7c923a2..65e4bab 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -155,12 +155,13 @@ tun_alloc(struct pmd_internals *pmd, uint16_t qid)
 #endif
 	{
 		ifr.ifr_flags |= IFF_ONE_QUEUE;
-		RTE_LOG(DEBUG, PMD, "Single queue only support\n");
+		RTE_LOG(DEBUG, PMD, "  Single queue only support\n");
 	}
 
 	/* Set the TUN/TAP configuration and set the name if needed */
 	if (ioctl(fd, TUNSETIFF, (void *)&ifr) < 0) {
-		RTE_LOG(ERR, PMD, "Unable to set TUNSETIFF for %s\n",
+		RTE_LOG(WARNING, PMD,
+			"Unable to set TUNSETIFF for %s\n",
 			ifr.ifr_name);
 		perror("TUNSETIFF");
 		goto error;
@@ -168,7 +169,9 @@ tun_alloc(struct pmd_internals *pmd, uint16_t qid)
 
 	/* Always set the file descriptor to non-blocking */
 	if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
-		RTE_LOG(ERR, PMD, "Unable to set to nonblocking\n");
+		RTE_LOG(WARNING, PMD,
+			"Unable to set %s to nonblocking\n",
+			ifr.ifr_name);
 		perror("F_SETFL, NONBLOCK");
 		goto error;
 	}
@@ -207,7 +210,7 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 		/* allocate the next mbuf */
 		mbuf = rte_pktmbuf_alloc(rxq->mp);
 		if (unlikely(!mbuf)) {
-			RTE_LOG(WARNING, PMD, "Unable to allocate mbuf\n");
+			RTE_LOG(WARNING, PMD, "TAP unable to allocate mbuf\n");
 			break;
 		}
 
@@ -276,7 +279,8 @@ pmd_tx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	return num_tx;
 }
 
-static int tap_link_set_flags(struct pmd_internals *pmd, short flags, int add)
+static int
+tap_link_set_flags(struct pmd_internals *pmd, short flags, int add)
 {
 	struct ifreq ifr;
 	int err, s;
@@ -296,8 +300,8 @@ static int tap_link_set_flags(struct pmd_internals *pmd, short flags, int add)
 	strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
 	err = ioctl(s, SIOCGIFFLAGS, &ifr);
 	if (err < 0) {
-		RTE_LOG(ERR, PMD, "Unable to get tap netdevice flags: %s\n",
-			strerror(errno));
+		RTE_LOG(WARNING, PMD, "Unable to get %s device flags: %s\n",
+			pmd->name, strerror(errno));
 		close(s);
 		return -1;
 	}
@@ -307,7 +311,7 @@ static int tap_link_set_flags(struct pmd_internals *pmd, short flags, int add)
 		ifr.ifr_flags &= ~flags;
 	err = ioctl(s, SIOCSIFFLAGS, &ifr);
 	if (err < 0) {
-		RTE_LOG(ERR, PMD, "Unable to %s flags 0x%x: %s\n",
+		RTE_LOG(WARNING, PMD, "Unable to %s flags 0x%x: %s\n",
 			add ? "set" : "unset", flags, strerror(errno));
 		close(s);
 		return -1;
@@ -511,8 +515,8 @@ tap_setup_queue(struct rte_eth_dev *dev,
 				pmd->name, qid);
 			fd = tun_alloc(pmd, qid);
 			if (fd < 0) {
-				RTE_LOG(ERR, PMD, "tun_alloc(%s) failed\n",
-					pmd->name);
+				RTE_LOG(ERR, PMD, "tun_alloc(%s, %d) failed\n",
+					pmd->name, qid);
 				return -1;
 			}
 		}
@@ -557,8 +561,8 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
 	int fd;
 
 	if ((rx_queue_id >= internals->nb_queues) || !mp) {
-		RTE_LOG(ERR, PMD, "nb_queues %d mp %p\n",
-			internals->nb_queues, mp);
+		RTE_LOG(WARNING, PMD, "nb_queues %d too small or mempool NULL\n",
+			internals->nb_queues);
 		return -1;
 	}
 
@@ -570,7 +574,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
 				RTE_PKTMBUF_HEADROOM);
 
 	if (buf_size < ETH_FRAME_LEN) {
-		RTE_LOG(ERR, PMD,
+		RTE_LOG(WARNING, PMD,
 			"%s: %d bytes will not fit in mbuf (%d bytes)\n",
 			dev->data->name, ETH_FRAME_LEN, buf_size);
 		return -ENOMEM;
@@ -580,7 +584,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
 	if (fd == -1)
 		return -1;
 
-	RTE_LOG(INFO, PMD, "RX TAP device name %s, qid %d on fd %d\n",
+	RTE_LOG(DEBUG, PMD, "  RX TAP device name %s, qid %d on fd %d\n",
 		internals->name, rx_queue_id, internals->rxq[rx_queue_id].fd);
 
 	return 0;
@@ -603,7 +607,7 @@ tap_tx_queue_setup(struct rte_eth_dev *dev,
 	if (ret == -1)
 		return -1;
 
-	RTE_LOG(INFO, PMD, "TX TAP device name %s, qid %d on fd %d\n",
+	RTE_LOG(DEBUG, PMD, "  TX TAP device name %s, qid %d on fd %d\n",
 		internals->name, tx_queue_id, internals->txq[tx_queue_id].fd);
 
 	return 0;
@@ -639,25 +643,23 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	struct rte_eth_dev_data *data = NULL;
 	int i;
 
-	RTE_LOG(INFO, PMD,
-		"%s: Create TAP Ethernet device with %d queues on numa %u\n",
-		 name, RTE_PMD_TAP_MAX_QUEUES, rte_socket_id());
+	RTE_LOG(DEBUG, PMD, "  TAP device on numa %u\n", rte_socket_id());
 
 	data = rte_zmalloc_socket(tap_name, sizeof(*data), 0, numa_node);
 	if (!data) {
-		RTE_LOG(ERR, PMD, "Failed to allocate data\n");
+		RTE_LOG(ERR, PMD, "TAP Failed to allocate data\n");
 		goto error_exit;
 	}
 
 	pmd = rte_zmalloc_socket(tap_name, sizeof(*pmd), 0, numa_node);
 	if (!pmd) {
-		RTE_LOG(ERR, PMD, "Unable to allocate internal struct\n");
+		RTE_LOG(ERR, PMD, "TAP Unable to allocate internal struct\n");
 		goto error_exit;
 	}
 
 	dev = rte_eth_dev_allocate(tap_name);
 	if (!dev) {
-		RTE_LOG(ERR, PMD, "Unable to allocate device struct\n");
+		RTE_LOG(ERR, PMD, "TAP Unable to allocate device struct\n");
 		goto error_exit;
 	}
 
@@ -694,7 +696,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	return 0;
 
 error_exit:
-	RTE_PMD_DEBUG_TRACE("Unable to initialize %s\n", name);
+	RTE_LOG(DEBUG, PMD, "TAP Unable to initialize %s\n", name);
 
 	rte_free(data);
 	rte_free(pmd);
@@ -745,7 +747,7 @@ rte_pmd_tap_probe(const char *name, const char *params)
 		 DEFAULT_TAP_NAME, tap_unit++);
 
 	if (params && (params[0] != '\0')) {
-		RTE_LOG(INFO, PMD, "paramaters (%s)\n", params);
+		RTE_LOG(DEBUG, PMD, "paramaters (%s)\n", params);
 
 		kvlist = rte_kvargs_parse(params, valid_arguments);
 		if (kvlist) {
@@ -770,14 +772,14 @@ rte_pmd_tap_probe(const char *name, const char *params)
 	}
 	pmd_link.link_speed = speed;
 
-	RTE_LOG(INFO, PMD, "Initializing pmd_tap for %s as %s\n",
+	RTE_LOG(NOTICE, PMD, "Initializing pmd_tap for %s as %s\n",
 		name, tap_name);
 
 	ret = eth_dev_tap_create(name, tap_name);
 
 leave:
 	if (ret == -1) {
-		RTE_LOG(INFO, PMD, "Failed to create pmd for %s as %s\n",
+		RTE_LOG(ERR, PMD, "Failed to create pmd for %s as %s\n",
 			name, tap_name);
 		tap_unit--;		/* Restore the unit number */
 	}
@@ -795,7 +797,7 @@ rte_pmd_tap_remove(const char *name)
 	struct pmd_internals *internals;
 	int i;
 
-	RTE_LOG(INFO, PMD, "Closing TUN/TAP Ethernet device on numa %u\n",
+	RTE_LOG(DEBUG, PMD, "Closing TUN/TAP Ethernet device on numa %u\n",
 		rte_socket_id());
 
 	/* find the ethdev entry */
-- 
2.8.0.GIT

[PATCH v2 6/6] net/tap: link set down must be before close

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 65e4bab..966e91a 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -353,10 +353,11 @@ tap_dev_stop(struct rte_eth_dev *dev)
 	int i;
 	struct pmd_internals *internals = dev->data->dev_private;
 
+	tap_link_set_down(dev);
+
 	for (i = 0; i < internals->nb_queues; i++)
 		if (internals->rxq[i].fd != -1)
 			close(internals->rxq[i].fd);
-	tap_link_set_down(dev);
 }
 
 static int
-- 
2.8.0.GIT

Re: [PATCH v2 6/6] net/tap: link set down must be before close

[Pascal Mazon]

On Sun,  5 Feb 2017 10:05:09 -0600
Keith Wiles <keith.wiles@intel.com> wrote:

> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
>  drivers/net/tap/rte_eth_tap.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 65e4bab..966e91a 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -353,10 +353,11 @@ tap_dev_stop(struct rte_eth_dev *dev)
>  	int i;
>  	struct pmd_internals *internals = dev->data->dev_private;
>  
> +	tap_link_set_down(dev);
> +
>  	for (i = 0; i < internals->nb_queues; i++)
>  		if (internals->rxq[i].fd != -1)
>  			close(internals->rxq[i].fd);
> -	tap_link_set_down(dev);
>  }
>  
>  static int

There's a word missing in your commit title.
Otherwise the patch is absolutely necessary.

Could you add this line to your patch, please, for traceability?

Fixes: ee418a25b0d3 ("net/tap: implement link up and down callbacks")

Thanks for fixing my bug.

Regards,
Pascal

Re: [PATCH v2 6/6] net/tap: link set down must be before close

[Wiles, Keith]

> On Feb 6, 2017, at 9:57 AM, Pascal Mazon <pascal.mazon@6wind.com> wrote:
> 
> Fixes: ee418a25b0d3 ("net/tap: implement link up and down callbacks”)

As I need to add these changes I will produce a v3 patch

Regards,
Keith

[PATCH v3 1/7] net/tap: fix tap docs for device name

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 doc/guides/nics/tap.rst | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
index 622b9e7..c4f207b 100644
--- a/doc/guides/nics/tap.rst
+++ b/doc/guides/nics/tap.rst
@@ -45,18 +45,18 @@ device.
 These TAP interfaces can be used with Wireshark or tcpdump or Pktgen-DPDK
 along with being able to be used as a network connection to the DPDK
 application. The method enable one or more interfaces is to use the
-``--vdev=net_tap`` option on the DPDK application command line. Each
-``--vdev=net_tap`` option give will create an interface named dtap0, dtap1,
+``--vdev=net_tap0`` option on the DPDK application command line. Each
+``--vdev=net_tap1`` option give will create an interface named dtap0, dtap1,
 and so on.
 
-The interfaced name can be changed by adding the ``iface=foo0``, for example::
+The interface name can be changed by adding the ``iface=foo0``, for example::
 
-   --vdev=net_tap,iface=foo0 --vdev=net_tap,iface=foo1, ...
+   --vdev=net_tap0,iface=foo0 --vdev=net_tap1,iface=foo1, ...
 
 Also the speed of the interface can be changed from 10G to whatever number
 needed, but the interface does not enforce that speed, for example::
 
-   --vdev=net_tap,iface=foo0,speed=25000
+   --vdev=net_tap0,iface=foo0,speed=25000
 
 After the DPDK application is started you can send and receive packets on the
 interface using the standard rx_burst/tx_burst APIs in DPDK. From the host
@@ -97,7 +97,7 @@ following::
 
     sudo ./app/app/x86_64-native-linuxapp-gcc/app/pktgen -l 1-5 -n 4        \
      --proc-type auto --log-level 8 --socket-mem 512,512 --file-prefix pg   \
-     --vdev=net_tap --vdev=net_tap -b 05:00.0 -b 05:00.1                    \
+     --vdev=net_tap0 --vdev=net_tap1 -b 05:00.0 -b 05:00.1                  \
      -b 04:00.0 -b 04:00.1 -b 04:00.2 -b 04:00.3                            \
      -b 81:00.0 -b 81:00.1 -b 81:00.2 -b 81:00.3                            \
      -b 82:00.0 -b 83:00.0 -- -T -P -m [2:3].0 -m [4:5].1                   \
@@ -131,6 +131,6 @@ time with ``start all``. The command ``str`` is an alias for ``start all`` and
 
 While running you should see the 64 byte counters increasing to verify the
 traffic is being looped back. You can use ``set all size XXX`` to change the
-size of the packets after you stop the traffic. Use the pktgen ``help``
+size of the packets after you stop the traffic. Use pktgen ``help``
 command to see a list of all commands. You can also use the ``-f`` option to
-load commands at startup.
+load commands at startup in command line or Lua script in pktgen.
-- 
2.8.0.GIT

[PATCH v3 2/7] net/tap: remove redundant fds array

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index c7b04bb..6d93eb7 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -103,7 +103,6 @@ struct pmd_internals {
 	struct ether_addr eth_addr;	/* Mac address of the device port */
 
 	int if_index;			/* IF_INDEX for the port */
-	int fds[RTE_PMD_TAP_MAX_QUEUES]; /* List of all file descriptors */
 
 	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 */
@@ -349,8 +348,8 @@ tap_dev_stop(struct rte_eth_dev *dev)
 	struct pmd_internals *internals = dev->data->dev_private;
 
 	for (i = 0; i < internals->nb_queues; i++)
-		if (internals->fds[i] != -1)
-			close(internals->fds[i]);
+		if (internals->rxq[i].fd != -1)
+			close(internals->rxq[i].fd);
 	tap_link_set_down(dev);
 }
 
@@ -583,7 +582,6 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
 	if (fd == -1)
 		return -1;
 
-	internals->fds[rx_queue_id] = fd;
 	RTE_LOG(INFO, PMD, "RX TAP device name %s, qid %d on fd %d\n",
 		internals->name, rx_queue_id, internals->rxq[rx_queue_id].fd);
 
@@ -720,7 +718,6 @@ eth_dev_tap_create(const char *name, char *tap_name)
 
 	/* Presetup the fds to -1 as being not working */
 	for (i = 1; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
-		pmd->fds[i] = -1;
 		pmd->rxq[i].fd = -1;
 		pmd->txq[i].fd = -1;
 	}
@@ -728,7 +725,6 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	/* Take the TUN/TAP fd and place in the first location */
 	pmd->rxq[0].fd = fd;
 	pmd->txq[0].fd = fd;
-	pmd->fds[0] = fd;
 
 	if (pmd_mac_address(fd, dev, &pmd->eth_addr) < 0) {
 		RTE_LOG(ERR, PMD, "Unable to get MAC address\n");
@@ -849,8 +845,8 @@ rte_pmd_tap_remove(const char *name)
 
 	internals = eth_dev->data->dev_private;
 	for (i = 0; i < internals->nb_queues; i++)
-		if (internals->fds[i] != -1)
-			close(internals->fds[i]);
+		if (internals->rxq[i].fd != -1)
+			close(internals->rxq[i].fd);
 
 	rte_free(eth_dev->data->dev_private);
 	rte_free(eth_dev->data);
-- 
2.8.0.GIT

[PATCH v3 3/7] net/tap: remove unused variable and minor cleanup

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 6d93eb7..61659bc 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -389,9 +389,7 @@ tap_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *tap_stats)
 		tap_stats->q_ibytes[i] = pmd->rxq[i].stats.ibytes;
 		rx_total += tap_stats->q_ipackets[i];
 		rx_bytes_total += tap_stats->q_ibytes[i];
-	}
 
-	for (i = 0; i < imax; i++) {
 		tap_stats->q_opackets[i] = pmd->txq[i].stats.opackets;
 		tap_stats->q_errors[i] = pmd->txq[i].stats.errs;
 		tap_stats->q_obytes[i] = pmd->txq[i].stats.obytes;
@@ -416,9 +414,7 @@ tap_stats_reset(struct rte_eth_dev *dev)
 	for (i = 0; i < pmd->nb_queues; i++) {
 		pmd->rxq[i].stats.ipackets = 0;
 		pmd->rxq[i].stats.ibytes = 0;
-	}
 
-	for (i = 0; i < pmd->nb_queues; i++) {
 		pmd->txq[i].stats.opackets = 0;
 		pmd->txq[i].stats.errs = 0;
 		pmd->txq[i].stats.obytes = 0;
@@ -633,11 +629,11 @@ static const struct eth_dev_ops ops = {
 };
 
 static int
-pmd_mac_address(int fd, struct rte_eth_dev *dev, struct ether_addr *addr)
+pmd_mac_address(int fd, struct ether_addr *addr)
 {
 	struct ifreq ifr;
 
-	if ((fd <= 0) || !dev || !addr)
+	if ((fd <= 0) || !addr)
 		return -1;
 
 	memset(&ifr, 0, sizeof(ifr));
@@ -726,7 +722,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	pmd->rxq[0].fd = fd;
 	pmd->txq[0].fd = fd;
 
-	if (pmd_mac_address(fd, dev, &pmd->eth_addr) < 0) {
+	if (pmd_mac_address(fd, &pmd->eth_addr) < 0) {
 		RTE_LOG(ERR, PMD, "Unable to get MAC address\n");
 		goto error_exit;
 	}
-- 
2.8.0.GIT

[PATCH v3 4/7] net/tap: fix multi-queue support

[Keith Wiles]

At the same time remove the code which created the first device queue
at probe time. Now all queues are created during queue setup calls.

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 104 ++++++++++++++----------------------------
 1 file changed, 34 insertions(+), 70 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 61659bc..7c923a2 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -57,7 +57,11 @@
 #define ETH_TAP_IFACE_ARG       "iface"
 #define ETH_TAP_SPEED_ARG       "speed"
 
+#ifdef IFF_MULTI_QUEUE
 #define RTE_PMD_TAP_MAX_QUEUES	16
+#else
+#define RTE_PMD_TAP_MAX_QUEUES	1
+#endif
 
 static struct rte_vdev_driver pmd_tap_drv;
 
@@ -114,17 +118,20 @@ struct pmd_internals {
  * supplied name.
  */
 static int
-tun_alloc(char *name)
+tun_alloc(struct pmd_internals *pmd, uint16_t qid)
 {
 	struct ifreq ifr;
+#ifdef IFF_MULTI_QUEUE
 	unsigned int features;
+#endif
 	int fd;
 
 	memset(&ifr, 0, sizeof(struct ifreq));
 
 	ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
-	if (name && name[0])
-		strncpy(ifr.ifr_name, name, IFNAMSIZ);
+	strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
+
+	RTE_LOG(DEBUG, PMD, "ifr_name '%s'\n", ifr.ifr_name);
 
 	fd = open(TUN_TAP_DEV_PATH, O_RDWR);
 	if (fd < 0) {
@@ -132,37 +139,26 @@ tun_alloc(char *name)
 		goto error;
 	}
 
-	/* Grab the TUN features to verify we can work */
+#ifdef IFF_MULTI_QUEUE
+	/* Grab the TUN features to verify we can work multi-queue */
 	if (ioctl(fd, TUNGETFEATURES, &features) < 0) {
-		RTE_LOG(ERR, PMD, "Unable to get TUN/TAP features\n");
+		RTE_LOG(ERR, PMD, "TAP unable to get TUN/TAP features\n");
 		goto error;
 	}
-	RTE_LOG(DEBUG, PMD, "TUN/TAP Features %08x\n", features);
+	RTE_LOG(DEBUG, PMD, "  TAP Features %08x\n", features);
 
-#ifdef IFF_MULTI_QUEUE
-	if (!(features & IFF_MULTI_QUEUE) && (RTE_PMD_TAP_MAX_QUEUES > 1)) {
-		RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n");
-		goto error;
-	} else if ((features & IFF_ONE_QUEUE) &&
-			(RTE_PMD_TAP_MAX_QUEUES == 1)) {
-		ifr.ifr_flags |= IFF_ONE_QUEUE;
-		RTE_LOG(DEBUG, PMD, "Single queue only support\n");
-	} else {
-		ifr.ifr_flags |= IFF_MULTI_QUEUE;
-		RTE_LOG(DEBUG, PMD, "Multi-queue support for %d queues\n",
+	if (features & IFF_MULTI_QUEUE) {
+		RTE_LOG(DEBUG, PMD, "  Multi-queue support for %d queues\n",
 			RTE_PMD_TAP_MAX_QUEUES);
-	}
-#else
-	if (RTE_PMD_TAP_MAX_QUEUES > 1) {
-		RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n");
-		goto error;
-	} else {
+		ifr.ifr_flags |= IFF_MULTI_QUEUE;
+	} else
+#endif
+	{
 		ifr.ifr_flags |= IFF_ONE_QUEUE;
 		RTE_LOG(DEBUG, PMD, "Single queue only support\n");
 	}
-#endif
 
-	/* Set the TUN/TAP configuration and get the name if needed */
+	/* Set the TUN/TAP configuration and set the name if needed */
 	if (ioctl(fd, TUNSETIFF, (void *)&ifr) < 0) {
 		RTE_LOG(ERR, PMD, "Unable to set TUNSETIFF for %s\n",
 			ifr.ifr_name);
@@ -177,9 +173,15 @@ tun_alloc(char *name)
 		goto error;
 	}
 
-	/* If the name is different that new name as default */
-	if (name && strcmp(name, ifr.ifr_name))
-		snprintf(name, RTE_ETH_NAME_MAX_LEN - 1, "%s", ifr.ifr_name);
+	if (qid == 0) {
+		if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
+			RTE_LOG(ERR, PMD, "ioctl failed (SIOCGIFHWADDR) (%s)\n",
+				ifr.ifr_name);
+			goto error;
+		}
+
+		rte_memcpy(&pmd->eth_addr, ifr.ifr_hwaddr.sa_data, ETH_ALEN);
+	}
 
 	return fd;
 
@@ -507,7 +509,7 @@ tap_setup_queue(struct rte_eth_dev *dev,
 		if (fd < 0) {
 			RTE_LOG(INFO, PMD, "Add queue to TAP %s for qid %d\n",
 				pmd->name, qid);
-			fd = tun_alloc(pmd->name);
+			fd = tun_alloc(pmd, qid);
 			if (fd < 0) {
 				RTE_LOG(ERR, PMD, "tun_alloc(%s) failed\n",
 					pmd->name);
@@ -629,34 +631,13 @@ static const struct eth_dev_ops ops = {
 };
 
 static int
-pmd_mac_address(int fd, struct ether_addr *addr)
-{
-	struct ifreq ifr;
-
-	if ((fd <= 0) || !addr)
-		return -1;
-
-	memset(&ifr, 0, sizeof(ifr));
-
-	if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
-		RTE_LOG(ERR, PMD, "ioctl failed (SIOCGIFHWADDR) (%s)\n",
-			ifr.ifr_name);
-		return -1;
-	}
-
-	rte_memcpy(addr, ifr.ifr_hwaddr.sa_data, ETH_ALEN);
-
-	return 0;
-}
-
-static int
 eth_dev_tap_create(const char *name, char *tap_name)
 {
 	int numa_node = rte_socket_id();
 	struct rte_eth_dev *dev = NULL;
 	struct pmd_internals *pmd = NULL;
 	struct rte_eth_dev_data *data = NULL;
-	int i, fd = -1;
+	int i;
 
 	RTE_LOG(INFO, PMD,
 		"%s: Create TAP Ethernet device with %d queues on numa %u\n",
@@ -674,7 +655,6 @@ eth_dev_tap_create(const char *name, char *tap_name)
 		goto error_exit;
 	}
 
-	/* Use the name and not the tap_name */
 	dev = rte_eth_dev_allocate(tap_name);
 	if (!dev) {
 		RTE_LOG(ERR, PMD, "Unable to allocate device struct\n");
@@ -705,28 +685,12 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	dev->tx_pkt_burst = pmd_tx_burst;
 	snprintf(dev->data->name, sizeof(dev->data->name), "%s", name);
 
-	/* Create the first Tap device */
-	fd = tun_alloc(tap_name);
-	if (fd < 0) {
-		RTE_LOG(ERR, PMD, "tun_alloc() failed\n");
-		goto error_exit;
-	}
-
-	/* Presetup the fds to -1 as being not working */
-	for (i = 1; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
+	/* Presetup the fds to -1 as being not valid */
+	for (i = 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
 		pmd->rxq[i].fd = -1;
 		pmd->txq[i].fd = -1;
 	}
 
-	/* Take the TUN/TAP fd and place in the first location */
-	pmd->rxq[0].fd = fd;
-	pmd->txq[0].fd = fd;
-
-	if (pmd_mac_address(fd, &pmd->eth_addr) < 0) {
-		RTE_LOG(ERR, PMD, "Unable to get MAC address\n");
-		goto error_exit;
-	}
-
 	return 0;
 
 error_exit:
-- 
2.8.0.GIT

[PATCH v3 5/7] net/tap: cleanup log messages

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 54 ++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 7c923a2..65e4bab 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -155,12 +155,13 @@ tun_alloc(struct pmd_internals *pmd, uint16_t qid)
 #endif
 	{
 		ifr.ifr_flags |= IFF_ONE_QUEUE;
-		RTE_LOG(DEBUG, PMD, "Single queue only support\n");
+		RTE_LOG(DEBUG, PMD, "  Single queue only support\n");
 	}
 
 	/* Set the TUN/TAP configuration and set the name if needed */
 	if (ioctl(fd, TUNSETIFF, (void *)&ifr) < 0) {
-		RTE_LOG(ERR, PMD, "Unable to set TUNSETIFF for %s\n",
+		RTE_LOG(WARNING, PMD,
+			"Unable to set TUNSETIFF for %s\n",
 			ifr.ifr_name);
 		perror("TUNSETIFF");
 		goto error;
@@ -168,7 +169,9 @@ tun_alloc(struct pmd_internals *pmd, uint16_t qid)
 
 	/* Always set the file descriptor to non-blocking */
 	if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
-		RTE_LOG(ERR, PMD, "Unable to set to nonblocking\n");
+		RTE_LOG(WARNING, PMD,
+			"Unable to set %s to nonblocking\n",
+			ifr.ifr_name);
 		perror("F_SETFL, NONBLOCK");
 		goto error;
 	}
@@ -207,7 +210,7 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 		/* allocate the next mbuf */
 		mbuf = rte_pktmbuf_alloc(rxq->mp);
 		if (unlikely(!mbuf)) {
-			RTE_LOG(WARNING, PMD, "Unable to allocate mbuf\n");
+			RTE_LOG(WARNING, PMD, "TAP unable to allocate mbuf\n");
 			break;
 		}
 
@@ -276,7 +279,8 @@ pmd_tx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	return num_tx;
 }
 
-static int tap_link_set_flags(struct pmd_internals *pmd, short flags, int add)
+static int
+tap_link_set_flags(struct pmd_internals *pmd, short flags, int add)
 {
 	struct ifreq ifr;
 	int err, s;
@@ -296,8 +300,8 @@ static int tap_link_set_flags(struct pmd_internals *pmd, short flags, int add)
 	strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
 	err = ioctl(s, SIOCGIFFLAGS, &ifr);
 	if (err < 0) {
-		RTE_LOG(ERR, PMD, "Unable to get tap netdevice flags: %s\n",
-			strerror(errno));
+		RTE_LOG(WARNING, PMD, "Unable to get %s device flags: %s\n",
+			pmd->name, strerror(errno));
 		close(s);
 		return -1;
 	}
@@ -307,7 +311,7 @@ static int tap_link_set_flags(struct pmd_internals *pmd, short flags, int add)
 		ifr.ifr_flags &= ~flags;
 	err = ioctl(s, SIOCSIFFLAGS, &ifr);
 	if (err < 0) {
-		RTE_LOG(ERR, PMD, "Unable to %s flags 0x%x: %s\n",
+		RTE_LOG(WARNING, PMD, "Unable to %s flags 0x%x: %s\n",
 			add ? "set" : "unset", flags, strerror(errno));
 		close(s);
 		return -1;
@@ -511,8 +515,8 @@ tap_setup_queue(struct rte_eth_dev *dev,
 				pmd->name, qid);
 			fd = tun_alloc(pmd, qid);
 			if (fd < 0) {
-				RTE_LOG(ERR, PMD, "tun_alloc(%s) failed\n",
-					pmd->name);
+				RTE_LOG(ERR, PMD, "tun_alloc(%s, %d) failed\n",
+					pmd->name, qid);
 				return -1;
 			}
 		}
@@ -557,8 +561,8 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
 	int fd;
 
 	if ((rx_queue_id >= internals->nb_queues) || !mp) {
-		RTE_LOG(ERR, PMD, "nb_queues %d mp %p\n",
-			internals->nb_queues, mp);
+		RTE_LOG(WARNING, PMD, "nb_queues %d too small or mempool NULL\n",
+			internals->nb_queues);
 		return -1;
 	}
 
@@ -570,7 +574,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
 				RTE_PKTMBUF_HEADROOM);
 
 	if (buf_size < ETH_FRAME_LEN) {
-		RTE_LOG(ERR, PMD,
+		RTE_LOG(WARNING, PMD,
 			"%s: %d bytes will not fit in mbuf (%d bytes)\n",
 			dev->data->name, ETH_FRAME_LEN, buf_size);
 		return -ENOMEM;
@@ -580,7 +584,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
 	if (fd == -1)
 		return -1;
 
-	RTE_LOG(INFO, PMD, "RX TAP device name %s, qid %d on fd %d\n",
+	RTE_LOG(DEBUG, PMD, "  RX TAP device name %s, qid %d on fd %d\n",
 		internals->name, rx_queue_id, internals->rxq[rx_queue_id].fd);
 
 	return 0;
@@ -603,7 +607,7 @@ tap_tx_queue_setup(struct rte_eth_dev *dev,
 	if (ret == -1)
 		return -1;
 
-	RTE_LOG(INFO, PMD, "TX TAP device name %s, qid %d on fd %d\n",
+	RTE_LOG(DEBUG, PMD, "  TX TAP device name %s, qid %d on fd %d\n",
 		internals->name, tx_queue_id, internals->txq[tx_queue_id].fd);
 
 	return 0;
@@ -639,25 +643,23 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	struct rte_eth_dev_data *data = NULL;
 	int i;
 
-	RTE_LOG(INFO, PMD,
-		"%s: Create TAP Ethernet device with %d queues on numa %u\n",
-		 name, RTE_PMD_TAP_MAX_QUEUES, rte_socket_id());
+	RTE_LOG(DEBUG, PMD, "  TAP device on numa %u\n", rte_socket_id());
 
 	data = rte_zmalloc_socket(tap_name, sizeof(*data), 0, numa_node);
 	if (!data) {
-		RTE_LOG(ERR, PMD, "Failed to allocate data\n");
+		RTE_LOG(ERR, PMD, "TAP Failed to allocate data\n");
 		goto error_exit;
 	}
 
 	pmd = rte_zmalloc_socket(tap_name, sizeof(*pmd), 0, numa_node);
 	if (!pmd) {
-		RTE_LOG(ERR, PMD, "Unable to allocate internal struct\n");
+		RTE_LOG(ERR, PMD, "TAP Unable to allocate internal struct\n");
 		goto error_exit;
 	}
 
 	dev = rte_eth_dev_allocate(tap_name);
 	if (!dev) {
-		RTE_LOG(ERR, PMD, "Unable to allocate device struct\n");
+		RTE_LOG(ERR, PMD, "TAP Unable to allocate device struct\n");
 		goto error_exit;
 	}
 
@@ -694,7 +696,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	return 0;
 
 error_exit:
-	RTE_PMD_DEBUG_TRACE("Unable to initialize %s\n", name);
+	RTE_LOG(DEBUG, PMD, "TAP Unable to initialize %s\n", name);
 
 	rte_free(data);
 	rte_free(pmd);
@@ -745,7 +747,7 @@ rte_pmd_tap_probe(const char *name, const char *params)
 		 DEFAULT_TAP_NAME, tap_unit++);
 
 	if (params && (params[0] != '\0')) {
-		RTE_LOG(INFO, PMD, "paramaters (%s)\n", params);
+		RTE_LOG(DEBUG, PMD, "paramaters (%s)\n", params);
 
 		kvlist = rte_kvargs_parse(params, valid_arguments);
 		if (kvlist) {
@@ -770,14 +772,14 @@ rte_pmd_tap_probe(const char *name, const char *params)
 	}
 	pmd_link.link_speed = speed;
 
-	RTE_LOG(INFO, PMD, "Initializing pmd_tap for %s as %s\n",
+	RTE_LOG(NOTICE, PMD, "Initializing pmd_tap for %s as %s\n",
 		name, tap_name);
 
 	ret = eth_dev_tap_create(name, tap_name);
 
 leave:
 	if (ret == -1) {
-		RTE_LOG(INFO, PMD, "Failed to create pmd for %s as %s\n",
+		RTE_LOG(ERR, PMD, "Failed to create pmd for %s as %s\n",
 			name, tap_name);
 		tap_unit--;		/* Restore the unit number */
 	}
@@ -795,7 +797,7 @@ rte_pmd_tap_remove(const char *name)
 	struct pmd_internals *internals;
 	int i;
 
-	RTE_LOG(INFO, PMD, "Closing TUN/TAP Ethernet device on numa %u\n",
+	RTE_LOG(DEBUG, PMD, "Closing TUN/TAP Ethernet device on numa %u\n",
 		rte_socket_id());
 
 	/* find the ethdev entry */
-- 
2.8.0.GIT

[PATCH v3 6/7] net/tap: link set down must be done before close

[Keith Wiles]

Fixes: ee418a25b0d3 ("net/tap: implement link up and down callbacks")

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 65e4bab..966e91a 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -353,10 +353,11 @@ tap_dev_stop(struct rte_eth_dev *dev)
 	int i;
 	struct pmd_internals *internals = dev->data->dev_private;
 
+	tap_link_set_down(dev);
+
 	for (i = 0; i < internals->nb_queues; i++)
 		if (internals->rxq[i].fd != -1)
 			close(internals->rxq[i].fd);
-	tap_link_set_down(dev);
 }
 
 static int
-- 
2.8.0.GIT

[PATCH v3 7/7] net/tap: move closing fds to pmd close from pmd stop

[Keith Wiles]

At the same time remove closing fds code from pmd stop routine.

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 966e91a..0a7f4af 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -350,14 +350,7 @@ tap_dev_start(struct rte_eth_dev *dev)
 static void
 tap_dev_stop(struct rte_eth_dev *dev)
 {
-	int i;
-	struct pmd_internals *internals = dev->data->dev_private;
-
 	tap_link_set_down(dev);
-
-	for (i = 0; i < internals->nb_queues; i++)
-		if (internals->rxq[i].fd != -1)
-			close(internals->rxq[i].fd);
 }
 
 static int
@@ -431,6 +424,17 @@ tap_stats_reset(struct rte_eth_dev *dev)
 static void
 tap_dev_close(struct rte_eth_dev *dev __rte_unused)
 {
+	int i;
+	struct pmd_internals *internals = dev->data->dev_private;
+
+	tap_link_set_down(dev);
+
+	for (i = 0; i < internals->nb_queues; i++) {
+		if (internals->rxq[i].fd != -1)
+			close(internals->rxq[i].fd);
+		internals->rxq[i].fd = -1;
+		internals->txq[i].fd = -1;
+	}
 }
 
 static void
-- 
2.8.0.GIT

Re: [PATCH v3 7/7] net/tap: move closing fds to pmd close from pmd stop

[Pascal Mazon]

On Mon,  6 Feb 2017 13:40:38 -0600
Keith Wiles <keith.wiles@intel.com> wrote:

> At the same time remove closing fds code from pmd stop routine.
> 
> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
>  drivers/net/tap/rte_eth_tap.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 966e91a..0a7f4af 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -350,14 +350,7 @@ tap_dev_start(struct rte_eth_dev *dev)
>  static void
>  tap_dev_stop(struct rte_eth_dev *dev)
>  {
> -	int i;
> -	struct pmd_internals *internals = dev->data->dev_private;
> -
>  	tap_link_set_down(dev);
> -
> -	for (i = 0; i < internals->nb_queues; i++)
> -		if (internals->rxq[i].fd != -1)
> -			close(internals->rxq[i].fd);
>  }
>  
>  static int
> @@ -431,6 +424,17 @@ tap_stats_reset(struct rte_eth_dev *dev)
>  static void
>  tap_dev_close(struct rte_eth_dev *dev __rte_unused)
>  {
> +	int i;
> +	struct pmd_internals *internals = dev->data->dev_private;
> +
> +	tap_link_set_down(dev);
> +
> +	for (i = 0; i < internals->nb_queues; i++) {
> +		if (internals->rxq[i].fd != -1)
> +			close(internals->rxq[i].fd);
> +		internals->rxq[i].fd = -1;
> +		internals->txq[i].fd = -1;
> +	}
>  }
>  
>  static void

Ok, the series looks good to me.

Acked-by: Pascal Mazon <pascal.mazon@6wind.com>

Re: [PATCH v3 7/7] net/tap: move closing fds to pmd close from pmd stop

[Ferruh Yigit]

On 2/7/2017 8:51 AM, Pascal Mazon wrote:
<...>

>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>

<...>

> Acked-by: Pascal Mazon <pascal.mazon@6wind.com>

Series applied to dpdk-next-net/master, thanks.

(
patch 1/7: docs: fix tap PMD docs for device name
patch 4/7: Fixes: 02f96a0a82d1 ("net/tap: add TUN/TAP device PMD")
patch 5/7: checkpatch warning fixed
patch 6/7: Fixes: 082fdb7361d7 ("net/tap: implement link up and down
callbacks")
)