[PATCH 0/6] net/tap: add additional management ops

[PATCH 0/6] net/tap: add additional management ops

[Pascal Mazon]

Add a few eth_dev ops to the tap PMD for completeness.
We want it to behave as much as possible as a standard PMD.

Pascal Mazon (6):
  net/tap: add MAC address management ops
  net/tap: add speed capabilities
  net/tap: add multicast addresses management
  net/tap: add MTU management
  net/tap: add packet type management
  net/tap: add flow control management

 doc/guides/nics/features/tap.ini |   6 ++
 drivers/net/tap/rte_eth_tap.c    | 207 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 200 insertions(+), 13 deletions(-)

-- 
2.8.0.rc0

[PATCH 1/6] net/tap: add MAC address management ops

[Pascal Mazon]

Set a random MAC address when probing the device, as to not leave an
empty MAC in pmd->eth_addr.

This MAC will be set on the tap netdevice as soon as it's been created
using tun_alloc(). As the tap_mac_add() function depend on the fd in
the first rxq, move code from tun_alloc() to tap_setup_queue(),
after it's been set.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 88 ++++++++++++++++++++++++++++++++++------
 2 files changed, 76 insertions(+), 13 deletions(-)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index f4aca6921ddc..d9b47a003654 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+Unicast MAC filter   = Y
 Other kdrv           = Y
 ARMv7                = Y
 ARMv8                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 14c345f07afa..994c8be701c8 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -63,6 +63,8 @@
 #define RTE_PMD_TAP_MAX_QUEUES	1
 #endif
 
+#define RTE_PMD_TAP_MAX_MAC_ADDRS 1
+
 static struct rte_vdev_driver pmd_tap_drv;
 
 static const char *valid_arguments[] = {
@@ -118,7 +120,7 @@ struct pmd_internals {
  * supplied name.
  */
 static int
-tun_alloc(struct pmd_internals *pmd, uint16_t qid)
+tun_alloc(struct pmd_internals *pmd)
 {
 	struct ifreq ifr;
 #ifdef IFF_MULTI_QUEUE
@@ -176,16 +178,6 @@ tun_alloc(struct pmd_internals *pmd, uint16_t qid)
 		goto error;
 	}
 
-	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;
 
 error:
@@ -365,7 +357,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	struct pmd_internals *internals = dev->data->dev_private;
 
 	dev_info->if_index = internals->if_index;
-	dev_info->max_mac_addrs = 1;
+	dev_info->max_mac_addrs = RTE_PMD_TAP_MAX_MAC_ADDRS;
 	dev_info->max_rx_pktlen = (uint32_t)ETHER_MAX_VLAN_FRAME_LEN;
 	dev_info->max_rx_queues = internals->nb_queues;
 	dev_info->max_tx_queues = internals->nb_queues;
@@ -502,6 +494,69 @@ tap_allmulti_disable(struct rte_eth_dev *dev)
 	tap_link_set_flags(pmd, IFF_ALLMULTI, 0);
 }
 
+static void
+tap_mac_remove(struct rte_eth_dev *dev __rte_unused,
+	       uint32_t index __rte_unused)
+{
+	/*
+	 * A single MAC is authorized on the device and it's not possible to
+	 * leave the device without a MAC. Don't do anything, then.
+	 */
+}
+
+static void
+tap_mac_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
+	    uint32_t index, uint32_t vmdq __rte_unused)
+{
+	struct pmd_internals *internals = dev->data->dev_private;
+	int fd = internals->rxq[0].fd;
+	struct ifreq ifr;
+
+	if (index > RTE_PMD_TAP_MAX_MAC_ADDRS - 1) {
+		RTE_LOG(ERR, PMD,
+			"%s: can't set MAC address: index %d > max %d\n",
+			dev->data->name, index, RTE_PMD_TAP_MAX_MAC_ADDRS - 1);
+		return;
+	}
+	if (is_zero_ether_addr(mac_addr)) {
+		RTE_LOG(ERR, PMD,
+			"%s: can't set an empty MAC address.\n",
+			dev->data->name);
+		return;
+	}
+	if (fd < 0) {
+		RTE_LOG(ERR, PMD,
+			"%s: can't set MAC address: device does not exist.\n",
+			dev->data->name);
+		return;
+	}
+	memset(&ifr, 0, sizeof(struct ifreq));
+	if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
+		RTE_LOG(ERR, PMD, "%s: couldn't get current MAC address (%s)\n",
+			dev->data->name, strerror(errno));
+		return;
+	}
+	rte_memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, ETHER_ADDR_LEN);
+	if (ioctl(fd, SIOCSIFHWADDR, &ifr) == -1) {
+		RTE_LOG(ERR, PMD, "%s: couldn't set current MAC address (%s)\n",
+			dev->data->name, strerror(errno));
+		return;
+	}
+	rte_memcpy(&dev->data->mac_addrs[index], mac_addr, ETHER_ADDR_LEN);
+}
+
+static void
+tap_mac_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
+{
+	if (is_zero_ether_addr(mac_addr)) {
+		RTE_LOG(ERR, PMD, "%s: can't set an empty MAC address\n",
+			dev->data->name);
+		return;
+	}
+	tap_mac_remove(dev, 0);
+	tap_mac_add(dev, mac_addr, 0, 0);
+}
+
 static int
 tap_setup_queue(struct rte_eth_dev *dev,
 		struct pmd_internals *internals,
@@ -518,7 +573,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, qid);
+			fd = tun_alloc(pmd);
 			if (fd < 0) {
 				RTE_LOG(ERR, PMD, "tun_alloc(%s, %d) failed\n",
 					pmd->name, qid);
@@ -530,6 +585,9 @@ tap_setup_queue(struct rte_eth_dev *dev,
 	rx->fd = fd;
 	tx->fd = fd;
 
+	if (qid == 0)
+		tap_mac_set(dev, &pmd->eth_addr);
+
 	return fd;
 }
 
@@ -636,6 +694,9 @@ static const struct eth_dev_ops ops = {
 	.promiscuous_disable    = tap_promisc_disable,
 	.allmulticast_enable    = tap_allmulti_enable,
 	.allmulticast_disable   = tap_allmulti_disable,
+	.mac_addr_remove        = tap_mac_remove,
+	.mac_addr_add           = tap_mac_add,
+	.mac_addr_set           = tap_mac_set,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
 };
@@ -681,6 +742,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	data->drv_name = pmd_tap_drv.driver.name;
 	data->numa_node = numa_node;
 
+	eth_random_addr((uint8_t *)&pmd->eth_addr);
 	data->dev_link = pmd_link;
 	data->mac_addrs = &pmd->eth_addr;
 	data->nb_rx_queues = pmd->nb_queues;
-- 
2.8.0.rc0

[PATCH 2/6] net/tap: add speed capabilities

[Pascal Mazon]

Tap PMD is flexible, it supports any speed.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index d9b47a003654..dad5a0561087 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+Speed capabilities   = Y
 Unicast MAC filter   = Y
 Other kdrv           = Y
 ARMv7                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 994c8be701c8..6670dfbb35ce 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev __rte_unused)
 	return 0;
 }
 
+static uint32_t
+tap_dev_speed_capa(void)
+{
+	uint32_t speed = pmd_link.link_speed;
+	uint32_t capa = 0;
+
+	if (speed >= ETH_SPEED_NUM_10M)
+		capa |= ETH_LINK_SPEED_10M;
+	if (speed >= ETH_SPEED_NUM_100M)
+		capa |= ETH_LINK_SPEED_100M;
+	if (speed >= ETH_SPEED_NUM_1G)
+		capa |= ETH_LINK_SPEED_1G;
+	if (speed >= ETH_SPEED_NUM_5G)
+		capa |= ETH_LINK_SPEED_2_5G;
+	if (speed >= ETH_SPEED_NUM_5G)
+		capa |= ETH_LINK_SPEED_5G;
+	if (speed >= ETH_SPEED_NUM_10G)
+		capa |= ETH_LINK_SPEED_10G;
+	if (speed >= ETH_SPEED_NUM_20G)
+		capa |= ETH_LINK_SPEED_20G;
+	if (speed >= ETH_SPEED_NUM_25G)
+		capa |= ETH_LINK_SPEED_25G;
+	if (speed >= ETH_SPEED_NUM_40G)
+		capa |= ETH_LINK_SPEED_40G;
+	if (speed >= ETH_SPEED_NUM_50G)
+		capa |= ETH_LINK_SPEED_50G;
+	if (speed >= ETH_SPEED_NUM_56G)
+		capa |= ETH_LINK_SPEED_56G;
+	if (speed >= ETH_SPEED_NUM_100G)
+		capa |= ETH_LINK_SPEED_100G;
+
+	return capa;
+}
+
 static void
 tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
@@ -363,6 +397,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_tx_queues = internals->nb_queues;
 	dev_info->min_rx_bufsize = 0;
 	dev_info->pci_dev = NULL;
+	dev_info->speed_capa = tap_dev_speed_capa();
 }
 
 static void
-- 
2.8.0.rc0

[PATCH 3/6] net/tap: add multicast addresses management

[Pascal Mazon]

A tap netdevice actually receives every packet, without any filtering
whatsoever. There is no need for any multicast address registration
to receive multicast packets.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index dad5a0561087..6878a9b8fd17 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
 Other kdrv           = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 6670dfbb35ce..131c09fbc1a5 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -712,6 +712,18 @@ tap_tx_queue_setup(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static int
+tap_set_mc_addr_list(struct rte_eth_dev *dev __rte_unused,
+		     struct ether_addr *mc_addr_set __rte_unused,
+		     uint32_t nb_mc_addr __rte_unused)
+{
+	/*
+	 * Nothing to do actually: the tap has no filtering whatsoever, every
+	 * packet is received.
+	 */
+	return 0;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -732,6 +744,7 @@ static const struct eth_dev_ops ops = {
 	.mac_addr_remove        = tap_mac_remove,
 	.mac_addr_add           = tap_mac_add,
 	.mac_addr_set           = tap_mac_set,
+	.set_mc_addr_list       = tap_set_mc_addr_list,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
 };
-- 
2.8.0.rc0

[PATCH 4/6] net/tap: add MTU management

[Pascal Mazon]

The MTU is assigned to the tap netdevice according to the argument, but
packet transmission and reception just write/read on an fd with the
default limit being the socket buffer size.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index 6878a9b8fd17..6aa11874e2bc 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+MTU update           = Y
 Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 131c09fbc1a5..64b84cd76321 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -724,6 +724,42 @@ tap_set_mc_addr_list(struct rte_eth_dev *dev __rte_unused,
 	return 0;
 }
 
+static int
+tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr;
+	int err, s;
+
+	s = socket(AF_INET, SOCK_DGRAM, 0);
+	if (s < 0) {
+		RTE_LOG(ERR, PMD,
+			"Unable to get a socket for %s to set flags: %s\n",
+			pmd->name, strerror(errno));
+		return -1;
+	}
+	memset(&ifr, 0, sizeof(ifr));
+	strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
+	err = ioctl(s, SIOCGIFMTU, &ifr);
+	if (err < 0) {
+		RTE_LOG(WARNING, PMD, "Unable to get %s device MTU: %s\n",
+			pmd->name, strerror(errno));
+		close(s);
+		return -1;
+	}
+	ifr.ifr_mtu = mtu;
+	err = ioctl(s, SIOCSIFMTU, &ifr);
+	if (err < 0) {
+		RTE_LOG(WARNING, PMD, "Unable to set %s mtu %d: %s\n",
+			pmd->name, mtu, strerror(errno));
+		close(s);
+		return -1;
+	}
+	close(s);
+	dev->data->mtu = mtu;
+	return 0;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -745,6 +781,7 @@ static const struct eth_dev_ops ops = {
 	.mac_addr_add           = tap_mac_add,
 	.mac_addr_set           = tap_mac_set,
 	.set_mc_addr_list       = tap_set_mc_addr_list,
+	.mtu_set                = tap_mtu_set,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
 };
-- 
2.8.0.rc0

[PATCH 5/6] net/tap: add packet type management

[Pascal Mazon]

Advertize RTE_PTYPE_UNKNOWN since tap does not report any packet type.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index 6aa11874e2bc..7f3f4d661dd7 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -13,6 +13,7 @@ MTU update           = Y
 Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
+Packet type parsing  = Y
 Other kdrv           = Y
 ARMv7                = Y
 ARMv8                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 64b84cd76321..e4af36a6d142 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -36,6 +36,7 @@
 #include <rte_malloc.h>
 #include <rte_vdev.h>
 #include <rte_kvargs.h>
+#include <rte_net.h>
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -216,6 +217,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 		mbuf->data_len = len;
 		mbuf->pkt_len = len;
 		mbuf->port = rxq->in_port;
+		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
+						      RTE_PTYPE_ALL_MASK);
 
 		/* account for the receive frame */
 		bufs[num_rx++] = mbuf;
@@ -760,6 +763,17 @@ tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	return 0;
 }
 
+static const uint32_t*
+tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
+{
+	static const uint32_t ptypes[] = {
+		RTE_PTYPE_UNKNOWN,
+
+	};
+
+	return ptypes;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -784,6 +798,7 @@ static const struct eth_dev_ops ops = {
 	.mtu_set                = tap_mtu_set,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
+	.dev_supported_ptypes_get = tap_dev_supported_ptypes_get,
 };
 
 static int
-- 
2.8.0.rc0

[PATCH 6/6] net/tap: add flow control management

[Pascal Mazon]

A tap netdevice does not support flow control; ensure nothing but
RTE_FC_NONE mode can be set.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index 7f3f4d661dd7..a51712dce066 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -14,6 +14,7 @@ Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
 Packet type parsing  = Y
+Flow control         = Y
 Other kdrv           = Y
 ARMv7                = Y
 ARMv8                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index e4af36a6d142..3fd057225ab3 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -774,6 +774,23 @@ tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
 	return ptypes;
 }
 
+static int
+tap_flow_ctrl_get(struct rte_eth_dev *dev __rte_unused,
+		  struct rte_eth_fc_conf *fc_conf)
+{
+	fc_conf->mode = RTE_FC_NONE;
+	return 0;
+}
+
+static int
+tap_flow_ctrl_set(struct rte_eth_dev *dev __rte_unused,
+		  struct rte_eth_fc_conf *fc_conf)
+{
+	if (fc_conf->mode != RTE_FC_NONE)
+		return -ENOTSUP;
+	return 0;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -784,6 +801,8 @@ static const struct eth_dev_ops ops = {
 	.tx_queue_setup         = tap_tx_queue_setup,
 	.rx_queue_release       = tap_rx_queue_release,
 	.tx_queue_release       = tap_tx_queue_release,
+	.flow_ctrl_get          = tap_flow_ctrl_get,
+	.flow_ctrl_set          = tap_flow_ctrl_set,
 	.link_update            = tap_link_update,
 	.dev_set_link_up        = tap_link_set_up,
 	.dev_set_link_down      = tap_link_set_down,
-- 
2.8.0.rc0

Re: [PATCH 4/6] net/tap: add MTU management

[Wiles, Keith]

> On Mar 3, 2017, at 3:46 AM, Pascal Mazon <pascal.mazon@6wind.com> wrote:
> 
> The MTU is assigned to the tap netdevice according to the argument, but
> packet transmission and reception just write/read on an fd with the
> default limit being the socket buffer size.
> 
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> ---
> doc/guides/nics/features/tap.ini |  1 +
> drivers/net/tap/rte_eth_tap.c    | 37 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 38 insertions(+)
> 
> diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
> index 6878a9b8fd17..6aa11874e2bc 100644
> --- a/doc/guides/nics/features/tap.ini
> +++ b/doc/guides/nics/features/tap.ini
> @@ -9,6 +9,7 @@ Jumbo frame          = Y
> Promiscuous mode     = Y
> Allmulticast mode    = Y
> Basic stats          = Y
> +MTU update           = Y
> Multicast MAC filter = Y
> Speed capabilities   = Y
> Unicast MAC filter   = Y
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 131c09fbc1a5..64b84cd76321 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -724,6 +724,42 @@ tap_set_mc_addr_list(struct rte_eth_dev *dev __rte_unused,
> 	return 0;
> }
> 
> +static int
> +tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> +{
> +	struct pmd_internals *pmd = dev->data->dev_private;
> +	struct ifreq ifr;
> +	int err, s;
> +
> +	s = socket(AF_INET, SOCK_DGRAM, 0);
> +	if (s < 0) {
> +		RTE_LOG(ERR, PMD,
> +			"Unable to get a socket for %s to set flags: %s\n",
> +			pmd->name, strerror(errno));
> +		return -1;
> +	}
> +	memset(&ifr, 0, sizeof(ifr));
> +	strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);

This needs to be converted to a snprintf() to avoid overflow.

> +	err = ioctl(s, SIOCGIFMTU, &ifr);
> +	if (err < 0) {
> +		RTE_LOG(WARNING, PMD, "Unable to get %s device MTU: %s\n",
> +			pmd->name, strerror(errno));
> +		close(s);
> +		return -1;
> +	}
> +	ifr.ifr_mtu = mtu;
> +	err = ioctl(s, SIOCSIFMTU, &ifr);
> +	if (err < 0) {
> +		RTE_LOG(WARNING, PMD, "Unable to set %s mtu %d: %s\n",
> +			pmd->name, mtu, strerror(errno));
> +		close(s);
> +		return -1;
> +	}
> +	close(s);
> +	dev->data->mtu = mtu;
> +	return 0;
> +}
> +
> static const struct eth_dev_ops ops = {
> 	.dev_start              = tap_dev_start,
> 	.dev_stop               = tap_dev_stop,
> @@ -745,6 +781,7 @@ static const struct eth_dev_ops ops = {
> 	.mac_addr_add           = tap_mac_add,
> 	.mac_addr_set           = tap_mac_set,
> 	.set_mc_addr_list       = tap_set_mc_addr_list,
> +	.mtu_set                = tap_mtu_set,
> 	.stats_get              = tap_stats_get,
> 	.stats_reset            = tap_stats_reset,
> };
> -- 
> 2.8.0.rc0
> 

Regards,
Keith

Re: [PATCH 2/6] net/tap: add speed capabilities

[Wiles, Keith]

> On Mar 3, 2017, at 3:46 AM, Pascal Mazon <pascal.mazon@6wind.com> wrote:
> 
> Tap PMD is flexible, it supports any speed.
> 
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> ---
> doc/guides/nics/features/tap.ini |  1 +
> drivers/net/tap/rte_eth_tap.c    | 35 +++++++++++++++++++++++++++++++++++
> 2 files changed, 36 insertions(+)
> 
> diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
> index d9b47a003654..dad5a0561087 100644
> --- a/doc/guides/nics/features/tap.ini
> +++ b/doc/guides/nics/features/tap.ini
> @@ -9,6 +9,7 @@ Jumbo frame          = Y
> Promiscuous mode     = Y
> Allmulticast mode    = Y
> Basic stats          = Y
> +Speed capabilities   = Y
> Unicast MAC filter   = Y
> Other kdrv           = Y
> ARMv7                = Y
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 994c8be701c8..6670dfbb35ce 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev __rte_unused)
> 	return 0;
> }
> 
> +static uint32_t
> +tap_dev_speed_capa(void)
> +{
> +	uint32_t speed = pmd_link.link_speed;
> +	uint32_t capa = 0;
> +
> +	if (speed >= ETH_SPEED_NUM_10M)
> +		capa |= ETH_LINK_SPEED_10M;
> +	if (speed >= ETH_SPEED_NUM_100M)
> +		capa |= ETH_LINK_SPEED_100M;
> +	if (speed >= ETH_SPEED_NUM_1G)
> +		capa |= ETH_LINK_SPEED_1G;
> +	if (speed >= ETH_SPEED_NUM_5G)
> +		capa |= ETH_LINK_SPEED_2_5G;
> +	if (speed >= ETH_SPEED_NUM_5G)
> +		capa |= ETH_LINK_SPEED_5G;
> +	if (speed >= ETH_SPEED_NUM_10G)
> +		capa |= ETH_LINK_SPEED_10G;
> +	if (speed >= ETH_SPEED_NUM_20G)
> +		capa |= ETH_LINK_SPEED_20G;
> +	if (speed >= ETH_SPEED_NUM_25G)
> +		capa |= ETH_LINK_SPEED_25G;
> +	if (speed >= ETH_SPEED_NUM_40G)
> +		capa |= ETH_LINK_SPEED_40G;
> +	if (speed >= ETH_SPEED_NUM_50G)
> +		capa |= ETH_LINK_SPEED_50G;
> +	if (speed >= ETH_SPEED_NUM_56G)
> +		capa |= ETH_LINK_SPEED_56G;
> +	if (speed >= ETH_SPEED_NUM_100G)
> +		capa |= ETH_LINK_SPEED_100G;

In the real world the NIC may only support 50G an not say 10M, so in that case this code would be wrong as it would set all of the speeds up to 50G. I do not think the code should be changed, but I add a comment to tell the developer the issue here. I do not want someone copying the code and thinking is i correct for a real device.

> +
> +	return capa;
> +}
> +
> static void
> tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
> {
> @@ -363,6 +397,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
> 	dev_info->max_tx_queues = internals->nb_queues;
> 	dev_info->min_rx_bufsize = 0;
> 	dev_info->pci_dev = NULL;
> +	dev_info->speed_capa = tap_dev_speed_capa();
> }
> 
> static void
> -- 
> 2.8.0.rc0
> 

Regards,
Keith

Re: [PATCH 5/6] net/tap: add packet type management

[Wiles, Keith]

> On Mar 3, 2017, at 3:46 AM, Pascal Mazon <pascal.mazon@6wind.com> wrote:
> 
> Advertize RTE_PTYPE_UNKNOWN since tap does not report any packet type.
> 
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> ---
> doc/guides/nics/features/tap.ini |  1 +
> drivers/net/tap/rte_eth_tap.c    | 15 +++++++++++++++
> 2 files changed, 16 insertions(+)
> 
> diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
> index 6aa11874e2bc..7f3f4d661dd7 100644
> --- a/doc/guides/nics/features/tap.ini
> +++ b/doc/guides/nics/features/tap.ini
> @@ -13,6 +13,7 @@ MTU update           = Y
> Multicast MAC filter = Y
> Speed capabilities   = Y
> Unicast MAC filter   = Y
> +Packet type parsing  = Y
> Other kdrv           = Y
> ARMv7                = Y
> ARMv8                = Y
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 64b84cd76321..e4af36a6d142 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -36,6 +36,7 @@
> #include <rte_malloc.h>
> #include <rte_vdev.h>
> #include <rte_kvargs.h>
> +#include <rte_net.h>
> 
> #include <sys/types.h>
> #include <sys/stat.h>
> @@ -216,6 +217,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
> 		mbuf->data_len = len;
> 		mbuf->pkt_len = len;
> 		mbuf->port = rxq->in_port;
> +		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
> +						      RTE_PTYPE_ALL_MASK);
> 
> 		/* account for the receive frame */
> 		bufs[num_rx++] = mbuf;
> @@ -760,6 +763,17 @@ tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> 	return 0;
> }
> 
> +static const uint32_t*
> +tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
> +{
> +	static const uint32_t ptypes[] = {
> +		RTE_PTYPE_UNKNOWN,
> +
> +	};
> +
> +	return ptypes;
> +}

Can we just add the code to grab the ptype value instead of just saying not supported.

The original code would just return an error from ethdev correct, what was wrong with that one. I would like to see the tap PMD just return the ptype would that not be more useful?

> +
> static const struct eth_dev_ops ops = {
> 	.dev_start              = tap_dev_start,
> 	.dev_stop               = tap_dev_stop,
> @@ -784,6 +798,7 @@ static const struct eth_dev_ops ops = {
> 	.mtu_set                = tap_mtu_set,
> 	.stats_get              = tap_stats_get,
> 	.stats_reset            = tap_stats_reset,
> +	.dev_supported_ptypes_get = tap_dev_supported_ptypes_get,
> };
> 
> static int
> -- 
> 2.8.0.rc0
> 

Regards,
Keith

Re: [PATCH 2/6] net/tap: add speed capabilities

[Pascal Mazon]

On Fri, 3 Mar 2017 15:27:12 +0000
"Wiles, Keith" <keith.wiles@intel.com> wrote:

> 
> > On Mar 3, 2017, at 3:46 AM, Pascal Mazon <pascal.mazon@6wind.com>
> > wrote:
> > 
> > Tap PMD is flexible, it supports any speed.
> > 
> > Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> > ---
> > doc/guides/nics/features/tap.ini |  1 +
> > drivers/net/tap/rte_eth_tap.c    | 35
> > +++++++++++++++++++++++++++++++++++ 2 files changed, 36
> > insertions(+)
> > 
> > diff --git a/doc/guides/nics/features/tap.ini
> > b/doc/guides/nics/features/tap.ini index d9b47a003654..dad5a0561087
> > 100644 --- a/doc/guides/nics/features/tap.ini
> > +++ b/doc/guides/nics/features/tap.ini
> > @@ -9,6 +9,7 @@ Jumbo frame          = Y
> > Promiscuous mode     = Y
> > Allmulticast mode    = Y
> > Basic stats          = Y
> > +Speed capabilities   = Y
> > Unicast MAC filter   = Y
> > Other kdrv           = Y
> > ARMv7                = Y
> > diff --git a/drivers/net/tap/rte_eth_tap.c
> > b/drivers/net/tap/rte_eth_tap.c index 994c8be701c8..6670dfbb35ce
> > 100644 --- a/drivers/net/tap/rte_eth_tap.c
> > +++ b/drivers/net/tap/rte_eth_tap.c
> > @@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev
> > __rte_unused) return 0;
> > }
> > 
> > +static uint32_t
> > +tap_dev_speed_capa(void)
> > +{
> > +	uint32_t speed = pmd_link.link_speed;
> > +	uint32_t capa = 0;
> > +
> > +	if (speed >= ETH_SPEED_NUM_10M)
> > +		capa |= ETH_LINK_SPEED_10M;
> > +	if (speed >= ETH_SPEED_NUM_100M)
> > +		capa |= ETH_LINK_SPEED_100M;
> > +	if (speed >= ETH_SPEED_NUM_1G)
> > +		capa |= ETH_LINK_SPEED_1G;
> > +	if (speed >= ETH_SPEED_NUM_5G)
> > +		capa |= ETH_LINK_SPEED_2_5G;
> > +	if (speed >= ETH_SPEED_NUM_5G)
> > +		capa |= ETH_LINK_SPEED_5G;
> > +	if (speed >= ETH_SPEED_NUM_10G)
> > +		capa |= ETH_LINK_SPEED_10G;
> > +	if (speed >= ETH_SPEED_NUM_20G)
> > +		capa |= ETH_LINK_SPEED_20G;
> > +	if (speed >= ETH_SPEED_NUM_25G)
> > +		capa |= ETH_LINK_SPEED_25G;
> > +	if (speed >= ETH_SPEED_NUM_40G)
> > +		capa |= ETH_LINK_SPEED_40G;
> > +	if (speed >= ETH_SPEED_NUM_50G)
> > +		capa |= ETH_LINK_SPEED_50G;
> > +	if (speed >= ETH_SPEED_NUM_56G)
> > +		capa |= ETH_LINK_SPEED_56G;
> > +	if (speed >= ETH_SPEED_NUM_100G)
> > +		capa |= ETH_LINK_SPEED_100G;
> 
> In the real world the NIC may only support 50G an not say 10M, so in
> that case this code would be wrong as it would set all of the speeds
> up to 50G. I do not think the code should be changed, but I add a
> comment to tell the developer the issue here. I do not want someone
> copying the code and thinking is i correct for a real device.
> 

That's true for actual hardware. But tap is completely virtual, so
actually it could support any speed (it is limited by userland-kernel
communication speed).

What speed would you rather have the tap PMD report as capable of?

Best regards,
Pascal

> > +
> > +	return capa;
> > +}
> > +
> > static void
> > tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info
> > *dev_info) {
> > @@ -363,6 +397,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct
> > rte_eth_dev_info *dev_info) dev_info->max_tx_queues =
> > internals->nb_queues; dev_info->min_rx_bufsize = 0;
> > 	dev_info->pci_dev = NULL;
> > +	dev_info->speed_capa = tap_dev_speed_capa();
> > }
> > 
> > static void
> > -- 
> > 2.8.0.rc0
> > 
> 
> Regards,
> Keith
> 

Re: [PATCH 4/6] net/tap: add MTU management

[Pascal Mazon]

On Fri, 3 Mar 2017 15:23:28 +0000
"Wiles, Keith" <keith.wiles@intel.com> wrote:

> 
> > On Mar 3, 2017, at 3:46 AM, Pascal Mazon <pascal.mazon@6wind.com>
> > wrote:
> > 
> > The MTU is assigned to the tap netdevice according to the argument,
> > but packet transmission and reception just write/read on an fd with
> > the default limit being the socket buffer size.
> > 
> > Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> > ---
> > doc/guides/nics/features/tap.ini |  1 +
> > drivers/net/tap/rte_eth_tap.c    | 37
> > +++++++++++++++++++++++++++++++++++++ 2 files changed, 38
> > insertions(+)
> > 
> > diff --git a/doc/guides/nics/features/tap.ini
> > b/doc/guides/nics/features/tap.ini index 6878a9b8fd17..6aa11874e2bc
> > 100644 --- a/doc/guides/nics/features/tap.ini
> > +++ b/doc/guides/nics/features/tap.ini
> > @@ -9,6 +9,7 @@ Jumbo frame          = Y
> > Promiscuous mode     = Y
> > Allmulticast mode    = Y
> > Basic stats          = Y
> > +MTU update           = Y
> > Multicast MAC filter = Y
> > Speed capabilities   = Y
> > Unicast MAC filter   = Y
> > diff --git a/drivers/net/tap/rte_eth_tap.c
> > b/drivers/net/tap/rte_eth_tap.c index 131c09fbc1a5..64b84cd76321
> > 100644 --- a/drivers/net/tap/rte_eth_tap.c
> > +++ b/drivers/net/tap/rte_eth_tap.c
> > @@ -724,6 +724,42 @@ tap_set_mc_addr_list(struct rte_eth_dev *dev
> > __rte_unused, return 0;
> > }
> > 
> > +static int
> > +tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> > +{
> > +	struct pmd_internals *pmd = dev->data->dev_private;
> > +	struct ifreq ifr;
> > +	int err, s;
> > +
> > +	s = socket(AF_INET, SOCK_DGRAM, 0);
> > +	if (s < 0) {
> > +		RTE_LOG(ERR, PMD,
> > +			"Unable to get a socket for %s to set
> > flags: %s\n",
> > +			pmd->name, strerror(errno));
> > +		return -1;
> > +	}
> > +	memset(&ifr, 0, sizeof(ifr));
> > +	strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
> 
> This needs to be converted to a snprintf() to avoid overflow.
> 

Ok, I'll do that in version 2.

Regards,
Pascal

> > +	err = ioctl(s, SIOCGIFMTU, &ifr);
> > +	if (err < 0) {
> > +		RTE_LOG(WARNING, PMD, "Unable to get %s device
> > MTU: %s\n",
> > +			pmd->name, strerror(errno));
> > +		close(s);
> > +		return -1;
> > +	}
> > +	ifr.ifr_mtu = mtu;
> > +	err = ioctl(s, SIOCSIFMTU, &ifr);
> > +	if (err < 0) {
> > +		RTE_LOG(WARNING, PMD, "Unable to set %s mtu %d:
> > %s\n",
> > +			pmd->name, mtu, strerror(errno));
> > +		close(s);
> > +		return -1;
> > +	}
> > +	close(s);
> > +	dev->data->mtu = mtu;
> > +	return 0;
> > +}
> > +
> > static const struct eth_dev_ops ops = {
> > 	.dev_start              = tap_dev_start,
> > 	.dev_stop               = tap_dev_stop,
> > @@ -745,6 +781,7 @@ static const struct eth_dev_ops ops = {
> > 	.mac_addr_add           = tap_mac_add,
> > 	.mac_addr_set           = tap_mac_set,
> > 	.set_mc_addr_list       = tap_set_mc_addr_list,
> > +	.mtu_set                = tap_mtu_set,
> > 	.stats_get              = tap_stats_get,
> > 	.stats_reset            = tap_stats_reset,
> > };
> > -- 
> > 2.8.0.rc0
> > 
> 
> Regards,
> Keith
> 

Re: [PATCH 5/6] net/tap: add packet type management

[Pascal Mazon]

On Fri, 3 Mar 2017 15:31:14 +0000
"Wiles, Keith" <keith.wiles@intel.com> wrote:

> 
> > On Mar 3, 2017, at 3:46 AM, Pascal Mazon <pascal.mazon@6wind.com>
> > wrote:
> > 
> > Advertize RTE_PTYPE_UNKNOWN since tap does not report any packet
> > type.
> > 
> > Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> > ---
> > doc/guides/nics/features/tap.ini |  1 +
> > drivers/net/tap/rte_eth_tap.c    | 15 +++++++++++++++
> > 2 files changed, 16 insertions(+)
> > 
> > diff --git a/doc/guides/nics/features/tap.ini
> > b/doc/guides/nics/features/tap.ini index 6aa11874e2bc..7f3f4d661dd7
> > 100644 --- a/doc/guides/nics/features/tap.ini
> > +++ b/doc/guides/nics/features/tap.ini
> > @@ -13,6 +13,7 @@ MTU update           = Y
> > Multicast MAC filter = Y
> > Speed capabilities   = Y
> > Unicast MAC filter   = Y
> > +Packet type parsing  = Y
> > Other kdrv           = Y
> > ARMv7                = Y
> > ARMv8                = Y
> > diff --git a/drivers/net/tap/rte_eth_tap.c
> > b/drivers/net/tap/rte_eth_tap.c index 64b84cd76321..e4af36a6d142
> > 100644 --- a/drivers/net/tap/rte_eth_tap.c
> > +++ b/drivers/net/tap/rte_eth_tap.c
> > @@ -36,6 +36,7 @@
> > #include <rte_malloc.h>
> > #include <rte_vdev.h>
> > #include <rte_kvargs.h>
> > +#include <rte_net.h>
> > 
> > #include <sys/types.h>
> > #include <sys/stat.h>
> > @@ -216,6 +217,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf
> > **bufs, uint16_t nb_pkts) mbuf->data_len = len;
> > 		mbuf->pkt_len = len;
> > 		mbuf->port = rxq->in_port;
> > +		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
> > +
> > RTE_PTYPE_ALL_MASK);
> > 
> > 		/* account for the receive frame */
> > 		bufs[num_rx++] = mbuf;
> > @@ -760,6 +763,17 @@ tap_mtu_set(struct rte_eth_dev *dev, uint16_t
> > mtu) return 0;
> > }
> > 
> > +static const uint32_t*
> > +tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
> > +{
> > +	static const uint32_t ptypes[] = {
> > +		RTE_PTYPE_UNKNOWN,
> > +
> > +	};
> > +
> > +	return ptypes;
> > +}
> 
> Can we just add the code to grab the ptype value instead of just
> saying not supported.
> 
> The original code would just return an error from ethdev correct,
> what was wrong with that one. I would like to see the tap PMD just
> return the ptype would that not be more useful?
> 

tap PMD depends on the rte_net_get_ptype(), which code may change in the
future to support more packet types. Those changes would then need to be
reflected on the tap PMD, to be consistent.

I reported only RTE_PTYPE_UNKNOWN to avoid keeping a tight sync with the
rte_net library. As we're allowed to be more precise in the packet types
we actually set, compared to those we declare as supported, I thought it
best.

Would you indeed rather we copied all currently supported packet types
from rte_net to tap_dev_supported_ptypes_get()?

Regards,
Pascal

> > +
> > static const struct eth_dev_ops ops = {
> > 	.dev_start              = tap_dev_start,
> > 	.dev_stop               = tap_dev_stop,
> > @@ -784,6 +798,7 @@ static const struct eth_dev_ops ops = {
> > 	.mtu_set                = tap_mtu_set,
> > 	.stats_get              = tap_stats_get,
> > 	.stats_reset            = tap_stats_reset,
> > +	.dev_supported_ptypes_get = tap_dev_supported_ptypes_get,
> > };
> > 
> > static int
> > -- 
> > 2.8.0.rc0
> > 
> 
> Regards,
> Keith
> 

Re: [PATCH 2/6] net/tap: add speed capabilities

[Wiles, Keith]

> On Mar 6, 2017, at 7:58 AM, Pascal Mazon <pascal.mazon@6wind.com> wrote:
> 
> On Fri, 3 Mar 2017 15:27:12 +0000
> "Wiles, Keith" <keith.wiles@intel.com> wrote:
> 
>> 
>>> On Mar 3, 2017, at 3:46 AM, Pascal Mazon <pascal.mazon@6wind.com>
>>> wrote:
>>> 
>>> Tap PMD is flexible, it supports any speed.
>>> 
>>> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
>>> ---
>>> doc/guides/nics/features/tap.ini |  1 +
>>> drivers/net/tap/rte_eth_tap.c    | 35
>>> +++++++++++++++++++++++++++++++++++ 2 files changed, 36
>>> insertions(+)
>>> 
>>> diff --git a/doc/guides/nics/features/tap.ini
>>> b/doc/guides/nics/features/tap.ini index d9b47a003654..dad5a0561087
>>> 100644 --- a/doc/guides/nics/features/tap.ini
>>> +++ b/doc/guides/nics/features/tap.ini
>>> @@ -9,6 +9,7 @@ Jumbo frame          = Y
>>> Promiscuous mode     = Y
>>> Allmulticast mode    = Y
>>> Basic stats          = Y
>>> +Speed capabilities   = Y
>>> Unicast MAC filter   = Y
>>> Other kdrv           = Y
>>> ARMv7                = Y
>>> diff --git a/drivers/net/tap/rte_eth_tap.c
>>> b/drivers/net/tap/rte_eth_tap.c index 994c8be701c8..6670dfbb35ce
>>> 100644 --- a/drivers/net/tap/rte_eth_tap.c
>>> +++ b/drivers/net/tap/rte_eth_tap.c
>>> @@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev
>>> __rte_unused) return 0;
>>> }
>>> 
>>> +static uint32_t
>>> +tap_dev_speed_capa(void)
>>> +{
>>> +	uint32_t speed = pmd_link.link_speed;
>>> +	uint32_t capa = 0;
>>> +
>>> +	if (speed >= ETH_SPEED_NUM_10M)
>>> +		capa |= ETH_LINK_SPEED_10M;
>>> +	if (speed >= ETH_SPEED_NUM_100M)
>>> +		capa |= ETH_LINK_SPEED_100M;
>>> +	if (speed >= ETH_SPEED_NUM_1G)
>>> +		capa |= ETH_LINK_SPEED_1G;
>>> +	if (speed >= ETH_SPEED_NUM_5G)
>>> +		capa |= ETH_LINK_SPEED_2_5G;
>>> +	if (speed >= ETH_SPEED_NUM_5G)
>>> +		capa |= ETH_LINK_SPEED_5G;
>>> +	if (speed >= ETH_SPEED_NUM_10G)
>>> +		capa |= ETH_LINK_SPEED_10G;
>>> +	if (speed >= ETH_SPEED_NUM_20G)
>>> +		capa |= ETH_LINK_SPEED_20G;
>>> +	if (speed >= ETH_SPEED_NUM_25G)
>>> +		capa |= ETH_LINK_SPEED_25G;
>>> +	if (speed >= ETH_SPEED_NUM_40G)
>>> +		capa |= ETH_LINK_SPEED_40G;
>>> +	if (speed >= ETH_SPEED_NUM_50G)
>>> +		capa |= ETH_LINK_SPEED_50G;
>>> +	if (speed >= ETH_SPEED_NUM_56G)
>>> +		capa |= ETH_LINK_SPEED_56G;
>>> +	if (speed >= ETH_SPEED_NUM_100G)
>>> +		capa |= ETH_LINK_SPEED_100G;
>> 
>> In the real world the NIC may only support 50G an not say 10M, so in
>> that case this code would be wrong as it would set all of the speeds
>> up to 50G. I do not think the code should be changed, but I add a
>> comment to tell the developer the issue here. I do not want someone
>> copying the code and thinking is i correct for a real device.
>> 
> 
> That's true for actual hardware. But tap is completely virtual, so
> actually it could support any speed (it is limited by userland-kernel
> communication speed).
> 
> What speed would you rather have the tap PMD report as capable of?

This note was a very minor picky point and you can ignore my comment here. It is not worth the time to deal with and you have better things to do :-)

> 
> Best regards,
> Pascal
> 
>>> +
>>> +	return capa;
>>> +}
>>> +
>>> static void
>>> tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info
>>> *dev_info) {
>>> @@ -363,6 +397,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct
>>> rte_eth_dev_info *dev_info) dev_info->max_tx_queues =
>>> internals->nb_queues; dev_info->min_rx_bufsize = 0;
>>> 	dev_info->pci_dev = NULL;
>>> +	dev_info->speed_capa = tap_dev_speed_capa();
>>> }
>>> 
>>> static void
>>> -- 
>>> 2.8.0.rc0
>>> 
>> 
>> Regards,
>> Keith
>> 
> 

Regards,
Keith

Re: [PATCH 5/6] net/tap: add packet type management

[Wiles, Keith]

> On Mar 6, 2017, at 8:10 AM, Pascal Mazon <pascal.mazon@6wind.com> wrote:
> 
> On Fri, 3 Mar 2017 15:31:14 +0000
> "Wiles, Keith" <keith.wiles@intel.com> wrote:
> 
>> 
>>> On Mar 3, 2017, at 3:46 AM, Pascal Mazon <pascal.mazon@6wind.com>
>>> wrote:
>>> 
>>> Advertize RTE_PTYPE_UNKNOWN since tap does not report any packet
>>> type.
>>> 
>>> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
>>> ---
>>> doc/guides/nics/features/tap.ini |  1 +
>>> drivers/net/tap/rte_eth_tap.c    | 15 +++++++++++++++
>>> 2 files changed, 16 insertions(+)
>>> 
>>> diff --git a/doc/guides/nics/features/tap.ini
>>> b/doc/guides/nics/features/tap.ini index 6aa11874e2bc..7f3f4d661dd7
>>> 100644 --- a/doc/guides/nics/features/tap.ini
>>> +++ b/doc/guides/nics/features/tap.ini
>>> @@ -13,6 +13,7 @@ MTU update           = Y
>>> Multicast MAC filter = Y
>>> Speed capabilities   = Y
>>> Unicast MAC filter   = Y
>>> +Packet type parsing  = Y
>>> Other kdrv           = Y
>>> ARMv7                = Y
>>> ARMv8                = Y
>>> diff --git a/drivers/net/tap/rte_eth_tap.c
>>> b/drivers/net/tap/rte_eth_tap.c index 64b84cd76321..e4af36a6d142
>>> 100644 --- a/drivers/net/tap/rte_eth_tap.c
>>> +++ b/drivers/net/tap/rte_eth_tap.c
>>> @@ -36,6 +36,7 @@
>>> #include <rte_malloc.h>
>>> #include <rte_vdev.h>
>>> #include <rte_kvargs.h>
>>> +#include <rte_net.h>
>>> 
>>> #include <sys/types.h>
>>> #include <sys/stat.h>
>>> @@ -216,6 +217,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf
>>> **bufs, uint16_t nb_pkts) mbuf->data_len = len;
>>> 		mbuf->pkt_len = len;
>>> 		mbuf->port = rxq->in_port;
>>> +		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
>>> +
>>> RTE_PTYPE_ALL_MASK);
>>> 
>>> 		/* account for the receive frame */
>>> 		bufs[num_rx++] = mbuf;
>>> @@ -760,6 +763,17 @@ tap_mtu_set(struct rte_eth_dev *dev, uint16_t
>>> mtu) return 0;
>>> }
>>> 
>>> +static const uint32_t*
>>> +tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
>>> +{
>>> +	static const uint32_t ptypes[] = {
>>> +		RTE_PTYPE_UNKNOWN,
>>> +
>>> +	};
>>> +
>>> +	return ptypes;
>>> +}
>> 
>> Can we just add the code to grab the ptype value instead of just
>> saying not supported.
>> 
>> The original code would just return an error from ethdev correct,
>> what was wrong with that one. I would like to see the tap PMD just
>> return the ptype would that not be more useful?
>> 
> 
> tap PMD depends on the rte_net_get_ptype(), which code may change in the
> future to support more packet types. Those changes would then need to be
> reflected on the tap PMD, to be consistent.
> 
> I reported only RTE_PTYPE_UNKNOWN to avoid keeping a tight sync with the
> rte_net library. As we're allowed to be more precise in the packet types
> we actually set, compared to those we declare as supported, I thought it
> best.
> 
> Would you indeed rather we copied all currently supported packet types
> from rte_net to tap_dev_supported_ptypes_get()?

I see your point here, it not reasonable to copy all of the ptypes from rte_net to here ignore my comment.

> 
> Regards,
> Pascal
> 
>>> +
>>> static const struct eth_dev_ops ops = {
>>> 	.dev_start              = tap_dev_start,
>>> 	.dev_stop               = tap_dev_stop,
>>> @@ -784,6 +798,7 @@ static const struct eth_dev_ops ops = {
>>> 	.mtu_set                = tap_mtu_set,
>>> 	.stats_get              = tap_stats_get,
>>> 	.stats_reset            = tap_stats_reset,
>>> +	.dev_supported_ptypes_get = tap_dev_supported_ptypes_get,
>>> };
>>> 
>>> static int
>>> -- 
>>> 2.8.0.rc0
>>> 
>> 
>> Regards,
>> Keith
>> 
> 

Regards,
Keith

[PATCH v2 0/6] net/tap: add additional management ops

[Pascal Mazon]

Add a few eth_dev ops to the tap PMD for completeness.
We want it to behave as much as possible as a standard PMD.

v2 change:
  - use snprintf in tap_mtu set

Pascal Mazon (6):
  net/tap: add MAC address management ops
  net/tap: add speed capabilities
  net/tap: add multicast addresses management
  net/tap: add MTU management
  net/tap: add packet type management
  net/tap: add flow control management

 doc/guides/nics/features/tap.ini |   6 ++
 drivers/net/tap/rte_eth_tap.c    | 207 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 200 insertions(+), 13 deletions(-)

-- 
2.8.0.rc0

[PATCH v2 1/6] net/tap: add MAC address management ops

[Pascal Mazon]

Set a random MAC address when probing the device, as to not leave an
empty MAC in pmd->eth_addr.

This MAC will be set on the tap netdevice as soon as it's been created
using tun_alloc(). As the tap_mac_add() function depend on the fd in
the first rxq, move code from tun_alloc() to tap_setup_queue(),
after it's been set.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 88 ++++++++++++++++++++++++++++++++++------
 2 files changed, 76 insertions(+), 13 deletions(-)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index f4aca6921ddc..d9b47a003654 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+Unicast MAC filter   = Y
 Other kdrv           = Y
 ARMv7                = Y
 ARMv8                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index ece3a5fcc897..020ae8251a94 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -63,6 +63,8 @@
 #define RTE_PMD_TAP_MAX_QUEUES	1
 #endif
 
+#define RTE_PMD_TAP_MAX_MAC_ADDRS 1
+
 static struct rte_vdev_driver pmd_tap_drv;
 
 static const char *valid_arguments[] = {
@@ -118,7 +120,7 @@ struct pmd_internals {
  * supplied name.
  */
 static int
-tun_alloc(struct pmd_internals *pmd, uint16_t qid)
+tun_alloc(struct pmd_internals *pmd)
 {
 	struct ifreq ifr;
 #ifdef IFF_MULTI_QUEUE
@@ -176,16 +178,6 @@ tun_alloc(struct pmd_internals *pmd, uint16_t qid)
 		goto error;
 	}
 
-	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;
 
 error:
@@ -365,7 +357,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	struct pmd_internals *internals = dev->data->dev_private;
 
 	dev_info->if_index = internals->if_index;
-	dev_info->max_mac_addrs = 1;
+	dev_info->max_mac_addrs = RTE_PMD_TAP_MAX_MAC_ADDRS;
 	dev_info->max_rx_pktlen = (uint32_t)ETHER_MAX_VLAN_FRAME_LEN;
 	dev_info->max_rx_queues = internals->nb_queues;
 	dev_info->max_tx_queues = internals->nb_queues;
@@ -502,6 +494,69 @@ tap_allmulti_disable(struct rte_eth_dev *dev)
 	tap_link_set_flags(pmd, IFF_ALLMULTI, 0);
 }
 
+static void
+tap_mac_remove(struct rte_eth_dev *dev __rte_unused,
+	       uint32_t index __rte_unused)
+{
+	/*
+	 * A single MAC is authorized on the device and it's not possible to
+	 * leave the device without a MAC. Don't do anything, then.
+	 */
+}
+
+static void
+tap_mac_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
+	    uint32_t index, uint32_t vmdq __rte_unused)
+{
+	struct pmd_internals *internals = dev->data->dev_private;
+	int fd = internals->rxq[0].fd;
+	struct ifreq ifr;
+
+	if (index > RTE_PMD_TAP_MAX_MAC_ADDRS - 1) {
+		RTE_LOG(ERR, PMD,
+			"%s: can't set MAC address: index %d > max %d\n",
+			dev->data->name, index, RTE_PMD_TAP_MAX_MAC_ADDRS - 1);
+		return;
+	}
+	if (is_zero_ether_addr(mac_addr)) {
+		RTE_LOG(ERR, PMD,
+			"%s: can't set an empty MAC address.\n",
+			dev->data->name);
+		return;
+	}
+	if (fd < 0) {
+		RTE_LOG(ERR, PMD,
+			"%s: can't set MAC address: device does not exist.\n",
+			dev->data->name);
+		return;
+	}
+	memset(&ifr, 0, sizeof(struct ifreq));
+	if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
+		RTE_LOG(ERR, PMD, "%s: couldn't get current MAC address (%s)\n",
+			dev->data->name, strerror(errno));
+		return;
+	}
+	rte_memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, ETHER_ADDR_LEN);
+	if (ioctl(fd, SIOCSIFHWADDR, &ifr) == -1) {
+		RTE_LOG(ERR, PMD, "%s: couldn't set current MAC address (%s)\n",
+			dev->data->name, strerror(errno));
+		return;
+	}
+	rte_memcpy(&dev->data->mac_addrs[index], mac_addr, ETHER_ADDR_LEN);
+}
+
+static void
+tap_mac_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
+{
+	if (is_zero_ether_addr(mac_addr)) {
+		RTE_LOG(ERR, PMD, "%s: can't set an empty MAC address\n",
+			dev->data->name);
+		return;
+	}
+	tap_mac_remove(dev, 0);
+	tap_mac_add(dev, mac_addr, 0, 0);
+}
+
 static int
 tap_setup_queue(struct rte_eth_dev *dev,
 		struct pmd_internals *internals,
@@ -518,7 +573,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, qid);
+			fd = tun_alloc(pmd);
 			if (fd < 0) {
 				RTE_LOG(ERR, PMD, "tun_alloc(%s, %d) failed\n",
 					pmd->name, qid);
@@ -530,6 +585,9 @@ tap_setup_queue(struct rte_eth_dev *dev,
 	rx->fd = fd;
 	tx->fd = fd;
 
+	if (qid == 0)
+		tap_mac_set(dev, &pmd->eth_addr);
+
 	return fd;
 }
 
@@ -636,6 +694,9 @@ static const struct eth_dev_ops ops = {
 	.promiscuous_disable    = tap_promisc_disable,
 	.allmulticast_enable    = tap_allmulti_enable,
 	.allmulticast_disable   = tap_allmulti_disable,
+	.mac_addr_remove        = tap_mac_remove,
+	.mac_addr_add           = tap_mac_add,
+	.mac_addr_set           = tap_mac_set,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
 };
@@ -683,6 +744,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	data->drv_name = pmd_tap_drv.driver.name;
 	data->numa_node = numa_node;
 
+	eth_random_addr((uint8_t *)&pmd->eth_addr);
 	data->dev_link = pmd_link;
 	data->mac_addrs = &pmd->eth_addr;
 	data->nb_rx_queues = pmd->nb_queues;
-- 
2.8.0.rc0

[PATCH v2 2/6] net/tap: add speed capabilities

[Pascal Mazon]

Tap PMD is flexible, it supports any speed.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index d9b47a003654..dad5a0561087 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+Speed capabilities   = Y
 Unicast MAC filter   = Y
 Other kdrv           = Y
 ARMv7                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 020ae8251a94..a1ca646305ea 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev __rte_unused)
 	return 0;
 }
 
+static uint32_t
+tap_dev_speed_capa(void)
+{
+	uint32_t speed = pmd_link.link_speed;
+	uint32_t capa = 0;
+
+	if (speed >= ETH_SPEED_NUM_10M)
+		capa |= ETH_LINK_SPEED_10M;
+	if (speed >= ETH_SPEED_NUM_100M)
+		capa |= ETH_LINK_SPEED_100M;
+	if (speed >= ETH_SPEED_NUM_1G)
+		capa |= ETH_LINK_SPEED_1G;
+	if (speed >= ETH_SPEED_NUM_5G)
+		capa |= ETH_LINK_SPEED_2_5G;
+	if (speed >= ETH_SPEED_NUM_5G)
+		capa |= ETH_LINK_SPEED_5G;
+	if (speed >= ETH_SPEED_NUM_10G)
+		capa |= ETH_LINK_SPEED_10G;
+	if (speed >= ETH_SPEED_NUM_20G)
+		capa |= ETH_LINK_SPEED_20G;
+	if (speed >= ETH_SPEED_NUM_25G)
+		capa |= ETH_LINK_SPEED_25G;
+	if (speed >= ETH_SPEED_NUM_40G)
+		capa |= ETH_LINK_SPEED_40G;
+	if (speed >= ETH_SPEED_NUM_50G)
+		capa |= ETH_LINK_SPEED_50G;
+	if (speed >= ETH_SPEED_NUM_56G)
+		capa |= ETH_LINK_SPEED_56G;
+	if (speed >= ETH_SPEED_NUM_100G)
+		capa |= ETH_LINK_SPEED_100G;
+
+	return capa;
+}
+
 static void
 tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
@@ -363,6 +397,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_tx_queues = internals->nb_queues;
 	dev_info->min_rx_bufsize = 0;
 	dev_info->pci_dev = NULL;
+	dev_info->speed_capa = tap_dev_speed_capa();
 }
 
 static void
-- 
2.8.0.rc0

[PATCH v2 3/6] net/tap: add multicast addresses management

[Pascal Mazon]

A tap netdevice actually receives every packet, without any filtering
whatsoever. There is no need for any multicast address registration
to receive multicast packets.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index dad5a0561087..6878a9b8fd17 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
 Other kdrv           = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index a1ca646305ea..957e57ad3466 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -712,6 +712,18 @@ tap_tx_queue_setup(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static int
+tap_set_mc_addr_list(struct rte_eth_dev *dev __rte_unused,
+		     struct ether_addr *mc_addr_set __rte_unused,
+		     uint32_t nb_mc_addr __rte_unused)
+{
+	/*
+	 * Nothing to do actually: the tap has no filtering whatsoever, every
+	 * packet is received.
+	 */
+	return 0;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -732,6 +744,7 @@ static const struct eth_dev_ops ops = {
 	.mac_addr_remove        = tap_mac_remove,
 	.mac_addr_add           = tap_mac_add,
 	.mac_addr_set           = tap_mac_set,
+	.set_mc_addr_list       = tap_set_mc_addr_list,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
 };
-- 
2.8.0.rc0

[PATCH v2 4/6] net/tap: add MTU management

[Pascal Mazon]

The MTU is assigned to the tap netdevice according to the argument, but
packet transmission and reception just write/read on an fd with the
default limit being the socket buffer size.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index 6878a9b8fd17..6aa11874e2bc 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+MTU update           = Y
 Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 957e57ad3466..e15a037785e9 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -724,6 +724,42 @@ tap_set_mc_addr_list(struct rte_eth_dev *dev __rte_unused,
 	return 0;
 }
 
+static int
+tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr;
+	int err, s;
+
+	s = socket(AF_INET, SOCK_DGRAM, 0);
+	if (s < 0) {
+		RTE_LOG(ERR, PMD,
+			"Unable to get a socket for %s to set flags: %s\n",
+			pmd->name, strerror(errno));
+		return -1;
+	}
+	memset(&ifr, 0, sizeof(ifr));
+	strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
+	err = ioctl(s, SIOCGIFMTU, &ifr);
+	if (err < 0) {
+		RTE_LOG(WARNING, PMD, "Unable to get %s device MTU: %s\n",
+			pmd->name, strerror(errno));
+		close(s);
+		return -1;
+	}
+	ifr.ifr_mtu = mtu;
+	err = ioctl(s, SIOCSIFMTU, &ifr);
+	if (err < 0) {
+		RTE_LOG(WARNING, PMD, "Unable to set %s mtu %d: %s\n",
+			pmd->name, mtu, strerror(errno));
+		close(s);
+		return -1;
+	}
+	close(s);
+	dev->data->mtu = mtu;
+	return 0;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -745,6 +781,7 @@ static const struct eth_dev_ops ops = {
 	.mac_addr_add           = tap_mac_add,
 	.mac_addr_set           = tap_mac_set,
 	.set_mc_addr_list       = tap_set_mc_addr_list,
+	.mtu_set                = tap_mtu_set,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
 };
-- 
2.8.0.rc0

[PATCH v2 5/6] net/tap: add packet type management

[Pascal Mazon]

Advertize RTE_PTYPE_UNKNOWN since tap does not report any packet type.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index 6aa11874e2bc..7f3f4d661dd7 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -13,6 +13,7 @@ MTU update           = Y
 Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
+Packet type parsing  = Y
 Other kdrv           = Y
 ARMv7                = Y
 ARMv8                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index e15a037785e9..4d0188d1dde1 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -36,6 +36,7 @@
 #include <rte_malloc.h>
 #include <rte_vdev.h>
 #include <rte_kvargs.h>
+#include <rte_net.h>
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -216,6 +217,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 		mbuf->data_len = len;
 		mbuf->pkt_len = len;
 		mbuf->port = rxq->in_port;
+		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
+						      RTE_PTYPE_ALL_MASK);
 
 		/* account for the receive frame */
 		bufs[num_rx++] = mbuf;
@@ -760,6 +763,17 @@ tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	return 0;
 }
 
+static const uint32_t*
+tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
+{
+	static const uint32_t ptypes[] = {
+		RTE_PTYPE_UNKNOWN,
+
+	};
+
+	return ptypes;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -784,6 +798,7 @@ static const struct eth_dev_ops ops = {
 	.mtu_set                = tap_mtu_set,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
+	.dev_supported_ptypes_get = tap_dev_supported_ptypes_get,
 };
 
 static int
-- 
2.8.0.rc0

[PATCH v2 6/6] net/tap: add flow control management

[Pascal Mazon]

A tap netdevice does not support flow control; ensure nothing but
RTE_FC_NONE mode can be set.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index 7f3f4d661dd7..a51712dce066 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -14,6 +14,7 @@ Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
 Packet type parsing  = Y
+Flow control         = Y
 Other kdrv           = Y
 ARMv7                = Y
 ARMv8                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 4d0188d1dde1..cc3194b8a69c 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -774,6 +774,23 @@ tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
 	return ptypes;
 }
 
+static int
+tap_flow_ctrl_get(struct rte_eth_dev *dev __rte_unused,
+		  struct rte_eth_fc_conf *fc_conf)
+{
+	fc_conf->mode = RTE_FC_NONE;
+	return 0;
+}
+
+static int
+tap_flow_ctrl_set(struct rte_eth_dev *dev __rte_unused,
+		  struct rte_eth_fc_conf *fc_conf)
+{
+	if (fc_conf->mode != RTE_FC_NONE)
+		return -ENOTSUP;
+	return 0;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -784,6 +801,8 @@ static const struct eth_dev_ops ops = {
 	.tx_queue_setup         = tap_tx_queue_setup,
 	.rx_queue_release       = tap_rx_queue_release,
 	.tx_queue_release       = tap_tx_queue_release,
+	.flow_ctrl_get          = tap_flow_ctrl_get,
+	.flow_ctrl_set          = tap_flow_ctrl_set,
 	.link_update            = tap_link_update,
 	.dev_set_link_up        = tap_link_set_up,
 	.dev_set_link_down      = tap_link_set_down,
-- 
2.8.0.rc0

[PATCH v3 0/6] net/tap: add additional management ops

[Pascal Mazon]

Add a few eth_dev ops to the tap PMD for completeness.
We want it to behave as much as possible as a standard PMD.

v2 change:
  - use snprintf in tap_mtu set

v3 change:
  - call tap_mac_set() only once in tap_setup_queue()

Pascal Mazon (6):
  net/tap: add MAC address management ops
  net/tap: add speed capabilities
  net/tap: add multicast addresses management
  net/tap: add MTU management
  net/tap: add packet type management
  net/tap: add flow control management

 doc/guides/nics/features/tap.ini |   6 ++
 drivers/net/tap/rte_eth_tap.c    | 216 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 209 insertions(+), 13 deletions(-)

-- 
2.8.0.rc0

[PATCH v3 1/6] net/tap: add MAC address management ops

[Pascal Mazon]

Set a random MAC address when probing the device, as to not leave an
empty MAC in pmd->eth_addr.

This MAC will be set on the tap netdevice as soon as it's been created
using tun_alloc(). As the tap_mac_add() function depend on the fd in
the first rxq, move code from tun_alloc() to tap_setup_queue(),
after it's been set.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 97 ++++++++++++++++++++++++++++++++++------
 2 files changed, 85 insertions(+), 13 deletions(-)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index f4aca6921ddc..d9b47a003654 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+Unicast MAC filter   = Y
 Other kdrv           = Y
 ARMv7                = Y
 ARMv8                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index ece3a5fcc897..1e46ee36efa2 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -63,6 +63,8 @@
 #define RTE_PMD_TAP_MAX_QUEUES	1
 #endif
 
+#define RTE_PMD_TAP_MAX_MAC_ADDRS 1
+
 static struct rte_vdev_driver pmd_tap_drv;
 
 static const char *valid_arguments[] = {
@@ -118,7 +120,7 @@ struct pmd_internals {
  * supplied name.
  */
 static int
-tun_alloc(struct pmd_internals *pmd, uint16_t qid)
+tun_alloc(struct pmd_internals *pmd)
 {
 	struct ifreq ifr;
 #ifdef IFF_MULTI_QUEUE
@@ -176,16 +178,6 @@ tun_alloc(struct pmd_internals *pmd, uint16_t qid)
 		goto error;
 	}
 
-	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;
 
 error:
@@ -365,7 +357,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	struct pmd_internals *internals = dev->data->dev_private;
 
 	dev_info->if_index = internals->if_index;
-	dev_info->max_mac_addrs = 1;
+	dev_info->max_mac_addrs = RTE_PMD_TAP_MAX_MAC_ADDRS;
 	dev_info->max_rx_pktlen = (uint32_t)ETHER_MAX_VLAN_FRAME_LEN;
 	dev_info->max_rx_queues = internals->nb_queues;
 	dev_info->max_tx_queues = internals->nb_queues;
@@ -502,6 +494,69 @@ tap_allmulti_disable(struct rte_eth_dev *dev)
 	tap_link_set_flags(pmd, IFF_ALLMULTI, 0);
 }
 
+static void
+tap_mac_remove(struct rte_eth_dev *dev __rte_unused,
+	       uint32_t index __rte_unused)
+{
+	/*
+	 * A single MAC is authorized on the device and it's not possible to
+	 * leave the device without a MAC. Don't do anything, then.
+	 */
+}
+
+static void
+tap_mac_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
+	    uint32_t index, uint32_t vmdq __rte_unused)
+{
+	struct pmd_internals *internals = dev->data->dev_private;
+	int fd = internals->rxq[0].fd;
+	struct ifreq ifr;
+
+	if (index > RTE_PMD_TAP_MAX_MAC_ADDRS - 1) {
+		RTE_LOG(ERR, PMD,
+			"%s: can't set MAC address: index %d > max %d\n",
+			dev->data->name, index, RTE_PMD_TAP_MAX_MAC_ADDRS - 1);
+		return;
+	}
+	if (is_zero_ether_addr(mac_addr)) {
+		RTE_LOG(ERR, PMD,
+			"%s: can't set an empty MAC address.\n",
+			dev->data->name);
+		return;
+	}
+	if (fd < 0) {
+		RTE_LOG(ERR, PMD,
+			"%s: can't set MAC address: device does not exist.\n",
+			dev->data->name);
+		return;
+	}
+	memset(&ifr, 0, sizeof(struct ifreq));
+	if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
+		RTE_LOG(ERR, PMD, "%s: couldn't get current MAC address (%s)\n",
+			dev->data->name, strerror(errno));
+		return;
+	}
+	rte_memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, ETHER_ADDR_LEN);
+	if (ioctl(fd, SIOCSIFHWADDR, &ifr) == -1) {
+		RTE_LOG(ERR, PMD, "%s: couldn't set current MAC address (%s)\n",
+			dev->data->name, strerror(errno));
+		return;
+	}
+	rte_memcpy(&dev->data->mac_addrs[index], mac_addr, ETHER_ADDR_LEN);
+}
+
+static void
+tap_mac_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
+{
+	if (is_zero_ether_addr(mac_addr)) {
+		RTE_LOG(ERR, PMD, "%s: can't set an empty MAC address\n",
+			dev->data->name);
+		return;
+	}
+	tap_mac_remove(dev, 0);
+	tap_mac_add(dev, mac_addr, 0, 0);
+}
+
 static int
 tap_setup_queue(struct rte_eth_dev *dev,
 		struct pmd_internals *internals,
@@ -518,7 +573,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, qid);
+			fd = tun_alloc(pmd);
 			if (fd < 0) {
 				RTE_LOG(ERR, PMD, "tun_alloc(%s, %d) failed\n",
 					pmd->name, qid);
@@ -530,6 +585,18 @@ tap_setup_queue(struct rte_eth_dev *dev,
 	rx->fd = fd;
 	tx->fd = fd;
 
+	if (qid == 0) {
+		/*
+		 * tap_setup_queue() is called for both tx and rx.
+		 * Let's use dev->data->r/tx_queues[qid] to determine if init
+		 * has already been done.
+		 */
+		if (dev->data->rx_queues[qid] && dev->data->tx_queues[qid])
+			return fd;
+
+		tap_mac_set(dev, &pmd->eth_addr);
+	}
+
 	return fd;
 }
 
@@ -636,6 +703,9 @@ static const struct eth_dev_ops ops = {
 	.promiscuous_disable    = tap_promisc_disable,
 	.allmulticast_enable    = tap_allmulti_enable,
 	.allmulticast_disable   = tap_allmulti_disable,
+	.mac_addr_remove        = tap_mac_remove,
+	.mac_addr_add           = tap_mac_add,
+	.mac_addr_set           = tap_mac_set,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
 };
@@ -683,6 +753,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	data->drv_name = pmd_tap_drv.driver.name;
 	data->numa_node = numa_node;
 
+	eth_random_addr((uint8_t *)&pmd->eth_addr);
 	data->dev_link = pmd_link;
 	data->mac_addrs = &pmd->eth_addr;
 	data->nb_rx_queues = pmd->nb_queues;
-- 
2.8.0.rc0

[PATCH v3 2/6] net/tap: add speed capabilities

[Pascal Mazon]

Tap PMD is flexible, it supports any speed.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index d9b47a003654..dad5a0561087 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+Speed capabilities   = Y
 Unicast MAC filter   = Y
 Other kdrv           = Y
 ARMv7                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 1e46ee36efa2..ef525a3f0826 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev __rte_unused)
 	return 0;
 }
 
+static uint32_t
+tap_dev_speed_capa(void)
+{
+	uint32_t speed = pmd_link.link_speed;
+	uint32_t capa = 0;
+
+	if (speed >= ETH_SPEED_NUM_10M)
+		capa |= ETH_LINK_SPEED_10M;
+	if (speed >= ETH_SPEED_NUM_100M)
+		capa |= ETH_LINK_SPEED_100M;
+	if (speed >= ETH_SPEED_NUM_1G)
+		capa |= ETH_LINK_SPEED_1G;
+	if (speed >= ETH_SPEED_NUM_5G)
+		capa |= ETH_LINK_SPEED_2_5G;
+	if (speed >= ETH_SPEED_NUM_5G)
+		capa |= ETH_LINK_SPEED_5G;
+	if (speed >= ETH_SPEED_NUM_10G)
+		capa |= ETH_LINK_SPEED_10G;
+	if (speed >= ETH_SPEED_NUM_20G)
+		capa |= ETH_LINK_SPEED_20G;
+	if (speed >= ETH_SPEED_NUM_25G)
+		capa |= ETH_LINK_SPEED_25G;
+	if (speed >= ETH_SPEED_NUM_40G)
+		capa |= ETH_LINK_SPEED_40G;
+	if (speed >= ETH_SPEED_NUM_50G)
+		capa |= ETH_LINK_SPEED_50G;
+	if (speed >= ETH_SPEED_NUM_56G)
+		capa |= ETH_LINK_SPEED_56G;
+	if (speed >= ETH_SPEED_NUM_100G)
+		capa |= ETH_LINK_SPEED_100G;
+
+	return capa;
+}
+
 static void
 tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
@@ -363,6 +397,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_tx_queues = internals->nb_queues;
 	dev_info->min_rx_bufsize = 0;
 	dev_info->pci_dev = NULL;
+	dev_info->speed_capa = tap_dev_speed_capa();
 }
 
 static void
-- 
2.8.0.rc0

[PATCH v3 3/6] net/tap: add multicast addresses management

[Pascal Mazon]

A tap netdevice actually receives every packet, without any filtering
whatsoever. There is no need for any multicast address registration
to receive multicast packets.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index dad5a0561087..6878a9b8fd17 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
 Other kdrv           = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index ef525a3f0826..301072f20930 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -721,6 +721,18 @@ tap_tx_queue_setup(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static int
+tap_set_mc_addr_list(struct rte_eth_dev *dev __rte_unused,
+		     struct ether_addr *mc_addr_set __rte_unused,
+		     uint32_t nb_mc_addr __rte_unused)
+{
+	/*
+	 * Nothing to do actually: the tap has no filtering whatsoever, every
+	 * packet is received.
+	 */
+	return 0;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -741,6 +753,7 @@ static const struct eth_dev_ops ops = {
 	.mac_addr_remove        = tap_mac_remove,
 	.mac_addr_add           = tap_mac_add,
 	.mac_addr_set           = tap_mac_set,
+	.set_mc_addr_list       = tap_set_mc_addr_list,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
 };
-- 
2.8.0.rc0

[PATCH v3 4/6] net/tap: add MTU management

[Pascal Mazon]

The MTU is assigned to the tap netdevice according to the argument, but
packet transmission and reception just write/read on an fd with the
default limit being the socket buffer size.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index 6878a9b8fd17..6aa11874e2bc 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+MTU update           = Y
 Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 301072f20930..d76f1dc83b03 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -733,6 +733,42 @@ tap_set_mc_addr_list(struct rte_eth_dev *dev __rte_unused,
 	return 0;
 }
 
+static int
+tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr;
+	int err, s;
+
+	s = socket(AF_INET, SOCK_DGRAM, 0);
+	if (s < 0) {
+		RTE_LOG(ERR, PMD,
+			"Unable to get a socket for %s to set flags: %s\n",
+			pmd->name, strerror(errno));
+		return -1;
+	}
+	memset(&ifr, 0, sizeof(ifr));
+	snprintf(ifr.ifr_name, IFNAMSIZ, "%s", pmd->name);
+	err = ioctl(s, SIOCGIFMTU, &ifr);
+	if (err < 0) {
+		RTE_LOG(WARNING, PMD, "Unable to get %s device MTU: %s\n",
+			pmd->name, strerror(errno));
+		close(s);
+		return -1;
+	}
+	ifr.ifr_mtu = mtu;
+	err = ioctl(s, SIOCSIFMTU, &ifr);
+	if (err < 0) {
+		RTE_LOG(WARNING, PMD, "Unable to set %s mtu %d: %s\n",
+			pmd->name, mtu, strerror(errno));
+		close(s);
+		return -1;
+	}
+	close(s);
+	dev->data->mtu = mtu;
+	return 0;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -754,6 +790,7 @@ static const struct eth_dev_ops ops = {
 	.mac_addr_add           = tap_mac_add,
 	.mac_addr_set           = tap_mac_set,
 	.set_mc_addr_list       = tap_set_mc_addr_list,
+	.mtu_set                = tap_mtu_set,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
 };
-- 
2.8.0.rc0

[PATCH v3 5/6] net/tap: add packet type management

[Pascal Mazon]

Advertize RTE_PTYPE_UNKNOWN since tap does not report any packet type.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index 6aa11874e2bc..7f3f4d661dd7 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -13,6 +13,7 @@ MTU update           = Y
 Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
+Packet type parsing  = Y
 Other kdrv           = Y
 ARMv7                = Y
 ARMv8                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index d76f1dc83b03..edb5d2a82f12 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -36,6 +36,7 @@
 #include <rte_malloc.h>
 #include <rte_vdev.h>
 #include <rte_kvargs.h>
+#include <rte_net.h>
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -216,6 +217,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 		mbuf->data_len = len;
 		mbuf->pkt_len = len;
 		mbuf->port = rxq->in_port;
+		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
+						      RTE_PTYPE_ALL_MASK);
 
 		/* account for the receive frame */
 		bufs[num_rx++] = mbuf;
@@ -769,6 +772,17 @@ tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	return 0;
 }
 
+static const uint32_t*
+tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
+{
+	static const uint32_t ptypes[] = {
+		RTE_PTYPE_UNKNOWN,
+
+	};
+
+	return ptypes;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -793,6 +807,7 @@ static const struct eth_dev_ops ops = {
 	.mtu_set                = tap_mtu_set,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
+	.dev_supported_ptypes_get = tap_dev_supported_ptypes_get,
 };
 
 static int
-- 
2.8.0.rc0

[PATCH v3 6/6] net/tap: add flow control management

[Pascal Mazon]

A tap netdevice does not support flow control; ensure nothing but
RTE_FC_NONE mode can be set.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index 7f3f4d661dd7..a51712dce066 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -14,6 +14,7 @@ Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
 Packet type parsing  = Y
+Flow control         = Y
 Other kdrv           = Y
 ARMv7                = Y
 ARMv8                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index edb5d2a82f12..cbfb3b9641c8 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -783,6 +783,23 @@ tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
 	return ptypes;
 }
 
+static int
+tap_flow_ctrl_get(struct rte_eth_dev *dev __rte_unused,
+		  struct rte_eth_fc_conf *fc_conf)
+{
+	fc_conf->mode = RTE_FC_NONE;
+	return 0;
+}
+
+static int
+tap_flow_ctrl_set(struct rte_eth_dev *dev __rte_unused,
+		  struct rte_eth_fc_conf *fc_conf)
+{
+	if (fc_conf->mode != RTE_FC_NONE)
+		return -ENOTSUP;
+	return 0;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -793,6 +810,8 @@ static const struct eth_dev_ops ops = {
 	.tx_queue_setup         = tap_tx_queue_setup,
 	.rx_queue_release       = tap_rx_queue_release,
 	.tx_queue_release       = tap_tx_queue_release,
+	.flow_ctrl_get          = tap_flow_ctrl_get,
+	.flow_ctrl_set          = tap_flow_ctrl_set,
 	.link_update            = tap_link_update,
 	.dev_set_link_up        = tap_link_set_up,
 	.dev_set_link_down      = tap_link_set_down,
-- 
2.8.0.rc0

Re: [PATCH v3 1/6] net/tap: add MAC address management ops

[Ferruh Yigit]

On 3/7/2017 4:31 PM, Pascal Mazon wrote:
> Set a random MAC address when probing the device, as to not leave an
> empty MAC in pmd->eth_addr.
> 
> This MAC will be set on the tap netdevice as soon as it's been created
> using tun_alloc(). As the tap_mac_add() function depend on the fd in
> the first rxq, move code from tun_alloc() to tap_setup_queue(),
> after it's been set.
> 
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> ---
>  doc/guides/nics/features/tap.ini |  1 +
>  drivers/net/tap/rte_eth_tap.c    | 97 ++++++++++++++++++++++++++++++++++------
>  2 files changed, 85 insertions(+), 13 deletions(-)
> 
> diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
> index f4aca6921ddc..d9b47a003654 100644
> --- a/doc/guides/nics/features/tap.ini
> +++ b/doc/guides/nics/features/tap.ini
> @@ -9,6 +9,7 @@ Jumbo frame          = Y
>  Promiscuous mode     = Y
>  Allmulticast mode    = Y
>  Basic stats          = Y
> +Unicast MAC filter   = Y
>  Other kdrv           = Y
>  ARMv7                = Y
>  ARMv8                = Y
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index ece3a5fcc897..1e46ee36efa2 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -63,6 +63,8 @@
>  #define RTE_PMD_TAP_MAX_QUEUES	1
>  #endif
>  
> +#define RTE_PMD_TAP_MAX_MAC_ADDRS 1

mac_addr_add and mac_addr_remove not really supported, because only one
MAC is supported. For mac_addr_add() all indexes other than 0 will give
an error. So only mac_addr_set is supported.

For this case what is the benefit of implementing these functions and
claim support, instead of just leaving mac_addr_add and mac_addr_remove
NULL?


<...>

> +	if (qid == 0) {
> +		/*
> +		 * tap_setup_queue() is called for both tx and rx.
> +		 * Let's use dev->data->r/tx_queues[qid] to determine if init
> +		 * has already been done.
> +		 */
> +		if (dev->data->rx_queues[qid] && dev->data->tx_queues[qid])
> +			return fd;
> +
> +		tap_mac_set(dev, &pmd->eth_addr);

What is the reason of changing behavior here?

Tap devices assigned random MAC by kernel, and previous implementation
was reading that value and using it for DPDK.

Now kernel assigns a random MAC, and DPDK overwrites it another random
MAC, previous implementation was simpler I think.

It is OK to move this code tap_setup_queue(), I just missed the benefit
of overwriting with DPDK random MAC?

<...>

Re: [PATCH v3 2/6] net/tap: add speed capabilities

[Ferruh Yigit]

On 3/7/2017 4:31 PM, Pascal Mazon wrote:
> Tap PMD is flexible, it supports any speed.
> 
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> ---
>  doc/guides/nics/features/tap.ini |  1 +
>  drivers/net/tap/rte_eth_tap.c    | 35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
> index d9b47a003654..dad5a0561087 100644
> --- a/doc/guides/nics/features/tap.ini
> +++ b/doc/guides/nics/features/tap.ini
> @@ -9,6 +9,7 @@ Jumbo frame          = Y
>  Promiscuous mode     = Y
>  Allmulticast mode    = Y
>  Basic stats          = Y
> +Speed capabilities   = Y
>  Unicast MAC filter   = Y
>  Other kdrv           = Y
>  ARMv7                = Y
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 1e46ee36efa2..ef525a3f0826 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev __rte_unused)
>  	return 0;
>  }
>  
> +static uint32_t
> +tap_dev_speed_capa(void)
> +{
> +	uint32_t speed = pmd_link.link_speed;

link_speed is already hardcoded into PMD, so there is nothing to detect
here. Would it be different if PMD directly return pmd_link.link_speed?

> +	uint32_t capa = 0;
> +
> +	if (speed >= ETH_SPEED_NUM_10M)
> +		capa |= ETH_LINK_SPEED_10M;
> +	if (speed >= ETH_SPEED_NUM_100M)
> +		capa |= ETH_LINK_SPEED_100M;
> +	if (speed >= ETH_SPEED_NUM_1G)
> +		capa |= ETH_LINK_SPEED_1G;
> +	if (speed >= ETH_SPEED_NUM_5G)
> +		capa |= ETH_LINK_SPEED_2_5G;
> +	if (speed >= ETH_SPEED_NUM_5G)
> +		capa |= ETH_LINK_SPEED_5G;
> +	if (speed >= ETH_SPEED_NUM_10G)
> +		capa |= ETH_LINK_SPEED_10G;
> +	if (speed >= ETH_SPEED_NUM_20G)
> +		capa |= ETH_LINK_SPEED_20G;
> +	if (speed >= ETH_SPEED_NUM_25G)
> +		capa |= ETH_LINK_SPEED_25G;
> +	if (speed >= ETH_SPEED_NUM_40G)
> +		capa |= ETH_LINK_SPEED_40G;
> +	if (speed >= ETH_SPEED_NUM_50G)
> +		capa |= ETH_LINK_SPEED_50G;
> +	if (speed >= ETH_SPEED_NUM_56G)
> +		capa |= ETH_LINK_SPEED_56G;
> +	if (speed >= ETH_SPEED_NUM_100G)
> +		capa |= ETH_LINK_SPEED_100G;

I would prefer switch here [1], but functionally both are same, it is
your call.

[1]
switch (speed) {
case ETH_SPEED_NUM_100G:
	capa |= ETH_LINK_SPEED_100G
	/* fallthrough */
case ETH_SPEED_NUM_56G:
	capa |= ETH_LINK_SPEED_56G
	/* fallthrough */
...
};


> +
> +	return capa;
> +}
> +
>  static void
>  tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>  {
> @@ -363,6 +397,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>  	dev_info->max_tx_queues = internals->nb_queues;
>  	dev_info->min_rx_bufsize = 0;
>  	dev_info->pci_dev = NULL;
> +	dev_info->speed_capa = tap_dev_speed_capa();
>  }
>  
>  static void
> 

Re: [PATCH v3 5/6] net/tap: add packet type management

[Ferruh Yigit]

On 3/7/2017 4:31 PM, Pascal Mazon wrote:
> Advertize RTE_PTYPE_UNKNOWN since tap does not report any packet type.
> 
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> ---
>  doc/guides/nics/features/tap.ini |  1 +
>  drivers/net/tap/rte_eth_tap.c    | 15 +++++++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
> index 6aa11874e2bc..7f3f4d661dd7 100644
> --- a/doc/guides/nics/features/tap.ini
> +++ b/doc/guides/nics/features/tap.ini
> @@ -13,6 +13,7 @@ MTU update           = Y
>  Multicast MAC filter = Y
>  Speed capabilities   = Y
>  Unicast MAC filter   = Y
> +Packet type parsing  = Y
>  Other kdrv           = Y
>  ARMv7                = Y
>  ARMv8                = Y
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index d76f1dc83b03..edb5d2a82f12 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -36,6 +36,7 @@
>  #include <rte_malloc.h>
>  #include <rte_vdev.h>
>  #include <rte_kvargs.h>
> +#include <rte_net.h>
>  
>  #include <sys/types.h>
>  #include <sys/stat.h>
> @@ -216,6 +217,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>  		mbuf->data_len = len;
>  		mbuf->pkt_len = len;
>  		mbuf->port = rxq->in_port;
> +		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
> +						      RTE_PTYPE_ALL_MASK);

Isn't PMD become inconsistent with this update. It reports
RTE_PTYPE_UNKNOWN, but sets mbuf->packet_type with various ptype.

Do we want software packet type parsing in PMD level? Any change some
users may not interested with this data at all.

>  
>  		/* account for the receive frame */
>  		bufs[num_rx++] = mbuf;
> @@ -769,6 +772,17 @@ tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>  	return 0;
>  }
>  
> +static const uint32_t*
> +tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
> +{
> +	static const uint32_t ptypes[] = {
> +		RTE_PTYPE_UNKNOWN,
> +
> +	};
> +
> +	return ptypes;
> +}
> +
>  static const struct eth_dev_ops ops = {
>  	.dev_start              = tap_dev_start,
>  	.dev_stop               = tap_dev_stop,
> @@ -793,6 +807,7 @@ static const struct eth_dev_ops ops = {
>  	.mtu_set                = tap_mtu_set,
>  	.stats_get              = tap_stats_get,
>  	.stats_reset            = tap_stats_reset,
> +	.dev_supported_ptypes_get = tap_dev_supported_ptypes_get,
>  };
>  
>  static int
> 

Re: [PATCH v3 2/6] net/tap: add speed capabilities

[Wiles, Keith]

> On Mar 9, 2017, at 8:18 AM, Yigit, Ferruh <ferruh.yigit@intel.com> wrote:
> 
> On 3/7/2017 4:31 PM, Pascal Mazon wrote:
>> Tap PMD is flexible, it supports any speed.
>> 
>> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
>> ---
>> doc/guides/nics/features/tap.ini |  1 +
>> drivers/net/tap/rte_eth_tap.c    | 35 +++++++++++++++++++++++++++++++++++
>> 2 files changed, 36 insertions(+)
>> 
>> diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
>> index d9b47a003654..dad5a0561087 100644
>> --- a/doc/guides/nics/features/tap.ini
>> +++ b/doc/guides/nics/features/tap.ini
>> @@ -9,6 +9,7 @@ Jumbo frame          = Y
>> Promiscuous mode     = Y
>> Allmulticast mode    = Y
>> Basic stats          = Y
>> +Speed capabilities   = Y
>> Unicast MAC filter   = Y
>> Other kdrv           = Y
>> ARMv7                = Y
>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>> index 1e46ee36efa2..ef525a3f0826 100644
>> --- a/drivers/net/tap/rte_eth_tap.c
>> +++ b/drivers/net/tap/rte_eth_tap.c
>> @@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev __rte_unused)
>> 	return 0;
>> }
>> 
>> +static uint32_t
>> +tap_dev_speed_capa(void)
>> +{
>> +	uint32_t speed = pmd_link.link_speed;
> 
> link_speed is already hardcoded into PMD, so there is nothing to detect
> here. Would it be different if PMD directly return pmd_link.link_speed?

The link speed is passed into the PMD via the command line, which means it can change per run.
> 
>> +	uint32_t capa = 0;
>> +
>> +	if (speed >= ETH_SPEED_NUM_10M)
>> +		capa |= ETH_LINK_SPEED_10M;
>> +	if (speed >= ETH_SPEED_NUM_100M)
>> +		capa |= ETH_LINK_SPEED_100M;
>> +	if (speed >= ETH_SPEED_NUM_1G)
>> +		capa |= ETH_LINK_SPEED_1G;
>> +	if (speed >= ETH_SPEED_NUM_5G)
>> +		capa |= ETH_LINK_SPEED_2_5G;
>> +	if (speed >= ETH_SPEED_NUM_5G)
>> +		capa |= ETH_LINK_SPEED_5G;
>> +	if (speed >= ETH_SPEED_NUM_10G)
>> +		capa |= ETH_LINK_SPEED_10G;
>> +	if (speed >= ETH_SPEED_NUM_20G)
>> +		capa |= ETH_LINK_SPEED_20G;
>> +	if (speed >= ETH_SPEED_NUM_25G)
>> +		capa |= ETH_LINK_SPEED_25G;
>> +	if (speed >= ETH_SPEED_NUM_40G)
>> +		capa |= ETH_LINK_SPEED_40G;
>> +	if (speed >= ETH_SPEED_NUM_50G)
>> +		capa |= ETH_LINK_SPEED_50G;
>> +	if (speed >= ETH_SPEED_NUM_56G)
>> +		capa |= ETH_LINK_SPEED_56G;
>> +	if (speed >= ETH_SPEED_NUM_100G)
>> +		capa |= ETH_LINK_SPEED_100G;
> 
> I would prefer switch here [1], but functionally both are same, it is
> your call.
> 
> [1]
> switch (speed) {
> case ETH_SPEED_NUM_100G:
> 	capa |= ETH_LINK_SPEED_100G
> 	/* fallthrough */
> case ETH_SPEED_NUM_56G:
> 	capa |= ETH_LINK_SPEED_56G
> 	/* fallthrough */
> ...
> };
> 
> 
>> +
>> +	return capa;
>> +}
>> +
>> static void
>> tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>> {
>> @@ -363,6 +397,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>> 	dev_info->max_tx_queues = internals->nb_queues;
>> 	dev_info->min_rx_bufsize = 0;
>> 	dev_info->pci_dev = NULL;
>> +	dev_info->speed_capa = tap_dev_speed_capa();
>> }
>> 
>> static void

Regards,
Keith

Re: [PATCH v3 2/6] net/tap: add speed capabilities

[Ferruh Yigit]

On 3/9/2017 2:36 PM, Wiles, Keith wrote:
> 
>> On Mar 9, 2017, at 8:18 AM, Yigit, Ferruh <ferruh.yigit@intel.com> wrote:
>>
>> On 3/7/2017 4:31 PM, Pascal Mazon wrote:
>>> Tap PMD is flexible, it supports any speed.
>>>
>>> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
>>> ---
>>> doc/guides/nics/features/tap.ini |  1 +
>>> drivers/net/tap/rte_eth_tap.c    | 35 +++++++++++++++++++++++++++++++++++
>>> 2 files changed, 36 insertions(+)
>>>
>>> diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
>>> index d9b47a003654..dad5a0561087 100644
>>> --- a/doc/guides/nics/features/tap.ini
>>> +++ b/doc/guides/nics/features/tap.ini
>>> @@ -9,6 +9,7 @@ Jumbo frame          = Y
>>> Promiscuous mode     = Y
>>> Allmulticast mode    = Y
>>> Basic stats          = Y
>>> +Speed capabilities   = Y
>>> Unicast MAC filter   = Y
>>> Other kdrv           = Y
>>> ARMv7                = Y
>>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>>> index 1e46ee36efa2..ef525a3f0826 100644
>>> --- a/drivers/net/tap/rte_eth_tap.c
>>> +++ b/drivers/net/tap/rte_eth_tap.c
>>> @@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev __rte_unused)
>>> 	return 0;
>>> }
>>>
>>> +static uint32_t
>>> +tap_dev_speed_capa(void)
>>> +{
>>> +	uint32_t speed = pmd_link.link_speed;
>>
>> link_speed is already hardcoded into PMD, so there is nothing to detect
>> here. Would it be different if PMD directly return pmd_link.link_speed?
> 
> The link speed is passed into the PMD via the command line, which means it can change per run.

Right, I missed that.

Re: [PATCH v3 1/6] net/tap: add MAC address management ops

[Pascal Mazon]

On Thu, 9 Mar 2017 14:05:51 +0000
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 3/7/2017 4:31 PM, Pascal Mazon wrote:
> > Set a random MAC address when probing the device, as to not leave an
> > empty MAC in pmd->eth_addr.
> > 
> > This MAC will be set on the tap netdevice as soon as it's been
> > created using tun_alloc(). As the tap_mac_add() function depend on
> > the fd in the first rxq, move code from tun_alloc() to
> > tap_setup_queue(), after it's been set.
> > 
> > Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> > ---
> >  doc/guides/nics/features/tap.ini |  1 +
> >  drivers/net/tap/rte_eth_tap.c    | 97
> > ++++++++++++++++++++++++++++++++++------ 2 files changed, 85
> > insertions(+), 13 deletions(-)
> > 
> > diff --git a/doc/guides/nics/features/tap.ini
> > b/doc/guides/nics/features/tap.ini index f4aca6921ddc..d9b47a003654
> > 100644 --- a/doc/guides/nics/features/tap.ini
> > +++ b/doc/guides/nics/features/tap.ini
> > @@ -9,6 +9,7 @@ Jumbo frame          = Y
> >  Promiscuous mode     = Y
> >  Allmulticast mode    = Y
> >  Basic stats          = Y
> > +Unicast MAC filter   = Y
> >  Other kdrv           = Y
> >  ARMv7                = Y
> >  ARMv8                = Y
> > diff --git a/drivers/net/tap/rte_eth_tap.c
> > b/drivers/net/tap/rte_eth_tap.c index ece3a5fcc897..1e46ee36efa2
> > 100644 --- a/drivers/net/tap/rte_eth_tap.c
> > +++ b/drivers/net/tap/rte_eth_tap.c
> > @@ -63,6 +63,8 @@
> >  #define RTE_PMD_TAP_MAX_QUEUES	1
> >  #endif
> >  
> > +#define RTE_PMD_TAP_MAX_MAC_ADDRS 1
> 
> mac_addr_add and mac_addr_remove not really supported, because only
> one MAC is supported. For mac_addr_add() all indexes other than 0
> will give an error. So only mac_addr_set is supported.
> 
> For this case what is the benefit of implementing these functions and
> claim support, instead of just leaving mac_addr_add and
> mac_addr_remove NULL?
> 

Well, I wanted to implement those along with the mac_addr_set, as they
all dealt with mac addresses. But you're right, I might as well leave
the ops NULL.

I'll send a new version reflecting this.

> 
> <...>
> 
> > +	if (qid == 0) {
> > +		/*
> > +		 * tap_setup_queue() is called for both tx and rx.
> > +		 * Let's use dev->data->r/tx_queues[qid] to
> > determine if init
> > +		 * has already been done.
> > +		 */
> > +		if (dev->data->rx_queues[qid] &&
> > dev->data->tx_queues[qid])
> > +			return fd;
> > +
> > +		tap_mac_set(dev, &pmd->eth_addr);
> 
> What is the reason of changing behavior here?
> 
> Tap devices assigned random MAC by kernel, and previous implementation
> was reading that value and using it for DPDK.
> 
> Now kernel assigns a random MAC, and DPDK overwrites it another random
> MAC, previous implementation was simpler I think.
> 
> It is OK to move this code tap_setup_queue(), I just missed the
> benefit of overwriting with DPDK random MAC?
> 
> <...>

As far as I remember, I did it because somewhere the mac_addr_set was
checked as part of reconfiguration, which happenened before queue setup
was done. The default mac address (dev->data->mac_addrs[0]) got set to 0
and later call for the default mac address tried using this mac address.
Or something along those lines.

I'll definitely re-take a closer look at all this for my next version.

Regards,
Pascal

Re: [PATCH v3 2/6] net/tap: add speed capabilities

[Pascal Mazon]

On Thu, 9 Mar 2017 16:05:47 +0000
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 3/9/2017 2:36 PM, Wiles, Keith wrote:
> > 
> >> On Mar 9, 2017, at 8:18 AM, Yigit, Ferruh <ferruh.yigit@intel.com>
> >> wrote:
> >>
> >> On 3/7/2017 4:31 PM, Pascal Mazon wrote:
> >>> Tap PMD is flexible, it supports any speed.
> >>>
> >>> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> >>> ---
> >>> doc/guides/nics/features/tap.ini |  1 +
> >>> drivers/net/tap/rte_eth_tap.c    | 35
> >>> +++++++++++++++++++++++++++++++++++ 2 files changed, 36
> >>> insertions(+)
> >>>
> >>> diff --git a/doc/guides/nics/features/tap.ini
> >>> b/doc/guides/nics/features/tap.ini index
> >>> d9b47a003654..dad5a0561087 100644 ---
> >>> a/doc/guides/nics/features/tap.ini +++
> >>> b/doc/guides/nics/features/tap.ini @@ -9,6 +9,7 @@ Jumbo
> >>> frame          = Y Promiscuous mode     = Y
> >>> Allmulticast mode    = Y
> >>> Basic stats          = Y
> >>> +Speed capabilities   = Y
> >>> Unicast MAC filter   = Y
> >>> Other kdrv           = Y
> >>> ARMv7                = Y
> >>> diff --git a/drivers/net/tap/rte_eth_tap.c
> >>> b/drivers/net/tap/rte_eth_tap.c index 1e46ee36efa2..ef525a3f0826
> >>> 100644 --- a/drivers/net/tap/rte_eth_tap.c
> >>> +++ b/drivers/net/tap/rte_eth_tap.c
> >>> @@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev
> >>> __rte_unused) return 0;
> >>> }
> >>>
> >>> +static uint32_t
> >>> +tap_dev_speed_capa(void)
> >>> +{
> >>> +	uint32_t speed = pmd_link.link_speed;
> >>
> >> link_speed is already hardcoded into PMD, so there is nothing to
> >> detect here. Would it be different if PMD directly return
> >> pmd_link.link_speed?
> > 
> > The link speed is passed into the PMD via the command line, which
> > means it can change per run.
> 
> Right, I missed that.

I'll use switch/case in the next version in any case.
But yes, as Keith said speed is a runtime option.

Regards,
Pascal

Re: [PATCH v3 2/6] net/tap: add speed capabilities

[Pascal Mazon]

On Fri, 10 Mar 2017 10:03:12 +0100
Pascal Mazon <pascal.mazon@6wind.com> wrote:

> On Thu, 9 Mar 2017 16:05:47 +0000
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
> > On 3/9/2017 2:36 PM, Wiles, Keith wrote:
> > > 
> > >> On Mar 9, 2017, at 8:18 AM, Yigit, Ferruh
> > >> <ferruh.yigit@intel.com> wrote:
> > >>
> > >> On 3/7/2017 4:31 PM, Pascal Mazon wrote:
> > >>> Tap PMD is flexible, it supports any speed.
> > >>>
> > >>> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> > >>> ---
> > >>> doc/guides/nics/features/tap.ini |  1 +
> > >>> drivers/net/tap/rte_eth_tap.c    | 35
> > >>> +++++++++++++++++++++++++++++++++++ 2 files changed, 36
> > >>> insertions(+)
> > >>>
> > >>> diff --git a/doc/guides/nics/features/tap.ini
> > >>> b/doc/guides/nics/features/tap.ini index
> > >>> d9b47a003654..dad5a0561087 100644 ---
> > >>> a/doc/guides/nics/features/tap.ini +++
> > >>> b/doc/guides/nics/features/tap.ini @@ -9,6 +9,7 @@ Jumbo
> > >>> frame          = Y Promiscuous mode     = Y
> > >>> Allmulticast mode    = Y
> > >>> Basic stats          = Y
> > >>> +Speed capabilities   = Y
> > >>> Unicast MAC filter   = Y
> > >>> Other kdrv           = Y
> > >>> ARMv7                = Y
> > >>> diff --git a/drivers/net/tap/rte_eth_tap.c
> > >>> b/drivers/net/tap/rte_eth_tap.c index 1e46ee36efa2..ef525a3f0826
> > >>> 100644 --- a/drivers/net/tap/rte_eth_tap.c
> > >>> +++ b/drivers/net/tap/rte_eth_tap.c
> > >>> @@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev
> > >>> __rte_unused) return 0;
> > >>> }
> > >>>
> > >>> +static uint32_t
> > >>> +tap_dev_speed_capa(void)
> > >>> +{
> > >>> +	uint32_t speed = pmd_link.link_speed;
> > >>
> > >> link_speed is already hardcoded into PMD, so there is nothing to
> > >> detect here. Would it be different if PMD directly return
> > >> pmd_link.link_speed?
> > > 
> > > The link speed is passed into the PMD via the command line, which
> > > means it can change per run.
> > 
> > Right, I missed that.
> 
> I'll use switch/case in the next version in any case.
> But yes, as Keith said speed is a runtime option.

Sorry, I've sent that mail a little too quick. Of course I can't use
switch/case as we're not checking exact values matching.

> 
> Regards,
> Pascal

Re: [PATCH v3 5/6] net/tap: add packet type management

[Pascal Mazon]

On Thu, 9 Mar 2017 14:26:08 +0000
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 3/7/2017 4:31 PM, Pascal Mazon wrote:
> > Advertize RTE_PTYPE_UNKNOWN since tap does not report any packet
> > type.
> > 
> > Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> > ---
> >  doc/guides/nics/features/tap.ini |  1 +
> >  drivers/net/tap/rte_eth_tap.c    | 15 +++++++++++++++
> >  2 files changed, 16 insertions(+)
> > 
> > diff --git a/doc/guides/nics/features/tap.ini
> > b/doc/guides/nics/features/tap.ini index 6aa11874e2bc..7f3f4d661dd7
> > 100644 --- a/doc/guides/nics/features/tap.ini
> > +++ b/doc/guides/nics/features/tap.ini
> > @@ -13,6 +13,7 @@ MTU update           = Y
> >  Multicast MAC filter = Y
> >  Speed capabilities   = Y
> >  Unicast MAC filter   = Y
> > +Packet type parsing  = Y
> >  Other kdrv           = Y
> >  ARMv7                = Y
> >  ARMv8                = Y
> > diff --git a/drivers/net/tap/rte_eth_tap.c
> > b/drivers/net/tap/rte_eth_tap.c index d76f1dc83b03..edb5d2a82f12
> > 100644 --- a/drivers/net/tap/rte_eth_tap.c
> > +++ b/drivers/net/tap/rte_eth_tap.c
> > @@ -36,6 +36,7 @@
> >  #include <rte_malloc.h>
> >  #include <rte_vdev.h>
> >  #include <rte_kvargs.h>
> > +#include <rte_net.h>
> >  
> >  #include <sys/types.h>
> >  #include <sys/stat.h>
> > @@ -216,6 +217,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf
> > **bufs, uint16_t nb_pkts) mbuf->data_len = len;
> >  		mbuf->pkt_len = len;
> >  		mbuf->port = rxq->in_port;
> > +		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
> > +
> > RTE_PTYPE_ALL_MASK);
> 
> Isn't PMD become inconsistent with this update. It reports
> RTE_PTYPE_UNKNOWN, but sets mbuf->packet_type with various ptype.

I discussed this briefly with Keith, but my argument was that the
librte_net did not provide a list of supported packet types, so I didn't
want to have to keep tap supported packet type list in sync with the
lib.
But it's actually just a few lines to add, I'll do that and also set a
comment to mention that it must be in sync with librte_net.

> 
> Do we want software packet type parsing in PMD level? Any change some
> users may not interested with this data at all.

Well, most PMDs support packet type parsing, and among the vdev, I
inspired that part from virtio, which does software ptype parsing.
An application coded with physical hardware PMD in mind doesn't have to
be recoded when switching to a tap PMD.

Furthermore, the tap PMD is already using syscalls in its datapath, the
cost of software packet parsing won't really change overall performance.

Regards,
Pascal

> 
> >  
> >  		/* account for the receive frame */
> >  		bufs[num_rx++] = mbuf;
> > @@ -769,6 +772,17 @@ tap_mtu_set(struct rte_eth_dev *dev, uint16_t
> > mtu) return 0;
> >  }
> >  
> > +static const uint32_t*
> > +tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
> > +{
> > +	static const uint32_t ptypes[] = {
> > +		RTE_PTYPE_UNKNOWN,
> > +
> > +	};
> > +
> > +	return ptypes;
> > +}
> > +
> >  static const struct eth_dev_ops ops = {
> >  	.dev_start              = tap_dev_start,
> >  	.dev_stop               = tap_dev_stop,
> > @@ -793,6 +807,7 @@ static const struct eth_dev_ops ops = {
> >  	.mtu_set                = tap_mtu_set,
> >  	.stats_get              = tap_stats_get,
> >  	.stats_reset            = tap_stats_reset,
> > +	.dev_supported_ptypes_get = tap_dev_supported_ptypes_get,
> >  };
> >  
> >  static int
> > 
> 

[PATCH v4 0/8] net/tap: add additional management ops

[Pascal Mazon]

Add a few eth_dev ops to the tap PMD for completeness.
We want it to behave as much as possible as a standard PMD.

v2 change:
  - use snprintf in tap_mtu set

v3 change:
  - call tap_mac_set() only once in tap_setup_queue()

v4 changes:
  - use new tap_ioctl function for shared behavior between ops
  - update supported packet types
  - remove IFF_NOARP flag from link status change
  - sync desired MTU to both remote and tap netdevices
  - remove support for mac_add and mac_remove ops

Pascal Mazon (8):
  net/tap: remove wrong IFF_NOARP flags
  net/tap: refactor ioctl calls
  net/tap: add MAC address management
  net/tap: add MTU management
  net/tap: add speed capabilities
  net/tap: add multicast addresses management
  net/tap: add packet type management
  net/tap: add flow control management

 doc/guides/nics/features/tap.ini |   6 +
 drivers/net/tap/rte_eth_tap.c    | 255 +++++++++++++++++++++++++++++++--------
 2 files changed, 214 insertions(+), 47 deletions(-)

-- 
2.8.0.rc0

[PATCH v4 1/8] net/tap: remove wrong IFF_NOARP flags

[Pascal Mazon]

There is no reason not to support ARP on a tap netdevice.
Focus on IFF_UP when a link status change is required.

Fixes: f457b472b1f2 ("net/tap: add link up and down operations")
Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 drivers/net/tap/rte_eth_tap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index ece3a5fcc897..b75b7542a329 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -327,7 +327,7 @@ tap_link_set_down(struct rte_eth_dev *dev)
 	struct pmd_internals *pmd = dev->data->dev_private;
 
 	dev->data->dev_link.link_status = ETH_LINK_DOWN;
-	return tap_link_set_flags(pmd, IFF_UP | IFF_NOARP, 0);
+	return tap_link_set_flags(pmd, IFF_UP, 0);
 }
 
 static int
@@ -336,7 +336,7 @@ tap_link_set_up(struct rte_eth_dev *dev)
 	struct pmd_internals *pmd = dev->data->dev_private;
 
 	dev->data->dev_link.link_status = ETH_LINK_UP;
-	return tap_link_set_flags(pmd, IFF_UP | IFF_NOARP, 1);
+	return tap_link_set_flags(pmd, IFF_UP, 1);
 }
 
 static int
-- 
2.8.0.rc0

[PATCH v4 2/8] net/tap: refactor ioctl calls

[Pascal Mazon]

Create a socket for ioctl at tap device creation instead of opening it
and closing it every call to tap_link_set_flags().

Use a common tap_ioctl() function that can be extended for various uses
(such as MTU change, MAC address change, ...).

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 drivers/net/tap/rte_eth_tap.c | 90 +++++++++++++++++++++++--------------------
 1 file changed, 48 insertions(+), 42 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index b75b7542a329..18edac57eead 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -107,6 +107,7 @@ struct pmd_internals {
 	struct ether_addr eth_addr;	/* Mac address of the device port */
 
 	int if_index;			/* IF_INDEX for the port */
+	int ioctl_sock;			/* socket for ioctl calls */
 
 	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 */
@@ -280,63 +281,55 @@ pmd_tx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 }
 
 static int
-tap_link_set_flags(struct pmd_internals *pmd, short flags, int add)
+tap_ioctl(struct pmd_internals *pmd, unsigned long request,
+	  struct ifreq *ifr, int set)
 {
-	struct ifreq ifr;
-	int err, s;
-
-	/*
-	 * An AF_INET/DGRAM socket is needed for
-	 * SIOCGIFFLAGS/SIOCSIFFLAGS, using fd won't work.
-	 */
-	s = socket(AF_INET, SOCK_DGRAM, 0);
-	if (s < 0) {
-		RTE_LOG(ERR, PMD,
-			"Unable to get a socket to set flags: %s\n",
-			strerror(errno));
-		return -1;
-	}
-	memset(&ifr, 0, sizeof(ifr));
-	snprintf(ifr.ifr_name, IFNAMSIZ, "%s", pmd->name);
-	err = ioctl(s, SIOCGIFFLAGS, &ifr);
-	if (err < 0) {
-		RTE_LOG(WARNING, PMD, "Unable to get %s device flags: %s\n",
-			pmd->name, strerror(errno));
-		close(s);
-		return -1;
-	}
-	if (add)
-		ifr.ifr_flags |= flags;
-	else
-		ifr.ifr_flags &= ~flags;
-	err = ioctl(s, SIOCSIFFLAGS, &ifr);
-	if (err < 0) {
-		RTE_LOG(WARNING, PMD, "Unable to %s flags 0x%x: %s\n",
-			add ? "set" : "unset", flags, strerror(errno));
-		close(s);
-		return -1;
-	}
-	close(s);
+	short req_flags = ifr->ifr_flags;
 
+	snprintf(ifr->ifr_name, IFNAMSIZ, "%s", pmd->name);
+	switch (request) {
+	case SIOCSIFFLAGS:
+		/* fetch current flags to leave other flags untouched */
+		if (ioctl(pmd->ioctl_sock, SIOCGIFFLAGS, ifr) < 0)
+			goto error;
+		if (set)
+			ifr->ifr_flags |= req_flags;
+		else
+			ifr->ifr_flags &= ~req_flags;
+		break;
+	default:
+		RTE_LOG(WARNING, PMD, "%s: ioctl() called with wrong arg\n",
+			pmd->name);
+		return -EINVAL;
+	}
+	if (ioctl(pmd->ioctl_sock, request, ifr) < 0)
+		goto error;
 	return 0;
+
+error:
+	RTE_LOG(ERR, PMD, "%s: ioctl(%lu) failed with error: %s\n",
+		ifr->ifr_name, request, strerror(errno));
+	return -errno;
 }
 
 static int
 tap_link_set_down(struct rte_eth_dev *dev)
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr = { .ifr_flags = IFF_UP };
 
 	dev->data->dev_link.link_status = ETH_LINK_DOWN;
-	return tap_link_set_flags(pmd, IFF_UP, 0);
+	return tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 0);
 }
 
 static int
 tap_link_set_up(struct rte_eth_dev *dev)
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr = { .ifr_flags = IFF_UP };
 
 	dev->data->dev_link.link_status = ETH_LINK_UP;
-	return tap_link_set_flags(pmd, IFF_UP, 1);
+	return tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 1);
 }
 
 static int
@@ -470,36 +463,40 @@ static void
 tap_promisc_enable(struct rte_eth_dev *dev)
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr = { .ifr_flags = IFF_PROMISC };
 
 	dev->data->promiscuous = 1;
-	tap_link_set_flags(pmd, IFF_PROMISC, 1);
+	tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 1);
 }
 
 static void
 tap_promisc_disable(struct rte_eth_dev *dev)
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr = { .ifr_flags = IFF_PROMISC };
 
 	dev->data->promiscuous = 0;
-	tap_link_set_flags(pmd, IFF_PROMISC, 0);
+	tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 0);
 }
 
 static void
 tap_allmulti_enable(struct rte_eth_dev *dev)
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr = { .ifr_flags = IFF_ALLMULTI };
 
 	dev->data->all_multicast = 1;
-	tap_link_set_flags(pmd, IFF_ALLMULTI, 1);
+	tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 1);
 }
 
 static void
 tap_allmulti_disable(struct rte_eth_dev *dev)
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr = { .ifr_flags = IFF_ALLMULTI };
 
 	dev->data->all_multicast = 0;
-	tap_link_set_flags(pmd, IFF_ALLMULTI, 0);
+	tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 0);
 }
 
 static int
@@ -675,6 +672,14 @@ eth_dev_tap_create(const char *name, char *tap_name)
 
 	pmd->nb_queues = RTE_PMD_TAP_MAX_QUEUES;
 
+	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));
+		goto error_exit;
+	}
+
 	/* Setup some default values */
 	data->dev_private = pmd;
 	data->port_id = dev->data->port_id;
@@ -817,6 +822,7 @@ rte_pmd_tap_remove(const char *name)
 		if (internals->rxq[i].fd != -1)
 			close(internals->rxq[i].fd);
 
+	close(internals->ioctl_sock);
 	rte_free(eth_dev->data->dev_private);
 	rte_free(eth_dev->data);
 
-- 
2.8.0.rc0

[PATCH v4 3/8] net/tap: add MAC address management

[Pascal Mazon]

As soon as the netdevice is created, update pmd->mac_addr with its
actual MAC address.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 39 +++++++++++++++++++++++++++++++++------
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index f4aca6921ddc..d9b47a003654 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+Unicast MAC filter   = Y
 Other kdrv           = Y
 ARMv7                = Y
 ARMv8                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 18edac57eead..06c1faa92dce 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -113,6 +113,10 @@ struct pmd_internals {
 	struct tx_queue txq[RTE_PMD_TAP_MAX_QUEUES];	/* List of TX queues */
 };
 
+static int
+tap_ioctl(struct pmd_internals *pmd, unsigned long request,
+	  struct ifreq *ifr, int set);
+
 /* Tun/Tap allocation routine
  *
  * name is the number of the interface to use, unless NULL to take the host
@@ -178,13 +182,12 @@ tun_alloc(struct pmd_internals *pmd, uint16_t qid)
 	}
 
 	if (qid == 0) {
-		if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
-			RTE_LOG(ERR, PMD, "ioctl failed (SIOCGIFHWADDR) (%s)\n",
-				ifr.ifr_name);
-			goto error;
-		}
+		struct ifreq ifr;
 
-		rte_memcpy(&pmd->eth_addr, ifr.ifr_hwaddr.sa_data, ETH_ALEN);
+		if (tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0) < 0)
+			goto error;
+		rte_memcpy(&pmd->eth_addr, ifr.ifr_hwaddr.sa_data,
+			   ETHER_ADDR_LEN);
 	}
 
 	return fd;
@@ -297,6 +300,9 @@ tap_ioctl(struct pmd_internals *pmd, unsigned long request,
 		else
 			ifr->ifr_flags &= ~req_flags;
 		break;
+	case SIOCGIFHWADDR:
+	case SIOCSIFHWADDR:
+		break;
 	default:
 		RTE_LOG(WARNING, PMD, "%s: ioctl() called with wrong arg\n",
 			pmd->name);
@@ -499,6 +505,26 @@ tap_allmulti_disable(struct rte_eth_dev *dev)
 	tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 0);
 }
 
+
+static void
+tap_mac_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
+{
+	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr;
+
+	if (is_zero_ether_addr(mac_addr)) {
+		RTE_LOG(ERR, PMD, "%s: can't set an empty MAC address\n",
+			dev->data->name);
+		return;
+	}
+
+	ifr.ifr_hwaddr.sa_family = AF_LOCAL;
+	rte_memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, ETHER_ADDR_LEN);
+	if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 1) < 0)
+		return;
+	rte_memcpy(&pmd->eth_addr, mac_addr, ETHER_ADDR_LEN);
+}
+
 static int
 tap_setup_queue(struct rte_eth_dev *dev,
 		struct pmd_internals *internals,
@@ -633,6 +659,7 @@ static const struct eth_dev_ops ops = {
 	.promiscuous_disable    = tap_promisc_disable,
 	.allmulticast_enable    = tap_allmulti_enable,
 	.allmulticast_disable   = tap_allmulti_disable,
+	.mac_addr_set           = tap_mac_set,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
 };
-- 
2.8.0.rc0

[PATCH v4 4/8] net/tap: add MTU management

[Pascal Mazon]

The MTU is assigned to the tap netdevice according to the argument, but
packet transmission and reception just write/read on an fd with the
default limit being the socket buffer size.

As a new rte_eth_dev_data is allocated during tap device init, ensure it
is set again dev->data->mtu.
Once the actual netdevice is created via tun_alloc(), make sure to apply
the desired MTU to the netdevice.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index d9b47a003654..b80577753240 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+MTU update           = Y
 Unicast MAC filter   = Y
 Other kdrv           = Y
 ARMv7                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 06c1faa92dce..3e61f7bda555 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -302,6 +302,7 @@ tap_ioctl(struct pmd_internals *pmd, unsigned long request,
 		break;
 	case SIOCGIFHWADDR:
 	case SIOCSIFHWADDR:
+	case SIOCSIFMTU:
 		break;
 	default:
 		RTE_LOG(WARNING, PMD, "%s: ioctl() called with wrong arg\n",
@@ -547,6 +548,15 @@ tap_setup_queue(struct rte_eth_dev *dev,
 					pmd->name, qid);
 				return -1;
 			}
+			if (qid == 0) {
+				struct ifreq ifr;
+
+				ifr.ifr_mtu = dev->data->mtu;
+				if (tap_ioctl(pmd, SIOCSIFMTU, &ifr, 1) < 0) {
+					close(fd);
+					return -1;
+				}
+			}
 		}
 	}
 
@@ -642,6 +652,20 @@ tap_tx_queue_setup(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static int
+tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr = { .ifr_mtu = mtu };
+	int err = 0;
+
+	err = tap_ioctl(pmd, SIOCSIFMTU, &ifr, 1);
+	if (!err)
+		dev->data->mtu = mtu;
+
+	return err;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -660,6 +684,7 @@ static const struct eth_dev_ops ops = {
 	.allmulticast_enable    = tap_allmulti_enable,
 	.allmulticast_disable   = tap_allmulti_disable,
 	.mac_addr_set           = tap_mac_set,
+	.mtu_set                = tap_mtu_set,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
 };
@@ -710,6 +735,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	/* Setup some default values */
 	data->dev_private = pmd;
 	data->port_id = dev->data->port_id;
+	data->mtu = dev->data->mtu;
 	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 	data->kdrv = RTE_KDRV_NONE;
 	data->drv_name = pmd_tap_drv.driver.name;
-- 
2.8.0.rc0

[PATCH v4 5/8] net/tap: add speed capabilities

[Pascal Mazon]

Tap PMD is flexible, it supports any speed.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index b80577753240..6c07352088bf 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -10,6 +10,7 @@ Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
 MTU update           = Y
+Speed capabilities   = Y
 Unicast MAC filter   = Y
 Other kdrv           = Y
 ARMv7                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 3e61f7bda555..86d2b3b4c106 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -359,6 +359,40 @@ tap_dev_configure(struct rte_eth_dev *dev __rte_unused)
 	return 0;
 }
 
+static uint32_t
+tap_dev_speed_capa(void)
+{
+	uint32_t speed = pmd_link.link_speed;
+	uint32_t capa = 0;
+
+	if (speed >= ETH_SPEED_NUM_10M)
+		capa |= ETH_LINK_SPEED_10M;
+	if (speed >= ETH_SPEED_NUM_100M)
+		capa |= ETH_LINK_SPEED_100M;
+	if (speed >= ETH_SPEED_NUM_1G)
+		capa |= ETH_LINK_SPEED_1G;
+	if (speed >= ETH_SPEED_NUM_5G)
+		capa |= ETH_LINK_SPEED_2_5G;
+	if (speed >= ETH_SPEED_NUM_5G)
+		capa |= ETH_LINK_SPEED_5G;
+	if (speed >= ETH_SPEED_NUM_10G)
+		capa |= ETH_LINK_SPEED_10G;
+	if (speed >= ETH_SPEED_NUM_20G)
+		capa |= ETH_LINK_SPEED_20G;
+	if (speed >= ETH_SPEED_NUM_25G)
+		capa |= ETH_LINK_SPEED_25G;
+	if (speed >= ETH_SPEED_NUM_40G)
+		capa |= ETH_LINK_SPEED_40G;
+	if (speed >= ETH_SPEED_NUM_50G)
+		capa |= ETH_LINK_SPEED_50G;
+	if (speed >= ETH_SPEED_NUM_56G)
+		capa |= ETH_LINK_SPEED_56G;
+	if (speed >= ETH_SPEED_NUM_100G)
+		capa |= ETH_LINK_SPEED_100G;
+
+	return capa;
+}
+
 static void
 tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
@@ -371,6 +405,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_tx_queues = internals->nb_queues;
 	dev_info->min_rx_bufsize = 0;
 	dev_info->pci_dev = NULL;
+	dev_info->speed_capa = tap_dev_speed_capa();
 }
 
 static void
-- 
2.8.0.rc0

[PATCH v4 6/8] net/tap: add multicast addresses management

[Pascal Mazon]

A tap netdevice actually receives every packet, without any filtering
whatsoever. There is no need for any multicast address registration
to receive multicast packets.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index 6c07352088bf..6aa11874e2bc 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -10,6 +10,7 @@ Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
 MTU update           = Y
+Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
 Other kdrv           = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 86d2b3b4c106..e9aa2f45ba54 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -701,6 +701,18 @@ tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	return err;
 }
 
+static int
+tap_set_mc_addr_list(struct rte_eth_dev *dev __rte_unused,
+		     struct ether_addr *mc_addr_set __rte_unused,
+		     uint32_t nb_mc_addr __rte_unused)
+{
+	/*
+	 * Nothing to do actually: the tap has no filtering whatsoever, every
+	 * packet is received.
+	 */
+	return 0;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -720,6 +732,7 @@ static const struct eth_dev_ops ops = {
 	.allmulticast_disable   = tap_allmulti_disable,
 	.mac_addr_set           = tap_mac_set,
 	.mtu_set                = tap_mtu_set,
+	.set_mc_addr_list       = tap_set_mc_addr_list,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
 };
-- 
2.8.0.rc0

[PATCH v4 7/8] net/tap: add packet type management

[Pascal Mazon]

Advertize packet types supported by the librte_net.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index 6aa11874e2bc..7f3f4d661dd7 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -13,6 +13,7 @@ MTU update           = Y
 Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
+Packet type parsing  = Y
 Other kdrv           = Y
 ARMv7                = Y
 ARMv8                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index e9aa2f45ba54..371249b0daed 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -36,6 +36,7 @@
 #include <rte_malloc.h>
 #include <rte_vdev.h>
 #include <rte_kvargs.h>
+#include <rte_net.h>
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -228,6 +229,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 		mbuf->data_len = len;
 		mbuf->pkt_len = len;
 		mbuf->port = rxq->in_port;
+		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
+						      RTE_PTYPE_ALL_MASK);
 
 		/* account for the receive frame */
 		bufs[num_rx++] = mbuf;
@@ -713,6 +716,37 @@ tap_set_mc_addr_list(struct rte_eth_dev *dev __rte_unused,
 	return 0;
 }
 
+static const uint32_t*
+tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
+{
+	static const uint32_t ptypes[] = {
+		RTE_PTYPE_INNER_L2_ETHER,
+		RTE_PTYPE_INNER_L2_ETHER_VLAN,
+		RTE_PTYPE_INNER_L2_ETHER_QINQ,
+		RTE_PTYPE_INNER_L3_IPV4,
+		RTE_PTYPE_INNER_L3_IPV4_EXT,
+		RTE_PTYPE_INNER_L3_IPV6,
+		RTE_PTYPE_INNER_L3_IPV6_EXT,
+		RTE_PTYPE_INNER_L4_FRAG,
+		RTE_PTYPE_INNER_L4_UDP,
+		RTE_PTYPE_INNER_L4_TCP,
+		RTE_PTYPE_INNER_L4_SCTP,
+		RTE_PTYPE_L2_ETHER,
+		RTE_PTYPE_L2_ETHER_VLAN,
+		RTE_PTYPE_L2_ETHER_QINQ,
+		RTE_PTYPE_L3_IPV4,
+		RTE_PTYPE_L3_IPV4_EXT,
+		RTE_PTYPE_L3_IPV6_EXT,
+		RTE_PTYPE_L3_IPV6,
+		RTE_PTYPE_L4_FRAG,
+		RTE_PTYPE_L4_UDP,
+		RTE_PTYPE_L4_TCP,
+		RTE_PTYPE_L4_SCTP,
+	};
+
+	return ptypes;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -735,6 +769,7 @@ static const struct eth_dev_ops ops = {
 	.set_mc_addr_list       = tap_set_mc_addr_list,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
+	.dev_supported_ptypes_get = tap_dev_supported_ptypes_get,
 };
 
 static int
-- 
2.8.0.rc0

[PATCH v4 8/8] net/tap: add flow control management

[Pascal Mazon]

A tap netdevice does not support flow control; ensure nothing but
RTE_FC_NONE mode can be set.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index 7f3f4d661dd7..a51712dce066 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -14,6 +14,7 @@ Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
 Packet type parsing  = Y
+Flow control         = Y
 Other kdrv           = Y
 ARMv7                = Y
 ARMv8                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 371249b0daed..7557abb2ebfc 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -747,6 +747,23 @@ tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
 	return ptypes;
 }
 
+static int
+tap_flow_ctrl_get(struct rte_eth_dev *dev __rte_unused,
+		  struct rte_eth_fc_conf *fc_conf)
+{
+	fc_conf->mode = RTE_FC_NONE;
+	return 0;
+}
+
+static int
+tap_flow_ctrl_set(struct rte_eth_dev *dev __rte_unused,
+		  struct rte_eth_fc_conf *fc_conf)
+{
+	if (fc_conf->mode != RTE_FC_NONE)
+		return -ENOTSUP;
+	return 0;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -757,6 +774,8 @@ static const struct eth_dev_ops ops = {
 	.tx_queue_setup         = tap_tx_queue_setup,
 	.rx_queue_release       = tap_rx_queue_release,
 	.tx_queue_release       = tap_tx_queue_release,
+	.flow_ctrl_get          = tap_flow_ctrl_get,
+	.flow_ctrl_set          = tap_flow_ctrl_set,
 	.link_update            = tap_link_update,
 	.dev_set_link_up        = tap_link_set_up,
 	.dev_set_link_down      = tap_link_set_down,
-- 
2.8.0.rc0

Re: [PATCH v4 7/8] net/tap: add packet type management

[Ferruh Yigit]

On 3/14/2017 8:22 AM, Pascal Mazon wrote:
> Advertize packet types supported by the librte_net.
> 
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>

<...>

>  
>  #include <sys/types.h>
>  #include <sys/stat.h>
> @@ -228,6 +229,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>  		mbuf->data_len = len;
>  		mbuf->pkt_len = len;
>  		mbuf->port = rxq->in_port;
> +		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
> +						      RTE_PTYPE_ALL_MASK);

This breaks shared library build [1], now librte_net also needs to be
linked against PMD, this can be done easily by adding library as
dependency [2] to PMD.


[1]
rte_eth_tap.o: In function `pmd_rx_burst':
.../drivers/net/tap/rte_eth_tap.c:(.text+0x863): undefined reference to
`rte_net_get_ptype'


[2]
--- a/drivers/net/tap/Makefile
+++ b/drivers/net/tap/Makefile
@@ -53,5 +53,6 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) += lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) += lib/librte_mempool
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) += lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) += lib/librte_kvargs
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) += lib/librte_net

 include $(RTE_SDK)/mk/rte.lib.mk

Re: [PATCH v4 7/8] net/tap: add packet type management

[Ferruh Yigit]

On 3/14/2017 8:22 AM, Pascal Mazon wrote:
> Advertize packet types supported by the librte_net.
> 
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>

<...>

>  
> +static const uint32_t*
> +tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
> +{
> +	static const uint32_t ptypes[] = {
> +		RTE_PTYPE_INNER_L2_ETHER,
> +		RTE_PTYPE_INNER_L2_ETHER_VLAN,
> +		RTE_PTYPE_INNER_L2_ETHER_QINQ,
> +		RTE_PTYPE_INNER_L3_IPV4,
> +		RTE_PTYPE_INNER_L3_IPV4_EXT,
> +		RTE_PTYPE_INNER_L3_IPV6,
> +		RTE_PTYPE_INNER_L3_IPV6_EXT,
> +		RTE_PTYPE_INNER_L4_FRAG,
> +		RTE_PTYPE_INNER_L4_UDP,
> +		RTE_PTYPE_INNER_L4_TCP,
> +		RTE_PTYPE_INNER_L4_SCTP,
> +		RTE_PTYPE_L2_ETHER,
> +		RTE_PTYPE_L2_ETHER_VLAN,
> +		RTE_PTYPE_L2_ETHER_QINQ,
> +		RTE_PTYPE_L3_IPV4,
> +		RTE_PTYPE_L3_IPV4_EXT,
> +		RTE_PTYPE_L3_IPV6_EXT,
> +		RTE_PTYPE_L3_IPV6,
> +		RTE_PTYPE_L4_FRAG,
> +		RTE_PTYPE_L4_UDP,
> +		RTE_PTYPE_L4_TCP,
> +		RTE_PTYPE_L4_SCTP,

Not for this patch, but it can be good if librte_net exports a macro
that lists rte_net_get_ptype() supported types.

Re: [PATCH v4 0/8] net/tap: add additional management ops

[Ferruh Yigit]

On 3/14/2017 8:22 AM, Pascal Mazon wrote:
> Add a few eth_dev ops to the tap PMD for completeness.
> We want it to behave as much as possible as a standard PMD.
> 
> v2 change:
>   - use snprintf in tap_mtu set
> 
> v3 change:
>   - call tap_mac_set() only once in tap_setup_queue()
> 
> v4 changes:
>   - use new tap_ioctl function for shared behavior between ops
>   - update supported packet types
>   - remove IFF_NOARP flag from link status change
>   - sync desired MTU to both remote and tap netdevices
>   - remove support for mac_add and mac_remove ops
> 
> Pascal Mazon (8):
>   net/tap: remove wrong IFF_NOARP flags
>   net/tap: refactor ioctl calls
>   net/tap: add MAC address management
>   net/tap: add MTU management
>   net/tap: add speed capabilities
>   net/tap: add multicast addresses management
>   net/tap: add packet type management
>   net/tap: add flow control management

Except from missing librte_net dependency, patchset looks good to me,

Series Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [PATCH v4 7/8] net/tap: add packet type management

[Pascal Mazon]

On Wed, 15 Mar 2017 13:35:44 +0000
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 3/14/2017 8:22 AM, Pascal Mazon wrote:
> > Advertize packet types supported by the librte_net.
> > 
> > Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> 
> <...>
> 
> >  
> >  #include <sys/types.h>
> >  #include <sys/stat.h>
> > @@ -228,6 +229,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf
> > **bufs, uint16_t nb_pkts) mbuf->data_len = len;
> >  		mbuf->pkt_len = len;
> >  		mbuf->port = rxq->in_port;
> > +		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
> > +
> > RTE_PTYPE_ALL_MASK);
> 
> This breaks shared library build [1], now librte_net also needs to be
> linked against PMD, this can be done easily by adding library as
> dependency [2] to PMD.
> 
> 
> [1]
> rte_eth_tap.o: In function `pmd_rx_burst':
> .../drivers/net/tap/rte_eth_tap.c:(.text+0x863): undefined reference
> to `rte_net_get_ptype'
> 
> 
> [2]
> --- a/drivers/net/tap/Makefile
> +++ b/drivers/net/tap/Makefile
> @@ -53,5 +53,6 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) +=
> lib/librte_mbuf DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) +=
> lib/librte_mempool DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) +=
> lib/librte_ether DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) +=
> lib/librte_kvargs +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) +=
> lib/librte_net
> 
>  include $(RTE_SDK)/mk/rte.lib.mk

Indeed!

I'll fix that and send a new version (rebased on the latest
next-net, following adrien's changes to tap PMD).

Regards,
Pascal

Re: [PATCH v4 7/8] net/tap: add packet type management

[Ferruh Yigit]

On 3/15/2017 1:44 PM, Pascal Mazon wrote:
> On Wed, 15 Mar 2017 13:35:44 +0000
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
>> On 3/14/2017 8:22 AM, Pascal Mazon wrote:
>>> Advertize packet types supported by the librte_net.
>>>
>>> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
>>
>> <...>
>>
>>>  
>>>  #include <sys/types.h>
>>>  #include <sys/stat.h>
>>> @@ -228,6 +229,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf
>>> **bufs, uint16_t nb_pkts) mbuf->data_len = len;
>>>  		mbuf->pkt_len = len;
>>>  		mbuf->port = rxq->in_port;
>>> +		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
>>> +
>>> RTE_PTYPE_ALL_MASK);
>>
>> This breaks shared library build [1], now librte_net also needs to be
>> linked against PMD, this can be done easily by adding library as
>> dependency [2] to PMD.
>>
>>
>> [1]
>> rte_eth_tap.o: In function `pmd_rx_burst':
>> .../drivers/net/tap/rte_eth_tap.c:(.text+0x863): undefined reference
>> to `rte_net_get_ptype'
>>
>>
>> [2]
>> --- a/drivers/net/tap/Makefile
>> +++ b/drivers/net/tap/Makefile
>> @@ -53,5 +53,6 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) +=
>> lib/librte_mbuf DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) +=
>> lib/librte_mempool DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) +=
>> lib/librte_ether DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) +=
>> lib/librte_kvargs +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) +=
>> lib/librte_net
>>
>>  include $(RTE_SDK)/mk/rte.lib.mk
> 
> Indeed!
> 
> I'll fix that and send a new version (rebased on the latest
> next-net, following adrien's changes to tap PMD).

Can you also rebase tap rte_flow one [1], after this one please?
Because that set effected from both this one and Adrien's changes.

[1]
http://dpdk.org/dev/patchwork/patch/21757/ [4 patches]

Thanks,
ferruh

> 
> Regards,
> Pascal
> 

[PATCH v5 0/8] net/tap: add additional management ops

[Pascal Mazon]

v2 change:
  - use snprintf in tap_mtu set

v3 change:
  - call tap_mac_set() only once in tap_setup_queue()

v4 changes:
  - use new tap_ioctl function for shared behavior between ops
  - update supported packet types
  - remove IFF_NOARP flag from link status change
  - sync desired MTU to both remote and tap netdevices
  - remove support for mac_add and mac_remove ops

v5 changes:
  - rebase after adrien's patches on Tx poll and Rx signaling
  - add dependency to librte_net for packet type

Pascal Mazon (8):
  net/tap: remove wrong IFF_NOARP flags
  net/tap: refactor ioctl calls
  net/tap: add MAC address management
  net/tap: add MTU management
  net/tap: add speed capabilities
  net/tap: add multicast addresses management
  net/tap: add packet type management
  net/tap: add flow control management

 doc/guides/nics/features/tap.ini |   6 +
 drivers/net/tap/Makefile         |   1 +
 drivers/net/tap/rte_eth_tap.c    | 255 +++++++++++++++++++++++++++++++--------
 3 files changed, 215 insertions(+), 47 deletions(-)

-- 
2.8.0.rc0

[PATCH v5 1/8] net/tap: remove wrong IFF_NOARP flags

[Pascal Mazon]

There is no reason not to support ARP on a tap netdevice.
Focus on IFF_UP when a link status change is required.

Fixes: f457b472b1f2 ("net/tap: add link up and down operations")
Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index a5cbb23e4cd2..fcf4a1dc3da1 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -375,7 +375,7 @@ tap_link_set_down(struct rte_eth_dev *dev)
 	struct pmd_internals *pmd = dev->data->dev_private;
 
 	dev->data->dev_link.link_status = ETH_LINK_DOWN;
-	return tap_link_set_flags(pmd, IFF_UP | IFF_NOARP, 0);
+	return tap_link_set_flags(pmd, IFF_UP, 0);
 }
 
 static int
@@ -384,7 +384,7 @@ tap_link_set_up(struct rte_eth_dev *dev)
 	struct pmd_internals *pmd = dev->data->dev_private;
 
 	dev->data->dev_link.link_status = ETH_LINK_UP;
-	return tap_link_set_flags(pmd, IFF_UP | IFF_NOARP, 1);
+	return tap_link_set_flags(pmd, IFF_UP, 1);
 }
 
 static int
-- 
2.8.0.rc0

[PATCH v5 2/8] net/tap: refactor ioctl calls

[Pascal Mazon]

Create a socket for ioctl at tap device creation instead of opening it
and closing it every call to tap_link_set_flags().

Use a common tap_ioctl() function that can be extended for various uses
(such as MTU change, MAC address change, ...).

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 90 +++++++++++++++++++++++--------------------
 1 file changed, 48 insertions(+), 42 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index fcf4a1dc3da1..50a07249dff2 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -114,6 +114,7 @@ struct pmd_internals {
 	struct ether_addr eth_addr;	/* Mac address of the device port */
 
 	int if_index;			/* IF_INDEX for the port */
+	int ioctl_sock;			/* socket for ioctl calls */
 
 	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 */
@@ -328,63 +329,55 @@ pmd_tx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 }
 
 static int
-tap_link_set_flags(struct pmd_internals *pmd, short flags, int add)
+tap_ioctl(struct pmd_internals *pmd, unsigned long request,
+	  struct ifreq *ifr, int set)
 {
-	struct ifreq ifr;
-	int err, s;
-
-	/*
-	 * An AF_INET/DGRAM socket is needed for
-	 * SIOCGIFFLAGS/SIOCSIFFLAGS, using fd won't work.
-	 */
-	s = socket(AF_INET, SOCK_DGRAM, 0);
-	if (s < 0) {
-		RTE_LOG(ERR, PMD,
-			"Unable to get a socket to set flags: %s\n",
-			strerror(errno));
-		return -1;
-	}
-	memset(&ifr, 0, sizeof(ifr));
-	snprintf(ifr.ifr_name, IFNAMSIZ, "%s", pmd->name);
-	err = ioctl(s, SIOCGIFFLAGS, &ifr);
-	if (err < 0) {
-		RTE_LOG(WARNING, PMD, "Unable to get %s device flags: %s\n",
-			pmd->name, strerror(errno));
-		close(s);
-		return -1;
-	}
-	if (add)
-		ifr.ifr_flags |= flags;
-	else
-		ifr.ifr_flags &= ~flags;
-	err = ioctl(s, SIOCSIFFLAGS, &ifr);
-	if (err < 0) {
-		RTE_LOG(WARNING, PMD, "Unable to %s flags 0x%x: %s\n",
-			add ? "set" : "unset", flags, strerror(errno));
-		close(s);
-		return -1;
-	}
-	close(s);
+	short req_flags = ifr->ifr_flags;
 
+	snprintf(ifr->ifr_name, IFNAMSIZ, "%s", pmd->name);
+	switch (request) {
+	case SIOCSIFFLAGS:
+		/* fetch current flags to leave other flags untouched */
+		if (ioctl(pmd->ioctl_sock, SIOCGIFFLAGS, ifr) < 0)
+			goto error;
+		if (set)
+			ifr->ifr_flags |= req_flags;
+		else
+			ifr->ifr_flags &= ~req_flags;
+		break;
+	default:
+		RTE_LOG(WARNING, PMD, "%s: ioctl() called with wrong arg\n",
+			pmd->name);
+		return -EINVAL;
+	}
+	if (ioctl(pmd->ioctl_sock, request, ifr) < 0)
+		goto error;
 	return 0;
+
+error:
+	RTE_LOG(ERR, PMD, "%s: ioctl(%lu) failed with error: %s\n",
+		ifr->ifr_name, request, strerror(errno));
+	return -errno;
 }
 
 static int
 tap_link_set_down(struct rte_eth_dev *dev)
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr = { .ifr_flags = IFF_UP };
 
 	dev->data->dev_link.link_status = ETH_LINK_DOWN;
-	return tap_link_set_flags(pmd, IFF_UP, 0);
+	return tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 0);
 }
 
 static int
 tap_link_set_up(struct rte_eth_dev *dev)
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr = { .ifr_flags = IFF_UP };
 
 	dev->data->dev_link.link_status = ETH_LINK_UP;
-	return tap_link_set_flags(pmd, IFF_UP, 1);
+	return tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 1);
 }
 
 static int
@@ -518,36 +511,40 @@ static void
 tap_promisc_enable(struct rte_eth_dev *dev)
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr = { .ifr_flags = IFF_PROMISC };
 
 	dev->data->promiscuous = 1;
-	tap_link_set_flags(pmd, IFF_PROMISC, 1);
+	tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 1);
 }
 
 static void
 tap_promisc_disable(struct rte_eth_dev *dev)
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr = { .ifr_flags = IFF_PROMISC };
 
 	dev->data->promiscuous = 0;
-	tap_link_set_flags(pmd, IFF_PROMISC, 0);
+	tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 0);
 }
 
 static void
 tap_allmulti_enable(struct rte_eth_dev *dev)
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr = { .ifr_flags = IFF_ALLMULTI };
 
 	dev->data->all_multicast = 1;
-	tap_link_set_flags(pmd, IFF_ALLMULTI, 1);
+	tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 1);
 }
 
 static void
 tap_allmulti_disable(struct rte_eth_dev *dev)
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr = { .ifr_flags = IFF_ALLMULTI };
 
 	dev->data->all_multicast = 0;
-	tap_link_set_flags(pmd, IFF_ALLMULTI, 0);
+	tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 0);
 }
 
 static int
@@ -724,6 +721,14 @@ eth_dev_tap_create(const char *name, char *tap_name)
 
 	pmd->nb_queues = RTE_PMD_TAP_MAX_QUEUES;
 
+	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));
+		goto error_exit;
+	}
+
 	/* Setup some default values */
 	data->dev_private = pmd;
 	data->port_id = dev->data->port_id;
@@ -866,6 +871,7 @@ rte_pmd_tap_remove(const char *name)
 		if (internals->rxq[i].fd != -1)
 			close(internals->rxq[i].fd);
 
+	close(internals->ioctl_sock);
 	rte_free(eth_dev->data->dev_private);
 	rte_free(eth_dev->data);
 
-- 
2.8.0.rc0

[PATCH v5 3/8] net/tap: add MAC address management

[Pascal Mazon]

As soon as the netdevice is created, update pmd->mac_addr with its
actual MAC address.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 39 +++++++++++++++++++++++++++++++++------
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index f4aca6921ddc..d9b47a003654 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+Unicast MAC filter   = Y
 Other kdrv           = Y
 ARMv7                = Y
 ARMv8                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 50a07249dff2..2cf8203ba24e 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -127,6 +127,10 @@ tap_trigger_cb(int sig __rte_unused)
 	tap_trigger = (tap_trigger + 1) | 0x80000000;
 }
 
+static int
+tap_ioctl(struct pmd_internals *pmd, unsigned long request,
+	  struct ifreq *ifr, int set);
+
 /* Tun/Tap allocation routine
  *
  * name is the number of the interface to use, unless NULL to take the host
@@ -229,13 +233,12 @@ tun_alloc(struct pmd_internals *pmd, uint16_t qid)
 	}
 
 	if (qid == 0) {
-		if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
-			RTE_LOG(ERR, PMD, "ioctl failed (SIOCGIFHWADDR) (%s)\n",
-				ifr.ifr_name);
-			goto error;
-		}
+		struct ifreq ifr;
 
-		rte_memcpy(&pmd->eth_addr, ifr.ifr_hwaddr.sa_data, ETH_ALEN);
+		if (tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0) < 0)
+			goto error;
+		rte_memcpy(&pmd->eth_addr, ifr.ifr_hwaddr.sa_data,
+			   ETHER_ADDR_LEN);
 	}
 
 	return fd;
@@ -345,6 +348,9 @@ tap_ioctl(struct pmd_internals *pmd, unsigned long request,
 		else
 			ifr->ifr_flags &= ~req_flags;
 		break;
+	case SIOCGIFHWADDR:
+	case SIOCSIFHWADDR:
+		break;
 	default:
 		RTE_LOG(WARNING, PMD, "%s: ioctl() called with wrong arg\n",
 			pmd->name);
@@ -547,6 +553,26 @@ tap_allmulti_disable(struct rte_eth_dev *dev)
 	tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 0);
 }
 
+
+static void
+tap_mac_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
+{
+	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr;
+
+	if (is_zero_ether_addr(mac_addr)) {
+		RTE_LOG(ERR, PMD, "%s: can't set an empty MAC address\n",
+			dev->data->name);
+		return;
+	}
+
+	ifr.ifr_hwaddr.sa_family = AF_LOCAL;
+	rte_memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, ETHER_ADDR_LEN);
+	if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 1) < 0)
+		return;
+	rte_memcpy(&pmd->eth_addr, mac_addr, ETHER_ADDR_LEN);
+}
+
 static int
 tap_setup_queue(struct rte_eth_dev *dev,
 		struct pmd_internals *internals,
@@ -682,6 +708,7 @@ static const struct eth_dev_ops ops = {
 	.promiscuous_disable    = tap_promisc_disable,
 	.allmulticast_enable    = tap_allmulti_enable,
 	.allmulticast_disable   = tap_allmulti_disable,
+	.mac_addr_set           = tap_mac_set,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
 };
-- 
2.8.0.rc0

[PATCH v5 4/8] net/tap: add MTU management

[Pascal Mazon]

The MTU is assigned to the tap netdevice according to the argument, but
packet transmission and reception just write/read on an fd with the
default limit being the socket buffer size.

As a new rte_eth_dev_data is allocated during tap device init, ensure it
is set again dev->data->mtu.
Once the actual netdevice is created via tun_alloc(), make sure to apply
the desired MTU to the netdevice.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index d9b47a003654..b80577753240 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -9,6 +9,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
+MTU update           = Y
 Unicast MAC filter   = Y
 Other kdrv           = Y
 ARMv7                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 2cf8203ba24e..b14bbabb84cb 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -350,6 +350,7 @@ tap_ioctl(struct pmd_internals *pmd, unsigned long request,
 		break;
 	case SIOCGIFHWADDR:
 	case SIOCSIFHWADDR:
+	case SIOCSIFMTU:
 		break;
 	default:
 		RTE_LOG(WARNING, PMD, "%s: ioctl() called with wrong arg\n",
@@ -595,6 +596,15 @@ tap_setup_queue(struct rte_eth_dev *dev,
 					pmd->name, qid);
 				return -1;
 			}
+			if (qid == 0) {
+				struct ifreq ifr;
+
+				ifr.ifr_mtu = dev->data->mtu;
+				if (tap_ioctl(pmd, SIOCSIFMTU, &ifr, 1) < 0) {
+					close(fd);
+					return -1;
+				}
+			}
 		}
 	}
 
@@ -691,6 +701,20 @@ tap_tx_queue_setup(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static int
+tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+	struct pmd_internals *pmd = dev->data->dev_private;
+	struct ifreq ifr = { .ifr_mtu = mtu };
+	int err = 0;
+
+	err = tap_ioctl(pmd, SIOCSIFMTU, &ifr, 1);
+	if (!err)
+		dev->data->mtu = mtu;
+
+	return err;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -709,6 +733,7 @@ static const struct eth_dev_ops ops = {
 	.allmulticast_enable    = tap_allmulti_enable,
 	.allmulticast_disable   = tap_allmulti_disable,
 	.mac_addr_set           = tap_mac_set,
+	.mtu_set                = tap_mtu_set,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
 };
@@ -759,6 +784,7 @@ eth_dev_tap_create(const char *name, char *tap_name)
 	/* Setup some default values */
 	data->dev_private = pmd;
 	data->port_id = dev->data->port_id;
+	data->mtu = dev->data->mtu;
 	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 	data->kdrv = RTE_KDRV_NONE;
 	data->drv_name = pmd_tap_drv.driver.name;
-- 
2.8.0.rc0

[PATCH v5 5/8] net/tap: add speed capabilities

[Pascal Mazon]

Tap PMD is flexible, it supports any speed.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index b80577753240..6c07352088bf 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -10,6 +10,7 @@ Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
 MTU update           = Y
+Speed capabilities   = Y
 Unicast MAC filter   = Y
 Other kdrv           = Y
 ARMv7                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index b14bbabb84cb..c531febfc6ce 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -407,6 +407,40 @@ tap_dev_configure(struct rte_eth_dev *dev __rte_unused)
 	return 0;
 }
 
+static uint32_t
+tap_dev_speed_capa(void)
+{
+	uint32_t speed = pmd_link.link_speed;
+	uint32_t capa = 0;
+
+	if (speed >= ETH_SPEED_NUM_10M)
+		capa |= ETH_LINK_SPEED_10M;
+	if (speed >= ETH_SPEED_NUM_100M)
+		capa |= ETH_LINK_SPEED_100M;
+	if (speed >= ETH_SPEED_NUM_1G)
+		capa |= ETH_LINK_SPEED_1G;
+	if (speed >= ETH_SPEED_NUM_5G)
+		capa |= ETH_LINK_SPEED_2_5G;
+	if (speed >= ETH_SPEED_NUM_5G)
+		capa |= ETH_LINK_SPEED_5G;
+	if (speed >= ETH_SPEED_NUM_10G)
+		capa |= ETH_LINK_SPEED_10G;
+	if (speed >= ETH_SPEED_NUM_20G)
+		capa |= ETH_LINK_SPEED_20G;
+	if (speed >= ETH_SPEED_NUM_25G)
+		capa |= ETH_LINK_SPEED_25G;
+	if (speed >= ETH_SPEED_NUM_40G)
+		capa |= ETH_LINK_SPEED_40G;
+	if (speed >= ETH_SPEED_NUM_50G)
+		capa |= ETH_LINK_SPEED_50G;
+	if (speed >= ETH_SPEED_NUM_56G)
+		capa |= ETH_LINK_SPEED_56G;
+	if (speed >= ETH_SPEED_NUM_100G)
+		capa |= ETH_LINK_SPEED_100G;
+
+	return capa;
+}
+
 static void
 tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
@@ -419,6 +453,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_tx_queues = internals->nb_queues;
 	dev_info->min_rx_bufsize = 0;
 	dev_info->pci_dev = NULL;
+	dev_info->speed_capa = tap_dev_speed_capa();
 }
 
 static void
-- 
2.8.0.rc0

[PATCH v5 6/8] net/tap: add multicast addresses management

[Pascal Mazon]

A tap netdevice actually receives every packet, without any filtering
whatsoever. There is no need for any multicast address registration
to receive multicast packets.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index 6c07352088bf..6aa11874e2bc 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -10,6 +10,7 @@ Promiscuous mode     = Y
 Allmulticast mode    = Y
 Basic stats          = Y
 MTU update           = Y
+Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
 Other kdrv           = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index c531febfc6ce..0a684813ed3c 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -750,6 +750,18 @@ tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	return err;
 }
 
+static int
+tap_set_mc_addr_list(struct rte_eth_dev *dev __rte_unused,
+		     struct ether_addr *mc_addr_set __rte_unused,
+		     uint32_t nb_mc_addr __rte_unused)
+{
+	/*
+	 * Nothing to do actually: the tap has no filtering whatsoever, every
+	 * packet is received.
+	 */
+	return 0;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -769,6 +781,7 @@ static const struct eth_dev_ops ops = {
 	.allmulticast_disable   = tap_allmulti_disable,
 	.mac_addr_set           = tap_mac_set,
 	.mtu_set                = tap_mtu_set,
+	.set_mc_addr_list       = tap_set_mc_addr_list,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
 };
-- 
2.8.0.rc0

[PATCH v5 7/8] net/tap: add packet type management

[Pascal Mazon]

Advertize packet types supported by the librte_net.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/Makefile         |  1 +
 drivers/net/tap/rte_eth_tap.c    | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index 6aa11874e2bc..7f3f4d661dd7 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -13,6 +13,7 @@ MTU update           = Y
 Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
+Packet type parsing  = Y
 Other kdrv           = Y
 ARMv7                = Y
 ARMv8                = Y
diff --git a/drivers/net/tap/Makefile b/drivers/net/tap/Makefile
index e18f30c56f52..ddf87232d335 100644
--- a/drivers/net/tap/Makefile
+++ b/drivers/net/tap/Makefile
@@ -53,5 +53,6 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) += lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) += lib/librte_mempool
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) += lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) += lib/librte_kvargs
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) += lib/librte_net
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 0a684813ed3c..7b70d62d5598 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -38,6 +38,7 @@
 #include <rte_malloc.h>
 #include <rte_vdev.h>
 #include <rte_kvargs.h>
+#include <rte_net.h>
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -285,6 +286,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 		mbuf->data_len = len;
 		mbuf->pkt_len = len;
 		mbuf->port = rxq->in_port;
+		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
+						      RTE_PTYPE_ALL_MASK);
 
 		/* account for the receive frame */
 		bufs[num_rx++] = mbuf;
@@ -762,6 +765,37 @@ tap_set_mc_addr_list(struct rte_eth_dev *dev __rte_unused,
 	return 0;
 }
 
+static const uint32_t*
+tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
+{
+	static const uint32_t ptypes[] = {
+		RTE_PTYPE_INNER_L2_ETHER,
+		RTE_PTYPE_INNER_L2_ETHER_VLAN,
+		RTE_PTYPE_INNER_L2_ETHER_QINQ,
+		RTE_PTYPE_INNER_L3_IPV4,
+		RTE_PTYPE_INNER_L3_IPV4_EXT,
+		RTE_PTYPE_INNER_L3_IPV6,
+		RTE_PTYPE_INNER_L3_IPV6_EXT,
+		RTE_PTYPE_INNER_L4_FRAG,
+		RTE_PTYPE_INNER_L4_UDP,
+		RTE_PTYPE_INNER_L4_TCP,
+		RTE_PTYPE_INNER_L4_SCTP,
+		RTE_PTYPE_L2_ETHER,
+		RTE_PTYPE_L2_ETHER_VLAN,
+		RTE_PTYPE_L2_ETHER_QINQ,
+		RTE_PTYPE_L3_IPV4,
+		RTE_PTYPE_L3_IPV4_EXT,
+		RTE_PTYPE_L3_IPV6_EXT,
+		RTE_PTYPE_L3_IPV6,
+		RTE_PTYPE_L4_FRAG,
+		RTE_PTYPE_L4_UDP,
+		RTE_PTYPE_L4_TCP,
+		RTE_PTYPE_L4_SCTP,
+	};
+
+	return ptypes;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -784,6 +818,7 @@ static const struct eth_dev_ops ops = {
 	.set_mc_addr_list       = tap_set_mc_addr_list,
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
+	.dev_supported_ptypes_get = tap_dev_supported_ptypes_get,
 };
 
 static int
-- 
2.8.0.rc0

[PATCH v5 8/8] net/tap: add flow control management

[Pascal Mazon]

A tap netdevice does not support flow control; ensure nothing but
RTE_FC_NONE mode can be set.

Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 doc/guides/nics/features/tap.ini |  1 +
 drivers/net/tap/rte_eth_tap.c    | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini
index 7f3f4d661dd7..a51712dce066 100644
--- a/doc/guides/nics/features/tap.ini
+++ b/doc/guides/nics/features/tap.ini
@@ -14,6 +14,7 @@ Multicast MAC filter = Y
 Speed capabilities   = Y
 Unicast MAC filter   = Y
 Packet type parsing  = Y
+Flow control         = Y
 Other kdrv           = Y
 ARMv7                = Y
 ARMv8                = Y
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 7b70d62d5598..f8d9cc7dc3b2 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -796,6 +796,23 @@ tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
 	return ptypes;
 }
 
+static int
+tap_flow_ctrl_get(struct rte_eth_dev *dev __rte_unused,
+		  struct rte_eth_fc_conf *fc_conf)
+{
+	fc_conf->mode = RTE_FC_NONE;
+	return 0;
+}
+
+static int
+tap_flow_ctrl_set(struct rte_eth_dev *dev __rte_unused,
+		  struct rte_eth_fc_conf *fc_conf)
+{
+	if (fc_conf->mode != RTE_FC_NONE)
+		return -ENOTSUP;
+	return 0;
+}
+
 static const struct eth_dev_ops ops = {
 	.dev_start              = tap_dev_start,
 	.dev_stop               = tap_dev_stop,
@@ -806,6 +823,8 @@ static const struct eth_dev_ops ops = {
 	.tx_queue_setup         = tap_tx_queue_setup,
 	.rx_queue_release       = tap_rx_queue_release,
 	.tx_queue_release       = tap_tx_queue_release,
+	.flow_ctrl_get          = tap_flow_ctrl_get,
+	.flow_ctrl_set          = tap_flow_ctrl_set,
 	.link_update            = tap_link_update,
 	.dev_set_link_up        = tap_link_set_up,
 	.dev_set_link_down      = tap_link_set_down,
-- 
2.8.0.rc0

Re: [PATCH v5 0/8] net/tap: add additional management ops

[Ferruh Yigit]

On 3/15/2017 2:48 PM, Pascal Mazon wrote:
> v2 change:
>   - use snprintf in tap_mtu set
> 
> v3 change:
>   - call tap_mac_set() only once in tap_setup_queue()
> 
> v4 changes:
>   - use new tap_ioctl function for shared behavior between ops
>   - update supported packet types
>   - remove IFF_NOARP flag from link status change
>   - sync desired MTU to both remote and tap netdevices
>   - remove support for mac_add and mac_remove ops
> 
> v5 changes:
>   - rebase after adrien's patches on Tx poll and Rx signaling
>   - add dependency to librte_net for packet type
> 
> Pascal Mazon (8):
>   net/tap: remove wrong IFF_NOARP flags
>   net/tap: refactor ioctl calls
>   net/tap: add MAC address management
>   net/tap: add MTU management
>   net/tap: add speed capabilities
>   net/tap: add multicast addresses management
>   net/tap: add packet type management
>   net/tap: add flow control management

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

Re: [PATCH v5 1/8] net/tap: remove wrong IFF_NOARP flags

[Wiles, Keith]

> On Mar 15, 2017, at 10:48 PM, Pascal Mazon <pascal.mazon@6wind.com> wrote:
> 
> There is no reason not to support ARP on a tap netdevice.
> Focus on IFF_UP when a link status change is required.
> 
> Fixes: f457b472b1f2 ("net/tap: add link up and down operations")
> Signed-off-by: Pascal Mazon <pascal.mazon@6wind.com>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Acked-by: Keith.Wiles@intel.com for the series.

> ---
> drivers/net/tap/rte_eth_tap.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index a5cbb23e4cd2..fcf4a1dc3da1 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -375,7 +375,7 @@ tap_link_set_down(struct rte_eth_dev *dev)
> 	struct pmd_internals *pmd = dev->data->dev_private;
> 
> 	dev->data->dev_link.link_status = ETH_LINK_DOWN;
> -	return tap_link_set_flags(pmd, IFF_UP | IFF_NOARP, 0);
> +	return tap_link_set_flags(pmd, IFF_UP, 0);
> }
> 
> static int
> @@ -384,7 +384,7 @@ tap_link_set_up(struct rte_eth_dev *dev)
> 	struct pmd_internals *pmd = dev->data->dev_private;
> 
> 	dev->data->dev_link.link_status = ETH_LINK_UP;
> -	return tap_link_set_flags(pmd, IFF_UP | IFF_NOARP, 1);
> +	return tap_link_set_flags(pmd, IFF_UP, 1);
> }
> 
> static int
> -- 
> 2.8.0.rc0
> 

Regards,
Keith

Re: [PATCH v5 0/8] net/tap: add additional management ops

[Wiles, Keith]

Please let me know if I have missed and Ack for the TAP PMD. I believe I have reviewed and agree with all of the changes to TAP at this time.

> On Mar 16, 2017, at 12:46 AM, Yigit, Ferruh <ferruh.yigit@intel.com> wrote:
> 
> On 3/15/2017 2:48 PM, Pascal Mazon wrote:
>> v2 change:
>>  - use snprintf in tap_mtu set
>> 
>> v3 change:
>>  - call tap_mac_set() only once in tap_setup_queue()
>> 
>> v4 changes:
>>  - use new tap_ioctl function for shared behavior between ops
>>  - update supported packet types
>>  - remove IFF_NOARP flag from link status change
>>  - sync desired MTU to both remote and tap netdevices
>>  - remove support for mac_add and mac_remove ops
>> 
>> v5 changes:
>>  - rebase after adrien's patches on Tx poll and Rx signaling
>>  - add dependency to librte_net for packet type
>> 
>> Pascal Mazon (8):
>>  net/tap: remove wrong IFF_NOARP flags
>>  net/tap: refactor ioctl calls
>>  net/tap: add MAC address management
>>  net/tap: add MTU management
>>  net/tap: add speed capabilities
>>  net/tap: add multicast addresses management
>>  net/tap: add packet type management
>>  net/tap: add flow control management
> 
> Series applied to dpdk-next-net/master, thanks.

Regards,
Keith