[dpdk-dev] [RFC][PATCH V2 0/3] example/vhost: Introduce Vswitch Framework

[dpdk-dev] [RFC][PATCH V2 0/3] example/vhost: Introduce Vswitch Framework

[Pankaj Chauhan]

Introduce generic vswitch framework in vhost-switch application. Following
are the goals/aim of the framework:

1. Make vhost-switch application generic so that it can support devices 
which don't support VMDQ.

2. Provide a framework so that any switching logic (generic in software or
vendor specefic like VMDQ) can work with vhost-switch. Thus making vhost-switch
applicable for multiple platforms of different vendors.

3. Make vhost-switch and switching logic scalable in terms of ports or policies
of doing rx/tx across the ports added to switch.

The patchset includes three patches:
1. "Add vswitch(generic switch) framework": This adds the generic framework, it provides
the APIs/accessor functions which the vhos-switch application uses without knowing
anything about underlying switching logic. The framework introduces the concept of
vswitch_device, vswitch_port, and vswitch_ops. The idea is that vhost-switch will
sit over the framework and different switching logics will plug into the framework
underneath it. Please refer the patch description for more details of devices, ports
and ops.

2. "Add vswitch command line options": Adds two new command line options for vswitch.
3. "Add VMDQ vswitch device": This patch basically delinks existing vhost/main.[c,h]
from VMDQ and adds VMDQ as a vswitch device implmentation which plugs into the vswitch
framework. Any other vendor logic for switching can also be plugged in same way.

Thanks to Tan Jianfeng, Yuanhan Liu, Maxime coquelin for early discussions and inputs
on this concept.

TODO list:
1. Addd constructor based logic for registration of Vswitch implmentations like VMDQ.
we will use similar mechanism as registration of PMD drivers (base on constructor function)
to register all the switch implmentations in vhost-switch and then select the required
implementation using command line option 'switch'.

2. Test VM2VM hardware mode: I tried following command it didn't work:

vhost-switch -c f -n 4 --socket-mem 1024 --huge-dir /mnt/huge  -- -p 0x1  --vm2vm 2 
  	--vlan-strip 1 --dev-basename usvhost
I tried same command on upstream master also but it didn't work, that means i am doing something
wrong, please help me with correct method to test it. Or now the patches are working, so
if possible please give a try to hardware VM2VM mode.

3. Changes for Maxime's comment on freeing of buffers in vs_looup_n_fwd(). I wasn't clear about
what is the right way to handle, we'll fix it after getting feedback from Maxime.

Change Log
==========

v1->v2
------
1. Tested following:
	- Ping and iperf from VM to external machine connected to Phys port.
	- Ping and iperf betweeen 2 VMs in software vm2vm mode

	- My testsetup is:
		-  vhost & VM machine: xeonD1521 with Network card: X552/X557-AT 10GBASE-T
		- Peer connected to VM machine: OptiPlex-790 with 82574L Gigabit card
2. Incorporated changes for all the comments from Maxime Coquelin. One comment is not yet
   taken care off, and it is listed in TODO list.

Pankaj Chauhan (3):
  examples/vhost: Add vswitch (generic switch) framework
  examples/vhost: Add vswitch command line options
  examples/vhost: Add VMDQ vswitch device

 examples/vhost/Makefile         |   2 +-
 examples/vhost/main.c           | 651 +++++++++++++-------------------------
 examples/vhost/main.h           |  10 +
 examples/vhost/vmdq.c           | 669 ++++++++++++++++++++++++++++++++++++++++
 examples/vhost/vmdq.h           |  57 ++++
 examples/vhost/vswitch_common.c | 499 ++++++++++++++++++++++++++++++
 examples/vhost/vswitch_common.h | 186 +++++++++++
 examples/vhost/vswitch_txrx.c   |  97 ++++++
 examples/vhost/vswitch_txrx.h   |  71 +++++
 9 files changed, 1800 insertions(+), 442 deletions(-)
 create mode 100644 examples/vhost/vmdq.c
 create mode 100644 examples/vhost/vmdq.h
 create mode 100644 examples/vhost/vswitch_common.c
 create mode 100644 examples/vhost/vswitch_common.h
 create mode 100644 examples/vhost/vswitch_txrx.c
 create mode 100644 examples/vhost/vswitch_txrx.h

-- 
1.9.1

[dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Pankaj Chauhan]

Introduce support for a generic framework for handling of switching between
physical and vhost devices. The vswitch framework introduces the following
concept:

1. vswitch_dev: Vswitch device is a logical switch which can have physical and
virtio devices. The devices are operated/used using standard rte_eth API for
physical devices and lib_vhost API for vhost devices, PMD API is not used
for vhost devices.

2. vswitch_port: Any physical or virtio device that is added to vswitch. The
port can have its own tx/rx functions for doing data transfer, which are exposed
to the framework using generic function pointers (vs_port->do_tx/do_rx). This way
the generic code can do tx/rx without understanding the type of device (Physical or
virtio). Similarly each port has its own functions to select tx/rx queues. The framework
provides default tx/rx queue selection functions to the port when port is added
(for both physical and vhost devices). But the framework allows the switch implementation
to override the queue selection functions (vs_port->get_txq/rxq) if required.

3. vswitch_ops: The ops is set of function pointers which are used to do operations
like learning, unlearning, add/delete port, lookup_and_forward. The user of vswitch
framework (vhost/main.[c,h])uses these function pointers to perform above mentioned
operations, thus it remains agnostic of the underlying implementation.

Different switching logics can implement their vswitch_device and vswitch_ops, and
register with the framework. This framework makes vhost-switch application scalable
in terms of:

1. Different switching logics (one of them is vmdq, vhost/vmdq.[c,h]
2. Number of ports.
3. Policies of selecting ports for rx and tx.

Signed-off-by: Pankaj Chauhan <pankaj.chauhan@nxp.com>
---
 examples/vhost/Makefile         |   2 +-
 examples/vhost/main.c           | 128 +++++++++--
 examples/vhost/vswitch_common.c | 499 ++++++++++++++++++++++++++++++++++++++++
 examples/vhost/vswitch_common.h | 186 +++++++++++++++
 examples/vhost/vswitch_txrx.c   |  97 ++++++++
 examples/vhost/vswitch_txrx.h   |  71 ++++++
 6 files changed, 966 insertions(+), 17 deletions(-)
 create mode 100644 examples/vhost/vswitch_common.c
 create mode 100644 examples/vhost/vswitch_common.h
 create mode 100644 examples/vhost/vswitch_txrx.c
 create mode 100644 examples/vhost/vswitch_txrx.h

diff --git a/examples/vhost/Makefile b/examples/vhost/Makefile
index e95c68a..1b58308 100644
--- a/examples/vhost/Makefile
+++ b/examples/vhost/Makefile
@@ -48,7 +48,7 @@ else
 APP = vhost-switch
 
 # all source are stored in SRCS-y
-SRCS-y := main.c
+SRCS-y := main.c vswitch_common.c vswitch_txrx.c vmdq.c
 
 CFLAGS += -O2 -D_FILE_OFFSET_BITS=64
 CFLAGS += $(WERROR_FLAGS)
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 92a9823..c949df4 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -811,9 +811,16 @@ static inline void __attribute__((always_inline))
 virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
 	    struct rte_mbuf *m)
 {
-	uint16_t ret;
+	uint16_t ret, txq;
+	struct vswitch_port *vs_port = dst_vdev->vs_port;
 
-	ret = rte_vhost_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ, &m, 1);
+	/* The switch implmentation decides the txq for each port itself.
+	 * this gives switch implmentations to choose thier own queue management
+	 * for each port
+	 */
+
+	txq = vs_port->get_txq(vs_port, rte_lcore_id());
+	ret = rte_vhost_enqueue_burst(dst_vdev->vid, txq, &m, 1);
 	if (enable_stats) {
 		rte_atomic64_inc(&dst_vdev->stats.rx_total_atomic);
 		rte_atomic64_add(&dst_vdev->stats.rx_atomic, ret);
@@ -832,7 +839,7 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 	struct ether_hdr *pkt_hdr;
 	struct vhost_dev *dst_vdev;
 
-	pkt_hdr = rte_pktmbuf_mtod(m, struct ether_hdr *);
+		pkt_hdr = rte_pktmbuf_mtod(m, struct ether_hdr *);
 
 	dst_vdev = find_vhost_dev(&pkt_hdr->d_addr);
 	if (!dst_vdev)
@@ -934,11 +941,26 @@ static inline void __attribute__((always_inline))
 do_drain_mbuf_table(struct mbuf_table *tx_q)
 {
 	uint16_t count;
+	struct vswitch_port *tx_port;
+
+	/* Let switch implmentation decide which physical port to do tx to.
+	 * Every switch implmentation may have it's own strategy, for example
+	 * VMDQ does tx to only one Physical port. Having a scheduler function
+	 * which is switch specefic give flexibility to have another strategy
+	 * for a switch
+	 */
+	tx_port = vs_sched_tx_port(vswitch_dev_g, VSWITCH_PTYPE_PHYS,
+				     rte_lcore_id());
+	if (unlikely(!tx_port))
+	    goto out;
 
-	count = rte_eth_tx_burst(ports[0], tx_q->txq_id,
-				 tx_q->m_table, tx_q->len);
-	if (unlikely(count < tx_q->len))
-		free_pkts(&tx_q->m_table[count], tx_q->len - count);
+	if (tx_q->len) {
+		count = tx_port->do_tx(tx_port, tx_q->txq_id, NULL, tx_q->m_table,
+					  tx_q->len);
+
+		if (unlikely(count < tx_q->len))
+			free_pkts(&tx_q->m_table[count], tx_q->len - count);
+	}
 
 	tx_q->len = 0;
 }
@@ -1060,6 +1082,28 @@ drain_eth_rx(struct vhost_dev *vdev)
 {
 	uint16_t rx_count, enqueue_count;
 	struct rte_mbuf *pkts[MAX_PKT_BURST];
+	uint16_t rxq, core_id;
+	struct vswitch_port *rx_port;
+
+
+	core_id = rte_lcore_id();
+	/* Let switch implmentation decide which physical port to do rx from.
+	 * Every switch implmentation may have it's own strategy, for example
+	 * VMDQ does rx from only one Physical port. Having a scheduler function
+	 * which is switch specefic give flexibility to have another strategy
+	 * for a switch
+	 */
+	rx_port = vs_sched_rx_port(vswitch_dev_g, VSWITCH_PTYPE_PHYS,
+				    core_id);
+	if (unlikely(!rx_port))
+	    goto out;
+
+	/* The switch implmentation decides the rxq for each port itself.
+	 * this gives switch implmentations to choose thier own queue management
+	 * for each port
+	 */
+	rxq = rx_port->get_rxq(rx_port, vdev, core_id);
+	rx_count = rx_port->do_rx(rx_port, rxq, NULL, pkts, MAX_PKT_BURST);
 
 	rx_count = rte_eth_rx_burst(ports[0], vdev->vmdq_rx_q,
 				    pkts, MAX_PKT_BURST);
@@ -1098,20 +1142,31 @@ static inline void __attribute__((always_inline))
 drain_virtio_tx(struct vhost_dev *vdev)
 {
 	struct rte_mbuf *pkts[MAX_PKT_BURST];
-	uint16_t count;
-	uint16_t i;
+	struct vswitch_port *vs_port = vdev->vs_port;
+	uint16_t count, rxq, core_id;
+
+	/* The switch implmentation decides the rxq for each port itself.
+	 * this gives switch implmentations to choose thier own queue management
+	 * for each port
+	 */
 
-	count = rte_vhost_dequeue_burst(vdev->vid, VIRTIO_TXQ, mbuf_pool,
+	core_id = rte_lcore_id();
+	rxq = vs_port->get_rxq(vs_port, NULL, core_id);
+	count = rte_vhost_dequeue_burst(vdev->vid, rxq, mbuf_pool,
 					pkts, MAX_PKT_BURST);
 
-	/* setup VMDq for the first packet */
-	if (unlikely(vdev->ready == DEVICE_MAC_LEARNING) && count) {
-		if (vdev->remove || link_vmdq(vdev, pkts[0]) == -1)
+	if (unlikely(!count))
+		goto out;
+
+	if (unlikely(vdev->ready == DEVICE_MAC_LEARNING)) {
+		if (vdev->remove || vs_learn_port(vs_port, pkts, count))
 			free_pkts(pkts, count);
 	}
 
-	for (i = 0; i < count; ++i)
-		virtio_tx_route(vdev, pkts[i], vlan_tags[vdev->vid]);
+	vs_lookup_n_fwd(vs_port, pkts, count, VIRTIO_RXQ);
+
+out:
+	return;
 }
 
 /*
@@ -1241,7 +1296,7 @@ static int
 new_device(int vid)
 {
 	int lcore, core_add = 0;
-	uint32_t device_num_min = num_devices;
+	uint32_t device_num_min;
 	struct vhost_dev *vdev;
 
 	vdev = rte_zmalloc("vhost device", sizeof(*vdev), RTE_CACHE_LINE_SIZE);
@@ -1252,6 +1307,16 @@ new_device(int vid)
 		return -1;
 	}
 	vdev->vid = vid;
+	device_num_min = vs_get_max_vdevs(vswitch_dev_g);
+	RTE_LOG(INFO, VHOST_PORT, "max virtio devices %d\n", device_num_min);
+
+	vs_port = vs_add_port(vswitch_dev_g, vid, VSWITCH_PTYPE_VIRTIO, vdev);
+
+	if (!vs_port) {
+		rte_exit(EXIT_FAILURE, "Failed to add port [%d] to vsdev %s\n",
+			 vs_port->port_id, vswitch_dev_g->name);
+	}
+	vdev->vs_port = vs_port;
 
 	TAILQ_INSERT_TAIL(&vhost_dev_list, vdev, global_vdev_entry);
 	vdev->vmdq_rx_q = vid * queues_per_pool + vmdq_queue_base;
@@ -1277,6 +1342,10 @@ new_device(int vid)
 	rte_vhost_enable_guest_notification(vid, VIRTIO_RXQ, 0);
 	rte_vhost_enable_guest_notification(vid, VIRTIO_TXQ, 0);
 
+	RTE_LOG(INFO, VHOST_PORT, "New virtio port %d\n", vid);
+
+	vs_port_start(vs_port);
+
 	RTE_LOG(INFO, VHOST_DATA,
 		"(%d) device has been added to data core %d\n",
 		vid, vdev->coreid);
@@ -1402,6 +1471,31 @@ create_mbuf_pool(uint16_t nr_port, uint32_t nr_switch_core, uint32_t mbuf_size,
 		rte_exit(EXIT_FAILURE, "Cannot create mbuf pool\n");
 }
 
+static uint64_t get_vswitch_conf_flags(void)
+{
+	uint64_t vs_conf_flags = 0;
+
+	if (vm2vm_mode == VM2VM_HARDWARE)
+		vs_conf_flags |= VS_CNF_FLG_VM2VM_HARDWARE;
+
+	if (vm2vm_mode == VM2VM_SOFTWARE)
+		vs_conf_flags |= VS_CNF_FLG_VM2VM_SOFTWARE;
+
+	if (promiscuous)
+		vs_conf_flags |= VS_CNF_FLG_PROMISCOUS_EN;
+
+	if (jumbo_frame_en)
+		vs_conf_flags |= VS_CNF_FLG_JUMBO_EN;
+
+	if (enable_stats)
+		vs_conf_flags |= VS_CNF_FLG_STATS_EN;
+
+	if (vlan_strip)
+		vs_conf_flags |= VS_CNF_FLG_VLAN_STRIP_EN;
+
+	return vs_conf_flags;
+}
+
 /*
  * Main function, does initialisation and calls the per-lcore functions. The CUSE
  * device is also registered here to handle the IOCTLs.
@@ -1484,6 +1578,7 @@ main(int argc, char *argv[])
 				"Cannot initialize network ports\n");
 	}
 
+	RTE_LOG(INFO, VHOST_CONFIG, "PORT init done \n");
 	/* Enable stats if the user option is set. */
 	if (enable_stats) {
 		ret = pthread_create(&tid, NULL, (void *)print_stats, NULL);
@@ -1499,6 +1594,7 @@ main(int argc, char *argv[])
 				"Cannot set print-stats name\n");
 	}
 
+	RTE_LOG(INFO, VHOST_CONFIG, "Launching switch_worker threads \n");
 	/* Launch all data cores. */
 	RTE_LCORE_FOREACH_SLAVE(lcore_id)
 		rte_eal_remote_launch(switch_worker, NULL, lcore_id);
diff --git a/examples/vhost/vswitch_common.c b/examples/vhost/vswitch_common.c
new file mode 100644
index 0000000..9f9ab87
--- /dev/null
+++ b/examples/vhost/vswitch_common.c
@@ -0,0 +1,499 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2016 Freescale Semiconductor. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Freescale Semiconductor nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <unistd.h>
+#include <rte_atomic.h>
+#include <rte_cycles.h>
+#include <rte_ethdev.h>
+#include <rte_log.h>
+#include <rte_string_fns.h>
+#include <rte_malloc.h>
+#include <rte_virtio_net.h>
+#include <rte_ip.h>
+#include <rte_tcp.h>
+
+#include <sys/queue.h>
+#include <strings.h>
+#include <errno.h>
+
+#include "vswitch_common.h"
+#include "vswitch_txrx.h"
+
+/* Meta data for vswitch. Since this is going to be used in this file only it
+ * is defined here instead of in a header file.
+ */
+struct vs_mdata {
+	rte_spinlock_t lock;
+	int dev_count;
+	LIST_HEAD(, vswitch_dev) head;
+};
+
+static struct vs_mdata *vs_mdata_g;
+
+struct vswitch_dev *vs_get_vswitch_dev(const char *name)
+{
+	struct vswitch_dev *temp, *vs_dev = NULL;
+
+	rte_spinlock_lock(&vs_mdata_g->lock);
+	LIST_FOREACH(temp, &vs_mdata_g->head, list) {
+		if (!strncmp(temp->name, name, VSWITCH_NAME_SIZE))
+			vs_dev = temp;
+	}
+	rte_spinlock_unlock(&vs_mdata_g->lock);
+
+	return vs_dev;
+}
+
+static struct vswitch_port *vs_get_free_port(struct vswitch_dev *vs_dev)
+{
+	int i, found = 0;
+	struct vswitch_port *vs_port = NULL;
+
+	rte_spinlock_lock(&vs_dev->lock);
+
+	for (i = 0; i < vs_dev->port_count; i++) {
+		vs_port = &vs_dev->ports[i];
+		if (vs_port->state == VSWITCH_PSTATE_NOT_INUSE) {
+			found = 1;
+			break;
+		}
+	}
+
+	if (found)
+		vs_port->state = VSWITCH_PSTATE_BEING_ALLOCATED;
+	else
+		vs_port = NULL;
+
+	rte_spinlock_unlock(&vs_dev->lock);
+	return vs_port;
+}
+
+static void vs_free_port(struct vswitch_port *vs_port)
+{
+	struct vswitch_dev *vs_dev = vs_port->vs_dev;
+
+	rte_spinlock_lock(&vs_dev->lock);
+	vs_port->state = VSWITCH_PSTATE_NOT_INUSE;
+	rte_spinlock_unlock(&vs_dev->lock);
+}
+
+static __attribute__((unused))struct vswitch_port *vs_get_port(
+			struct vswitch_dev *vs_dev, unsigned int port_id,
+			enum vswitch_port_type type)
+{
+	int i;
+	struct vswitch_port *vs_port = NULL;
+
+	for (i = 0; i < vs_dev->port_count; i++) {
+		vs_port = &vs_dev->ports[i];
+		if ((vs_port->port_id == port_id) && (vs_port->type == type))
+			return vs_port;
+	}
+
+	return NULL;
+}
+
+int vs_port_start(struct vswitch_port *vs_port)
+{
+	struct vswitch_ops *vs_ops = vs_port->vs_dev->ops;
+	int rc = 0;
+
+	if (vs_ops->port_start)
+		rc = vs_ops->port_start(vs_port);
+
+	if (!rc)
+		vs_port->state = VSWITCH_PSTATE_LEARNING;
+
+	return rc;
+}
+
+int vs_port_stop(struct vswitch_port *vs_port)
+{
+	struct vswitch_ops *vs_ops = vs_port->vs_dev->ops;
+	int rc = 0;
+
+	if (vs_ops->port_stop)
+		rc = vs_ops->port_stop(vs_port);
+
+	if (!rc)
+		vs_port->state = VSWITCH_PSTATE_ADDED;
+
+	return rc;
+}
+
+struct vswitch_port *vs_add_port(struct vswitch_dev *vs_dev, int port_id,
+		enum vswitch_port_type type, void *priv)
+{
+	int rc = 0;
+	struct vswitch_port *vs_port = NULL;
+	struct vswitch_ops *vs_ops = vs_dev->ops;
+
+	vs_port = vs_get_free_port(vs_dev);
+	if (!vs_port) {
+		RTE_LOG(DEBUG, VHOST_CONFIG, "Failed get free port in \
+			vswitch %s\n", vs_dev->name);
+		rc = -EBUSY;
+		goto out;
+	}
+
+	vs_port->port_id = port_id;
+	vs_port->type = type;
+	vs_port->priv = priv;
+
+	/* Initialize default port operations. It should be noted that
+	 * The switch ops->add_port can replace them with switch specefic
+	 * operations if required. This gives us more flexibility in switch
+	 * implementations.
+	 */
+
+	switch (type) {
+	case VSWITCH_PTYPE_PHYS:
+	       vs_port->do_tx = vs_do_tx_phys_port;
+	       vs_port->do_rx = vs_do_rx_phys_port;
+	       vs_port->get_txq = vs_get_txq_phys_port;
+	       vs_port->get_rxq = vs_get_rxq_phys_port;
+	       break;
+	case VSWITCH_PTYPE_VIRTIO:
+	       vs_port->do_tx = vs_do_tx_virtio_port;
+	       vs_port->do_rx = vs_do_rx_virtio_port;
+	       vs_port->get_txq = vs_get_txq_virtio_port;
+	       vs_port->get_rxq = vs_get_rxq_virtio_port;
+	       break;
+	default:
+		RTE_LOG(DEBUG, VHOST_CONFIG, "Invalid port [id %d, type %d]",
+				port_id, type);
+	       rc = -EINVAL;
+	       goto out;
+	}
+
+	if (vs_ops->add_port)
+		rc = vs_ops->add_port(vs_port);
+
+	if (rc)
+		goto out;
+
+	vs_port->state = VSWITCH_PSTATE_ADDED;
+
+	rte_eth_macaddr_get(vs_port->port_id, &vs_port->mac_addr);
+	RTE_LOG(INFO, VHOST_PORT, "Port %u MAC: %02"PRIx8" %02"PRIx8" %02"PRIx8
+			" %02"PRIx8" %02"PRIx8" %02"PRIx8"\n",
+			(unsigned)port_id,
+			vs_port->mac_addr.addr_bytes[0],
+			vs_port->mac_addr.addr_bytes[1],
+			vs_port->mac_addr.addr_bytes[2],
+			vs_port->mac_addr.addr_bytes[3],
+			vs_port->mac_addr.addr_bytes[4],
+			vs_port->mac_addr.addr_bytes[5]);
+
+	RTE_LOG(DEBUG, VHOST_CONFIG, "Added port [%d, type %d] to \
+			vswitch %s\n", vs_port->port_id, type, vs_dev->name);
+out:
+	if (rc){
+		RTE_LOG(INFO, VHOST_CONFIG, "Failed to Add port [%d, type %d] to \
+			vswitch %s\n", port_id, type, vs_dev->name);
+		if (vs_port)
+			vs_free_port(vs_port);
+		vs_port = NULL;
+	}
+
+	return vs_port;
+}
+
+int vs_del_port(struct vswitch_port *vs_port)
+{
+	int rc = 0;
+	struct vswitch_ops *vs_ops = vs_port->vs_dev->ops;
+
+	if (vs_ops->del_port)
+		rc = vs_ops->del_port(vs_port);
+
+	if (!rc)
+		vs_port->state = VSWITCH_PSTATE_NOT_INUSE;
+
+
+	/*TBD:XXX: may be put a dummy function which frees all packets without
+	 * any tx/rx, NULL looks ugly!
+	 */
+	vs_port->do_tx = vs_port->do_rx = NULL;
+
+	RTE_LOG(DEBUG, VHOST_CONFIG, "Removed port [%d, type %d] from \
+			vswitch %s\n", vs_port->port_id, vs_port->type,
+			vs_port->vs_dev->name);
+
+	return rc;
+}
+
+int vs_learn_port(struct vswitch_port *vs_port, struct rte_mbuf
+				**pkts, uint16_t count)
+{
+	struct vswitch_ops *vs_ops = vs_port->vs_dev->ops;
+	int rc;
+
+	rc = vs_ops->learn_port(vs_port, pkts, count);
+	if (!rc)
+		vs_port->state = VSWITCH_PSTATE_FORWARDING;
+
+	return rc;
+}
+
+int vs_unlearn_port(struct vswitch_port *vs_port)
+{
+	struct vswitch_ops *vs_ops = vs_port->vs_dev->ops;
+
+	vs_ops->unlearn_port(vs_port);
+	vs_port->state = VSWITCH_PSTATE_LEARNING;
+
+	return 0;
+}
+
+
+int vs_lookup_n_fwd(struct vswitch_port *vs_port, struct rte_mbuf **pkts,
+		    uint16_t count, uint16_t in_rxq)
+{
+	int rc, i;
+	struct vswitch_ops *vs_ops = vs_port->vs_dev->ops;
+
+	rc = vs_ops->lookup_n_fwd(vs_port, pkts, count, in_rxq);
+
+	if (rc) {
+		for (i = 0; i < count; i++)
+			rte_pktmbuf_free(pkts[i]);
+	}
+
+	return rc;
+}
+
+int vs_do_broadcast_fwd(struct vswitch_dev *vs_dev,
+			struct vswitch_port *in_port, uint32_t txport_type_mask,
+			struct rte_mbuf *mbuf)
+{
+	int i = 0, tx_q;
+	struct vswitch_port *dest_port;
+
+	for (i = 0; i < vs_dev->port_count; i++) {
+		dest_port = &vs_dev->ports[i];
+		/*Do not broadcast on incomming port */
+		if ((in_port->type == dest_port->type) &&
+			(in_port->port_id == dest_port->port_id))
+			continue;
+		if (!(VS_PTYPE_MASK(dest_port->type) & txport_type_mask))
+			continue;
+
+		tx_q = dest_port->get_txq(dest_port, rte_lcore_id());
+		dest_port->do_tx(dest_port, tx_q, NULL, &mbuf, 1);
+	}
+
+	return 0;
+}
+
+struct vswitch_port *vs_sched_rx_port(struct vswitch_dev *vs_dev, enum
+				      vswitch_port_type ptype, uint16_t core_id)
+{
+	struct vswitch_port *vs_port = NULL;
+	struct vswitch_ops *vs_ops = vs_dev->ops;
+
+	if (vs_ops->sched_rx_port)
+		vs_port = vs_ops->sched_rx_port(vs_dev, ptype, core_id);
+
+	return vs_port;
+}
+
+struct vswitch_port *vs_sched_tx_port(struct vswitch_dev *vs_dev, enum
+				      vswitch_port_type ptype, uint16_t core_id)
+{
+	struct vswitch_port *vs_port = NULL;
+	struct vswitch_ops *vs_ops = vs_dev->ops;
+
+	if (vs_ops->sched_tx_port)
+		vs_port = vs_ops->sched_tx_port(vs_dev, ptype, core_id);
+
+	return vs_port;
+}
+
+static int vs_check_mandatory_ops(struct vswitch_ops *ops)
+{
+	int rc = 0;
+
+	if (!ops->lookup_n_fwd){
+		RTE_LOG(ERR, VHOST_CONFIG, "lookup_n_fwd is NULL");
+		rc = -EINVAL;
+	}
+	if (!ops->learn_port){
+		RTE_LOG(ERR, VHOST_CONFIG, "learn_port is NULL");
+		rc = -EINVAL;
+	}
+
+	if (!ops->unlearn_port){
+		RTE_LOG(ERR, VHOST_CONFIG, "unlearn_port is NULL");
+		rc = -EINVAL;
+	}
+
+
+	return rc;
+}
+
+struct vswitch_dev *vs_register_switch(const char *name, int priv_size,
+				int max_ports, struct vswitch_ops *ops)
+{
+	struct vswitch_dev *vs_dev;
+	struct vswitch_port *vs_port;
+	int size, i;
+
+	if (vs_check_mandatory_ops(ops)) {
+		RTE_LOG(ERR, VHOST_CONFIG, "%s: mandatory ops missing\n",
+			name);
+		return NULL;
+	}
+
+	size = priv_size + sizeof(struct vswitch_dev);
+	vs_dev = rte_malloc(NULL, size, 64);
+	if (!vs_dev) {
+		RTE_LOG(DEBUG, VHOST_CONFIG, "Failed to vswitch device\n");
+		goto out;
+	}
+
+	strncpy(vs_dev->name, name, VSWITCH_NAME_SIZE);
+	vs_dev->priv = (void *) (vs_dev + 1);
+
+	size = max_ports * sizeof(struct vswitch_port);
+	vs_dev->ports = rte_malloc(NULL, size, 64);
+	if (!vs_dev->ports) {
+		RTE_LOG(DEBUG, VHOST_CONFIG,
+			"Failed allocate %d ports for vswitch %s\n",
+			max_ports, vs_dev->name);
+		goto out;
+	}
+
+	memset(vs_dev->ports, 0, size);
+	for (i = 0; i < max_ports; i++)
+	{
+		vs_port = &vs_dev->ports[i];
+		vs_port->state = VSWITCH_PSTATE_NOT_INUSE;
+		vs_port->vs_dev = vs_dev;
+	}
+
+	vs_dev->port_count = max_ports;
+	vs_dev->ops = ops;
+	rte_spinlock_init(&vs_dev->lock);
+
+	rte_spinlock_lock(&vs_mdata_g->lock);
+	LIST_INSERT_HEAD(&vs_mdata_g->head, vs_dev, list);
+	vs_mdata_g->dev_count++;
+	rte_spinlock_unlock(&vs_mdata_g->lock);
+
+	RTE_LOG(INFO, VHOST_CONFIG, "Added vswitch %s [0x%p], max_ports %d\n",
+		vs_dev->name, vs_dev, vs_dev->port_count);
+
+	return vs_dev;
+
+out:
+	if (vs_dev && vs_dev->ports)
+		rte_free(vs_dev->ports);
+	if (vs_dev)
+		rte_free(vs_dev);
+
+	return NULL;
+}
+
+int  vs_unregister_switch(struct vswitch_dev *vs_dev)
+{
+	struct vswitch_dev *temp;
+	int removed = 0, rc;
+
+	rte_spinlock_lock(&vs_mdata_g->lock);
+	LIST_FOREACH(temp, &vs_mdata_g->head, list) {
+		if (!strncmp(temp->name, vs_dev->name, VSWITCH_NAME_SIZE)){
+			LIST_REMOVE(temp, list);
+			removed = 1;
+		}
+	}
+	rte_spinlock_unlock(&vs_mdata_g->lock);
+
+	if (!removed) {
+		RTE_LOG(DEBUG, VHOST_CONFIG, "vswitch [%s] not found\n",
+			vs_dev->name);
+		rc = -ENODEV;
+	}
+
+	RTE_LOG(DEBUG, VHOST_CONFIG, "Unregistering and freeing vswitch [%s]\n",
+		vs_dev->name);
+
+	if (vs_dev->ports)
+		rte_free(vs_dev->ports);
+
+	rte_free(vs_dev);
+
+	return rc;
+}
+
+int  vs_get_max_vdevs(struct vswitch_dev *vs_dev)
+{
+	struct vswitch_ops *ops = vs_dev->ops;
+
+	return ops->get_max_vdevs(vs_dev);
+}
+
+int vs_switch_dev_init(struct vswitch_dev *vs_dev, uint16_t conf_flags)
+{
+	struct vswitch_ops *ops = vs_dev->ops;
+	int rc = 0;
+
+	if (ops->switch_init)
+		rc = ops->switch_init(vs_dev, conf_flags);
+
+	if (!rc)
+		vs_dev->conf_flags = conf_flags;
+
+	return rc;
+}
+
+int vs_vswitch_init(void)
+{
+	int rc = 0;
+
+	vs_mdata_g = rte_malloc(NULL, sizeof(struct vs_mdata), 64);
+	if (!vs_mdata_g)
+	{
+		RTE_LOG(ERR, VHOST_CONFIG,
+				"vhost vswitch mdata allocation failed\n");
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	memset(vs_mdata_g, 0, sizeof(struct vs_mdata));
+	LIST_INIT(&vs_mdata_g->head);
+	rte_spinlock_init(&vs_mdata_g->lock);
+
+out:
+	return rc;
+}
diff --git a/examples/vhost/vswitch_common.h b/examples/vhost/vswitch_common.h
new file mode 100644
index 0000000..a5800df
--- /dev/null
+++ b/examples/vhost/vswitch_common.h
@@ -0,0 +1,186 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2016 Freescale Semiconductor. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Freescale Semiconductor nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef __VHOST_SWITCH_COMMON_H__
+#define __VHOST_SWITCH_COMMON_H__
+
+#include <sys/queue.h>
+#include "main.h"
+
+#define VSWITCH_NAME_SIZE	32
+
+enum vswitch_port_type {
+	VSWITCH_PTYPE_PHYS = 1,
+	VSWITCH_PTYPE_VIRTIO,
+	VSWITCH_PTYPE_END,
+};
+
+#define VS_PTYPE_MASK(type) (1 << type)
+
+enum vswitch_port_state {
+	VSWITCH_PSTATE_ADDED = 1,
+	VSWITCH_PSTATE_LEARNING,
+	VSWITCH_PSTATE_FORWARDING,
+	VSWITCH_PSTATE_NOT_INUSE,
+	VSWITCH_PSTATE_BEING_ALLOCATED,
+	VSWITCH_PSTATE_END,
+};
+
+struct vswitch_port {
+	enum vswitch_port_type type;
+	enum vswitch_port_state state;
+	unsigned int port_id;
+	struct rte_eth_conf port_conf;
+	struct vswitch_dev *vs_dev;	/*switch to which this port belongs */
+	void *priv;			/*Private for port specefic data*/
+	struct ether_addr mac_addr;
+	int phys_port_rxq;
+	uint16_t (*do_tx) (struct vswitch_port *port, uint16_t tx_q,
+			   __attribute__((unused))struct rte_mempool *mbuf_pool,
+			   struct rte_mbuf **tx_pkts,	uint16_t pkt_count);
+	uint16_t (*do_rx) (struct vswitch_port *port, uint16_t rx_q,
+			   __attribute__((unused))struct rte_mempool *mbuf_pool,
+			   struct rte_mbuf **rx_pkts,	uint16_t pkt_count);
+	uint16_t (*get_rxq) (struct vswitch_port *port, struct vhost_dev *vdev,
+				uint16_t core_id);
+	uint16_t (*get_txq) (struct vswitch_port *port, uint16_t core_id);
+};
+
+struct vswitch_dev {
+	char name[VSWITCH_NAME_SIZE];
+	void *priv;				/* Private for switch specefic
+						 * data */
+	uint64_t conf_flags;
+	rte_spinlock_t lock;
+	LIST_ENTRY (vswitch_dev) list;
+	struct vswitch_port *ports;
+	int port_count;
+	struct vswitch_ops *ops;
+};
+
+/* Function typedefs */
+typedef int (*vs_switch_init_t) (struct vswitch_dev *, uint64_t conf_flags);
+typedef int (*vs_add_port_t) (struct vswitch_port *vs_port);
+typedef int (*vs_del_port_t) (struct vswitch_port *port);
+typedef int (*vs_learn_port_t) (struct vswitch_port *port, struct rte_mbuf
+				**pkts, uint16_t count);
+typedef int (*vs_unlearn_port_t) (struct vswitch_port *port);
+typedef int (*vs_lookup_n_fwd_t)(struct vswitch_port *in_port, struct rte_mbuf
+			      **pkts, uint16_t pkt_count, uint16_t rxq);
+#if 0
+typedef uint16_t (*vs_do_tx_t) (struct vswitch_port *port, uint16_t tx_q, struct
+			   rte_mbuf **tx_pkts,	uint16_t pkt_count);
+
+typedef uint16_t (*vs_do_rx_t) (struct vswitch_port *port, uint16_t rx_q, struct
+			   rte_mbuf **rx_pkts,	uint16_t pkt_count);
+#endif
+typedef struct vswitch_port* (*vs_sched_tx_port_t)(struct vswitch_dev *vs_dev,
+				enum vswitch_port_type ptype, uint16_t core_id);
+
+typedef struct vswitch_port* (*vs_sched_rx_port_t)(struct vswitch_dev *vs_dev,
+				enum vswitch_port_type ptype, uint16_t core_id);
+
+typedef int (*vs_port_start_t) (struct vswitch_port *port);
+typedef int (*vs_get_max_vdevs_t) (struct vswitch_dev *);
+
+struct vswitch_ops {
+	vs_add_port_t add_port;
+	vs_del_port_t del_port;
+	vs_lookup_n_fwd_t lookup_n_fwd;
+	vs_learn_port_t learn_port;
+	vs_unlearn_port_t unlearn_port;
+	vs_port_start_t port_start;
+	vs_port_start_t port_stop;
+	vs_switch_init_t switch_init;
+	vs_sched_tx_port_t sched_tx_port;
+	vs_sched_rx_port_t sched_rx_port;
+	vs_get_max_vdevs_t get_max_vdevs;
+};
+
+/*VSWITCH conf flags */
+#define VS_CNF_FLG_VM2VM_HARDWARE	(1 << 0)
+#define VS_CNF_FLG_VM2VM_SOFTWARE	(1 << 1)
+#define VS_CNF_FLG_PROMISCOUS_EN	(1 << 2)
+#define VS_CNF_FLG_JUMBO_EN		(1 << 3)
+#define VS_CNF_FLG_VLAN_STRIP_EN	(1 << 4)
+#define VS_CNF_FLG_STATS_EN		(1 << 5)
+
+/*API for vswitch implementations*/
+struct vswitch_dev *vs_register_switch(const char *name, int priv_size,
+				int max_ports, struct vswitch_ops *ops);
+int  vs_unregister_switch(struct vswitch_dev *dev);
+
+int vs_vswitch_init(void);
+
+
+/*API for user of vswitch like vhost-switch application */
+
+struct vswitch_dev *vs_get_vswitch_dev(const char *name);
+struct vswitch_port *vs_add_port(struct vswitch_dev *vs_dev, int port_id,
+		enum vswitch_port_type type, void *priv);
+
+int vs_del_port(struct vswitch_port *port);
+
+int vs_switch_dev_init(struct vswitch_dev *vs_dev, uint16_t conf_flags);
+
+int vs_port_start(struct vswitch_port *vs_port);
+int vs_port_stop(struct vswitch_port *vs_port);
+int vs_learn_port(struct vswitch_port *port, struct rte_mbuf
+				**pkts, uint16_t count);
+int vs_unlearn_port(struct vswitch_port *port);
+int vs_lookup_n_fwd(struct vswitch_port *in_port, struct rte_mbuf
+			      **pkts, uint16_t count, uint16_t in_rxq);
+
+int vs_do_broadcast_fwd(struct vswitch_dev *vs_dev,
+		struct vswitch_port *in_port, uint32_t txport_type_mask,
+		struct rte_mbuf *mbuf);
+
+struct vswitch_port *vs_sched_tx_port(struct vswitch_dev *vs_dev, enum
+				vswitch_port_type ptype, uint16_t core_id);
+
+struct vswitch_port *vs_sched_rx_port(struct vswitch_dev *vs_dev, enum
+				vswitch_port_type ptype, uint16_t core_id);
+int vs_get_max_vdevs(struct vswitch_dev *vs_dev);
+
+/*Extern APIs from vhost/main.c */
+
+extern struct mbuf_table *vhost_switch_get_txq(uint16_t core_id);
+extern int virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m);
+extern struct vhost_dev *find_vhost_dev(struct ether_addr *mac);
+void do_drain_mbuf_table(struct mbuf_table *tx_q);
+
+/* TBD:XXX: This needs to be removed here, when constructor mechanism
+ * for registering swittches is in place
+ */
+extern void vmdq_switch_impl_init(void);
+#endif
diff --git a/examples/vhost/vswitch_txrx.c b/examples/vhost/vswitch_txrx.c
new file mode 100644
index 0000000..23b38b9
--- /dev/null
+++ b/examples/vhost/vswitch_txrx.c
@@ -0,0 +1,97 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2016 Freescale Semiconductor. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Freescale Semiconductor nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <unistd.h>
+#include <rte_ethdev.h>
+#include <rte_log.h>
+#include <rte_virtio_net.h>
+
+#include "vswitch_common.h"
+#include "vswitch_txrx.h"
+
+uint16_t vs_do_tx_phys_port(struct vswitch_port *port, uint16_t tx_q,
+			__attribute__((unused))struct rte_mempool *mbuf_pool,
+				   struct rte_mbuf **pkts, uint16_t pkt_count)
+{
+	return rte_eth_tx_burst(port->port_id, tx_q, pkts, pkt_count);
+}
+
+
+uint16_t vs_do_tx_virtio_port(struct vswitch_port *port, uint16_t tx_q,
+			__attribute__((unused)) struct rte_mempool *mbuf_pool,
+			struct rte_mbuf **pkts, uint16_t pkt_count)
+{
+	return rte_vhost_enqueue_burst(port->port_id, tx_q, pkts, pkt_count);
+}
+
+uint16_t vs_do_rx_phys_port (struct vswitch_port *port, uint16_t rx_q,
+			__attribute__((unused))struct rte_mempool *mbuf_pool,
+				 struct rte_mbuf **rx_pkts, uint16_t pkt_count)
+{
+	return rte_eth_rx_burst(port->port_id, rx_q, rx_pkts, pkt_count);
+}
+
+uint16_t vs_do_rx_virtio_port (struct vswitch_port *port, uint16_t rx_q,
+				struct rte_mempool *mbuf_pool,
+				struct rte_mbuf **rx_pkts, uint16_t pkt_count)
+{
+	return rte_vhost_dequeue_burst(port->port_id, rx_q, mbuf_pool,
+				       rx_pkts, pkt_count);
+}
+
+uint16_t vs_get_rxq_phys_port(__attribute__((unused)) struct vswitch_port *port,
+	struct vhost_dev *vdev,	__attribute__((unused))uint16_t core_id)
+{
+	struct vswitch_port *vdev_vs_port = vdev->vs_port;
+
+	return vdev_vs_port->phys_port_rxq;
+}
+
+uint16_t vs_get_txq_phys_port(__attribute__((unused)) struct vswitch_port *port,
+	uint16_t core_id)
+{
+	return core_id;
+}
+
+uint16_t vs_get_rxq_virtio_port(__attribute__((unused)) struct vswitch_port *port,
+	__attribute__((unused))struct vhost_dev *vdev,
+	__attribute__((unused)) uint16_t core_id)
+{
+	return VIRTIO_TXQ;
+}
+
+uint16_t vs_get_txq_virtio_port(__attribute__((unused))struct vswitch_port *port,
+	__attribute__((unused)) uint16_t core_id)
+{
+	return VIRTIO_RXQ;
+}
diff --git a/examples/vhost/vswitch_txrx.h b/examples/vhost/vswitch_txrx.h
new file mode 100644
index 0000000..39d2dab
--- /dev/null
+++ b/examples/vhost/vswitch_txrx.h
@@ -0,0 +1,71 @@
+
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2016 Freescale Semiconductor. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Freescale Semiconductor nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#ifndef __VSWTICH_TXRX_H__
+#define __VSWTICH_TXRX_H__
+
+#include <unistd.h>
+#include <rte_ethdev.h>
+
+#include "vswitch_common.h"
+#include "vswitch_txrx.h"
+
+uint16_t vs_do_tx_phys_port(struct vswitch_port *port, uint16_t tx_q,
+			__attribute__((unused))struct rte_mempool *mbuf_pool,
+				   struct rte_mbuf **pkts, uint16_t pkt_count);
+
+
+uint16_t vs_do_tx_virtio_port(struct vswitch_port *port, uint16_t tx_q,
+			__attribute__((unused)) struct rte_mempool *mbuf_pool,
+				struct rte_mbuf **pkts, uint16_t pkt_count);
+
+uint16_t vs_do_rx_phys_port (struct vswitch_port *port, uint16_t rx_q,
+			__attribute__((unused))struct rte_mempool *mbuf_pool,
+				 struct rte_mbuf **rx_pkts, uint16_t pkt_count);
+
+uint16_t vs_do_rx_virtio_port (struct vswitch_port *port, uint16_t rx_q,
+				struct rte_mempool *mbuf_pool,
+				struct rte_mbuf **rx_pkts, uint16_t pkt_count);
+
+uint16_t vs_get_rxq_phys_port(struct vswitch_port *port,
+	struct vhost_dev *vdev,	uint16_t core_id);
+
+uint16_t vs_get_txq_phys_port(struct vswitch_port *port,
+	uint16_t core_id);
+
+uint16_t vs_get_rxq_virtio_port(struct vswitch_port *port,
+	struct vhost_dev *vdev,	uint16_t core_id);
+
+uint16_t vs_get_txq_virtio_port(struct vswitch_port *port,
+	uint16_t core_id);
+#endif
-- 
1.9.1

Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Tan, Jianfeng]

Hi Pankaj,

Thanks for the massive and great work.

On 9/5/2016 6:54 PM, Pankaj Chauhan wrote:
> Introduce support for a generic framework for handling of switching between
> physical and vhost devices. The vswitch framework introduces the following
> concept:
>
> 1. vswitch_dev: Vswitch device is a logical switch which can have physical and
> virtio devices. The devices are operated/used using standard rte_eth API for
> physical devices and lib_vhost API for vhost devices, PMD API is not used
> for vhost devices.
>
> 2. vswitch_port: Any physical or virtio device that is added to vswitch. The
> port can have its own tx/rx functions for doing data transfer, which are exposed
> to the framework using generic function pointers (vs_port->do_tx/do_rx). This way
> the generic code can do tx/rx without understanding the type of device (Physical or
> virtio). Similarly each port has its own functions to select tx/rx queues. The framework
> provides default tx/rx queue selection functions to the port when port is added
> (for both physical and vhost devices). But the framework allows the switch implementation
> to override the queue selection functions (vs_port->get_txq/rxq) if required.
>
> 3. vswitch_ops: The ops is set of function pointers which are used to do operations
> like learning, unlearning, add/delete port, lookup_and_forward. The user of vswitch
> framework (vhost/main.[c,h])uses these function pointers to perform above mentioned
> operations, thus it remains agnostic of the underlying implementation.
>
> Different switching logics can implement their vswitch_device and vswitch_ops, and
> register with the framework. This framework makes vhost-switch application scalable
> in terms of:
>
> 1. Different switching logics (one of them is vmdq, vhost/vmdq.[c,h]
> 2. Number of ports.
> 3. Policies of selecting ports for rx and tx.
>
> Signed-off-by: Pankaj Chauhan <pankaj.chauhan@nxp.com>

After this patch set, how's mapping relationship between cores and 
vswitch_dev? Old vhost example does not have the concept of switch, so 
each core is binded with some VDEVs. Now, we still keep original logic?

(a) If yes, provided that phys device could has no direct relationship 
with vdevs in other switching logics, we may need to bind those physical 
devices to cores too? In that case, switch_worker() will receiving pkts 
from all devices (phys or virtual) and dispatch, which looks like:

switch_worker() {
     FOR each port binding to this core {
          rx(port, pkts[], count);
          vs_lookup_n_fwd( information_needed );
     }
}

(b) If no, we may bind each core with n switches? If so, switch_worker() 
also need to be reworked to keep receiving pkts from all ports of the 
switch, and dispatch.

switch_worker() {
     FOR each switch binding to this core {
          FOR each port of switch {
              rx(port, pkts[], count);
              vs_lookup_n_fwd( information_needed );
         }
     }
}

In all, (1) we'd better not use vdev to find phys dev in switch_worker 
any more; (2) we'd better not differentiate phys device and virtual 
device in generic framework (it's just an attribute of vswitch_port.

What do you think?

Thanks,
Jianfeng

Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Pankaj Chauhan]

On 9/9/2016 2:26 PM, Tan, Jianfeng wrote:
> Hi Pankaj,
>
> Thanks for the massive and great work.

Hi Jianfeng,

Thanks for the review.
>
> On 9/5/2016 6:54 PM, Pankaj Chauhan wrote:
>> Introduce support for a generic framework for handling of switching
>> between
>> physical and vhost devices. The vswitch framework introduces the
>> following
>> concept:
>>
>> 1. vswitch_dev: Vswitch device is a logical switch which can have
>> physical and
>> virtio devices. The devices are operated/used using standard rte_eth
>> API for
>> physical devices and lib_vhost API for vhost devices, PMD API is not used
>> for vhost devices.
>>
>> 2. vswitch_port: Any physical or virtio device that is added to
>> vswitch. The
>> port can have its own tx/rx functions for doing data transfer, which
>> are exposed
>> to the framework using generic function pointers
>> (vs_port->do_tx/do_rx). This way
>> the generic code can do tx/rx without understanding the type of device
>> (Physical or
>> virtio). Similarly each port has its own functions to select tx/rx
>> queues. The framework
>> provides default tx/rx queue selection functions to the port when port
>> is added
>> (for both physical and vhost devices). But the framework allows the
>> switch implementation
>> to override the queue selection functions (vs_port->get_txq/rxq) if
>> required.
>>
>> 3. vswitch_ops: The ops is set of function pointers which are used to
>> do operations
>> like learning, unlearning, add/delete port, lookup_and_forward. The
>> user of vswitch
>> framework (vhost/main.[c,h])uses these function pointers to perform
>> above mentioned
>> operations, thus it remains agnostic of the underlying implementation.
>>
>> Different switching logics can implement their vswitch_device and
>> vswitch_ops, and
>> register with the framework. This framework makes vhost-switch
>> application scalable
>> in terms of:
>>
>> 1. Different switching logics (one of them is vmdq, vhost/vmdq.[c,h]
>> 2. Number of ports.
>> 3. Policies of selecting ports for rx and tx.
>>
>> Signed-off-by: Pankaj Chauhan <pankaj.chauhan@nxp.com>
>
> After this patch set, how's mapping relationship between cores and
> vswitch_dev? Old vhost example does not have the concept of switch, so
> each core is binded with some VDEVs. Now, we still keep original logic?
>
> (a) If yes, provided that phys device could has no direct relationship
> with vdevs in other switching logics, we may need to bind those physical
> devices to cores too? In that case, switch_worker() will receiving pkts
> from all devices (phys or virtual) and dispatch, which looks like:
>
> switch_worker() {
>     FOR each port binding to this core {
>          rx(port, pkts[], count);
>          vs_lookup_n_fwd( information_needed );
>     }
> }

Since we support only one switch device running at one time. We bind the 
ports of the switchdev to the core. But The switch might have it's own 
logic to bind the port to the core. For example
VMDQ only supports one Physical port, other switch can support more than 
one Physical port. In order to take care of that i have added two ops in 
swithdev_ops:

1. vs_sched_rx_port:

struct vswitch_port *vs_sched_rx_port(struct vswitch_dev *vs_dev, enum
                                       vswitch_port_type ptype, uint16_t 
core_id)

2. vs_sched_tx_port:

struct vswitch_port *vs_sched_tx_port(struct vswitch_dev *vs_dev, enum
                                       vswitch_port_type ptype, uint16_t
core_id)

The idea of providing these functions is that vhost/main requests the 
underlying switch implementation to schedule a port for rx or Tx, the 
current running core_id is also passed as parameter. So the switch can
take a decision which port to do rx or tx based on core id, and may be 
some other custom policy.

For example VMDQ always returns the one single Physical port in response 
to these functions called from any core. The other switch
can return the ports bound to the cores.

Similarly there are two port operations (vs_port->get_rxq and 
vs_port->get_txq), here also we pass core_id as parameter so that
the underlying switch implementation can schedule the rx or tx queue 
based on the core_id.

The above mentioned ops are used in drain_eth_rx() and 
do_drain_mbuf_table() (main.c) currently, and they leave binding of 
physical port and the queues to the underlying implementation. This
way we can accommodate VMDQ which uses only one physical port and 
rxqueues based on VMDQ, OR any other switch which uses multiple physical 
port and rx/tx queue scheduling based on some other logic.

Please suggest if this way of scheduling ports and tx/rx queues is fine 
or not?
>
> (b) If no, we may bind each core with n switches? If so, switch_worker()
> also need to be reworked to keep receiving pkts from all ports of the
> switch, and dispatch.
>
> switch_worker() {
>     FOR each switch binding to this core {
>          FOR each port of switch {
>              rx(port, pkts[], count);
>              vs_lookup_n_fwd( information_needed );
>         }
>     }
> }
Since we currently support only one switch_dev in a running instance of
vhost_switch() we can use binding of ports to core as you suggested in #(a).

>
> In all, (1) we'd better not use vdev to find phys dev in switch_worker
> any more;

I agree with you. I have tried to do following in drain_eth_rx ():


         core_id = rte_lcore_id();
         rx_port = vs_sched_rx_port(vswitch_dev_g, VSWITCH_PTYPE_PHYS,
                                     core_id);
         if (unlikely(!rx_port))
             goto out;

        rxq = rx_port->get_rxq(rx_port, vdev, core_id);
         rx_count = rx_port->do_rx(rx_port, rxq, NULL, pkts, 
MAX_PKT_BURST);

Here we don't assume any relation between vdev and the physical device 
or rxq.  But pass vdev to the underlying switch so that it can choose
the rxq for this vdev. In case of VMDQ it will return VMDQ queue number 
for this vdev. In case of any other switch implementation it can have 
their own logic to decide rxq.

(2) we'd better not differentiate phys device and virtual
> device in generic framework (it's just an attribute of vswitch_port.
>
> What do you think?

I agree with your thought that given the current API in this patchset we
should aim for making switch_worker agnostic of the port type. Ideally 
it should look something like this:

switch_worker() {

		rx_port mask = VSWITCH_PTYPE_PHYS | VSWITCH_PTYPE_PHYS;

		rx_port = vs_sched_rx_port(vswit_dev_g, rx_port_mask, core_id)
		rx_q = rx_port->get_rxq(vs_port, vdev, code_id);
		rx_port->do_rx(rx_port, rxq, NULL, pktss, MAX_PKT_BURST);

		vs_lookup_n_fwd(rx_port, pkts, rx_count, rxq);

}

Here i have two problems in achieving switch_worker as mentioned above:

1. while doing Rx from physical port, we need the associated vdev to 
know the VMDQ rx_q. That was the reason i kept current logic of keeping 
vdevs bound to the core in the switch_worker.  If we don't keep list of 
vdevs assigned to the core, how we' will get the rxq on phsyical device 
in case of VMDQ switch implementation.

2. drain_mbuf_table() currently assumes that there is only one physical 
device on which we have to transmit. We may have to rethink where to 
keep the tx queues (&lcore_tx_queue[core_id]), i case we have multiple 
physical ports we might have to move tx queues to each port.

Since i didn't have good solution currently for #1 and #2 mentioned 
above i kept assumption of one single physical port as it is and tried 
to integrate VMDQ in the framework with leaving these assumptions (thus
the main structure of switch_worker) as it it.

Please suggest if you have some solution to above mentioned points in 
your mind. Ideally i agree we should have switch_worker() as i mentioned 
in pseudo-code i gave above.
>
> Thanks,
> Jianfeng
>

Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Tan, Jianfeng]

Hi Pankaj,


On 9/12/2016 6:55 PM, Pankaj Chauhan wrote:
> On 9/9/2016 2:26 PM, Tan, Jianfeng wrote:
>> Hi Pankaj,
>>
>> Thanks for the massive and great work.
>
> Hi Jianfeng,
>
> Thanks for the review.
>>
>> On 9/5/2016 6:54 PM, Pankaj Chauhan wrote:
>>> Introduce support for a generic framework for handling of switching
>>> between
>>> physical and vhost devices. The vswitch framework introduces the
>>> following
>>> concept:
>>>
>>> 1. vswitch_dev: Vswitch device is a logical switch which can have
>>> physical and
>>> virtio devices. The devices are operated/used using standard rte_eth
>>> API for
>>> physical devices and lib_vhost API for vhost devices, PMD API is not 
>>> used
>>> for vhost devices.
>>>
>>> 2. vswitch_port: Any physical or virtio device that is added to
>>> vswitch. The
>>> port can have its own tx/rx functions for doing data transfer, which
>>> are exposed
>>> to the framework using generic function pointers
>>> (vs_port->do_tx/do_rx). This way
>>> the generic code can do tx/rx without understanding the type of device
>>> (Physical or
>>> virtio). Similarly each port has its own functions to select tx/rx
>>> queues. The framework
>>> provides default tx/rx queue selection functions to the port when port
>>> is added
>>> (for both physical and vhost devices). But the framework allows the
>>> switch implementation
>>> to override the queue selection functions (vs_port->get_txq/rxq) if
>>> required.
>>>
>>> 3. vswitch_ops: The ops is set of function pointers which are used to
>>> do operations
>>> like learning, unlearning, add/delete port, lookup_and_forward. The
>>> user of vswitch
>>> framework (vhost/main.[c,h])uses these function pointers to perform
>>> above mentioned
>>> operations, thus it remains agnostic of the underlying implementation.
>>>
>>> Different switching logics can implement their vswitch_device and
>>> vswitch_ops, and
>>> register with the framework. This framework makes vhost-switch
>>> application scalable
>>> in terms of:
>>>
>>> 1. Different switching logics (one of them is vmdq, vhost/vmdq.[c,h]
>>> 2. Number of ports.
>>> 3. Policies of selecting ports for rx and tx.
>>>
>>> Signed-off-by: Pankaj Chauhan <pankaj.chauhan@nxp.com>
>>
>> After this patch set, how's mapping relationship between cores and
>> vswitch_dev? Old vhost example does not have the concept of switch, so
>> each core is binded with some VDEVs. Now, we still keep original logic?
>>
>> (a) If yes, provided that phys device could has no direct relationship
>> with vdevs in other switching logics, we may need to bind those physical
>> devices to cores too? In that case, switch_worker() will receiving pkts
>> from all devices (phys or virtual) and dispatch, which looks like:
>>
>> switch_worker() {
>>     FOR each port binding to this core {
>>          rx(port, pkts[], count);
>>          vs_lookup_n_fwd( information_needed );
>>     }
>> }
>
> Since we support only one switch device running at one time. We bind 
> the ports of the switchdev to the core. But The switch might have it's 
> own logic to bind the port to the core. For example
> VMDQ only supports one Physical port, other switch can support more 
> than one Physical port. In order to take care of that i have added two 
> ops in swithdev_ops:
>
> 1. vs_sched_rx_port:
>
> struct vswitch_port *vs_sched_rx_port(struct vswitch_dev *vs_dev, enum
>                                       vswitch_port_type ptype, 
> uint16_t core_id)
>
> 2. vs_sched_tx_port:
>
> struct vswitch_port *vs_sched_tx_port(struct vswitch_dev *vs_dev, enum
>                                       vswitch_port_type ptype, uint16_t
> core_id)
>
> The idea of providing these functions is that vhost/main requests the 
> underlying switch implementation to schedule a port for rx or Tx, the 
> current running core_id is also passed as parameter. So the switch can
> take a decision which port to do rx or tx based on core id, and may be 
> some other custom policy.
>
> For example VMDQ always returns the one single Physical port in 
> response to these functions called from any core. The other switch
> can return the ports bound to the cores.
>
> Similarly there are two port operations (vs_port->get_rxq and 
> vs_port->get_txq), here also we pass core_id as parameter so that
> the underlying switch implementation can schedule the rx or tx queue 
> based on the core_id.
>
> The above mentioned ops are used in drain_eth_rx() and 
> do_drain_mbuf_table() (main.c) currently, and they leave binding of 
> physical port and the queues to the underlying implementation. This
> way we can accommodate VMDQ which uses only one physical port and 
> rxqueues based on VMDQ, OR any other switch which uses multiple 
> physical port and rx/tx queue scheduling based on some other logic.
>
> Please suggest if this way of scheduling ports and tx/rx queues is 
> fine or not?

According to above explanation, in VMDQ switch, we cannot schedule two 
queues (belongs to the same port) on the same core, right?


>>
>> (b) If no, we may bind each core with n switches? If so, switch_worker()
>> also need to be reworked to keep receiving pkts from all ports of the
>> switch, and dispatch.
>>
>> switch_worker() {
>>     FOR each switch binding to this core {
>>          FOR each port of switch {
>>              rx(port, pkts[], count);
>>              vs_lookup_n_fwd( information_needed );
>>         }
>>     }
>> }
> Since we currently support only one switch_dev in a running instance of
> vhost_switch() we can use binding of ports to core as you suggested in 
> #(a).

OK.

>
>>
>> In all, (1) we'd better not use vdev to find phys dev in switch_worker
>> any more;
>
> I agree with you. I have tried to do following in drain_eth_rx ():
>
>
>         core_id = rte_lcore_id();
>         rx_port = vs_sched_rx_port(vswitch_dev_g, VSWITCH_PTYPE_PHYS,
>                                     core_id);
>         if (unlikely(!rx_port))
>             goto out;
>
>        rxq = rx_port->get_rxq(rx_port, vdev, core_id);
>         rx_count = rx_port->do_rx(rx_port, rxq, NULL, pkts, 
> MAX_PKT_BURST);
>
> Here we don't assume any relation between vdev and the physical device 
> or rxq.  But pass vdev to the underlying switch so that it can choose
> the rxq for this vdev. In case of VMDQ it will return VMDQ queue 
> number for this vdev. In case of any other switch implementation it 
> can have their own logic to decide rxq.

Thanks for explanation.

>
> (2) we'd better not differentiate phys device and virtual
>> device in generic framework (it's just an attribute of vswitch_port.
>>
>> What do you think?
>
> I agree with your thought that given the current API in this patchset we
> should aim for making switch_worker agnostic of the port type. Ideally 
> it should look something like this:
>
> switch_worker() {
>
>         rx_port mask = VSWITCH_PTYPE_PHYS | VSWITCH_PTYPE_PHYS;
>
>         rx_port = vs_sched_rx_port(vswit_dev_g, rx_port_mask, core_id)
>         rx_q = rx_port->get_rxq(vs_port, vdev, code_id);
>         rx_port->do_rx(rx_port, rxq, NULL, pktss, MAX_PKT_BURST);

Can we hide queues inside struct vswitch_port? I mean:
For VMDQ switch, treat (port_id, queue_id) as a vswitch_port, so far 
you've already stored "struct vhost_dev *" into vswitch_port.priv when 
it's a virtual port, how about store queue_id into vswitch_port.priv 
when it's a physical port.
For arp_learning switch, make (port_id, all_enabled_queues) as a 
vswitch_port.
Summarize above two: we treat (port_id, all_enabled_queues[]) as a 
vswitch_port.

How about it?

>
>         vs_lookup_n_fwd(rx_port, pkts, rx_count, rxq);
>
> }
>
> Here i have two problems in achieving switch_worker as mentioned above:
>
> 1. while doing Rx from physical port, we need the associated vdev to 
> know the VMDQ rx_q. That was the reason i kept current logic of 
> keeping vdevs bound to the core in the switch_worker.  If we don't 
> keep list of vdevs assigned to the core, how we' will get the rxq on 
> phsyical device in case of VMDQ switch implementation.

I think my proposal above can solve this problem.

>
> 2. drain_mbuf_table() currently assumes that there is only one 
> physical device on which we have to transmit. We may have to rethink 
> where to keep the tx queues (&lcore_tx_queue[core_id]), i case we have 
> multiple physical ports we might have to move tx queues to each port.

This function, drain_mbuf_table(), and its related data structures, are 
used for cache on tx direction. But it's not only useful for physical 
port, but also virtual port (vhost_dev). I suggest to make such cache a 
field of per vswitch_port. And each again, all vswitch_ports are 
connected to a core, each core will periodically clean those vswitch_ports.

Thanks,
Jianfeng

>
> Since i didn't have good solution currently for #1 and #2 mentioned 
> above i kept assumption of one single physical port as it is and tried 
> to integrate VMDQ in the framework with leaving these assumptions (thus
> the main structure of switch_worker) as it it.
>
> Please suggest if you have some solution to above mentioned points in 
> your mind. Ideally i agree we should have switch_worker() as i 
> mentioned in pseudo-code i gave above.
>>
>> Thanks,
>> Jianfeng
>>
>
>

Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Pankaj Chauhan]

On 9/13/2016 12:21 PM, Tan, Jianfeng wrote:
Hi Jianfeng,

>>>
>>> On 9/5/2016 6:54 PM, Pankaj Chauhan wrote:
>>>> Introduce support for a generic framework for handling of switching
>>>> between
>>>> physical and vhost devices. The vswitch framework introduces the
>>>> following
>>>> concept:
>>>>
>>>> 1. vswitch_dev: Vswitch device is a logical switch which can have
>>>> physical and
>>>> virtio devices. The devices are operated/used using standard rte_eth
>>>> API for
>>>> physical devices and lib_vhost API for vhost devices, PMD API is not
>>>> used
>>>> for vhost devices.
>>>>
>>>> 2. vswitch_port: Any physical or virtio device that is added to
>>>> vswitch. The
>>>> port can have its own tx/rx functions for doing data transfer, which
>>>> are exposed
>>>> to the framework using generic function pointers
>>>> (vs_port->do_tx/do_rx). This way
>>>> the generic code can do tx/rx without understanding the type of device
>>>> (Physical or
>>>> virtio). Similarly each port has its own functions to select tx/rx
>>>> queues. The framework
>>>> provides default tx/rx queue selection functions to the port when port
>>>> is added
>>>> (for both physical and vhost devices). But the framework allows the
>>>> switch implementation
>>>> to override the queue selection functions (vs_port->get_txq/rxq) if
>>>> required.
>>>>
>>>> 3. vswitch_ops: The ops is set of function pointers which are used to
>>>> do operations
>>>> like learning, unlearning, add/delete port, lookup_and_forward. The
>>>> user of vswitch
>>>> framework (vhost/main.[c,h])uses these function pointers to perform
>>>> above mentioned
>>>> operations, thus it remains agnostic of the underlying implementation.
>>>>
>>>> Different switching logics can implement their vswitch_device and
>>>> vswitch_ops, and
>>>> register with the framework. This framework makes vhost-switch
>>>> application scalable
>>>> in terms of:
>>>>
>>>> 1. Different switching logics (one of them is vmdq, vhost/vmdq.[c,h]
>>>> 2. Number of ports.
>>>> 3. Policies of selecting ports for rx and tx.
>>>>
>>>> Signed-off-by: Pankaj Chauhan <pankaj.chauhan@nxp.com>
>>>
>>> After this patch set, how's mapping relationship between cores and
>>> vswitch_dev? Old vhost example does not have the concept of switch, so
>>> each core is binded with some VDEVs. Now, we still keep original logic?
>>>
>>> (a) If yes, provided that phys device could has no direct relationship
>>> with vdevs in other switching logics, we may need to bind those physical
>>> devices to cores too? In that case, switch_worker() will receiving pkts
>>> from all devices (phys or virtual) and dispatch, which looks like:
>>>
>>> switch_worker() {
>>>     FOR each port binding to this core {
>>>          rx(port, pkts[], count);
>>>          vs_lookup_n_fwd( information_needed );
>>>     }
>>> }
>>
>> Since we support only one switch device running at one time. We bind
>> the ports of the switchdev to the core. But The switch might have it's
>> own logic to bind the port to the core. For example
>> VMDQ only supports one Physical port, other switch can support more
>> than one Physical port. In order to take care of that i have added two
>> ops in swithdev_ops:
>>
>> 1. vs_sched_rx_port:
>>
>> struct vswitch_port *vs_sched_rx_port(struct vswitch_dev *vs_dev, enum
>>                                       vswitch_port_type ptype,
>> uint16_t core_id)
>>
>> 2. vs_sched_tx_port:
>>
>> struct vswitch_port *vs_sched_tx_port(struct vswitch_dev *vs_dev, enum
>>                                       vswitch_port_type ptype, uint16_t
>> core_id)
>>
>> The idea of providing these functions is that vhost/main requests the
>> underlying switch implementation to schedule a port for rx or Tx, the
>> current running core_id is also passed as parameter. So the switch can
>> take a decision which port to do rx or tx based on core id, and may be
>> some other custom policy.
>>
>> For example VMDQ always returns the one single Physical port in
>> response to these functions called from any core. The other switch
>> can return the ports bound to the cores.
>>
>> Similarly there are two port operations (vs_port->get_rxq and
>> vs_port->get_txq), here also we pass core_id as parameter so that
>> the underlying switch implementation can schedule the rx or tx queue
>> based on the core_id.
>>
>> The above mentioned ops are used in drain_eth_rx() and
>> do_drain_mbuf_table() (main.c) currently, and they leave binding of
>> physical port and the queues to the underlying implementation. This
>> way we can accommodate VMDQ which uses only one physical port and
>> rxqueues based on VMDQ, OR any other switch which uses multiple
>> physical port and rx/tx queue scheduling based on some other logic.
>>
>> Please suggest if this way of scheduling ports and tx/rx queues is
>> fine or not?
>
> According to above explanation, in VMDQ switch, we cannot schedule two
> queues (belongs to the same port) on the same core, right?
>

Yes. This would be a limitation i believe which will not be acceptable.
The method that you suggested further in the email (tread port + 
queue_id as a vs_port) can solve this.

>
>>>
>>> (b) If no, we may bind each core with n switches? If so, switch_worker()
>>> also need to be reworked to keep receiving pkts from all ports of the
>>> switch, and dispatch.
>>>
>>> switch_worker() {
>>>     FOR each switch binding to this core {
>>>          FOR each port of switch {
>>>              rx(port, pkts[], count);
>>>              vs_lookup_n_fwd( information_needed );
>>>         }
>>>     }
>>> }
>> Since we currently support only one switch_dev in a running instance of
>> vhost_switch() we can use binding of ports to core as you suggested in
>> #(a).
>
> OK.
>
>>
>>>
>>> In all, (1) we'd better not use vdev to find phys dev in switch_worker
>>> any more;
>>
>> I agree with you. I have tried to do following in drain_eth_rx ():
>>
>>
>>         core_id = rte_lcore_id();
>>         rx_port = vs_sched_rx_port(vswitch_dev_g, VSWITCH_PTYPE_PHYS,
>>                                     core_id);
>>         if (unlikely(!rx_port))
>>             goto out;
>>
>>        rxq = rx_port->get_rxq(rx_port, vdev, core_id);
>>         rx_count = rx_port->do_rx(rx_port, rxq, NULL, pkts,
>> MAX_PKT_BURST);
>>
>> Here we don't assume any relation between vdev and the physical device
>> or rxq.  But pass vdev to the underlying switch so that it can choose
>> the rxq for this vdev. In case of VMDQ it will return VMDQ queue
>> number for this vdev. In case of any other switch implementation it
>> can have their own logic to decide rxq.
>
> Thanks for explanation.
>
>>
>> (2) we'd better not differentiate phys device and virtual
>>> device in generic framework (it's just an attribute of vswitch_port.
>>>
>>> What do you think?
>>
>> I agree with your thought that given the current API in this patchset we
>> should aim for making switch_worker agnostic of the port type. Ideally
>> it should look something like this:
>>
>> switch_worker() {
>>
>>         rx_port mask = VSWITCH_PTYPE_PHYS | VSWITCH_PTYPE_PHYS;
>>
>>         rx_port = vs_sched_rx_port(vswit_dev_g, rx_port_mask, core_id)
>>         rx_q = rx_port->get_rxq(vs_port, vdev, code_id);
>>         rx_port->do_rx(rx_port, rxq, NULL, pktss, MAX_PKT_BURST);
>
> Can we hide queues inside struct vswitch_port? I mean:
> For VMDQ switch, treat (port_id, queue_id) as a vswitch_port, so far
> you've already stored "struct vhost_dev *" into vswitch_port.priv when
> it's a virtual port, how about store queue_id into vswitch_port.priv
> when it's a physical port.
> For arp_learning switch, make (port_id, all_enabled_queues) as a
> vswitch_port.
> Summarize above two: we treat (port_id, all_enabled_queues[]) as a
> vswitch_port.
>
> How about it?

Thanks for wonderful suggestion! Yes it should be possible, instead of 
having one vs_port for physical device we could create one vs_port for 
each phys device + queue_id combination.

Then we can bind the vs_ports to the cores, and do away with binding 
vdevs to the cores.

In order to add extra vs_ports (port + queue_id) there are two methods:

1. vhost/main.c (or vswitch_common.c) queries the underlying switch 
about how many rx queues to support thourgh a new accessor. VMDQ will 
return the number of vmdq queues, in this case. After getting the number 
of rx queues, vhost/main.c creates the extra ports i.e one vs_port for 
each physical port and queue id combination.

2. Second method is that the switch implementation for example vmdq.c 
create the extra ports (port + queue_id) as part of vs_port->add_port 
operation for the physical port.

Which method do you think we should follow? I think #1 should be done so 
that same logic can be used for other switches also.
>
>>
>>         vs_lookup_n_fwd(rx_port, pkts, rx_count, rxq);
>>
>> }
>>
>> Here i have two problems in achieving switch_worker as mentioned above:
>>
>> 1. while doing Rx from physical port, we need the associated vdev to
>> know the VMDQ rx_q. That was the reason i kept current logic of
>> keeping vdevs bound to the core in the switch_worker.  If we don't
>> keep list of vdevs assigned to the core, how we' will get the rxq on
>> phsyical device in case of VMDQ switch implementation.
>
> I think my proposal above can solve this problem.

Yes thanks!
>
>>
>> 2. drain_mbuf_table() currently assumes that there is only one
>> physical device on which we have to transmit. We may have to rethink
>> where to keep the tx queues (&lcore_tx_queue[core_id]), i case we have
>> multiple physical ports we might have to move tx queues to each port.
>
> This function, drain_mbuf_table(), and its related data structures, are
> used for cache on tx direction. But it's not only useful for physical
> port, but also virtual port (vhost_dev). I suggest to make such cache a
> field of per vswitch_port. And each again, all vswitch_ports are
> connected to a core, each core will periodically clean those vswitch_ports.
>

okay. So we add similar tx_q structures (one per core) to each 
vswitch_port, correct?

Thanks,
Pankaj

Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Tan, Jianfeng]

Hi Pankaj,

>> Can we hide queues inside struct vswitch_port? I mean:
>> For VMDQ switch, treat (port_id, queue_id) as a vswitch_port, so far
>> you've already stored "struct vhost_dev *" into vswitch_port.priv when
>> it's a virtual port, how about store queue_id into vswitch_port.priv
>> when it's a physical port.
>> For arp_learning switch, make (port_id, all_enabled_queues) as a
>> vswitch_port.
>> Summarize above two: we treat (port_id, all_enabled_queues[]) as a
>> vswitch_port.
>>
>> How about it?
>
> Thanks for wonderful suggestion! Yes it should be possible, instead of 
> having one vs_port for physical device we could create one vs_port for 
> each phys device + queue_id combination.
>
> Then we can bind the vs_ports to the cores, and do away with binding 
> vdevs to the cores.
>
> In order to add extra vs_ports (port + queue_id) there are two methods:
>
> 1. vhost/main.c (or vswitch_common.c) queries the underlying switch 
> about how many rx queues to support thourgh a new accessor. VMDQ will 
> return the number of vmdq queues, in this case. After getting the 
> number of rx queues, vhost/main.c creates the extra ports i.e one 
> vs_port for each physical port and queue id combination.
>
> 2. Second method is that the switch implementation for example vmdq.c 
> create the extra ports (port + queue_id) as part of vs_port->add_port 
> operation for the physical port.
>
> Which method do you think we should follow? I think #1 should be done 
> so that same logic can be used for other switches also.

Although VMDQs are created at initialization, we will not connect all of 
them to switch until a virtio device is added. So how about creating 
extra vs_ports (port + queue_id) as part of vmdq_add_port_virtio()?

Another thing, why we need the accessor, port_start()? For physical 
ports, it's initialized and started at port_init(); for virtual ports, 
no need to get started, right?

Thanks,
Jianfeng

Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Pankaj Chauhan]

On 9/19/2016 6:12 PM, Tan, Jianfeng wrote:

> Hi Pankaj,
>

Hi Jianfeng,

Sorry for delayed reply.
>>> Can we hide queues inside struct vswitch_port? I mean:
>>> For VMDQ switch, treat (port_id, queue_id) as a vswitch_port, so far
>>> you've already stored "struct vhost_dev *" into vswitch_port.priv when
>>> it's a virtual port, how about store queue_id into vswitch_port.priv
>>> when it's a physical port.
>>> For arp_learning switch, make (port_id, all_enabled_queues) as a
>>> vswitch_port.
>>> Summarize above two: we treat (port_id, all_enabled_queues[]) as a
>>> vswitch_port.
>>>
>>> How about it?
>>
>> Thanks for wonderful suggestion! Yes it should be possible, instead of
>> having one vs_port for physical device we could create one vs_port for
>> each phys device + queue_id combination.
>>
>> Then we can bind the vs_ports to the cores, and do away with binding
>> vdevs to the cores.
>>
>> In order to add extra vs_ports (port + queue_id) there are two methods:
>>
>> 1. vhost/main.c (or vswitch_common.c) queries the underlying switch
>> about how many rx queues to support thourgh a new accessor. VMDQ will
>> return the number of vmdq queues, in this case. After getting the
>> number of rx queues, vhost/main.c creates the extra ports i.e one
>> vs_port for each physical port and queue id combination.
>>
>> 2. Second method is that the switch implementation for example vmdq.c
>> create the extra ports (port + queue_id) as part of vs_port->add_port
>> operation for the physical port.
>>
>> Which method do you think we should follow? I think #1 should be done
>> so that same logic can be used for other switches also.
>
> Although VMDQs are created at initialization, we will not connect all of
> them to switch until a virtio device is added. So how about creating
> extra vs_ports (port + queue_id) as part of vmdq_add_port_virtio()?
>

Yes we can do that, it is similar to #2 which is proposed above. But I 
am little bit confused regarding this approach that we were planning to 
take, the approach of considering port_id + queue_id as one vswitch port 
because Yliu mentioned in one of his comments that we should not follow 
this approach and keep the concept of port simple.

So which approach do we take now? are we fine with keeping vswitch port 
simple and keeping some intelligence regarding vhost devices in the main 
(main.c, the generic code).

> Another thing, why we need the accessor, port_start()? For physical
> ports, it's initialized and started at port_init(); for virtual ports,
> no need to get started, right?
>

Intention of port_start was to have some control when the port starts 
doing tx/rx. For physical port it translates to rte_eth_dev_start(), and 
for vhost_port it does nothing as you said. Please note that in order to 
support this i've moved rte_eth_dev_start() from port_init() to 
vs_port->port_start.


Thanks,
Pankaj

Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Yuanhan Liu]

Firstly, sorry for being late on this discussion: I just got a chance
to follow what you guys were talking about.

On Tue, Sep 13, 2016 at 02:51:31PM +0800, Tan, Jianfeng wrote:
> >(2) we'd better not differentiate phys device and virtual

Agreed.

> >>device in generic framework (it's just an attribute of vswitch_port.
> >>
> >>What do you think?
> >
> >I agree with your thought that given the current API in this patchset we
> >should aim for making switch_worker agnostic of the port type. Ideally it
> >should look something like this:
> >
> >switch_worker() {
> >
> >        rx_port mask = VSWITCH_PTYPE_PHYS | VSWITCH_PTYPE_PHYS;
> >
> >        rx_port = vs_sched_rx_port(vswit_dev_g, rx_port_mask, core_id)
> >        rx_q = rx_port->get_rxq(vs_port, vdev, code_id);
> >        rx_port->do_rx(rx_port, rxq, NULL, pktss, MAX_PKT_BURST);
> 
> Can we hide queues inside struct vswitch_port? I mean:
> For VMDQ switch, treat (port_id, queue_id) as a vswitch_port, so far you've
> already stored "struct vhost_dev *" into vswitch_port.priv when it's a
> virtual port, how about store queue_id into vswitch_port.priv when it's a
> physical port.

Well, note that vhost-user also supports multiple queue; it's just
haven't been enabled yet. So, storing "vdev" for virtio port and
"queue_id" for phys port doesn't make too much sense.

> For arp_learning switch, make (port_id, all_enabled_queues) as a
> vswitch_port.
> Summarize above two: we treat (port_id, all_enabled_queues[]) as a
> vswitch_port.
> 
> How about it?

Sorry, I don't quite like the idea. It's weird to use "vswitch_port + queue_id"
combination to represent a port. A vswitch_port should be just a port: let's
keep the logic that simple.

	--yliu

Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Pankaj Chauhan]

On 9/19/2016 8:13 PM, Yuanhan Liu wrote:
> Firstly, sorry for being late on this discussion: I just got a chance
> to follow what you guys were talking about.
>
> On Tue, Sep 13, 2016 at 02:51:31PM +0800, Tan, Jianfeng wrote:
>>> (2) we'd better not differentiate phys device and virtual
>
> Agreed.
>
>>>> device in generic framework (it's just an attribute of vswitch_port.
>>>>
>>>> What do you think?
>>>
>>> I agree with your thought that given the current API in this patchset we
>>> should aim for making switch_worker agnostic of the port type. Ideally it
>>> should look something like this:
>>>
>>> switch_worker() {
>>>
>>>        rx_port mask = VSWITCH_PTYPE_PHYS | VSWITCH_PTYPE_PHYS;
>>>
>>>        rx_port = vs_sched_rx_port(vswit_dev_g, rx_port_mask, core_id)
>>>        rx_q = rx_port->get_rxq(vs_port, vdev, code_id);
>>>        rx_port->do_rx(rx_port, rxq, NULL, pktss, MAX_PKT_BURST);
>>
>> Can we hide queues inside struct vswitch_port? I mean:
>> For VMDQ switch, treat (port_id, queue_id) as a vswitch_port, so far you've
>> already stored "struct vhost_dev *" into vswitch_port.priv when it's a
>> virtual port, how about store queue_id into vswitch_port.priv when it's a
>> physical port.
>
> Well, note that vhost-user also supports multiple queue; it's just
> haven't been enabled yet. So, storing "vdev" for virtio port and
> "queue_id" for phys port doesn't make too much sense.
>
>> For arp_learning switch, make (port_id, all_enabled_queues) as a
>> vswitch_port.
>> Summarize above two: we treat (port_id, all_enabled_queues[]) as a
>> vswitch_port.
>>
>> How about it?
>
> Sorry, I don't quite like the idea. It's weird to use "vswitch_port + queue_id"
> combination to represent a port. A vswitch_port should be just a port: let's
> keep the logic that simple.
>

We wanted to take that approach to make vhost/main.c agnostic port type 
and have common code for rx/tx processing. The current version of 
patchset (v2) takes care of multiqueue, as it calls 
vs_port->get_txq/get_rxq to get the queue on which rx/tx has to be 
performed. This way the underlying switch can decide the queue based on 
core_id and vs_port.

But in the v2 patchset we still bind vhost_dev to the cores, and pass it 
to vs_port->get_rxq() to get the rx_queue corresponding to vhost_dev. 
Jianfeng had suggested to remove vhost_dev to core binding, and bind 
vs_port to the cores. Creating one vswitch_port for a physical port + 
queue_id was a step in that direction, thus creating very generic code 
in vhost/main.c.

YLiu/Jianfeng,

Please suggest what approach we should take here? Should we keep the 
logic of binding vhost_dev to core (as in V2 patchset), thus leaving 
some intelligence about vhost_dev in vhost/main.c.

Or What other options do you suggest if we want to achieve port type 
agnostic vhost/main.c

Thanks,
Pankaj

Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Yuanhan Liu]

On Tue, Sep 20, 2016 at 02:28:17PM +0530, Pankaj Chauhan wrote:
> On 9/19/2016 8:13 PM, Yuanhan Liu wrote:
> >Firstly, sorry for being late on this discussion: I just got a chance
> >to follow what you guys were talking about.
> >
> >On Tue, Sep 13, 2016 at 02:51:31PM +0800, Tan, Jianfeng wrote:
> >>>(2) we'd better not differentiate phys device and virtual
> >
> >Agreed.
> >
> >>>>device in generic framework (it's just an attribute of vswitch_port.
> >>>>
> >>>>What do you think?
> >>>
> >>>I agree with your thought that given the current API in this patchset we
> >>>should aim for making switch_worker agnostic of the port type. Ideally it
> >>>should look something like this:
> >>>
> >>>switch_worker() {
> >>>
> >>>       rx_port mask = VSWITCH_PTYPE_PHYS | VSWITCH_PTYPE_PHYS;
> >>>
> >>>       rx_port = vs_sched_rx_port(vswit_dev_g, rx_port_mask, core_id)
> >>>       rx_q = rx_port->get_rxq(vs_port, vdev, code_id);
> >>>       rx_port->do_rx(rx_port, rxq, NULL, pktss, MAX_PKT_BURST);
> >>
> >>Can we hide queues inside struct vswitch_port? I mean:
> >>For VMDQ switch, treat (port_id, queue_id) as a vswitch_port, so far you've
> >>already stored "struct vhost_dev *" into vswitch_port.priv when it's a
> >>virtual port, how about store queue_id into vswitch_port.priv when it's a
> >>physical port.
> >
> >Well, note that vhost-user also supports multiple queue; it's just
> >haven't been enabled yet. So, storing "vdev" for virtio port and
> >"queue_id" for phys port doesn't make too much sense.
> >
> >>For arp_learning switch, make (port_id, all_enabled_queues) as a
> >>vswitch_port.
> >>Summarize above two: we treat (port_id, all_enabled_queues[]) as a
> >>vswitch_port.
> >>
> >>How about it?
> >
> >Sorry, I don't quite like the idea. It's weird to use "vswitch_port + queue_id"
> >combination to represent a port. A vswitch_port should be just a port: let's
> >keep the logic that simple.
> >
> 
> We wanted to take that approach to make vhost/main.c agnostic port type and
> have common code for rx/tx processing. The current version of patchset (v2)
> takes care of multiqueue, as it calls vs_port->get_txq/get_rxq to get the
> queue on which rx/tx has to be performed. This way the underlying switch can
> decide the queue based on core_id and vs_port.
> 
> But in the v2 patchset we still bind vhost_dev to the cores, and pass it to
> vs_port->get_rxq() to get the rx_queue corresponding to vhost_dev. Jianfeng
> had suggested to remove vhost_dev to core binding, and bind vs_port to the
> cores. Creating one vswitch_port for a physical port + queue_id was a step
> in that direction, thus creating very generic code in vhost/main.c.
> 
> YLiu/Jianfeng,
> 
> Please suggest what approach we should take here? Should we keep the logic
> of binding vhost_dev to core (as in V2 patchset), thus leaving some
> intelligence about vhost_dev in vhost/main.c.
> 
> Or What other options do you suggest if we want to achieve port type
> agnostic vhost/main.c

Hi Pankaj,

Again, apologize for late response: you see I was busy ;) Besides, I
need some time to think about it.

Generally, I think your ideal proposal looks good to me (well, I don't
see the need of port mask):

    switch_worker() {
           rx_port = vs_sched_rx_port(vswit_dev_g, core_id)
           rx_q = rx_port->get_rxq(vs_port, vdev, code_id);
           rx_port->do_rx(rx_port, rxq, NULL, pktss, MAX_PKT_BURST);

           vs_lookup_n_fwd(rx_port, pkts, rx_count, rxq);
    }

The issue is, as you stated, VMDq it's bit tricky to handle. How about
the following proposal then?

We don't have to register the nic queues while VMDq is used, since a
phys queue is bond to a virtio queue in this mode. That means only
virtio queues will be scheduled.

The virtio do_rx might look like below then:

    vmdq_rx() {
            rte_eth_rx_burst(port, queue_bond_to_this_virtio_queue, ...);
            rte_vhost_enqueue_burst(...) if any;
            
            rte_vhost_dequeue_burst(...);
    }

	--yliu

Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Pankaj Chauhan]

On 9/26/2016 9:26 AM, Yuanhan Liu wrote:
> On Tue, Sep 20, 2016 at 02:28:17PM +0530, Pankaj Chauhan wrote:
>> On 9/19/2016 8:13 PM, Yuanhan Liu wrote:
>>> Firstly, sorry for being late on this discussion: I just got a chance
>>> to follow what you guys were talking about.
>>>
>>> On Tue, Sep 13, 2016 at 02:51:31PM +0800, Tan, Jianfeng wrote:
>>>>> (2) we'd better not differentiate phys device and virtual
>>>
>>> Agreed.
>>>
>>>>>> device in generic framework (it's just an attribute of vswitch_port.
>>>>>>
>>>>>> What do you think?
>>>>>
>>>>> I agree with your thought that given the current API in this patchset we
>>>>> should aim for making switch_worker agnostic of the port type. Ideally it
>>>>> should look something like this:
>>>>>
>>>>> switch_worker() {
>>>>>
>>>>>       rx_port mask = VSWITCH_PTYPE_PHYS | VSWITCH_PTYPE_PHYS;
>>>>>
>>>>>       rx_port = vs_sched_rx_port(vswit_dev_g, rx_port_mask, core_id)
>>>>>       rx_q = rx_port->get_rxq(vs_port, vdev, code_id);
>>>>>       rx_port->do_rx(rx_port, rxq, NULL, pktss, MAX_PKT_BURST);
>>>>
>>>> Can we hide queues inside struct vswitch_port? I mean:
>>>> For VMDQ switch, treat (port_id, queue_id) as a vswitch_port, so far you've
>>>> already stored "struct vhost_dev *" into vswitch_port.priv when it's a
>>>> virtual port, how about store queue_id into vswitch_port.priv when it's a
>>>> physical port.
>>>
>>> Well, note that vhost-user also supports multiple queue; it's just
>>> haven't been enabled yet. So, storing "vdev" for virtio port and
>>> "queue_id" for phys port doesn't make too much sense.
>>>
>>>> For arp_learning switch, make (port_id, all_enabled_queues) as a
>>>> vswitch_port.
>>>> Summarize above two: we treat (port_id, all_enabled_queues[]) as a
>>>> vswitch_port.
>>>>
>>>> How about it?
>>>
>>> Sorry, I don't quite like the idea. It's weird to use "vswitch_port + queue_id"
>>> combination to represent a port. A vswitch_port should be just a port: let's
>>> keep the logic that simple.
>>>
>>
>> We wanted to take that approach to make vhost/main.c agnostic port type and
>> have common code for rx/tx processing. The current version of patchset (v2)
>> takes care of multiqueue, as it calls vs_port->get_txq/get_rxq to get the
>> queue on which rx/tx has to be performed. This way the underlying switch can
>> decide the queue based on core_id and vs_port.
>>
>> But in the v2 patchset we still bind vhost_dev to the cores, and pass it to
>> vs_port->get_rxq() to get the rx_queue corresponding to vhost_dev. Jianfeng
>> had suggested to remove vhost_dev to core binding, and bind vs_port to the
>> cores. Creating one vswitch_port for a physical port + queue_id was a step
>> in that direction, thus creating very generic code in vhost/main.c.
>>
>> YLiu/Jianfeng,
>>
>> Please suggest what approach we should take here? Should we keep the logic
>> of binding vhost_dev to core (as in V2 patchset), thus leaving some
>> intelligence about vhost_dev in vhost/main.c.
>>
>> Or What other options do you suggest if we want to achieve port type
>> agnostic vhost/main.c
>
> Hi Pankaj,
>
> Again, apologize for late response: you see I was busy ;) Besides, I
> need some time to think about it.
>

Hi YLiu,

No issues with delayed response :)

> Generally, I think your ideal proposal looks good to me (well, I don't
> see the need of port mask):

The idea of port mask was to give ability to the caller to choose which 
type of port to do rx from, Physical port or vhost port.
>
>     switch_worker() {
>            rx_port = vs_sched_rx_port(vswit_dev_g, core_id)
>            rx_q = rx_port->get_rxq(vs_port, vdev, code_id);
>            rx_port->do_rx(rx_port, rxq, NULL, pktss, MAX_PKT_BURST);
>
>            vs_lookup_n_fwd(rx_port, pkts, rx_count, rxq);
>     }
>
> The issue is, as you stated, VMDq it's bit tricky to handle. How about
> the following proposal then?
>
> We don't have to register the nic queues while VMDq is used, since a
> phys queue is bond to a virtio queue in this mode. That means only
> virtio queues will be scheduled.
>
> The virtio do_rx might look like below then:
>
>     vmdq_rx() {
>             rte_eth_rx_burst(port, queue_bond_to_this_virtio_queue, ...);
>             rte_vhost_enqueue_burst(...) if any;
>
>             rte_vhost_dequeue_burst(...);
>     }
>

Okay so in that case, we won't do any rte_eth_rx_burst() when 
physical_port->do_rx is called, Correct?. If yes then in vmdq.c we'll 
overwrite vs_port->do_rx of physical port with a vmdq_do_rx_phys() which 
does nothing. Or we can even have an option that vmdq.c doesn't return 
the physical port when vs_sched_rx_port() is called, i think this later 
option is better to save some CPU cycles.

I think it is possible but i would prefer to overwrite vs_port->do_rx() 
for vmdq (in vmdq.c) with the implementation that you suggested. The 
framework provides this option, i.e the switch implementation can 
overwrite the vs_port->do_rx/do_tx if required to handle any special 
cases for example the case of vmdq <> vdev boding.

Thanks,
Pankaj
> 	--yliu
>

Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Yuanhan Liu]

On Tue, Sep 27, 2016 at 05:05:41PM +0530, Pankaj Chauhan wrote:
> >Hi Pankaj,
> >
> >Again, apologize for late response: you see I was busy ;) Besides, I
> >need some time to think about it.
> >
> 
> Hi YLiu,
> 
> No issues with delayed response :)

Thanks!

> 
> >Generally, I think your ideal proposal looks good to me (well, I don't
> >see the need of port mask):
> 
> The idea of port mask was to give ability to the caller to choose which type
> of port to do rx from, Physical port or vhost port.

What's the need of it? If you register a port, don't you need pull
packets from it?

And you don't have to distinguish whether it's a physical port or vhost
port or not. If a port is registered, just pull it. Simple, right?

> >
> >    switch_worker() {
> >           rx_port = vs_sched_rx_port(vswit_dev_g, core_id)
> >           rx_q = rx_port->get_rxq(vs_port, vdev, code_id);
> >           rx_port->do_rx(rx_port, rxq, NULL, pktss, MAX_PKT_BURST);
> >
> >           vs_lookup_n_fwd(rx_port, pkts, rx_count, rxq);
> >    }
> >
> >The issue is, as you stated, VMDq it's bit tricky to handle. How about
> >the following proposal then?
> >
> >We don't have to register the nic queues while VMDq is used, since a
> >phys queue is bond to a virtio queue in this mode. That means only
> >virtio queues will be scheduled.
> >
> >The virtio do_rx might look like below then:
> >
> >    vmdq_rx() {
> >            rte_eth_rx_burst(port, queue_bond_to_this_virtio_queue, ...);
> >            rte_vhost_enqueue_burst(...) if any;
> >
> >            rte_vhost_dequeue_burst(...);
> >    }
> >
> 
> Okay so in that case, we won't do any rte_eth_rx_burst() when
> physical_port->do_rx is called, Correct?.

The physical port do_rx will not be called at all, since it will
not registered. In the VMDq case, only virtio port will be registered
(by your vs_add_port function).

> If yes then in vmdq.c we'll
> overwrite vs_port->do_rx of physical port with a vmdq_do_rx_phys() which
> does nothing. Or we can even have an option that vmdq.c doesn't return the
> physical port when vs_sched_rx_port() is called,

As stated, if you don't register it, then vs_sched_rx_port will return no
physical port.

	--yliu

> i think this later option
> is better to save some CPU cycles.
> 
> I think it is possible but i would prefer to overwrite vs_port->do_rx() for
> vmdq (in vmdq.c) with the implementation that you suggested. The framework
> provides this option, i.e the switch implementation can overwrite the
> vs_port->do_rx/do_tx if required to handle any special cases for example the
> case of vmdq <> vdev boding.
> 
> Thanks,
> Pankaj
> >	--yliu
> >
> 

Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Yuanhan Liu]

On Mon, Sep 05, 2016 at 04:24:29PM +0530, Pankaj Chauhan wrote:
> Introduce support for a generic framework for handling of switching between
> physical and vhost devices. The vswitch framework introduces the following
> concept:
> 
> 1. vswitch_dev: Vswitch device is a logical switch which can have physical and
> virtio devices. The devices are operated/used using standard rte_eth API for
> physical devices and lib_vhost API for vhost devices, PMD API is not used
> for vhost devices.
> 
> 2. vswitch_port: Any physical or virtio device that is added to vswitch. The
> port can have its own tx/rx functions for doing data transfer, which are exposed
> to the framework using generic function pointers (vs_port->do_tx/do_rx). This way
> the generic code can do tx/rx without understanding the type of device (Physical or
> virtio). Similarly each port has its own functions to select tx/rx queues. The framework
> provides default tx/rx queue selection functions to the port when port is added
> (for both physical and vhost devices). But the framework allows the switch implementation
> to override the queue selection functions (vs_port->get_txq/rxq) if required.
> 
> 3. vswitch_ops: The ops is set of function pointers which are used to do operations
> like learning, unlearning, add/delete port, lookup_and_forward. The user of vswitch
> framework (vhost/main.[c,h])uses these function pointers to perform above mentioned
> operations, thus it remains agnostic of the underlying implementation.
> 
> Different switching logics can implement their vswitch_device and vswitch_ops, and
> register with the framework. This framework makes vhost-switch application scalable
> in terms of:
> 
> 1. Different switching logics (one of them is vmdq, vhost/vmdq.[c,h]
> 2. Number of ports.
> 3. Policies of selecting ports for rx and tx.
> 
> Signed-off-by: Pankaj Chauhan <pankaj.chauhan@nxp.com>

Hi,

FYI, my testrobot caught some errors when this patch is applied.
(And sorry for the late review; hopefully I can make it next week).

        --yliu

---
examples/vhost/main.c: In function 'virtio_xmit':
examples/vhost/main.c:815:41: error: 'struct vhost_dev' has no member named 'vs_port'
  struct vswitch_port *vs_port = dst_vdev->vs_port;
                                         ^
examples/vhost/main.c:822:15: error: dereferencing pointer to incomplete type 'struct vswitch_port'
  txq = vs_port->get_txq(vs_port, rte_lcore_id());
               ^
examples/vhost/main.c: In function 'do_drain_mbuf_table':
examples/vhost/main.c:952:12: error: implicit declaration of function 'vs_sched_tx_port' [-Werror=implicit-function-declaration]
  tx_port = vs_sched_tx_port(vswitch_dev_g, VSWITCH_PTYPE_PHYS,
            ^
examples/vhost/main.c:952:2: error: nested extern declaration of 'vs_sched_tx_port' [-Werror=nested-externs]
  tx_port = vs_sched_tx_port(vswitch_dev_g, VSWITCH_PTYPE_PHYS,
  ^
examples/vhost/main.c:952:29: error: 'vswitch_dev_g' undeclared (first use in this function)
  tx_port = vs_sched_tx_port(vswitch_dev_g, VSWITCH_PTYPE_PHYS,
                             ^
examples/vhost/main.c:952:29: note: each undeclared identifier is reported only once for each function it appears in
examples/vhost/main.c:952:44: error: 'VSWITCH_PTYPE_PHYS' undeclared (first use in this function)
  tx_port = vs_sched_tx_port(vswitch_dev_g, VSWITCH_PTYPE_PHYS,
                                            ^
examples/vhost/main.c:958:18: error: dereferencing pointer to incomplete type 'struct vswitch_port'
   count = tx_port->do_tx(tx_port, tx_q->txq_id, NULL, tx_q->m_table,
                  ^
examples/vhost/main.c:955:6: error: label 'out' used but not defined
      goto out;
      ^
examples/vhost/main.c: In function 'drain_eth_rx':
examples/vhost/main.c:1096:12: error: implicit declaration of function 'vs_sched_rx_port' [-Werror=implicit-function-declaration]
  rx_port = vs_sched_rx_port(vswitch_dev_g, VSWITCH_PTYPE_PHYS,
            ^
examples/vhost/main.c:1096:2: error: nested extern declaration of 'vs_sched_rx_port' [-Werror=nested-externs]
  rx_port = vs_sched_rx_port(vswitch_dev_g, VSWITCH_PTYPE_PHYS,
  ^
examples/vhost/main.c:1096:29: error: 'vswitch_dev_g' undeclared (first use in this function)
  rx_port = vs_sched_rx_port(vswitch_dev_g, VSWITCH_PTYPE_PHYS,
                             ^
examples/vhost/main.c:1096:44: error: 'VSWITCH_PTYPE_PHYS' undeclared (first use in this function)
  rx_port = vs_sched_rx_port(vswitch_dev_g, VSWITCH_PTYPE_PHYS,
                                            ^
examples/vhost/main.c:1105:15: error: dereferencing pointer to incomplete type 'struct vswitch_port'
  rxq = rx_port->get_rxq(rx_port, vdev, core_id);
               ^
examples/vhost/main.c:1099:6: error: label 'out' used but not defined
      goto out;
      ^
examples/vhost/main.c: In function 'drain_virtio_tx':
examples/vhost/main.c:1145:37: error: 'struct vhost_dev' has no member named 'vs_port'
  struct vswitch_port *vs_port = vdev->vs_port;
                                     ^
examples/vhost/main.c:1154:15: error: dereferencing pointer to incomplete type 'struct vswitch_port'
  rxq = vs_port->get_rxq(vs_port, NULL, core_id);
               ^
examples/vhost/main.c:1162:23: error: implicit declaration of function 'vs_learn_port' [-Werror=implicit-function-declaration]
   if (vdev->remove || vs_learn_port(vs_port, pkts, count))
                       ^
examples/vhost/main.c:1162:3: error: nested extern declaration of 'vs_learn_port' [-Werror=nested-externs]
   if (vdev->remove || vs_learn_port(vs_port, pkts, count))
   ^
examples/vhost/main.c:1166:2: error: implicit declaration of function 'vs_lookup_n_fwd' [-Werror=implicit-function-declaration]
  vs_lookup_n_fwd(vs_port, pkts, count, VIRTIO_RXQ);
  ^
examples/vhost/main.c:1166:2: error: nested extern declaration of 'vs_lookup_n_fwd' [-Werror=nested-externs]
examples/vhost/main.c: In function 'new_device':
examples/vhost/main.c:1310:19: error: implicit declaration of function 'vs_get_max_vdevs' [-Werror=implicit-function-declaration]
  device_num_min = vs_get_max_vdevs(vswitch_dev_g);
                   ^
examples/vhost/main.c:1310:2: error: nested extern declaration of 'vs_get_max_vdevs' [-Werror=nested-externs]
  device_num_min = vs_get_max_vdevs(vswitch_dev_g);
  ^
examples/vhost/main.c:1310:36: error: 'vswitch_dev_g' undeclared (first use in this function)
  device_num_min = vs_get_max_vdevs(vswitch_dev_g);
                                    ^
examples/vhost/main.c:1313:2: error: 'vs_port' undeclared (first use in this function)
  vs_port = vs_add_port(vswitch_dev_g, vid, VSWITCH_PTYPE_VIRTIO, vdev);
  ^
examples/vhost/main.c:1313:12: error: implicit declaration of function 'vs_add_port' [-Werror=implicit-function-declaration]
  vs_port = vs_add_port(vswitch_dev_g, vid, VSWITCH_PTYPE_VIRTIO, vdev);
            ^
examples/vhost/main.c:1313:2: error: nested extern declaration of 'vs_add_port' [-Werror=nested-externs]
  vs_port = vs_add_port(vswitch_dev_g, vid, VSWITCH_PTYPE_VIRTIO, vdev);
  ^
examples/vhost/main.c:1313:44: error: 'VSWITCH_PTYPE_VIRTIO' undeclared (first use in this function)
  vs_port = vs_add_port(vswitch_dev_g, vid, VSWITCH_PTYPE_VIRTIO, vdev);
                                            ^
examples/vhost/main.c:1319:6: error: 'struct vhost_dev' has no member named 'vs_port'
  vdev->vs_port = vs_port;
      ^
examples/vhost/main.c:1347:2: error: implicit declaration of function 'vs_port_start' [-Werror=implicit-function-declaration]
  vs_port_start(vs_port);
  ^
examples/vhost/main.c:1347:2: error: nested extern declaration of 'vs_port_start' [-Werror=nested-externs]
examples/vhost/main.c: In function 'get_vswitch_conf_flags':
examples/vhost/main.c:1479:20: error: 'VS_CNF_FLG_VM2VM_HARDWARE' undeclared (first use in this function)
   vs_conf_flags |= VS_CNF_FLG_VM2VM_HARDWARE;
                    ^
examples/vhost/main.c:1482:20: error: 'VS_CNF_FLG_VM2VM_SOFTWARE' undeclared (first use in this function)
   vs_conf_flags |= VS_CNF_FLG_VM2VM_SOFTWARE;
                    ^
examples/vhost/main.c:1485:20: error: 'VS_CNF_FLG_PROMISCOUS_EN' undeclared (first use in this function)
   vs_conf_flags |= VS_CNF_FLG_PROMISCOUS_EN;
                    ^
examples/vhost/main.c:1487:6: error: 'jumbo_frame_en' undeclared (first use in this function)
  if (jumbo_frame_en)
      ^
examples/vhost/main.c:1488:20: error: 'VS_CNF_FLG_JUMBO_EN' undeclared (first use in this function)
   vs_conf_flags |= VS_CNF_FLG_JUMBO_EN;
                    ^
examples/vhost/main.c:1491:20: error: 'VS_CNF_FLG_STATS_EN' undeclared (first use in this function)
   vs_conf_flags |= VS_CNF_FLG_STATS_EN;
                    ^
examples/vhost/main.c:1494:20: error: 'VS_CNF_FLG_VLAN_STRIP_EN' undeclared (first use in this function)
   vs_conf_flags |= VS_CNF_FLG_VLAN_STRIP_EN;
                    ^
examples/vhost/main.c: At top level:
examples/vhost/main.c:726:1: error: 'link_vmdq' defined but not used [-Werror=unused-function]
 link_vmdq(struct vhost_dev *vdev, struct rte_mbuf *m)
 ^
examples/vhost/main.c:1474:17: error: 'get_vswitch_conf_flags' defined but not used [-Werror=unused-function]
 static uint64_t get_vswitch_conf_flags(void)
                 ^
cc1: all warnings being treated as errors
make[1]: *** [main.o] Error 1
make: *** [all] Error 2
error: build examples/vhost failed
error: build failed

Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Pankaj Chauhan]

On 9/11/2016 5:51 PM, Yuanhan Liu wrote:
> On Mon, Sep 05, 2016 at 04:24:29PM +0530, Pankaj Chauhan wrote:
>> Introduce support for a generic framework for handling of switching between
>> physical and vhost devices. The vswitch framework introduces the following
>> concept:
>>
>> 1. vswitch_dev: Vswitch device is a logical switch which can have physical and
>> virtio devices. The devices are operated/used using standard rte_eth API for
>> physical devices and lib_vhost API for vhost devices, PMD API is not used
>> for vhost devices.
>>
>> 2. vswitch_port: Any physical or virtio device that is added to vswitch. The
>> port can have its own tx/rx functions for doing data transfer, which are exposed
>> to the framework using generic function pointers (vs_port->do_tx/do_rx). This way
>> the generic code can do tx/rx without understanding the type of device (Physical or
>> virtio). Similarly each port has its own functions to select tx/rx queues. The framework
>> provides default tx/rx queue selection functions to the port when port is added
>> (for both physical and vhost devices). But the framework allows the switch implementation
>> to override the queue selection functions (vs_port->get_txq/rxq) if required.
>>
>> 3. vswitch_ops: The ops is set of function pointers which are used to do operations
>> like learning, unlearning, add/delete port, lookup_and_forward. The user of vswitch
>> framework (vhost/main.[c,h])uses these function pointers to perform above mentioned
>> operations, thus it remains agnostic of the underlying implementation.
>>
>> Different switching logics can implement their vswitch_device and vswitch_ops, and
>> register with the framework. This framework makes vhost-switch application scalable
>> in terms of:
>>
>> 1. Different switching logics (one of them is vmdq, vhost/vmdq.[c,h]
>> 2. Number of ports.
>> 3. Policies of selecting ports for rx and tx.
>>
>> Signed-off-by: Pankaj Chauhan <pankaj.chauhan@nxp.com>
>
> Hi,
>
> FYI, my testrobot caught some errors when this patch is applied.
> (And sorry for the late review; hopefully I can make it next week).
>
>         --yliu

Hi YLiu,

Thanks for the review. I am sorry it was my mistake in this patchset, 
the header defining few structures and macros used in patch 0001 are 
defined in patch 0003. Because of this 0001 was not individually 
compilable but all three are compliable as set, it was mistake on my 
part , will fix it and will take care of it in further patchsets.

Do you want me to send another version for review, or i can fix it
in the next version i send after review of v2 is complete?

Thanks,
Pankaj
>

Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Yuanhan Liu]

Besides the VMDq proposal, I got few more comments for you.

On Mon, Sep 05, 2016 at 04:24:29PM +0530, Pankaj Chauhan wrote:
> Introduce support for a generic framework for handling of switching between
> physical and vhost devices. The vswitch framework introduces the following
> concept:
> 
> 1. vswitch_dev: Vswitch device

It looks a bit confusing to me, to claim it as a "device": it's neither a
physical nic device nor a virtio net device. Something like "vswitch_unit",
or even "vswitch" is better and enough.

> Signed-off-by: Pankaj Chauhan <pankaj.chauhan@nxp.com>
> ---
>  examples/vhost/Makefile         |   2 +-
>  examples/vhost/main.c           | 128 +++++++++--
>  examples/vhost/vswitch_common.c | 499 ++++++++++++++++++++++++++++++++++++++++
>  examples/vhost/vswitch_common.h | 186 +++++++++++++++
>  examples/vhost/vswitch_txrx.c   |  97 ++++++++
>  examples/vhost/vswitch_txrx.h   |  71 ++++++

Seems that you forgot to include the file to implment all those ops for
"switch" vswitch mode? I mean, I just see a vs_lookup_n_fwd implmentation
of VMDq.

> @@ -1241,7 +1296,7 @@ static int
>  new_device(int vid)
>  {
>  	int lcore, core_add = 0;
> -	uint32_t device_num_min = num_devices;
> +	uint32_t device_num_min;
>  	struct vhost_dev *vdev;
>  
>  	vdev = rte_zmalloc("vhost device", sizeof(*vdev), RTE_CACHE_LINE_SIZE);
> @@ -1252,6 +1307,16 @@ new_device(int vid)
>  		return -1;
>  	}
>  	vdev->vid = vid;
> +	device_num_min = vs_get_max_vdevs(vswitch_dev_g);
> +	RTE_LOG(INFO, VHOST_PORT, "max virtio devices %d\n", device_num_min);
> +
> +	vs_port = vs_add_port(vswitch_dev_g, vid, VSWITCH_PTYPE_VIRTIO, vdev);

Note that "vid" does not equal "port". They are two different counters
and both start from 0. That means, you will get unexpected results from
following piece of code ---->

> +struct vswitch_port *vs_add_port(struct vswitch_dev *vs_dev, int port_id,
> +		enum vswitch_port_type type, void *priv)
> +{
> +	int rc = 0;
> +	struct vswitch_port *vs_port = NULL;
> +	struct vswitch_ops *vs_ops = vs_dev->ops;
> +
> +	vs_port = vs_get_free_port(vs_dev);
> +	if (!vs_port) {
> +		RTE_LOG(DEBUG, VHOST_CONFIG, "Failed get free port in \
> +			vswitch %s\n", vs_dev->name);
> +		rc = -EBUSY;
> +		goto out;
> +	}
> +
> +	vs_port->port_id = port_id;
> +	vs_port->type = type;
> +	vs_port->priv = priv;
> +
> +	/* Initialize default port operations. It should be noted that
> +	 * The switch ops->add_port can replace them with switch specefic
> +	 * operations if required. This gives us more flexibility in switch
> +	 * implementations.
> +	 */
> +
> +	switch (type) {
> +	case VSWITCH_PTYPE_PHYS:
> +	       vs_port->do_tx = vs_do_tx_phys_port;
> +	       vs_port->do_rx = vs_do_rx_phys_port;
> +	       vs_port->get_txq = vs_get_txq_phys_port;
> +	       vs_port->get_rxq = vs_get_rxq_phys_port;
> +	       break;
> +	case VSWITCH_PTYPE_VIRTIO:
> +	       vs_port->do_tx = vs_do_tx_virtio_port;
> +	       vs_port->do_rx = vs_do_rx_virtio_port;
> +	       vs_port->get_txq = vs_get_txq_virtio_port;
> +	       vs_port->get_rxq = vs_get_rxq_virtio_port;
> +	       break;
> +	default:
> +		RTE_LOG(DEBUG, VHOST_CONFIG, "Invalid port [id %d, type %d]",
> +				port_id, type);
> +	       rc = -EINVAL;
> +	       goto out;
> +	}
> +
> +	if (vs_ops->add_port)
> +		rc = vs_ops->add_port(vs_port);
> +
> +	if (rc)
> +		goto out;
> +
> +	vs_port->state = VSWITCH_PSTATE_ADDED;
> +
> +	rte_eth_macaddr_get(vs_port->port_id, &vs_port->mac_addr);

<--- here.

	--yliu

> +	RTE_LOG(INFO, VHOST_PORT, "Port %u MAC: %02"PRIx8" %02"PRIx8" %02"PRIx8
> +			" %02"PRIx8" %02"PRIx8" %02"PRIx8"\n",
> +			(unsigned)port_id,
> +			vs_port->mac_addr.addr_bytes[0],
> +			vs_port->mac_addr.addr_bytes[1],
> +			vs_port->mac_addr.addr_bytes[2],
> +			vs_port->mac_addr.addr_bytes[3],
> +			vs_port->mac_addr.addr_bytes[4],
> +			vs_port->mac_addr.addr_bytes[5]);
> +
> +	RTE_LOG(DEBUG, VHOST_CONFIG, "Added port [%d, type %d] to \
> +			vswitch %s\n", vs_port->port_id, type, vs_dev->name);
> +out:
> +	if (rc){
> +		RTE_LOG(INFO, VHOST_CONFIG, "Failed to Add port [%d, type %d] to \
> +			vswitch %s\n", port_id, type, vs_dev->name);
> +		if (vs_port)
> +			vs_free_port(vs_port);
> +		vs_port = NULL;
> +	}
> +
> +	return vs_port;
> +}

Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Pankaj Chauhan]

On 9/26/2016 9:42 AM, Yuanhan Liu wrote:
> Besides the VMDq proposal, I got few more comments for you.
>
> On Mon, Sep 05, 2016 at 04:24:29PM +0530, Pankaj Chauhan wrote:
>> Introduce support for a generic framework for handling of switching between
>> physical and vhost devices. The vswitch framework introduces the following
>> concept:
>>
>> 1. vswitch_dev: Vswitch device
>
> It looks a bit confusing to me, to claim it as a "device": it's neither a
> physical nic device nor a virtio net device. Something like "vswitch_unit",
> or even "vswitch" is better and enough.
>

Yes we can change it to 'vswitch' it suites better, i'll do that in v3.

>> Signed-off-by: Pankaj Chauhan <pankaj.chauhan@nxp.com>
>> ---
>>  examples/vhost/Makefile         |   2 +-
>>  examples/vhost/main.c           | 128 +++++++++--
>>  examples/vhost/vswitch_common.c | 499 ++++++++++++++++++++++++++++++++++++++++
>>  examples/vhost/vswitch_common.h | 186 +++++++++++++++
>>  examples/vhost/vswitch_txrx.c   |  97 ++++++++
>>  examples/vhost/vswitch_txrx.h   |  71 ++++++
>
> Seems that you forgot to include the file to implment all those ops for
> "switch" vswitch mode? I mean, I just see a vs_lookup_n_fwd implmentation
> of VMDq.
>

No i didn't forget to include the file but wanted to implement, get 
reviewed and included (hopefully :)) the implementation of following first:

1. vswitch framework
2. vmdq implementation plugged into the vswitch framework.

After above two i am planning to send the 'software switch' 
implementation in a separate patch, i hope that is fine.

>> @@ -1241,7 +1296,7 @@ static int
>>  new_device(int vid)
>>  {
>>  	int lcore, core_add = 0;
>> -	uint32_t device_num_min = num_devices;
>> +	uint32_t device_num_min;
>>  	struct vhost_dev *vdev;
>>
>>  	vdev = rte_zmalloc("vhost device", sizeof(*vdev), RTE_CACHE_LINE_SIZE);
>> @@ -1252,6 +1307,16 @@ new_device(int vid)
>>  		return -1;
>>  	}
>>  	vdev->vid = vid;
>> +	device_num_min = vs_get_max_vdevs(vswitch_dev_g);
>> +	RTE_LOG(INFO, VHOST_PORT, "max virtio devices %d\n", device_num_min);
>> +
>> +	vs_port = vs_add_port(vswitch_dev_g, vid, VSWITCH_PTYPE_VIRTIO, vdev);
>
> Note that "vid" does not equal "port". They are two different counters
> and both start from 0. That means, you will get unexpected results from
> following piece of code ---->
>
Sorry i didn't get the inconsistency completely, please help me 
understand it.

I agree both port_id and vid counters start from zero. But when we add 
these as vswitch_port we'll pass different port type 
(VSWITCH_PTYPE_VIRTIO or VSWITCH_PTYPE_PHYS). And while searching for 
any vswitch port we use vs_port->port_id && vs_port->type as the key, 
thus we'll not get confused between ports even when both have same port_id.

Can you please help me understand the inconsistency that you thought we 
may have?

Thanks,
Pankaj
>> +struct vswitch_port *vs_add_port(struct vswitch_dev *vs_dev, int port_id,
>> +		enum vswitch_port_type type, void *priv)
>> +{
>> +	int rc = 0;
>> +	struct vswitch_port *vs_port = NULL;
>> +	struct vswitch_ops *vs_ops = vs_dev->ops;
>> +
>> +	vs_port = vs_get_free_port(vs_dev);
>> +	if (!vs_port) {
>> +		RTE_LOG(DEBUG, VHOST_CONFIG, "Failed get free port in \
>> +			vswitch %s\n", vs_dev->name);
>> +		rc = -EBUSY;
>> +		goto out;
>> +	}
>> +
>> +	vs_port->port_id = port_id;
>> +	vs_port->type = type;
>> +	vs_port->priv = priv;
>> +
>> +	/* Initialize default port operations. It should be noted that
>> +	 * The switch ops->add_port can replace them with switch specefic
>> +	 * operations if required. This gives us more flexibility in switch
>> +	 * implementations.
>> +	 */
>> +
>> +	switch (type) {
>> +	case VSWITCH_PTYPE_PHYS:
>> +	       vs_port->do_tx = vs_do_tx_phys_port;
>> +	       vs_port->do_rx = vs_do_rx_phys_port;
>> +	       vs_port->get_txq = vs_get_txq_phys_port;
>> +	       vs_port->get_rxq = vs_get_rxq_phys_port;
>> +	       break;
>> +	case VSWITCH_PTYPE_VIRTIO:
>> +	       vs_port->do_tx = vs_do_tx_virtio_port;
>> +	       vs_port->do_rx = vs_do_rx_virtio_port;
>> +	       vs_port->get_txq = vs_get_txq_virtio_port;
>> +	       vs_port->get_rxq = vs_get_rxq_virtio_port;
>> +	       break;
>> +	default:
>> +		RTE_LOG(DEBUG, VHOST_CONFIG, "Invalid port [id %d, type %d]",
>> +				port_id, type);
>> +	       rc = -EINVAL;
>> +	       goto out;
>> +	}
>> +
>> +	if (vs_ops->add_port)
>> +		rc = vs_ops->add_port(vs_port);
>> +
>> +	if (rc)
>> +		goto out;
>> +
>> +	vs_port->state = VSWITCH_PSTATE_ADDED;
>> +
>> +	rte_eth_macaddr_get(vs_port->port_id, &vs_port->mac_addr);
>
> <--- here.
>
> 	--yliu
>
>> +	RTE_LOG(INFO, VHOST_PORT, "Port %u MAC: %02"PRIx8" %02"PRIx8" %02"PRIx8
>> +			" %02"PRIx8" %02"PRIx8" %02"PRIx8"\n",
>> +			(unsigned)port_id,
>> +			vs_port->mac_addr.addr_bytes[0],
>> +			vs_port->mac_addr.addr_bytes[1],
>> +			vs_port->mac_addr.addr_bytes[2],
>> +			vs_port->mac_addr.addr_bytes[3],
>> +			vs_port->mac_addr.addr_bytes[4],
>> +			vs_port->mac_addr.addr_bytes[5]);
>> +
>> +	RTE_LOG(DEBUG, VHOST_CONFIG, "Added port [%d, type %d] to \
>> +			vswitch %s\n", vs_port->port_id, type, vs_dev->name);
>> +out:
>> +	if (rc){
>> +		RTE_LOG(INFO, VHOST_CONFIG, "Failed to Add port [%d, type %d] to \
>> +			vswitch %s\n", port_id, type, vs_dev->name);
>> +		if (vs_port)
>> +			vs_free_port(vs_port);
>> +		vs_port = NULL;
>> +	}
>> +
>> +	return vs_port;
>> +}
>

Re: [dpdk-dev] [RFC][PATCH V2 1/3] examples/vhost: Add vswitch (generic switch) framework

[Yuanhan Liu]

On Tue, Sep 27, 2016 at 05:14:44PM +0530, Pankaj Chauhan wrote:
> On 9/26/2016 9:42 AM, Yuanhan Liu wrote:
> >Besides the VMDq proposal, I got few more comments for you.
> >
> >On Mon, Sep 05, 2016 at 04:24:29PM +0530, Pankaj Chauhan wrote:
> >>Introduce support for a generic framework for handling of switching between
> >>physical and vhost devices. The vswitch framework introduces the following
> >>concept:
> >>
> >>1. vswitch_dev: Vswitch device
> >
> >It looks a bit confusing to me, to claim it as a "device": it's neither a
> >physical nic device nor a virtio net device. Something like "vswitch_unit",
> >or even "vswitch" is better and enough.
> >
> 
> Yes we can change it to 'vswitch' it suites better, i'll do that in v3.
> 
> >>Signed-off-by: Pankaj Chauhan <pankaj.chauhan@nxp.com>
> >>---
> >> examples/vhost/Makefile         |   2 +-
> >> examples/vhost/main.c           | 128 +++++++++--
> >> examples/vhost/vswitch_common.c | 499 ++++++++++++++++++++++++++++++++++++++++
> >> examples/vhost/vswitch_common.h | 186 +++++++++++++++
> >> examples/vhost/vswitch_txrx.c   |  97 ++++++++
> >> examples/vhost/vswitch_txrx.h   |  71 ++++++
> >
> >Seems that you forgot to include the file to implment all those ops for
> >"switch" vswitch mode? I mean, I just see a vs_lookup_n_fwd implmentation
> >of VMDq.
> >
> 
> No i didn't forget to include the file but wanted to implement, get reviewed
> and included (hopefully :)) the implementation of following first:
> 
> 1. vswitch framework
> 2. vmdq implementation plugged into the vswitch framework.
> 
> After above two i am planning to send the 'software switch' implementation
> in a separate patch, i hope that is fine.

It's better to shipt them together, so that we could do review once.
Besides, it helps to understand your framework design.

> 
> >>@@ -1241,7 +1296,7 @@ static int
> >> new_device(int vid)
> >> {
> >> 	int lcore, core_add = 0;
> >>-	uint32_t device_num_min = num_devices;
> >>+	uint32_t device_num_min;
> >> 	struct vhost_dev *vdev;
> >>
> >> 	vdev = rte_zmalloc("vhost device", sizeof(*vdev), RTE_CACHE_LINE_SIZE);
> >>@@ -1252,6 +1307,16 @@ new_device(int vid)
> >> 		return -1;
> >> 	}
> >> 	vdev->vid = vid;
> >>+	device_num_min = vs_get_max_vdevs(vswitch_dev_g);
> >>+	RTE_LOG(INFO, VHOST_PORT, "max virtio devices %d\n", device_num_min);
> >>+
> >>+	vs_port = vs_add_port(vswitch_dev_g, vid, VSWITCH_PTYPE_VIRTIO, vdev);
> >
> >Note that "vid" does not equal "port". They are two different counters
> >and both start from 0. That means, you will get unexpected results from
> >following piece of code ---->
> >
> Sorry i didn't get the inconsistency completely, please help me understand
> it.
> 
> I agree both port_id and vid counters start from zero. But when we add these
> as vswitch_port we'll pass different port type (VSWITCH_PTYPE_VIRTIO or
> VSWITCH_PTYPE_PHYS). And while searching for any vswitch port we use
> vs_port->port_id && vs_port->type as the key, thus we'll not get confused
> between ports even when both have same port_id.
> 
> Can you please help me understand the inconsistency that you thought we may
> have?
...
> >>+struct vswitch_port *vs_add_port(struct vswitch_dev *vs_dev, int port_id,
...
> >>+	rte_eth_macaddr_get(vs_port->port_id, &vs_port->mac_addr);

Will a virtio port invoke above function in your logic?

	--yliu
> >
> ><--- here.
> >
> >	--yliu
> >
> >>+	RTE_LOG(INFO, VHOST_PORT, "Port %u MAC: %02"PRIx8" %02"PRIx8" %02"PRIx8
> >>+			" %02"PRIx8" %02"PRIx8" %02"PRIx8"\n",
> >>+			(unsigned)port_id,
> >>+			vs_port->mac_addr.addr_bytes[0],
> >>+			vs_port->mac_addr.addr_bytes[1],
> >>+			vs_port->mac_addr.addr_bytes[2],
> >>+			vs_port->mac_addr.addr_bytes[3],
> >>+			vs_port->mac_addr.addr_bytes[4],
> >>+			vs_port->mac_addr.addr_bytes[5]);
> >>+
> >>+	RTE_LOG(DEBUG, VHOST_CONFIG, "Added port [%d, type %d] to \
> >>+			vswitch %s\n", vs_port->port_id, type, vs_dev->name);
> >>+out:
> >>+	if (rc){
> >>+		RTE_LOG(INFO, VHOST_CONFIG, "Failed to Add port [%d, type %d] to \
> >>+			vswitch %s\n", port_id, type, vs_dev->name);
> >>+		if (vs_port)
> >>+			vs_free_port(vs_port);
> >>+		vs_port = NULL;
> >>+	}
> >>+
> >>+	return vs_port;
> >>+}
> >
> 

[dpdk-dev] [RFC][PATCH V2 2/3] examples/vhost: Add vswitch command line options

[Pankaj Chauhan]

Add command line options for selecting switch implementation
and maximum ports for the vswitch.following are two new command
line options:

--switch <switch_name> [char string, Selects the switch imlementation]
--max-ports<port_count> [int, selects maximum number of ports to support]

For example:

$ ./vhost-switch -c 3 -n 2 --socket-mem 1024 --huge-dir /hugetlbfs -- -p
0x1 --dev-basename sock1 --switch "vmdq" --max-ports 3

Signed-off-by: Pankaj Chauhan <pankaj.chauhan@nxp.com>
---
 examples/vhost/main.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index c949df4..a4e51ae 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -142,6 +142,10 @@ static uint32_t burst_rx_retry_num = BURST_RX_RETRIES;
 /* Character device basename. Can be set by user. */
 static char dev_basename[MAX_BASENAME_SZ] = "vhost-net";
 
+/* vswitch device name and maximum number of ports */
+static char switch_dev[MAX_BASENAME_SZ] = "vmdq";
+static uint32_t switch_max_ports = MAX_DEVICES;
+
 /* empty vmdq configuration structure. Filled in programatically */
 static struct rte_eth_conf vmdq_conf_default = {
 	.rxmode = {
@@ -408,6 +412,22 @@ us_vhost_parse_basename(const char *q_arg)
 }
 
 /*
+ * Set switch device name.
+ */
+static int
+us_vhost_parse_switch_name(const char *q_arg)
+{
+	/* parse number string */
+
+	if (strnlen(q_arg, MAX_BASENAME_SZ) > MAX_BASENAME_SZ)
+		return -1;
+
+	snprintf(&switch_dev[0], MAX_BASENAME_SZ, "%s", q_arg);
+
+	return 0;
+}
+
+/*
  * Parse the portmask provided at run time.
  */
 static int
@@ -501,6 +521,8 @@ us_vhost_parse_args(int argc, char **argv)
 		{"tx-csum", required_argument, NULL, 0},
 		{"tso", required_argument, NULL, 0},
 		{"client", no_argument, &client_mode, 1},
+		{"switch", required_argument, NULL, 0},
+		{"max-ports", required_argument, NULL, 0},
 		{NULL, 0, 0, 0},
 	};
 
@@ -655,6 +677,27 @@ us_vhost_parse_args(int argc, char **argv)
 				}
 			}
 
+			/* Set vswitch_driver name */
+			if (!strncmp(long_option[option_index].name, "switch", MAX_LONG_OPT_SZ)) {
+				if (us_vhost_parse_switch_name(optarg) == -1) {
+					RTE_LOG(INFO, VHOST_CONFIG,
+						"Invalid argument for switch (Max len %d )\n", MAX_BASENAME_SZ);
+					us_vhost_usage(prgname);
+					return -EINVAL;
+				}
+			}
+
+			/* Specify Max ports in vswitch. */
+			if (!strncmp(long_option[option_index].name, "max-ports", MAX_LONG_OPT_SZ)) {
+				ret = parse_num_opt(optarg, INT32_MAX);
+				if (ret == -1) {
+					RTE_LOG(INFO, VHOST_CONFIG, "Invalid argument for switch max ports [0-N]\n");
+					us_vhost_usage(prgname);
+					return -1;
+				}
+				switch_max_ports = ret;
+			}
+
 			break;
 
 			/* Invalid option - print options. */
-- 
1.9.1

Re: [dpdk-dev] [RFC][PATCH V2 2/3] examples/vhost: Add vswitch command line options

[Yuanhan Liu]

On Mon, Sep 05, 2016 at 04:24:30PM +0530, Pankaj Chauhan wrote:
> Add command line options for selecting switch implementation
> and maximum ports for the vswitch.following are two new command
> line options:
> 
> --switch <switch_name> [char string, Selects the switch imlementation]
> --max-ports<port_count> [int, selects maximum number of ports to support]
> 
> For example:
> 
> $ ./vhost-switch -c 3 -n 2 --socket-mem 1024 --huge-dir /hugetlbfs -- -p
> 0x1 --dev-basename sock1

That means you were basing on the master branch. You should base on
next-virtio instead: http://dpdk.org/browse/next/dpdk-next-virtio/

> --switch "vmdq" --max-ports 3

Normally, we should keep the old behaviour first. Say, making the vmdq
as the default switch mode.

However, actually, I don't quite like to make the vhost-switch to bind
to a hardare feature that tightly, that you may want to make a standalone
patch as the last one in this patchset to make the "switch" mode be the
default one.

> 
> Signed-off-by: Pankaj Chauhan <pankaj.chauhan@nxp.com>
> ---
>  examples/vhost/main.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index c949df4..a4e51ae 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -142,6 +142,10 @@ static uint32_t burst_rx_retry_num = BURST_RX_RETRIES;
>  /* Character device basename. Can be set by user. */
>  static char dev_basename[MAX_BASENAME_SZ] = "vhost-net";
>  
> +/* vswitch device name and maximum number of ports */
> +static char switch_dev[MAX_BASENAME_SZ] = "vmdq";

First of all, limiting it with MAX_BASENAME_SZ makes no sense here.

Secondly, you don't have to represent the switch mode with string,
you could simply use some numeric macros, or enum.

Besides, I just had a quick glimplse of your patches (still couldn't
make a detailed look so far), and I'd ask you to do few more things
for v3:

- I'm hoping you could split this patchset further, say **maybe**
  one patch to introduce vswitch_port, one to introduce vswitch_ops
  and another one to introduce vswitch_dev. This helps review.

- make sure each commit is buldable.


And few more generic comments to the whole set:

- use "__rte_unused" but not "__attribute__((unused))"
  And we normally use it after but not before the key word.

- follow the DPDK prefered way to define a function, the return type
  and function name takes two lines.

- run scripts/{checkpatches.sh,check-git-log.sh} and fix real warnings
  if any before sending them out.

  This would at least help you catch "line over 80 chars" issue. 

Thanks.
	--yliu

Re: [dpdk-dev] [RFC][PATCH V2 2/3] examples/vhost: Add vswitch command line options

[Pankaj Chauhan]

On 9/13/2016 5:44 PM, Yuanhan Liu wrote:
> On Mon, Sep 05, 2016 at 04:24:30PM +0530, Pankaj Chauhan wrote:
>> Add command line options for selecting switch implementation
>> and maximum ports for the vswitch.following are two new command
>> line options:
>>
>> --switch <switch_name> [char string, Selects the switch imlementation]
>> --max-ports<port_count> [int, selects maximum number of ports to support]
>>
>> For example:
>>
>> $ ./vhost-switch -c 3 -n 2 --socket-mem 1024 --huge-dir /hugetlbfs -- -p
>> 0x1 --dev-basename sock1
>
> That means you were basing on the master branch. You should base on
> next-virtio instead: http://dpdk.org/browse/next/dpdk-next-virtio/

Yes i were basing it on master, i will base the next version on 
dpdk-next-virtio. Sorry for this, i am bit new to dpdk upstream 
development .
>
>> --switch "vmdq" --max-ports 3
>
> Normally, we should keep the old behaviour first. Say, making the vmdq
> as the default switch mode.

Yes if we don't give the '--switch' option then default is vmdq.
>
> However, actually, I don't quite like to make the vhost-switch to bind
> to a hardare feature that tightly, that you may want to make a standalone
> patch as the last one in this patchset to make the "switch" mode be the
> default one.

Sure i will split the patch to take care of this.
>
>>
>> Signed-off-by: Pankaj Chauhan <pankaj.chauhan@nxp.com>
>> ---
>>  examples/vhost/main.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 43 insertions(+)
>>
>> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
>> index c949df4..a4e51ae 100644
>> --- a/examples/vhost/main.c
>> +++ b/examples/vhost/main.c
>> @@ -142,6 +142,10 @@ static uint32_t burst_rx_retry_num = BURST_RX_RETRIES;
>>  /* Character device basename. Can be set by user. */
>>  static char dev_basename[MAX_BASENAME_SZ] = "vhost-net";
>>
>> +/* vswitch device name and maximum number of ports */
>> +static char switch_dev[MAX_BASENAME_SZ] = "vmdq";
>
> First of all, limiting it with MAX_BASENAME_SZ makes no sense here.
>

I will fix it, define a different constant for switch_dev name.

> Secondly, you don't have to represent the switch mode with string,
> you could simply use some numeric macros, or enum.
>

I used the string because the registration function (vs_register_switch) 
uses the switch name as string to register the switches and find the 
matching switch_dev given in command line. If we use macro or enum then 
we will have to add the MACRO for the switch id in common code every 
time a new switch implementation is added. Keeping a string as name, 
saves us from touching the common code for adding a new switch 
implementation.

> Besides, I just had a quick glimplse of your patches (still couldn't
> make a detailed look so far), and I'd ask you to do few more things
> for v3:
>
> - I'm hoping you could split this patchset further, say **maybe**
>   one patch to introduce vswitch_port, one to introduce vswitch_ops
>   and another one to introduce vswitch_dev. This helps review.
>

Do you want me to split the vswitch_ops, vswitch_dev and vswitch_port 
introduction (mainly vswitch_common.h) in three patches with the patch 
body explaining them in bit detail, correct ?

Their usage i.e vswitch_common.c can be kept in separate patch (4th 
one), is this fine? otherwise it fill be tricky to make vswitch_common.c 
to compilable without complete definition of vswitch_port, vswitch_dev 
and vswitch_ops

> - make sure each commit is buldable.
>
I will take care of this in v3
>
> And few more generic comments to the whole set:
>
> - use "__rte_unused" but not "__attribute__((unused))"
>   And we normally use it after but not before the key word.

I will take care of this in v3
>
> - follow the DPDK prefered way to define a function, the return type
>   and function name takes two lines.
I will take care of this in v3

>
> - run scripts/{checkpatches.sh,check-git-log.sh} and fix real warnings
>   if any before sending them out.
>
>   This would at least help you catch "line over 80 chars" issue.

Sure, will take care of this in v3 and onwards.

Thanks,
Pankaj
>
> Thanks.
> 	--yliu
>

[dpdk-dev] [RFC][PATCH V2 3/3] examples/vhost: Add VMDQ vswitch device

[Pankaj Chauhan]

Add support for VMDQ vswitch device. This patch takes
out all VMDQ specefic code from vhost/main.[c,h] and
move it to vmdq.[c,h]. Moreover vmdq.[c,h] files plug
the VMDQ vswitch device implmentation to the vhost-switch
using vswitch framework.

The main vhost/main.[c,h] code is now generic and can support
any switch implementation that conforms with vswitch framework.

Please note that the core VMDQ logic remains as it is, as it
was in vhost/main.c, this patch just moves it to different
file and fits into ops provided by framework.

Signed-off-by: Pankaj Chauhan <pankaj.chauhan@nxp.com>
---
 examples/vhost/main.c | 486 +++++-------------------------------
 examples/vhost/main.h |  10 +
 examples/vhost/vmdq.c | 669 ++++++++++++++++++++++++++++++++++++++++++++++++++
 examples/vhost/vmdq.h |  57 +++++
 4 files changed, 794 insertions(+), 428 deletions(-)
 create mode 100644 examples/vhost/vmdq.c
 create mode 100644 examples/vhost/vmdq.h

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index a4e51ae..096339b 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -54,6 +54,7 @@
 #include <rte_tcp.h>
 
 #include "main.h"
+#include "vswitch_common.h"
 
 #ifndef MAX_QUEUES
 #define MAX_QUEUES 128
@@ -65,7 +66,6 @@
 #define MBUF_CACHE_SIZE	128
 #define MBUF_DATA_SIZE	RTE_MBUF_DEFAULT_BUF_SIZE
 
-#define MAX_PKT_BURST 32		/* Max burst size for RX/TX */
 #define BURST_TX_DRAIN_US 100	/* TX drain every ~100us */
 
 #define BURST_RX_WAIT_US 15	/* Defines how long we wait between retries on RX */
@@ -103,7 +103,6 @@ static uint32_t enabled_port_mask = 0;
 static uint32_t promiscuous;
 
 /* number of devices/queues to support*/
-static uint32_t num_queues = 0;
 static uint32_t num_devices;
 
 static struct rte_mempool *mbuf_pool;
@@ -112,6 +111,8 @@ static int mergeable;
 /* Do vlan strip on host, enabled on default */
 static uint32_t vlan_strip = 1;
 
+static uint32_t jumbo_frame_en = 0;
+
 /* Enable VM2VM communications. If this is disabled then the MAC address compare is skipped. */
 typedef enum {
 	VM2VM_DISABLED = 0,
@@ -146,74 +147,16 @@ static char dev_basename[MAX_BASENAME_SZ] = "vhost-net";
 static char switch_dev[MAX_BASENAME_SZ] = "vmdq";
 static uint32_t switch_max_ports = MAX_DEVICES;
 
-/* empty vmdq configuration structure. Filled in programatically */
-static struct rte_eth_conf vmdq_conf_default = {
-	.rxmode = {
-		.mq_mode        = ETH_MQ_RX_VMDQ_ONLY,
-		.split_hdr_size = 0,
-		.header_split   = 0, /**< Header Split disabled */
-		.hw_ip_checksum = 0, /**< IP checksum offload disabled */
-		.hw_vlan_filter = 0, /**< VLAN filtering disabled */
-		/*
-		 * It is necessary for 1G NIC such as I350,
-		 * this fixes bug of ipv4 forwarding in guest can't
-		 * forward pakets from one virtio dev to another virtio dev.
-		 */
-		.hw_vlan_strip  = 1, /**< VLAN strip enabled. */
-		.jumbo_frame    = 0, /**< Jumbo Frame Support disabled */
-		.hw_strip_crc   = 0, /**< CRC stripped by hardware */
-	},
-
-	.txmode = {
-		.mq_mode = ETH_MQ_TX_NONE,
-	},
-	.rx_adv_conf = {
-		/*
-		 * should be overridden separately in code with
-		 * appropriate values
-		 */
-		.vmdq_rx_conf = {
-			.nb_queue_pools = ETH_8_POOLS,
-			.enable_default_pool = 0,
-			.default_pool = 0,
-			.nb_pool_maps = 0,
-			.pool_map = {{0, 0},},
-		},
-	},
-};
-
+struct vswitch_dev *vswitch_dev_g;
 static unsigned lcore_ids[RTE_MAX_LCORE];
 static uint8_t ports[RTE_MAX_ETHPORTS];
 static unsigned num_ports = 0; /**< The number of ports specified in command line */
-static uint16_t num_pf_queues, num_vmdq_queues;
-static uint16_t vmdq_pool_base, vmdq_queue_base;
-static uint16_t queues_per_pool;
-
-const uint16_t vlan_tags[] = {
-	1000, 1001, 1002, 1003, 1004, 1005, 1006, 1007,
-	1008, 1009, 1010, 1011,	1012, 1013, 1014, 1015,
-	1016, 1017, 1018, 1019, 1020, 1021, 1022, 1023,
-	1024, 1025, 1026, 1027, 1028, 1029, 1030, 1031,
-	1032, 1033, 1034, 1035, 1036, 1037, 1038, 1039,
-	1040, 1041, 1042, 1043, 1044, 1045, 1046, 1047,
-	1048, 1049, 1050, 1051, 1052, 1053, 1054, 1055,
-	1056, 1057, 1058, 1059, 1060, 1061, 1062, 1063,
-};
-
-/* ethernet addresses of ports */
-static struct ether_addr vmdq_ports_eth_addr[RTE_MAX_ETHPORTS];
 
 static struct vhost_dev_tailq_list vhost_dev_list =
 	TAILQ_HEAD_INITIALIZER(vhost_dev_list);
 
 static struct lcore_info lcore_info[RTE_MAX_LCORE];
 
-/* Used for queueing bursts of TX packets. */
-struct mbuf_table {
-	unsigned len;
-	unsigned txq_id;
-	struct rte_mbuf *m_table[MAX_PKT_BURST];
-};
 
 /* TX queue for each data core. */
 struct mbuf_table lcore_tx_queue[RTE_MAX_LCORE];
@@ -223,35 +166,6 @@ struct mbuf_table lcore_tx_queue[RTE_MAX_LCORE];
 #define VLAN_HLEN       4
 
 /*
- * Builds up the correct configuration for VMDQ VLAN pool map
- * according to the pool & queue limits.
- */
-static inline int
-get_eth_conf(struct rte_eth_conf *eth_conf, uint32_t num_devices)
-{
-	struct rte_eth_vmdq_rx_conf conf;
-	struct rte_eth_vmdq_rx_conf *def_conf =
-		&vmdq_conf_default.rx_adv_conf.vmdq_rx_conf;
-	unsigned i;
-
-	memset(&conf, 0, sizeof(conf));
-	conf.nb_queue_pools = (enum rte_eth_nb_pools)num_devices;
-	conf.nb_pool_maps = num_devices;
-	conf.enable_loop_back = def_conf->enable_loop_back;
-	conf.rx_mode = def_conf->rx_mode;
-
-	for (i = 0; i < conf.nb_pool_maps; i++) {
-		conf.pool_map[i].vlan_id = vlan_tags[ i ];
-		conf.pool_map[i].pools = (1UL << i);
-	}
-
-	(void)(rte_memcpy(eth_conf, &vmdq_conf_default, sizeof(*eth_conf)));
-	(void)(rte_memcpy(&eth_conf->rx_adv_conf.vmdq_rx_conf, &conf,
-		   sizeof(eth_conf->rx_adv_conf.vmdq_rx_conf)));
-	return 0;
-}
-
-/*
  * Validate the device number according to the max pool number gotten form
  * dev_info. If the device number is invalid, give the error message and
  * return -1. Each device must have its own pool.
@@ -274,16 +188,25 @@ static inline int
 port_init(uint8_t port)
 {
 	struct rte_eth_dev_info dev_info;
-	struct rte_eth_conf port_conf;
 	struct rte_eth_rxconf *rxconf;
 	struct rte_eth_txconf *txconf;
 	int16_t rx_rings, tx_rings;
 	uint16_t rx_ring_size, tx_ring_size;
+	struct vswitch_port *vs_port;
 	int retval;
 	uint16_t q;
 
+	if (port >= rte_eth_dev_count()) return -1;
+
+	vs_port = vs_add_port(vswitch_dev_g, port, VSWITCH_PTYPE_PHYS, NULL);
+
+	if (!vs_port) {
+		rte_exit(EXIT_FAILURE, "Failed to add port [%d] to vsdev %s\n",
+			 port, vswitch_dev_g->name);
+	}
+
 	/* The max pool number from dev_info will be used to validate the pool number specified in cmd line */
-	rte_eth_dev_info_get (port, &dev_info);
+	rte_eth_dev_info_get (vs_port->port_id, &dev_info);
 
 	if (dev_info.max_rx_queues > MAX_QUEUES) {
 		rte_exit(EXIT_FAILURE,
@@ -298,33 +221,10 @@ port_init(uint8_t port)
 	/* Enable vlan offload */
 	txconf->txq_flags &= ~ETH_TXQ_FLAGS_NOVLANOFFL;
 
-	/*configure the number of supported virtio devices based on VMDQ limits */
-	num_devices = dev_info.max_vmdq_pools;
-
 	rx_ring_size = RTE_TEST_RX_DESC_DEFAULT;
 	tx_ring_size = RTE_TEST_TX_DESC_DEFAULT;
 	tx_rings = (uint16_t)rte_lcore_count();
 
-	retval = validate_num_devices(MAX_DEVICES);
-	if (retval < 0)
-		return retval;
-
-	/* Get port configuration. */
-	retval = get_eth_conf(&port_conf, num_devices);
-	if (retval < 0)
-		return retval;
-	/* NIC queues are divided into pf queues and vmdq queues.  */
-	num_pf_queues = dev_info.max_rx_queues - dev_info.vmdq_queue_num;
-	queues_per_pool = dev_info.vmdq_queue_num / dev_info.max_vmdq_pools;
-	num_vmdq_queues = num_devices * queues_per_pool;
-	num_queues = num_pf_queues + num_vmdq_queues;
-	vmdq_queue_base = dev_info.vmdq_queue_base;
-	vmdq_pool_base  = dev_info.vmdq_pool_base;
-	printf("pf queue num: %u, configured vmdq pool num: %u, each vmdq pool has %u queues\n",
-		num_pf_queues, num_devices, queues_per_pool);
-
-	if (port >= rte_eth_dev_count()) return -1;
-
 	if (enable_tx_csum == 0)
 		rte_vhost_feature_disable(1ULL << VIRTIO_NET_F_CSUM);
 
@@ -337,10 +237,11 @@ port_init(uint8_t port)
 
 	rx_rings = (uint16_t)dev_info.max_rx_queues;
 	/* Configure ethernet device. */
-	retval = rte_eth_dev_configure(port, rx_rings, tx_rings, &port_conf);
+	retval = rte_eth_dev_configure(vs_port->port_id, rx_rings, tx_rings,
+				       &vs_port->port_conf);
 	if (retval != 0) {
 		RTE_LOG(ERR, VHOST_PORT, "Failed to configure port %u: %s.\n",
-			port, strerror(-retval));
+			vs_port->port_id, strerror(-retval));
 		return retval;
 	}
 
@@ -353,7 +254,7 @@ port_init(uint8_t port)
 		if (retval < 0) {
 			RTE_LOG(ERR, VHOST_PORT,
 				"Failed to setup rx queue %u of port %u: %s.\n",
-				q, port, strerror(-retval));
+				q, vs_port->port_id, strerror(-retval));
 			return retval;
 		}
 	}
@@ -369,28 +270,10 @@ port_init(uint8_t port)
 		}
 	}
 
-	/* Start the device. */
-	retval  = rte_eth_dev_start(port);
-	if (retval < 0) {
-		RTE_LOG(ERR, VHOST_PORT, "Failed to start port %u: %s\n",
-			port, strerror(-retval));
-		return retval;
-	}
-
 	if (promiscuous)
 		rte_eth_promiscuous_enable(port);
 
-	rte_eth_macaddr_get(port, &vmdq_ports_eth_addr[port]);
-	RTE_LOG(INFO, VHOST_PORT, "Max virtio devices supported: %u\n", num_devices);
-	RTE_LOG(INFO, VHOST_PORT, "Port %u MAC: %02"PRIx8" %02"PRIx8" %02"PRIx8
-			" %02"PRIx8" %02"PRIx8" %02"PRIx8"\n",
-			(unsigned)port,
-			vmdq_ports_eth_addr[port].addr_bytes[0],
-			vmdq_ports_eth_addr[port].addr_bytes[1],
-			vmdq_ports_eth_addr[port].addr_bytes[2],
-			vmdq_ports_eth_addr[port].addr_bytes[3],
-			vmdq_ports_eth_addr[port].addr_bytes[4],
-			vmdq_ports_eth_addr[port].addr_bytes[5]);
+	vs_port_start(vs_port);
 
 	return 0;
 }
@@ -542,9 +425,6 @@ us_vhost_parse_args(int argc, char **argv)
 
 		case 'P':
 			promiscuous = 1;
-			vmdq_conf_default.rx_adv_conf.vmdq_rx_conf.rx_mode =
-				ETH_VMDQ_ACCEPT_BROADCAST |
-				ETH_VMDQ_ACCEPT_MULTICAST;
 			rte_vhost_feature_enable(1ULL << VIRTIO_NET_F_CTRL_RX);
 
 			break;
@@ -632,11 +512,8 @@ us_vhost_parse_args(int argc, char **argv)
 					return -1;
 				} else {
 					mergeable = !!ret;
-					if (ret) {
-						vmdq_conf_default.rxmode.jumbo_frame = 1;
-						vmdq_conf_default.rxmode.max_rx_pkt_len
-							= JUMBO_FRAME_MAX_SIZE;
-					}
+					if (ret)
+						jumbo_frame_en = 1;
 				}
 			}
 
@@ -651,8 +528,6 @@ us_vhost_parse_args(int argc, char **argv)
 					return -1;
 				} else {
 					vlan_strip = !!ret;
-					vmdq_conf_default.rxmode.hw_vlan_strip =
-						vlan_strip;
 				}
 			}
 
@@ -747,8 +622,7 @@ static unsigned check_ports_num(unsigned nb_ports)
 	return valid_num_ports;
 }
 
-static inline struct vhost_dev *__attribute__((always_inline))
-find_vhost_dev(struct ether_addr *mac)
+struct vhost_dev *find_vhost_dev(struct ether_addr *mac)
 {
 	struct vhost_dev *vdev;
 
@@ -761,95 +635,6 @@ find_vhost_dev(struct ether_addr *mac)
 	return NULL;
 }
 
-/*
- * This function learns the MAC address of the device and registers this along with a
- * vlan tag to a VMDQ.
- */
-static int
-link_vmdq(struct vhost_dev *vdev, struct rte_mbuf *m)
-{
-	struct ether_hdr *pkt_hdr;
-	int i, ret;
-
-	/* Learn MAC address of guest device from packet */
-	pkt_hdr = rte_pktmbuf_mtod(m, struct ether_hdr *);
-
-	if (find_vhost_dev(&pkt_hdr->s_addr)) {
-		RTE_LOG(ERR, VHOST_DATA,
-			"(%d) device is using a registered MAC!\n",
-			vdev->vid);
-		return -1;
-	}
-
-	for (i = 0; i < ETHER_ADDR_LEN; i++)
-		vdev->mac_address.addr_bytes[i] = pkt_hdr->s_addr.addr_bytes[i];
-
-	/* vlan_tag currently uses the device_id. */
-	vdev->vlan_tag = vlan_tags[vdev->vid];
-
-	/* Print out VMDQ registration info. */
-	RTE_LOG(INFO, VHOST_DATA,
-		"(%d) mac %02x:%02x:%02x:%02x:%02x:%02x and vlan %d registered\n",
-		vdev->vid,
-		vdev->mac_address.addr_bytes[0], vdev->mac_address.addr_bytes[1],
-		vdev->mac_address.addr_bytes[2], vdev->mac_address.addr_bytes[3],
-		vdev->mac_address.addr_bytes[4], vdev->mac_address.addr_bytes[5],
-		vdev->vlan_tag);
-
-	/* Register the MAC address. */
-	ret = rte_eth_dev_mac_addr_add(ports[0], &vdev->mac_address,
-				(uint32_t)vdev->vid + vmdq_pool_base);
-	if (ret)
-		RTE_LOG(ERR, VHOST_DATA,
-			"(%d) failed to add device MAC address to VMDQ\n",
-			vdev->vid);
-
-	/* Enable stripping of the vlan tag as we handle routing. */
-	if (vlan_strip)
-		rte_eth_dev_set_vlan_strip_on_queue(ports[0],
-			(uint16_t)vdev->vmdq_rx_q, 1);
-
-	/* Set device as ready for RX. */
-	vdev->ready = DEVICE_RX;
-
-	return 0;
-}
-
-/*
- * Removes MAC address and vlan tag from VMDQ. Ensures that nothing is adding buffers to the RX
- * queue before disabling RX on the device.
- */
-static inline void
-unlink_vmdq(struct vhost_dev *vdev)
-{
-	unsigned i = 0;
-	unsigned rx_count;
-	struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
-
-	if (vdev->ready == DEVICE_RX) {
-		/*clear MAC and VLAN settings*/
-		rte_eth_dev_mac_addr_remove(ports[0], &vdev->mac_address);
-		for (i = 0; i < 6; i++)
-			vdev->mac_address.addr_bytes[i] = 0;
-
-		vdev->vlan_tag = 0;
-
-		/*Clear out the receive buffers*/
-		rx_count = rte_eth_rx_burst(ports[0],
-					(uint16_t)vdev->vmdq_rx_q, pkts_burst, MAX_PKT_BURST);
-
-		while (rx_count) {
-			for (i = 0; i < rx_count; i++)
-				rte_pktmbuf_free(pkts_burst[i]);
-
-			rx_count = rte_eth_rx_burst(ports[0],
-					(uint16_t)vdev->vmdq_rx_q, pkts_burst, MAX_PKT_BURST);
-		}
-
-		vdev->ready = DEVICE_MAC_LEARNING;
-	}
-}
-
 static inline void __attribute__((always_inline))
 virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
 	    struct rte_mbuf *m)
@@ -876,8 +661,7 @@ virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
  * Check if the packet destination MAC address is for a local device. If so then put
  * the packet on that devices RX queue. If not then return.
  */
-static inline int __attribute__((always_inline))
-virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
+int virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 {
 	struct ether_hdr *pkt_hdr;
 	struct vhost_dev *dst_vdev;
@@ -908,69 +692,9 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 	return 0;
 }
 
-/*
- * Check if the destination MAC of a packet is one local VM,
- * and get its vlan tag, and offset if it is.
- */
-static inline int __attribute__((always_inline))
-find_local_dest(struct vhost_dev *vdev, struct rte_mbuf *m,
-	uint32_t *offset, uint16_t *vlan_tag)
+struct mbuf_table *vhost_switch_get_txq(uint16_t core_id)
 {
-	struct vhost_dev *dst_vdev;
-	struct ether_hdr *pkt_hdr = rte_pktmbuf_mtod(m, struct ether_hdr *);
-
-	dst_vdev = find_vhost_dev(&pkt_hdr->d_addr);
-	if (!dst_vdev)
-		return 0;
-
-	if (vdev->vid == dst_vdev->vid) {
-		RTE_LOG(DEBUG, VHOST_DATA,
-			"(%d) TX: src and dst MAC is same. Dropping packet.\n",
-			vdev->vid);
-		return -1;
-	}
-
-	/*
-	 * HW vlan strip will reduce the packet length
-	 * by minus length of vlan tag, so need restore
-	 * the packet length by plus it.
-	 */
-	*offset  = VLAN_HLEN;
-	*vlan_tag = vlan_tags[vdev->vid];
-
-	RTE_LOG(DEBUG, VHOST_DATA,
-		"(%d) TX: pkt to local VM device id: (%d), vlan tag: %u.\n",
-		vdev->vid, dst_vdev->vid, *vlan_tag);
-
-	return 0;
-}
-
-static uint16_t
-get_psd_sum(void *l3_hdr, uint64_t ol_flags)
-{
-	if (ol_flags & PKT_TX_IPV4)
-		return rte_ipv4_phdr_cksum(l3_hdr, ol_flags);
-	else /* assume ethertype == ETHER_TYPE_IPv6 */
-		return rte_ipv6_phdr_cksum(l3_hdr, ol_flags);
-}
-
-static void virtio_tx_offload(struct rte_mbuf *m)
-{
-	void *l3_hdr;
-	struct ipv4_hdr *ipv4_hdr = NULL;
-	struct tcp_hdr *tcp_hdr = NULL;
-	struct ether_hdr *eth_hdr = rte_pktmbuf_mtod(m, struct ether_hdr *);
-
-	l3_hdr = (char *)eth_hdr + m->l2_len;
-
-	if (m->ol_flags & PKT_TX_IPV4) {
-		ipv4_hdr = l3_hdr;
-		ipv4_hdr->hdr_checksum = 0;
-		m->ol_flags |= PKT_TX_IP_CKSUM;
-	}
-
-	tcp_hdr = (struct tcp_hdr *)((char *)l3_hdr + m->l3_len);
-	tcp_hdr->cksum = get_psd_sum(l3_hdr, m->ol_flags);
+	return &lcore_tx_queue[core_id];
 }
 
 static inline void
@@ -980,8 +704,7 @@ free_pkts(struct rte_mbuf **pkts, uint16_t n)
 		rte_pktmbuf_free(pkts[n]);
 }
 
-static inline void __attribute__((always_inline))
-do_drain_mbuf_table(struct mbuf_table *tx_q)
+void do_drain_mbuf_table(struct mbuf_table *tx_q)
 {
 	uint16_t count;
 	struct vswitch_port *tx_port;
@@ -1006,100 +729,10 @@ do_drain_mbuf_table(struct mbuf_table *tx_q)
 	}
 
 	tx_q->len = 0;
+out:
+	return;
 }
 
-/*
- * This function routes the TX packet to the correct interface. This
- * may be a local device or the physical port.
- */
-static inline void __attribute__((always_inline))
-virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m, uint16_t vlan_tag)
-{
-	struct mbuf_table *tx_q;
-	unsigned offset = 0;
-	const uint16_t lcore_id = rte_lcore_id();
-	struct ether_hdr *nh;
-
-
-	nh = rte_pktmbuf_mtod(m, struct ether_hdr *);
-	if (unlikely(is_broadcast_ether_addr(&nh->d_addr))) {
-		struct vhost_dev *vdev2;
-
-		TAILQ_FOREACH(vdev2, &vhost_dev_list, global_vdev_entry) {
-			virtio_xmit(vdev2, vdev, m);
-		}
-		goto queue2nic;
-	}
-
-	/*check if destination is local VM*/
-	if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev, m) == 0)) {
-		rte_pktmbuf_free(m);
-		return;
-	}
-
-	if (unlikely(vm2vm_mode == VM2VM_HARDWARE)) {
-		if (unlikely(find_local_dest(vdev, m, &offset,
-					     &vlan_tag) != 0)) {
-			rte_pktmbuf_free(m);
-			return;
-		}
-	}
-
-	RTE_LOG(DEBUG, VHOST_DATA,
-		"(%d) TX: MAC address is external\n", vdev->vid);
-
-queue2nic:
-
-	/*Add packet to the port tx queue*/
-	tx_q = &lcore_tx_queue[lcore_id];
-
-	nh = rte_pktmbuf_mtod(m, struct ether_hdr *);
-	if (unlikely(nh->ether_type == rte_cpu_to_be_16(ETHER_TYPE_VLAN))) {
-		/* Guest has inserted the vlan tag. */
-		struct vlan_hdr *vh = (struct vlan_hdr *) (nh + 1);
-		uint16_t vlan_tag_be = rte_cpu_to_be_16(vlan_tag);
-		if ((vm2vm_mode == VM2VM_HARDWARE) &&
-			(vh->vlan_tci != vlan_tag_be))
-			vh->vlan_tci = vlan_tag_be;
-	} else {
-		m->ol_flags |= PKT_TX_VLAN_PKT;
-
-		/*
-		 * Find the right seg to adjust the data len when offset is
-		 * bigger than tail room size.
-		 */
-		if (unlikely(vm2vm_mode == VM2VM_HARDWARE)) {
-			if (likely(offset <= rte_pktmbuf_tailroom(m)))
-				m->data_len += offset;
-			else {
-				struct rte_mbuf *seg = m;
-
-				while ((seg->next != NULL) &&
-					(offset > rte_pktmbuf_tailroom(seg)))
-					seg = seg->next;
-
-				seg->data_len += offset;
-			}
-			m->pkt_len += offset;
-		}
-
-		m->vlan_tci = vlan_tag;
-	}
-
-	if (m->ol_flags & PKT_TX_TCP_SEG)
-		virtio_tx_offload(m);
-
-	tx_q->m_table[tx_q->len++] = m;
-	if (enable_stats) {
-		vdev->stats.tx_total++;
-		vdev->stats.tx++;
-	}
-
-	if (unlikely(tx_q->len == MAX_PKT_BURST))
-		do_drain_mbuf_table(tx_q);
-}
-
-
 static inline void __attribute__((always_inline))
 drain_mbuf_table(struct mbuf_table *tx_q)
 {
@@ -1123,7 +756,7 @@ drain_mbuf_table(struct mbuf_table *tx_q)
 static inline void __attribute__((always_inline))
 drain_eth_rx(struct vhost_dev *vdev)
 {
-	uint16_t rx_count, enqueue_count;
+	uint16_t rx_count;
 	struct rte_mbuf *pkts[MAX_PKT_BURST];
 	uint16_t rxq, core_id;
 	struct vswitch_port *rx_port;
@@ -1148,37 +781,18 @@ drain_eth_rx(struct vhost_dev *vdev)
 	rxq = rx_port->get_rxq(rx_port, vdev, core_id);
 	rx_count = rx_port->do_rx(rx_port, rxq, NULL, pkts, MAX_PKT_BURST);
 
-	rx_count = rte_eth_rx_burst(ports[0], vdev->vmdq_rx_q,
-				    pkts, MAX_PKT_BURST);
 	if (!rx_count)
 		return;
 
-	/*
-	 * When "enable_retry" is set, here we wait and retry when there
-	 * is no enough free slots in the queue to hold @rx_count packets,
-	 * to diminish packet loss.
-	 */
-	if (enable_retry &&
-	    unlikely(rx_count > rte_vhost_avail_entries(vdev->vid,
-			VIRTIO_RXQ))) {
-		uint32_t retry;
-
-		for (retry = 0; retry < burst_rx_retry_num; retry++) {
-			rte_delay_us(burst_rx_delay_time);
-			if (rx_count <= rte_vhost_avail_entries(vdev->vid,
-					VIRTIO_RXQ))
-				break;
-		}
-	}
+	vs_lookup_n_fwd(rx_port, pkts, rx_count, rxq);
 
-	enqueue_count = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
-						pkts, rx_count);
 	if (enable_stats) {
 		rte_atomic64_add(&vdev->stats.rx_total_atomic, rx_count);
-		rte_atomic64_add(&vdev->stats.rx_atomic, enqueue_count);
 	}
 
 	free_pkts(pkts, rx_count);
+out:
+	return;
 }
 
 static inline void __attribute__((always_inline))
@@ -1263,7 +877,7 @@ switch_worker(void *arg __rte_unused)
 		TAILQ_FOREACH(vdev, &lcore_info[lcore_id].vdev_list,
 			      lcore_vdev_entry) {
 			if (unlikely(vdev->remove)) {
-				unlink_vmdq(vdev);
+				vs_unlearn_port(vdev->vs_port);
 				vdev->ready = DEVICE_SAFE_REMOVE;
 				continue;
 			}
@@ -1289,6 +903,7 @@ static void
 destroy_device(int vid)
 {
 	struct vhost_dev *vdev = NULL;
+	struct vswitch_port *vs_port;
 	int lcore;
 
 	TAILQ_FOREACH(vdev, &vhost_dev_list, global_vdev_entry) {
@@ -1324,6 +939,10 @@ destroy_device(int vid)
 
 	lcore_info[vdev->coreid].device_num--;
 
+	vs_port = vdev->vs_port;
+	vs_port_stop(vs_port);
+	vs_del_port(vs_port);
+
 	RTE_LOG(INFO, VHOST_DATA,
 		"(%d) device has been removed from data core\n",
 		vdev->vid);
@@ -1341,6 +960,7 @@ new_device(int vid)
 	int lcore, core_add = 0;
 	uint32_t device_num_min;
 	struct vhost_dev *vdev;
+	struct vswitch_port *vs_port;
 
 	vdev = rte_zmalloc("vhost device", sizeof(*vdev), RTE_CACHE_LINE_SIZE);
 	if (vdev == NULL) {
@@ -1362,7 +982,6 @@ new_device(int vid)
 	vdev->vs_port = vs_port;
 
 	TAILQ_INSERT_TAIL(&vhost_dev_list, vdev, global_vdev_entry);
-	vdev->vmdq_rx_q = vid * queues_per_pool + vmdq_queue_base;
 
 	/*reset ready flag*/
 	vdev->ready = DEVICE_MAC_LEARNING;
@@ -1552,7 +1171,7 @@ main(int argc, char *argv[])
 	uint8_t portid;
 	static pthread_t tid;
 	char thread_name[RTE_MAX_THREAD_NAME_LEN];
-	uint64_t flags = 0;
+	uint64_t flags = 0, vswitch_conf_flags;
 
 	signal(SIGINT, sigint_handler);
 
@@ -1563,11 +1182,19 @@ main(int argc, char *argv[])
 	argc -= ret;
 	argv += ret;
 
+	vs_vswitch_init();
+
+	/* TBD:XXX: This needs to be removed here, when constructor mechanism
+	 * for registering swittches is in place
+	 */
+	vmdq_switch_impl_init();
+
 	/* parse app arguments */
 	ret = us_vhost_parse_args(argc, argv);
 	if (ret < 0)
 		rte_exit(EXIT_FAILURE, "Invalid argument\n");
 
+	/*TBD:XXX: vdev list or the vdev ports */
 	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id ++)
 		TAILQ_INIT(&lcore_info[lcore_id].vdev_list);
 
@@ -1592,6 +1219,13 @@ main(int argc, char *argv[])
 		return -1;
 	}
 
+	vswitch_dev_g = vs_get_vswitch_dev(switch_dev);
+	if (!vswitch_dev_g) {
+		RTE_LOG(INFO, VHOST_CONFIG, "switch dev %s not supported\n",
+			switch_dev);
+		return -1;
+	}
+
 	/*
 	 * FIXME: here we are trying to allocate mbufs big enough for
 	 * @MAX_QUEUES, but the truth is we're never going to use that
@@ -1601,12 +1235,8 @@ main(int argc, char *argv[])
 	create_mbuf_pool(valid_num_ports, rte_lcore_count() - 1, MBUF_DATA_SIZE,
 			 MAX_QUEUES, RTE_TEST_RX_DESC_DEFAULT, MBUF_CACHE_SIZE);
 
-	if (vm2vm_mode == VM2VM_HARDWARE) {
-		/* Enable VT loop back to let L2 switch to do it. */
-		vmdq_conf_default.rx_adv_conf.vmdq_rx_conf.enable_loop_back = 1;
-		RTE_LOG(DEBUG, VHOST_CONFIG,
-			"Enable loop back for L2 switch in vmdq.\n");
-	}
+	vswitch_conf_flags = get_vswitch_conf_flags();
+	vs_switch_dev_init(vswitch_dev_g, vswitch_conf_flags);
 
 	/* initialize all ports */
 	for (portid = 0; portid < nb_ports; portid++) {
diff --git a/examples/vhost/main.h b/examples/vhost/main.h
index 6bb42e8..d6b23f6 100644
--- a/examples/vhost/main.h
+++ b/examples/vhost/main.h
@@ -68,6 +68,7 @@ struct vhost_dev {
 	struct device_statistics stats;
 	TAILQ_ENTRY(vhost_dev) global_vdev_entry;
 	TAILQ_ENTRY(vhost_dev) lcore_vdev_entry;
+	struct vswitch_port *vs_port;
 } __rte_cache_aligned;
 
 TAILQ_HEAD(vhost_dev_tailq_list, vhost_dev);
@@ -88,4 +89,13 @@ struct lcore_info {
 	struct vhost_dev_tailq_list vdev_list;
 };
 
+#define MAX_PKT_BURST 32		/* Max burst size for RX/TX */
+
+/* Used for queueing bursts of TX packets. */
+struct mbuf_table {
+	unsigned len;
+	unsigned txq_id;
+	struct rte_mbuf *m_table[MAX_PKT_BURST];
+};
+
 #endif /* _MAIN_H_ */
diff --git a/examples/vhost/vmdq.c b/examples/vhost/vmdq.c
new file mode 100644
index 0000000..ca26195
--- /dev/null
+++ b/examples/vhost/vmdq.c
@@ -0,0 +1,669 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#include <rte_atomic.h>
+#include <rte_cycles.h>
+#include <rte_ethdev.h>
+#include <rte_log.h>
+#include <rte_string_fns.h>
+#include <rte_malloc.h>
+#include <rte_virtio_net.h>
+#include <rte_ip.h>
+#include <rte_tcp.h>
+
+#include "vswitch_common.h"
+#include "vmdq.h"
+
+#define JUMBO_FRAME_MAX_SIZE    0x2600
+/* State of virtio device. */
+#define DEVICE_MAC_LEARNING 0
+#define DEVICE_RX			1
+#define DEVICE_SAFE_REMOVE	2
+
+#define VLAN_HLEN       4
+
+static struct vswitch_dev *vmdq_switch_dev_g;
+
+const uint16_t vlan_tags[] = {
+	1000, 1001, 1002, 1003, 1004, 1005, 1006, 1007,
+	1008, 1009, 1010, 1011,	1012, 1013, 1014, 1015,
+	1016, 1017, 1018, 1019, 1020, 1021, 1022, 1023,
+	1024, 1025, 1026, 1027, 1028, 1029, 1030, 1031,
+	1032, 1033, 1034, 1035, 1036, 1037, 1038, 1039,
+	1040, 1041, 1042, 1043, 1044, 1045, 1046, 1047,
+	1048, 1049, 1050, 1051, 1052, 1053, 1054, 1055,
+	1056, 1057, 1058, 1059, 1060, 1061, 1062, 1063,
+};
+
+/* empty vmdq configuration structure. Filled in programatically */
+static struct rte_eth_conf vmdq_conf_default = {
+	.rxmode = {
+		.mq_mode        = ETH_MQ_RX_VMDQ_ONLY,
+		.split_hdr_size = 0,
+		.header_split   = 0, /**< Header Split disabled */
+		.hw_ip_checksum = 0, /**< IP checksum offload disabled */
+		.hw_vlan_filter = 0, /**< VLAN filtering disabled */
+		/*
+		 * It is necessary for 1G NIC such as I350,
+		 * this fixes bug of ipv4 forwarding in guest can't
+		 * forward pakets from one virtio dev to another virtio dev.
+		 */
+		.hw_vlan_strip  = 1, /**< VLAN strip enabled. */
+		.jumbo_frame    = 0, /**< Jumbo Frame Support disabled */
+		.hw_strip_crc   = 0, /**< CRC stripped by hardware */
+	},
+
+	.txmode = {
+		.mq_mode = ETH_MQ_TX_NONE,
+	},
+	.rx_adv_conf = {
+		/*
+		 * should be overridden separately in code with
+		 * appropriate values
+		 */
+		.vmdq_rx_conf = {
+			.nb_queue_pools = ETH_8_POOLS,
+			.enable_default_pool = 0,
+			.default_pool = 0,
+			.nb_pool_maps = 0,
+			.pool_map = {{0, 0},},
+		},
+	},
+};
+
+
+static int vmdq_switch_init(__attribute__((unused))struct vswitch_dev *vs_dev,
+			    uint64_t conf_flags)
+{
+	uint32_t enable;
+
+	if (conf_flags & VS_CNF_FLG_VM2VM_HARDWARE) {
+		/* Enable VT loop back to let L2 switch to do it. */
+		vmdq_conf_default.rx_adv_conf.vmdq_rx_conf.enable_loop_back = 1;
+		RTE_LOG(DEBUG, VHOST_CONFIG,
+			"Enable loop back for L2 switch in vmdq.\n");
+	}
+
+	if (conf_flags & VS_CNF_FLG_PROMISCOUS_EN) {
+		vmdq_conf_default.rx_adv_conf.vmdq_rx_conf.rx_mode =
+				ETH_VMDQ_ACCEPT_BROADCAST |
+				ETH_VMDQ_ACCEPT_MULTICAST;
+	}
+
+	if (conf_flags & VS_CNF_FLG_JUMBO_EN) {
+		vmdq_conf_default.rxmode.jumbo_frame = 1;
+		vmdq_conf_default.rxmode.max_rx_pkt_len = JUMBO_FRAME_MAX_SIZE;
+	}
+
+	enable = !!(conf_flags & VS_CNF_FLG_VLAN_STRIP_EN);
+	vmdq_conf_default.rxmode.hw_vlan_strip = enable;
+
+	return 0;
+}
+
+
+static int vmdq_get_max_vdevs(struct vswitch_dev *vs_dev)
+{
+	struct vmdq_switch_priv *priv = vs_dev->priv;
+
+	return priv->num_devices;
+}
+
+/*
+ * Builds up the correct configuration for VMDQ VLAN pool map
+ * according to the pool & queue limits.
+ */
+static inline int
+vmdq_get_eth_conf(struct vswitch_port *vs_port, struct rte_eth_conf *eth_conf)
+{
+	struct rte_eth_vmdq_rx_conf conf;
+	struct rte_eth_vmdq_rx_conf *def_conf =
+		&vmdq_conf_default.rx_adv_conf.vmdq_rx_conf;
+	struct vmdq_switch_priv *priv = vs_port->vs_dev->priv;
+	unsigned i;
+
+	memset(&conf, 0, sizeof(conf));
+	conf.nb_queue_pools = (enum rte_eth_nb_pools)priv->num_devices;
+	conf.nb_pool_maps = priv->num_devices;
+	conf.enable_loop_back = def_conf->enable_loop_back;
+	conf.rx_mode = def_conf->rx_mode;
+
+	for (i = 0; i < conf.nb_pool_maps; i++) {
+		conf.pool_map[i].vlan_id = vlan_tags[ i ];
+		conf.pool_map[i].pools = (1UL << i);
+	}
+
+	(void)(rte_memcpy(eth_conf, &vmdq_conf_default, sizeof(*eth_conf)));
+	(void)(rte_memcpy(&eth_conf->rx_adv_conf.vmdq_rx_conf, &conf,
+		   sizeof(eth_conf->rx_adv_conf.vmdq_rx_conf)));
+	return 0;
+}
+
+static int vmdq_add_port_phys(struct vswitch_port *vs_port)
+{
+	struct rte_eth_dev_info dev_info;
+	struct vmdq_switch_priv *priv = vs_port->vs_dev->priv;
+	uint16_t queues_per_pool;
+	int rc = 0;
+
+	if (priv->phys_port_count >= VMDQ_MAX_PHYS_PORTS) {
+		RTE_LOG(INFO, VHOST_CONFIG,
+			"Physical ports greater than max devices(%d)\n",
+			VMDQ_MAX_PHYS_PORTS);
+		rc = -EBUSY;
+		goto out;
+	}
+
+	rte_eth_dev_info_get (vs_port->port_id, &dev_info);
+	if (dev_info.max_vmdq_pools > VMDQ_MAX_VIRTIO_PORTS) {
+		RTE_LOG(INFO, VHOST_CONFIG,
+			"Num devices (%d) greater than Max (%d)\n",
+			dev_info.max_vmdq_pools, VMDQ_MAX_VIRTIO_PORTS);
+		rc = -EINVAL;
+		goto out;
+
+	}
+
+	priv->num_devices = dev_info.max_vmdq_pools;
+	priv->phys_port_count++;
+
+	priv->num_pf_queues = dev_info.max_rx_queues - dev_info.vmdq_queue_num;
+	queues_per_pool =  dev_info.vmdq_queue_num / dev_info.max_vmdq_pools;
+	priv->queues_per_pool = queues_per_pool;
+	priv->num_vmdq_queues = priv->phys_port_count * queues_per_pool;
+	priv->num_queues = priv->num_pf_queues + priv->num_vmdq_queues;
+	priv->vmdq_queue_base = dev_info.vmdq_queue_base;
+	priv->vmdq_pool_base  = dev_info.vmdq_pool_base;
+
+	rc = vmdq_get_eth_conf(vs_port, &vs_port->port_conf);
+	if (rc < 0) {
+		goto out;
+	}
+
+	/*In VMDQ vhost_switch only one physical port is required, keep it
+	 * global so that it can be accessed while doing tx/rx to physical port
+	 */
+	priv->phys_port = vs_port;
+
+
+	printf("pf queue num: %u, configured vmdq pool num: %u, each vmdq pool has %u queues\n",
+		priv->num_pf_queues, priv->num_devices, priv->queues_per_pool);
+
+	RTE_LOG(INFO, VHOST_PORT, "Max virtio devices supported: %u\n",
+		priv->num_devices);
+out:
+	if (rc) {
+		priv->phys_port_count--;
+	}
+	return rc;
+}
+
+static int vmdq_add_port_virtio(struct vswitch_port *vs_port)
+{
+	struct vmdq_switch_priv *priv = vs_port->vs_dev->priv;
+	uint16_t rxq;
+
+	rxq = vs_port->port_id * priv->queues_per_pool + priv->vmdq_queue_base;
+	vs_port->phys_port_rxq = rxq;
+
+	priv->virtio_port_map[rxq] = vs_port;
+
+	RTE_LOG(INFO, VHOST_PORT, "Added virtio port %d, vmdq rxq %d\n",
+	vs_port->port_id, rxq);
+
+	return 0;
+}
+
+static int vmdq_add_port(struct vswitch_port *port)
+{
+	int rc = 0;
+
+	switch(port->type) {
+	case VSWITCH_PTYPE_PHYS:
+		rc = vmdq_add_port_phys(port);
+		break;
+	case VSWITCH_PTYPE_VIRTIO:
+		rc = vmdq_add_port_virtio(port);
+		break;
+	default:
+		RTE_LOG(INFO, VHOST_CONFIG, "Unkown port[id %d] type %d\n",
+			port->port_id, port->type);
+		rc = -EINVAL;
+	}
+
+	return rc;
+}
+
+static int vmdq_port_start(struct vswitch_port *port)
+{
+	int rc = 0;
+
+	switch(port->type) {
+	case VSWITCH_PTYPE_PHYS:
+		rc  = rte_eth_dev_start(port->port_id);
+		RTE_LOG(INFO, VHOST_PORT, "Started PHYS port %d, rc %d\n", port->port_id, rc);
+		break;
+	case VSWITCH_PTYPE_VIRTIO:
+		/*No specefic function to start virtio dev (?? check) */
+		rc = 0;
+		break;
+	default:
+		RTE_LOG(INFO, VHOST_CONFIG, "Unkown port[id %d] type %d\n",
+			port->port_id, port->type);
+		rc = -EINVAL;
+	}
+
+
+	/* Start the device. */
+	if (rc) {
+		RTE_LOG(ERR, VHOST_PORT, "Failed to init port[id %d, typ %d]\n",
+			port->port_id, port->type);
+	}
+
+	return rc;
+}
+
+/*
+ * This function learns the MAC address of the device and registers this along with a
+ * vlan tag to a VMDQ.
+ */
+static int
+link_vmdq(struct vswitch_port *vs_port, struct rte_mbuf *m)
+{
+	struct vhost_dev *vdev = vs_port->priv;
+	struct vmdq_switch_priv *priv = vs_port->vs_dev->priv;
+	struct vswitch_dev *vs_dev = vs_port->vs_dev;
+	struct vswitch_port *phys_port = priv->phys_port;
+	struct ether_hdr *pkt_hdr;
+	int i, ret;
+
+	/* Learn MAC address of guest device from packet */
+	pkt_hdr = rte_pktmbuf_mtod(m, struct ether_hdr *);
+
+	if (find_vhost_dev(&pkt_hdr->s_addr)) {
+		RTE_LOG(ERR, VHOST_DATA,
+			"(%d) device is using a registered MAC!\n",
+			vdev->vid);
+		return -1;
+	}
+
+	for (i = 0; i < ETHER_ADDR_LEN; i++) {
+		vdev->mac_address.addr_bytes[i] = pkt_hdr->s_addr.addr_bytes[i];
+		vs_port->mac_addr.addr_bytes[i] = pkt_hdr->s_addr.addr_bytes[i];
+	}
+
+	/* vlan_tag currently uses the device_id. */
+	vdev->vlan_tag = vlan_tags[vdev->vid];
+
+	/* Print out VMDQ registration info. */
+	RTE_LOG(INFO, VHOST_DATA,
+		"(%d) mac %02x:%02x:%02x:%02x:%02x:%02x and vlan %d registered\n",
+		vdev->vid,
+		vdev->mac_address.addr_bytes[0], vdev->mac_address.addr_bytes[1],
+		vdev->mac_address.addr_bytes[2], vdev->mac_address.addr_bytes[3],
+		vdev->mac_address.addr_bytes[4], vdev->mac_address.addr_bytes[5],
+		vdev->vlan_tag);
+
+	/* Register the MAC address. */
+	ret = rte_eth_dev_mac_addr_add(phys_port->port_id, &vdev->mac_address,
+				(uint32_t)vdev->vid + priv->vmdq_pool_base);
+	if (ret)
+		RTE_LOG(ERR, VHOST_DATA,
+			"(%d) failed to add device MAC address to VMDQ\n",
+			vdev->vid);
+
+	/* Enable stripping of the vlan tag as we handle routing. */
+	if (vs_dev->conf_flags & VS_CNF_FLG_VLAN_STRIP_EN)
+		rte_eth_dev_set_vlan_strip_on_queue(phys_port->port_id,
+			(uint16_t)vs_port->phys_port_rxq, 1);
+
+	/* Set device as ready for RX. */
+	vdev->ready = DEVICE_RX;
+
+	return 0;
+}
+
+static int vmdq_learn_port (struct vswitch_port *vs_port,
+			    struct rte_mbuf **pkts,
+		     __attribute__((unused))uint16_t count)
+{
+	int rc  = 0;
+
+	if (vs_port->type == VSWITCH_PTYPE_VIRTIO)
+		rc = link_vmdq(vs_port, pkts[0]);
+
+	return rc;
+}
+
+/*
+ * Removes MAC address and vlan tag from VMDQ. Ensures that nothing is adding buffers to the RX
+ * queue before disabling RX on the device.
+ */
+static void unlink_vmdq(struct vswitch_port *vs_port)
+{
+	struct vhost_dev *vdev = vs_port->priv;
+	struct vmdq_switch_priv *priv = vs_port->vs_dev->priv;
+	struct vswitch_port *phys_port = priv->phys_port;
+	unsigned i = 0;
+	unsigned rx_count;
+	struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
+
+	if (vdev->ready == DEVICE_RX) {
+		/*clear MAC and VLAN settings*/
+		rte_eth_dev_mac_addr_remove(phys_port->port_id,
+					    &vdev->mac_address);
+		for (i = 0; i < 6; i++) {
+			vdev->mac_address.addr_bytes[i] = 0;
+			vs_port->mac_addr.addr_bytes[i] = 0;
+		}
+
+		vdev->vlan_tag = 0;
+
+		/*Clear out the receive buffers*/
+		rx_count = rte_eth_rx_burst(phys_port->port_id,
+					(uint16_t)vs_port->phys_port_rxq,
+					pkts_burst, MAX_PKT_BURST);
+
+		while (rx_count) {
+			for (i = 0; i < rx_count; i++)
+				rte_pktmbuf_free(pkts_burst[i]);
+
+			rx_count = rte_eth_rx_burst(phys_port->port_id,
+					(uint16_t)vdev->vmdq_rx_q, pkts_burst,
+					MAX_PKT_BURST);
+		}
+
+		vdev->ready = DEVICE_MAC_LEARNING;
+	}
+}
+
+static int vmdq_unlearn_port (struct vswitch_port *vs_port)
+{
+	int rc = 0;
+
+	if (vs_port->type == VSWITCH_PTYPE_VIRTIO)
+		unlink_vmdq(vs_port);
+
+	return rc;
+}
+
+/*
+ * Check if the destination MAC of a packet is one local VM,
+ * and get its vlan tag, and offset if it is.
+ */
+static inline int __attribute__((always_inline))
+find_local_dest(struct vhost_dev *vdev, struct rte_mbuf *m,
+	uint32_t *offset, uint16_t *vlan_tag)
+{
+	struct vhost_dev *dst_vdev;
+	struct ether_hdr *pkt_hdr = rte_pktmbuf_mtod(m, struct ether_hdr *);
+
+	dst_vdev = find_vhost_dev(&pkt_hdr->d_addr);
+	if (!dst_vdev)
+		return 0;
+
+	if (vdev->vid == dst_vdev->vid) {
+		RTE_LOG(DEBUG, VHOST_DATA,
+			"(%d) TX: src and dst MAC is same. Dropping packet.\n",
+			vdev->vid);
+		return -1;
+	}
+
+	/*
+	 * HW vlan strip will reduce the packet length
+	 * by minus length of vlan tag, so need restore
+	 * the packet length by plus it.
+	 */
+	*offset  = VLAN_HLEN;
+	*vlan_tag = vlan_tags[vdev->vid];
+
+	RTE_LOG(DEBUG, VHOST_DATA,
+		"(%d) TX: pkt to local VM device id: (%d), vlan tag: %u.\n",
+		vdev->vid, dst_vdev->vid, *vlan_tag);
+
+	return 0;
+}
+
+static uint16_t
+get_psd_sum(void *l3_hdr, uint64_t ol_flags)
+{
+	if (ol_flags & PKT_TX_IPV4)
+		return rte_ipv4_phdr_cksum(l3_hdr, ol_flags);
+	else /* assume ethertype == ETHER_TYPE_IPv6 */
+		return rte_ipv6_phdr_cksum(l3_hdr, ol_flags);
+}
+
+static void virtio_tx_offload(struct rte_mbuf *m)
+{
+	void *l3_hdr;
+	struct ipv4_hdr *ipv4_hdr = NULL;
+	struct tcp_hdr *tcp_hdr = NULL;
+	struct ether_hdr *eth_hdr = rte_pktmbuf_mtod(m, struct ether_hdr *);
+
+	l3_hdr = (char *)eth_hdr + m->l2_len;
+
+	if (m->ol_flags & PKT_TX_IPV4) {
+		ipv4_hdr = l3_hdr;
+		ipv4_hdr->hdr_checksum = 0;
+		m->ol_flags |= PKT_TX_IP_CKSUM;
+	}
+
+	tcp_hdr = (struct tcp_hdr *)((char *)l3_hdr + m->l3_len);
+	tcp_hdr->cksum = get_psd_sum(l3_hdr, m->ol_flags);
+}
+
+/*
+ * This function routes the TX packet to the correct interface. This
+ * may be a local device or the physical port.
+ */
+static inline void __attribute__((always_inline))
+virtio_tx_route(struct vswitch_port *vs_port,
+		struct rte_mbuf *m, uint16_t vlan_tag)
+{
+	struct vhost_dev *vdev = vs_port->priv;
+	struct vswitch_dev *vs_dev = vs_port->vs_dev;
+	struct mbuf_table *tx_q;
+	unsigned offset = 0;
+	const uint16_t lcore_id = rte_lcore_id();
+	struct ether_hdr *nh;
+	uint32_t tx_ptype_mask;
+
+	nh = rte_pktmbuf_mtod(m, struct ether_hdr *);
+	if (unlikely(is_broadcast_ether_addr(&nh->d_addr))) {
+
+		/*broad cast to virtio ports only, for physical port
+		 * qeueu2nic will do it after adding vlan tag of device
+		 */
+		tx_ptype_mask = VS_PTYPE_MASK(VSWITCH_PTYPE_VIRTIO);
+		vs_do_broadcast_fwd(vs_port->vs_dev, vs_port,
+			tx_ptype_mask, m);
+
+		goto queue2nic;
+	}
+
+	/*check if destination is local VM*/
+	if ((vs_dev->conf_flags & VS_CNF_FLG_VM2VM_SOFTWARE)) {
+		if (!virtio_tx_local(vdev, m)) {
+			rte_pktmbuf_free(m);
+			return;
+		}
+	}
+
+	if (unlikely(vs_dev->conf_flags & VS_CNF_FLG_VM2VM_HARDWARE)) {
+		if (unlikely(find_local_dest(vdev, m, &offset,
+					     &vlan_tag) != 0)) {
+			rte_pktmbuf_free(m);
+			return;
+		}
+	}
+
+	RTE_LOG(DEBUG, VHOST_DATA,
+		"(%d) TX: MAC address is external\n", vdev->vid);
+
+queue2nic:
+
+	/*Add packet to the port tx queue*/
+	tx_q = vhost_switch_get_txq(lcore_id);
+
+	nh = rte_pktmbuf_mtod(m, struct ether_hdr *);
+	if (unlikely(nh->ether_type == rte_cpu_to_be_16(ETHER_TYPE_VLAN))) {
+		/* Guest has inserted the vlan tag. */
+		struct vlan_hdr *vh = (struct vlan_hdr *) (nh + 1);
+		uint16_t vlan_tag_be = rte_cpu_to_be_16(vlan_tag);
+		if ((vs_dev->conf_flags & VS_CNF_FLG_VM2VM_HARDWARE) &&
+			(vh->vlan_tci != vlan_tag_be))
+			vh->vlan_tci = vlan_tag_be;
+	} else {
+		m->ol_flags |= PKT_TX_VLAN_PKT;
+
+		/*
+		 * Find the right seg to adjust the data len when offset is
+		 * bigger than tail room size.
+		 */
+		if (unlikely((vs_dev->conf_flags & VS_CNF_FLG_VM2VM_HARDWARE))){
+			if (likely(offset <= rte_pktmbuf_tailroom(m)))
+				m->data_len += offset;
+			else {
+				struct rte_mbuf *seg = m;
+
+				while ((seg->next != NULL) &&
+					(offset > rte_pktmbuf_tailroom(seg)))
+					seg = seg->next;
+
+				seg->data_len += offset;
+			}
+			m->pkt_len += offset;
+		}
+
+		m->vlan_tci = vlan_tag;
+	}
+
+	if (m->ol_flags & PKT_TX_TCP_SEG)
+		virtio_tx_offload(m);
+
+	tx_q->m_table[tx_q->len++] = m;
+	if (vs_dev->conf_flags & VS_CNF_FLG_STATS_EN) {
+		vdev->stats.tx_total++;
+		vdev->stats.tx++;
+	}
+
+	if (unlikely(tx_q->len == MAX_PKT_BURST))
+		do_drain_mbuf_table(tx_q);
+}
+
+static int vmdq_lookup_n_fwd_virtio(struct vswitch_port *vs_port,
+			struct rte_mbuf **pkts, uint16_t count,
+			__attribute__((unused)) uint16_t in_rxq)
+{
+	int i;
+	struct vhost_dev *vdev = vs_port->priv;
+
+	for (i = 0; i < count; ++i)
+		virtio_tx_route(vs_port, pkts[i], vlan_tags[vdev->vid]);
+
+	return 0;
+}
+
+static int vmdq_lookup_n_fwd_phys(struct vswitch_port *vs_port,
+			struct rte_mbuf **pkts, uint16_t count, uint16_t in_rxq)
+{
+	struct vswitch_port *dest_port;
+	struct vhost_dev *dest_vdev;
+	struct vmdq_switch_priv *priv = vs_port->vs_dev->priv;
+	uint16_t enqueue_count;
+
+	dest_port = priv->virtio_port_map[in_rxq];
+	dest_vdev = (struct vhost_dev *)dest_port->priv;
+	enqueue_count = dest_port->do_tx(dest_port, VIRTIO_RXQ,
+					 NULL, pkts, count);
+
+	rte_atomic64_add(&dest_vdev->stats.rx_atomic, enqueue_count);
+
+	return 0;
+}
+
+static int vmdq_lookup_n_fwd(struct vswitch_port *vs_port,
+		struct rte_mbuf **pkts, uint16_t count, uint16_t in_rxq)
+{
+	int rc;
+
+	switch(vs_port->type) {
+	case VSWITCH_PTYPE_VIRTIO:
+		rc = vmdq_lookup_n_fwd_virtio(vs_port, pkts, count, in_rxq);
+		break;
+	case VSWITCH_PTYPE_PHYS:
+		rc = vmdq_lookup_n_fwd_phys(vs_port, pkts, count, in_rxq);
+		break;
+	default:
+		rc = -EINVAL;
+		break;
+	}
+
+	return rc;
+}
+
+static struct vswitch_port *vmdq_sched_phys_port(struct vswitch_dev *vs_dev,
+			__attribute__((unused))enum vswitch_port_type ptype,
+			__attribute__((unused))uint16_t core_id)
+{
+	struct vmdq_switch_priv *priv = vs_dev->priv;
+
+	/*With VMDQ do rx/tx with the only one physical port (non virtio)*/
+
+	return priv->phys_port;
+}
+
+struct vswitch_ops vmdq_switch_ops = {
+	.add_port = vmdq_add_port,
+	.lookup_n_fwd = vmdq_lookup_n_fwd,
+	.port_start = vmdq_port_start,
+	.switch_init = vmdq_switch_init,
+	.learn_port = vmdq_learn_port,
+	.unlearn_port = vmdq_unlearn_port,
+	.sched_rx_port = vmdq_sched_phys_port,
+	.sched_tx_port = vmdq_sched_phys_port,
+	.get_max_vdevs = vmdq_get_max_vdevs,
+};
+
+void vmdq_switch_impl_init(void)
+{
+	vmdq_switch_dev_g = vs_register_switch("vmdq",
+			sizeof(struct vmdq_switch_priv), VMDQ_MAX_VIRTIO_PORTS,
+			&vmdq_switch_ops);
+	if (!vmdq_switch_dev_g) {
+		RTE_LOG(DEBUG, VHOST_CONFIG, "VMDQ switch registration failure\n");
+		goto out;
+	}
+
+out:
+	return;
+}
diff --git a/examples/vhost/vmdq.h b/examples/vhost/vmdq.h
new file mode 100644
index 0000000..bff8535
--- /dev/null
+++ b/examples/vhost/vmdq.h
@@ -0,0 +1,57 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2016 Freescale Semiconductor. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Freescale Semiconductor nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef __VHOST_VMDQ_H__
+#define __VHOST_VMDQ_H__
+
+#include "vswitch_common.h"
+
+#define VMDQ_MAX_PHYS_PORTS	1
+#define VMDQ_MAX_VIRTIO_PORTS	64
+
+struct vmdq_switch_priv {
+	struct vswitch_dev *vs_dev;
+	struct vswitch_port *phys_port;
+	struct vswitch_port *virtio_port_map[VMDQ_MAX_VIRTIO_PORTS];
+	int phys_port_count;
+	int num_devices;
+	uint16_t num_pf_queues;
+	uint16_t num_queues;
+	uint16_t num_vmdq_queues;
+	uint16_t vmdq_pool_base;
+	uint16_t vmdq_queue_base;
+	uint16_t queues_per_pool;
+	uint16_t conf_flags;
+};
+
+#endif
-- 
1.9.1