[dpdk-dev,2/2] net/tap: add tun log and documnetation

Message ID 1522705068-18198-2-git-send-email-vipin.varghese@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Varghese, Vipin April 2, 2018, 9:37 p.m. UTC
  The changes add TUN|TAP specific logs and documentation support.

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
---
 doc/guides/nics/tap.rst       | 15 +++++++++++++--
 drivers/net/tap/rte_eth_tap.c | 28 ++++++++++++++++------------
 2 files changed, 29 insertions(+), 14 deletions(-)
  

Comments

Pascal Mazon April 3, 2018, 8:27 a.m. UTC | #1
Hi,

I'm not sure why you split this patch in two.
It seemed to me that it was fine to change the doc+logs in the same
patch as the code.
Especially since documentation change is very small.

Anyway, there's just the "documnetation" typo in the commit title,
otherwise it's fine by me.

Best regards,
Pascal

On 02/04/2018 23:37, Vipin Varghese wrote:
> The changes add TUN|TAP specific logs and documentation support.
>
> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> ---
>  doc/guides/nics/tap.rst       | 15 +++++++++++++--
>  drivers/net/tap/rte_eth_tap.c | 28 ++++++++++++++++------------
>  2 files changed, 29 insertions(+), 14 deletions(-)
>
> diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
> index 76eb0bd..c97786a 100644
> --- a/doc/guides/nics/tap.rst
> +++ b/doc/guides/nics/tap.rst
> @@ -1,8 +1,8 @@
>  ..  SPDX-License-Identifier: BSD-3-Clause
>      Copyright(c) 2016 Intel Corporation.
>  
> -Tap Poll Mode Driver
> -====================
> +Tun|Tap Poll Mode Driver
> +========================
>  
>  The ``rte_eth_tap.c`` PMD creates a device using TAP interfaces on the
>  local host. The PMD allows for DPDK and the host to communicate using a raw
> @@ -83,6 +83,17 @@ can utilize that stack to handle the network protocols. Plus you would be able
>  to address the interface using an IP address assigned to the internal
>  interface.
>  
> +The TUN PMD allows user to create a TUN device on host. The PMD allows user
> +to transmit and receive packets via DPDK API calls with L3 header and payload.
> +The devices in host can be accessed via ``ifconfig`` or ``ip`` command. TUN
> +interfaces are passed to DPDK ``rte_eal_init`` arguments as ``--vdev=net_tunX``,
> +where X stands for unique id, example::
> +
> +   --vdev=net_tun0 --vdev=net_tun1,iface=foo1, ...
> +
> +Unlike TAP PMD, TUN PMD does not support user arguments as ``MAC`` or ``remote`` user
> +options. Default interface name is ``dtunX``, where X stands for unique id.
> +
>  Flow API support
>  ----------------
>  
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 295db3c..7c6704b 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -121,17 +121,19 @@ enum ioctl_mode {
>  
>  	fd = open(TUN_TAP_DEV_PATH, O_RDWR);
>  	if (fd < 0) {
> -		RTE_LOG(ERR, PMD, "Unable to create TAP interface\n");
> +		RTE_LOG(ERR, PMD, "Unable to create %s interface\n",
> +				tuntap_name);
>  		goto error;
>  	}
>  
>  #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, "TAP unable to get TUN/TAP features\n");
> +		RTE_LOG(ERR, PMD, "%s unable to get TUN/TAP features\n",
> +				tuntap_name);
>  		goto error;
>  	}
> -	RTE_LOG(DEBUG, PMD, "  TAP Features %08x\n", features);
> +	RTE_LOG(DEBUG, PMD, "%s Features %08x\n", tuntap_name, features);
>  
>  	if (features & IFF_MULTI_QUEUE) {
>  		RTE_LOG(DEBUG, PMD, "  Multi-queue support for %d queues\n",
> @@ -1133,7 +1135,7 @@ enum ioctl_mode {
>  		tmp = &(*tmp)->next;
>  	}
>  
> -	RTE_LOG(DEBUG, PMD, "  RX TAP device name %s, qid %d on fd %d\n",
> +	RTE_LOG(DEBUG, PMD, "  RX TUNTAP device name %s, qid %d on fd %d\n",
>  		internals->name, rx_queue_id, internals->rxq[rx_queue_id].fd);
>  
>  	return 0;
> @@ -1188,7 +1190,7 @@ enum ioctl_mode {
>  	if (ret == -1)
>  		return -1;
>  	RTE_LOG(DEBUG, PMD,
> -		"  TX TAP device name %s, qid %d on fd %d csum %s\n",
> +		"  TX TUNTAP device name %s, qid %d on fd %d csum %s\n",
>  		internals->name, tx_queue_id, internals->txq[tx_queue_id].fd,
>  		txq->csum ? "on" : "off");
>  
> @@ -1371,17 +1373,19 @@ enum ioctl_mode {
>  	struct ifreq ifr;
>  	int i;
>  
> -	RTE_LOG(DEBUG, PMD, "  TAP device on numa %u\n", rte_socket_id());
> +	RTE_LOG(DEBUG, PMD, "%s device on numa %u\n",
> +			tuntap_name, rte_socket_id());
>  
>  	data = rte_zmalloc_socket(tap_name, sizeof(*data), 0, numa_node);
>  	if (!data) {
> -		RTE_LOG(ERR, PMD, "TAP Failed to allocate data\n");
> +		RTE_LOG(ERR, PMD, "%s Failed to allocate data\n", tuntap_name);
>  		goto error_exit_nodev;
>  	}
>  
>  	dev = rte_eth_vdev_allocate(vdev, sizeof(*pmd));
>  	if (!dev) {
> -		RTE_LOG(ERR, PMD, "TAP Unable to allocate device struct\n");
> +		RTE_LOG(ERR, PMD, "%s Unable to allocate device struct\n",
> +				tuntap_name);
>  		goto error_exit_nodev;
>  	}
>  
> @@ -1392,8 +1396,8 @@ enum ioctl_mode {
>  	pmd->ioctl_sock = socket(AF_INET, SOCK_DGRAM, 0);
>  	if (pmd->ioctl_sock == -1) {
>  		RTE_LOG(ERR, PMD,
> -			"TAP Unable to get a socket for management: %s\n",
> -			strerror(errno));
> +			"%s Unable to get a socket for management: %s\n",
> +			tuntap_name, strerror(errno));
>  		goto error_exit;
>  	}
>  
> @@ -1557,8 +1561,8 @@ enum ioctl_mode {
>  	rte_eth_dev_release_port(dev);
>  
>  error_exit_nodev:
> -	RTE_LOG(ERR, PMD, "TAP Unable to initialize %s\n",
> -		rte_vdev_device_name(vdev));
> +	RTE_LOG(ERR, PMD, "%s Unable to initialize %s\n",
> +		tuntap_name, rte_vdev_device_name(vdev));
>  
>  	rte_free(data);
>  	return -EINVAL;
  
Ferruh Yigit April 3, 2018, 10:05 a.m. UTC | #2
On 4/3/2018 9:27 AM, Pascal Mazon wrote:
> Hi,
> 
> I'm not sure why you split this patch in two.
> It seemed to me that it was fine to change the doc+logs in the same
> patch as the code.
> Especially since documentation change is very small.

It was my suggestion indeed, to separate actual functional change from routine
log update noise.

> 
> Anyway, there's just the "documnetation" typo in the commit title,
> otherwise it's fine by me.
> 
> Best regards,
> Pascal
> 
> On 02/04/2018 23:37, Vipin Varghese wrote:
>> The changes add TUN|TAP specific logs and documentation support.
>>
>> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>

<...>
  
Ferruh Yigit April 3, 2018, 10:05 a.m. UTC | #3
On 4/3/2018 9:27 AM, Pascal Mazon wrote:
> Hi,
> 
> I'm not sure why you split this patch in two.
> It seemed to me that it was fine to change the doc+logs in the same
> patch as the code.
> Especially since documentation change is very small.

It was my suggestion indeed, to separate actual functional change from routine
log update noise.

> 
> Anyway, there's just the "documnetation" typo in the commit title,
> otherwise it's fine by me.
> 
> Best regards,
> Pascal
> 
> On 02/04/2018 23:37, Vipin Varghese wrote:
>> The changes add TUN|TAP specific logs and documentation support.
>>
>> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>

<...>
  
Pascal Mazon April 3, 2018, 10:53 a.m. UTC | #4
Ok Ferruh, I missed that.
Apart for the type in "documentation" the patches are ok with me.

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

On 03/04/2018 12:05, Ferruh Yigit wrote:
> On 4/3/2018 9:27 AM, Pascal Mazon wrote:
>> Hi,
>>
>> I'm not sure why you split this patch in two.
>> It seemed to me that it was fine to change the doc+logs in the same
>> patch as the code.
>> Especially since documentation change is very small.
> It was my suggestion indeed, to separate actual functional change from routine
> log update noise.
>
>> Anyway, there's just the "documnetation" typo in the commit title,
>> otherwise it's fine by me.
>>
>> Best regards,
>> Pascal
>>
>> On 02/04/2018 23:37, Vipin Varghese wrote:
>>> The changes add TUN|TAP specific logs and documentation support.
>>>
>>> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> <...>
  

Patch

diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
index 76eb0bd..c97786a 100644
--- a/doc/guides/nics/tap.rst
+++ b/doc/guides/nics/tap.rst
@@ -1,8 +1,8 @@ 
 ..  SPDX-License-Identifier: BSD-3-Clause
     Copyright(c) 2016 Intel Corporation.
 
-Tap Poll Mode Driver
-====================
+Tun|Tap Poll Mode Driver
+========================
 
 The ``rte_eth_tap.c`` PMD creates a device using TAP interfaces on the
 local host. The PMD allows for DPDK and the host to communicate using a raw
@@ -83,6 +83,17 @@  can utilize that stack to handle the network protocols. Plus you would be able
 to address the interface using an IP address assigned to the internal
 interface.
 
+The TUN PMD allows user to create a TUN device on host. The PMD allows user
+to transmit and receive packets via DPDK API calls with L3 header and payload.
+The devices in host can be accessed via ``ifconfig`` or ``ip`` command. TUN
+interfaces are passed to DPDK ``rte_eal_init`` arguments as ``--vdev=net_tunX``,
+where X stands for unique id, example::
+
+   --vdev=net_tun0 --vdev=net_tun1,iface=foo1, ...
+
+Unlike TAP PMD, TUN PMD does not support user arguments as ``MAC`` or ``remote`` user
+options. Default interface name is ``dtunX``, where X stands for unique id.
+
 Flow API support
 ----------------
 
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 295db3c..7c6704b 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -121,17 +121,19 @@  enum ioctl_mode {
 
 	fd = open(TUN_TAP_DEV_PATH, O_RDWR);
 	if (fd < 0) {
-		RTE_LOG(ERR, PMD, "Unable to create TAP interface\n");
+		RTE_LOG(ERR, PMD, "Unable to create %s interface\n",
+				tuntap_name);
 		goto error;
 	}
 
 #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, "TAP unable to get TUN/TAP features\n");
+		RTE_LOG(ERR, PMD, "%s unable to get TUN/TAP features\n",
+				tuntap_name);
 		goto error;
 	}
-	RTE_LOG(DEBUG, PMD, "  TAP Features %08x\n", features);
+	RTE_LOG(DEBUG, PMD, "%s Features %08x\n", tuntap_name, features);
 
 	if (features & IFF_MULTI_QUEUE) {
 		RTE_LOG(DEBUG, PMD, "  Multi-queue support for %d queues\n",
@@ -1133,7 +1135,7 @@  enum ioctl_mode {
 		tmp = &(*tmp)->next;
 	}
 
-	RTE_LOG(DEBUG, PMD, "  RX TAP device name %s, qid %d on fd %d\n",
+	RTE_LOG(DEBUG, PMD, "  RX TUNTAP device name %s, qid %d on fd %d\n",
 		internals->name, rx_queue_id, internals->rxq[rx_queue_id].fd);
 
 	return 0;
@@ -1188,7 +1190,7 @@  enum ioctl_mode {
 	if (ret == -1)
 		return -1;
 	RTE_LOG(DEBUG, PMD,
-		"  TX TAP device name %s, qid %d on fd %d csum %s\n",
+		"  TX TUNTAP device name %s, qid %d on fd %d csum %s\n",
 		internals->name, tx_queue_id, internals->txq[tx_queue_id].fd,
 		txq->csum ? "on" : "off");
 
@@ -1371,17 +1373,19 @@  enum ioctl_mode {
 	struct ifreq ifr;
 	int i;
 
-	RTE_LOG(DEBUG, PMD, "  TAP device on numa %u\n", rte_socket_id());
+	RTE_LOG(DEBUG, PMD, "%s device on numa %u\n",
+			tuntap_name, rte_socket_id());
 
 	data = rte_zmalloc_socket(tap_name, sizeof(*data), 0, numa_node);
 	if (!data) {
-		RTE_LOG(ERR, PMD, "TAP Failed to allocate data\n");
+		RTE_LOG(ERR, PMD, "%s Failed to allocate data\n", tuntap_name);
 		goto error_exit_nodev;
 	}
 
 	dev = rte_eth_vdev_allocate(vdev, sizeof(*pmd));
 	if (!dev) {
-		RTE_LOG(ERR, PMD, "TAP Unable to allocate device struct\n");
+		RTE_LOG(ERR, PMD, "%s Unable to allocate device struct\n",
+				tuntap_name);
 		goto error_exit_nodev;
 	}
 
@@ -1392,8 +1396,8 @@  enum ioctl_mode {
 	pmd->ioctl_sock = socket(AF_INET, SOCK_DGRAM, 0);
 	if (pmd->ioctl_sock == -1) {
 		RTE_LOG(ERR, PMD,
-			"TAP Unable to get a socket for management: %s\n",
-			strerror(errno));
+			"%s Unable to get a socket for management: %s\n",
+			tuntap_name, strerror(errno));
 		goto error_exit;
 	}
 
@@ -1557,8 +1561,8 @@  enum ioctl_mode {
 	rte_eth_dev_release_port(dev);
 
 error_exit_nodev:
-	RTE_LOG(ERR, PMD, "TAP Unable to initialize %s\n",
-		rte_vdev_device_name(vdev));
+	RTE_LOG(ERR, PMD, "%s Unable to initialize %s\n",
+		tuntap_name, rte_vdev_device_name(vdev));
 
 	rte_free(data);
 	return -EINVAL;