[dpdk-dev] [PATCH v6] drivers/net:new PMD using tun/tap host interface

[Ferruh Yigit]

On 10/12/2016 9:54 PM, Keith Wiles wrote:
> The rte_eth_tap.c PMD creates a device using TUN/TAP interfaces
> on the local host. The PMD allows for DPDK and the host to
> communicate using a raw device interface on the host and in
> the DPDK application. The device created is a Tap device with
> a L2 packet header.
> 
> v6 - fixed the checkpatch issues
> v5 - merge in changes from list review see related emails
>      fixed many minor edits
> v4 - merge with latest driver changes
> v3 - fix includes by removing ifdef for other type besides Linux
>      Fix the copyright notice in the Makefile
> v2 - merge all of the patches into one patch
>      Fix a typo on naming the tap device
>      Update the maintainers list
> 
> Signed-off-by: Keith Wiles <keith.wiles at intel.com>
> ---
<...>

> diff --git a/config/common_base b/config/common_base
...
> +
> +#
> +# Set TAP PMD to 'n' as it is only supported in Linux for now.
> +#

In final .config file, this comment says TAP PMD set to "n", but it is
set to "y"
Also this behavior is not unique to this config option, many config
items used as described in the comment, and they don't have same comment.

<...>

> diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
...
> +
> +   Also the speed of the interface can be changed from 10G to whatever number
> +   needed, but the interface does not enforce that speed.
> +   e.g. --vdev=eth_tap,iface=foo0,speed=25000

Same comment with previous review, eth_tap should be net_tap, for all
occurrences in this document

<...>

> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
...
> +struct tap_info {
> +	char name[RTE_ETH_NAME_MAX_LEN]; /* Interface name supplied/given */
> +	int speed;			 /* Speed of interface */
> +};

Same comment with previous review, this struct is not used at all.

<...>

> +static int
> +eth_dev_tap_create(const char *name, char *tap_name)
...
> +	dev->data = data;
> +	dev->dev_ops = &ops;
> +	dev->driver = NULL;
> +	dev->rx_pkt_burst = pmd_rx_burst;
> +	dev->tx_pkt_burst = pmd_tx_burst;
> +	snprintf(dev->data->name, sizeof(dev->data->name), "%s", tap_name);

[1]

<...>

> + */
> +static int
> +rte_pmd_tap_remove(const char *name)
...
> +
> +	/* find the ethdev entry */
> +	eth_dev = rte_eth_dev_allocated(name);

This still won't work, please see [1], dev->name overwritten by
tap_name, and this function does simply a strcmp with name and dev->name.

<...>

Thanks,
ferruh