[dpdk-dev] [RFC PATCH 00/17] Single virtio implementation

[dpdk-dev] [RFC PATCH 00/17] Single virtio implementation

[Ouyang Changchun]

This is RFC patch for single virtio implementation.
 
Why we need single virtio?
============================
As we know currently there are at least 3 virtio PMD driver implementations:
A) lib/librte_pmd_virtio(refer as virtio A);
B) virtio_net_pmd by 6wind(refer as virtio B);
C) virtio by Brocade/vyatta(refer as virtio C);
 
Integrating 3 implementations into one could reduce the maintaining cost and time,
in other hand, user don't need practice their application on 3 variant one by one to see
which one is the best for them;
 
 
What's the status?
====================
Currently virtio A has covered most features of virtio B, we could regard they have
similar behavior as virtio driver. But there are some differences between
virtio A and virtio C, so it need integrate features/codes from virtio C into virtio A.
This patch set bases on two original RFC patch sets from Stephen Hemminger[stephen@networkplumber.org]
Refer to [http://dpdk.org/ml/archives/dev/2014-August/004845.html ] for the original one.
This patch set also resolves some conflict with latest codes and removed duplicated codes.
 
 
What this patch set contains:
===============================
  1) virtio: Rearrange resource initialization, it extracts a function to setup PCI resources;
  2) virtio: Use weaker barriers, as DPDK driver only has to deal with the case of running on PCI
     and with SMP, In this case, the code can use the weaker barriers instead of using hard (fence)
     barriers. This may help performance a bit;
  3) virtio: Allow starting with link down, other driver has similar behavior;
  4) virtio: Add support for Link State interrupt;
  5) ether: Add soft vlan encap/decap functions, it helps if HW don't support vlan strip;
  6) virtio: Use software vlan stripping;
  7) virtio: Remove unnecessary adapter structure;
  8) virtio: Remove redundant vq_alignment, as vq alignment is always 4K, so use constant when needed;
  9) virtio: Fix how states are handled during initialization, this is to match Linux kernel;
  10) virtio: Make vtpci_get_status a local function as it is used in one file;
  11) virtio: Check for packet headroom at compile time;
  12) virtio: Move allocation before initialization to avoid being stuck in middle of virtio init;
  13) virtio: Add support for vlan filtering;
  14) virtio: Add support for multiple mac addresses;
  15) virtio: Add ability to set MAC address;
  16) virtio: Free mbuf's with threshold, this makes its behavior more like ixgbe;
  17) virtio: Use port IO to get PCI resource for security reasons and match virtio-net-pmd.
 
Any feedback and comments for this RFC are welcome.

Changchun Ouyang (17):
  virtio: Rearrange resource initialization
  virtio: Use weaker barriers
  virtio: Allow starting with link down
  virtio: Add support for Link State interrupt
  ether: Add soft vlan encap/decap functions
  virtio: Use software vlan stripping
  virtio: Remove unnecessary adapter structure
  virtio: Remove redundant vq_alignment
  virtio: Fix how states are handled during initialization
  virtio: Make vtpci_get_status local
  virtio: Check for packet headroom at compile time
  virtio: Move allocation before initialization
  virtio: Add support for vlan filtering
  virtio: Add suport for multiple mac addresses
  virtio: Add ability to set MAC address
  virtio: Free mbuf's with threshold
  virtio: Use port IO to get PCI resource.

 lib/librte_eal/common/include/rte_pci.h |   2 +
 lib/librte_eal/linuxapp/eal/eal_pci.c   |   3 +-
 lib/librte_ether/rte_ethdev.h           |   8 +
 lib/librte_ether/rte_ether.h            |  76 +++++
 lib/librte_pmd_virtio/virtio_ethdev.c   | 479 ++++++++++++++++++++++++--------
 lib/librte_pmd_virtio/virtio_ethdev.h   |  12 +-
 lib/librte_pmd_virtio/virtio_pci.c      |  20 +-
 lib/librte_pmd_virtio/virtio_pci.h      |   8 +-
 lib/librte_pmd_virtio/virtio_rxtx.c     | 101 +++++--
 lib/librte_pmd_virtio/virtqueue.h       |  59 +++-
 10 files changed, 614 insertions(+), 154 deletions(-)

-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 01/17] virtio: Rearrange resource initialization

[Ouyang Changchun]

For clarity make the setup of PCI resources for Linux into a function rather
than block of code #ifdef'd in middle of dev_init.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_virtio/virtio_ethdev.c | 76 ++++++++++++++++++++---------------
 1 file changed, 43 insertions(+), 33 deletions(-)

diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index c009f2a..6c31598 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -794,6 +794,41 @@ virtio_has_msix(const struct rte_pci_addr *loc)
 
 	return (d != NULL);
 }
+
+/* Extract I/O port numbers from sysfs */
+static int virtio_resource_init(struct rte_pci_device *pci_dev)
+{
+	char dirname[PATH_MAX];
+	char filename[PATH_MAX];
+	unsigned long start, size;
+
+	if (get_uio_dev(&pci_dev->addr, dirname, sizeof(dirname)) < 0)
+		return -1;
+
+	/* get portio size */
+	snprintf(filename, sizeof(filename),
+		     "%s/portio/port0/size", dirname);
+	if (parse_sysfs_value(filename, &size) < 0) {
+		PMD_INIT_LOG(ERR, "%s(): cannot parse size",
+			     __func__);
+		return -1;
+	}
+
+	/* get portio start */
+	snprintf(filename, sizeof(filename),
+		 "%s/portio/port0/start", dirname);
+	if (parse_sysfs_value(filename, &start) < 0) {
+		PMD_INIT_LOG(ERR, "%s(): cannot parse portio start",
+			     __func__);
+		return -1;
+	}
+	pci_dev->mem_resource[0].addr = (void *)(uintptr_t)start;
+	pci_dev->mem_resource[0].len =  (uint64_t)size;
+	PMD_INIT_LOG(DEBUG,
+		     "PCI Port IO found start=0x%lx with size=0x%lx",
+		     start, size);
+	return 0;
+}
 #else
 static int
 virtio_has_msix(const struct rte_pci_addr *loc __rte_unused)
@@ -801,6 +836,12 @@ virtio_has_msix(const struct rte_pci_addr *loc __rte_unused)
 	/* nic_uio does not enable interrupts, return 0 (false). */
 	return 0;
 }
+
+static int virtio_resource_init(struct rte_pci_device *pci_dev __rte_unused)
+{
+	/* no setup required */
+	return 0;
+}
 #endif
 
 /*
@@ -831,40 +872,9 @@ eth_virtio_dev_init(__rte_unused struct eth_driver *eth_drv,
 		return 0;
 
 	pci_dev = eth_dev->pci_dev;
+	if (virtio_resource_init(pci_dev) < 0)
+		return -1;
 
-#ifdef RTE_EXEC_ENV_LINUXAPP
-	{
-		char dirname[PATH_MAX];
-		char filename[PATH_MAX];
-		unsigned long start, size;
-
-		if (get_uio_dev(&pci_dev->addr, dirname, sizeof(dirname)) < 0)
-			return -1;
-
-		/* get portio size */
-		snprintf(filename, sizeof(filename),
-			     "%s/portio/port0/size", dirname);
-		if (parse_sysfs_value(filename, &size) < 0) {
-			PMD_INIT_LOG(ERR, "%s(): cannot parse size",
-				     __func__);
-			return -1;
-		}
-
-		/* get portio start */
-		snprintf(filename, sizeof(filename),
-			     "%s/portio/port0/start", dirname);
-		if (parse_sysfs_value(filename, &start) < 0) {
-			PMD_INIT_LOG(ERR, "%s(): cannot parse portio start",
-				     __func__);
-			return -1;
-		}
-		pci_dev->mem_resource[0].addr = (void *)(uintptr_t)start;
-		pci_dev->mem_resource[0].len =  (uint64_t)size;
-		PMD_INIT_LOG(DEBUG,
-			     "PCI Port IO found start=0x%lx with size=0x%lx",
-			     start, size);
-	}
-#endif
 	hw->use_msix = virtio_has_msix(&pci_dev->addr);
 	hw->io_base = (uint32_t)(uintptr_t)pci_dev->mem_resource[0].addr;
 
-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 02/17] virtio: Use weaker barriers

[Ouyang Changchun]

The DPDK driver only has to deal with the case of running on PCI
and with SMP. In this case, the code can use the weaker barriers
instead of using hard (fence) barriers. This will help performance.
The rationale is explained in Linux kernel virtio_ring.h.

To make it clearer that this is a virtio thing and not some generic
barrier, prefix the barrier calls with virtio_.

Add missing (and needed) barrier between updating ring data
structure and notifying host.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_virtio/virtio_ethdev.c |  2 +-
 lib/librte_pmd_virtio/virtio_rxtx.c   |  8 +++++---
 lib/librte_pmd_virtio/virtqueue.h     | 19 ++++++++++++++-----
 3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index 6c31598..78018f9 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -175,7 +175,7 @@ virtio_send_command(struct virtqueue *vq, struct virtio_pmd_ctrl *ctrl,
 		uint32_t idx, desc_idx, used_idx;
 		struct vring_used_elem *uep;
 
-		rmb();
+		virtio_rmb();
 
 		used_idx = (uint32_t)(vq->vq_used_cons_idx
 				& (vq->vq_nentries - 1));
diff --git a/lib/librte_pmd_virtio/virtio_rxtx.c b/lib/librte_pmd_virtio/virtio_rxtx.c
index 3f6bad2..f878c62 100644
--- a/lib/librte_pmd_virtio/virtio_rxtx.c
+++ b/lib/librte_pmd_virtio/virtio_rxtx.c
@@ -456,7 +456,7 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 
 	nb_used = VIRTQUEUE_NUSED(rxvq);
 
-	rmb();
+	virtio_rmb();
 
 	num = (uint16_t)(likely(nb_used <= nb_pkts) ? nb_used : nb_pkts);
 	num = (uint16_t)(likely(num <= VIRTIO_MBUF_BURST_SZ) ? num : VIRTIO_MBUF_BURST_SZ);
@@ -516,6 +516,7 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	}
 
 	if (likely(nb_enqueued)) {
+		virtio_wmb();
 		if (unlikely(virtqueue_kick_prepare(rxvq))) {
 			virtqueue_notify(rxvq);
 			PMD_RX_LOG(DEBUG, "Notified\n");
@@ -547,7 +548,7 @@ virtio_recv_mergeable_pkts(void *rx_queue,
 
 	nb_used = VIRTQUEUE_NUSED(rxvq);
 
-	rmb();
+	virtio_rmb();
 
 	if (nb_used == 0)
 		return 0;
@@ -694,7 +695,7 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 	PMD_TX_LOG(DEBUG, "%d packets to xmit", nb_pkts);
 	nb_used = VIRTQUEUE_NUSED(txvq);
 
-	rmb();
+	virtio_rmb();
 
 	num = (uint16_t)(likely(nb_used < VIRTIO_MBUF_BURST_SZ) ? nb_used : VIRTIO_MBUF_BURST_SZ);
 
@@ -735,6 +736,7 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 		}
 	}
 	vq_update_avail_idx(txvq);
+	virtio_wmb();
 
 	txvq->packets += nb_tx;
 
diff --git a/lib/librte_pmd_virtio/virtqueue.h b/lib/librte_pmd_virtio/virtqueue.h
index fdee054..f6ad98d 100644
--- a/lib/librte_pmd_virtio/virtqueue.h
+++ b/lib/librte_pmd_virtio/virtqueue.h
@@ -46,9 +46,18 @@
 #include "virtio_ring.h"
 #include "virtio_logs.h"
 
-#define mb()  rte_mb()
-#define wmb() rte_wmb()
-#define rmb() rte_rmb()
+/*
+ * Per virtio_config.h in Linux.
+ *     For virtio_pci on SMP, we don't need to order with respect to MMIO
+ *     accesses through relaxed memory I/O windows, so smp_mb() et al are
+ *     sufficient.
+ *
+ * This driver is for virtio_pci on SMP and therefore can assume
+ * weaker (compiler barriers)
+ */
+#define virtio_mb()	rte_mb()
+#define virtio_rmb()	rte_compiler_barrier()
+#define virtio_wmb()	rte_compiler_barrier()
 
 #ifdef RTE_PMD_PACKET_PREFETCH
 #define rte_packet_prefetch(p)  rte_prefetch1(p)
@@ -225,7 +234,7 @@ virtqueue_full(const struct virtqueue *vq)
 static inline void
 vq_update_avail_idx(struct virtqueue *vq)
 {
-	rte_compiler_barrier();
+	virtio_rmb();
 	vq->vq_ring.avail->idx = vq->vq_avail_idx;
 }
 
@@ -255,7 +264,7 @@ static inline void
 virtqueue_notify(struct virtqueue *vq)
 {
 	/*
-	 * Ensure updated avail->idx is visible to host. mb() necessary?
+	 * Ensure updated avail->idx is visible to host.
 	 * For virtio on IA, the notificaiton is through io port operation
 	 * which is a serialization instruction itself.
 	 */
-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 03/17] virtio: Allow starting with link down

[Ouyang Changchun]

Starting driver with link down should be ok, it is with every
other driver. So just allow it.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_virtio/virtio_ethdev.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index 78018f9..4bff0fe 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -1057,14 +1057,12 @@ virtio_dev_start(struct rte_eth_dev *dev)
 		vtpci_read_dev_config(hw,
 				offsetof(struct virtio_net_config, status),
 				&status, sizeof(status));
-		if ((status & VIRTIO_NET_S_LINK_UP) == 0) {
+		if ((status & VIRTIO_NET_S_LINK_UP) == 0)
 			PMD_INIT_LOG(ERR, "Port: %d Link is DOWN",
 				     dev->data->port_id);
-			return -EIO;
-		} else {
+		else
 			PMD_INIT_LOG(DEBUG, "Port: %d Link is UP",
 				     dev->data->port_id);
-		}
 	}
 	vtpci_reinit_complete(hw);
 
-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 04/17] virtio: Add support for Link State interrupt

[Ouyang Changchun]

Virtio has link state interrupt which can be used.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_virtio/virtio_ethdev.c | 78 +++++++++++++++++++++++++++--------
 lib/librte_pmd_virtio/virtio_pci.c    | 22 ++++++++++
 lib/librte_pmd_virtio/virtio_pci.h    |  4 ++
 3 files changed, 86 insertions(+), 18 deletions(-)

diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index 4bff0fe..d37f2e9 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -845,6 +845,34 @@ static int virtio_resource_init(struct rte_pci_device *pci_dev __rte_unused)
 #endif
 
 /*
+ * Process Virtio Config changed interrupt and call the callback
+ * if link state changed.
+ */
+static void
+virtio_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
+			 void *param)
+{
+	struct rte_eth_dev *dev = param;
+	struct virtio_hw *hw =
+		VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint8_t isr;
+
+	/* Read interrupt status which clears interrupt */
+	isr = vtpci_isr(hw);
+	PMD_DRV_LOG(INFO, "interrupt status = %#x", isr);
+
+	if (rte_intr_enable(&dev->pci_dev->intr_handle) < 0)
+		PMD_DRV_LOG(ERR, "interrupt enable failed");
+
+	if (isr & VIRTIO_PCI_ISR_CONFIG) {
+		if (virtio_dev_link_update(dev, 0) == 0)
+			_rte_eth_dev_callback_process(dev,
+						      RTE_ETH_EVENT_INTR_LSC);
+	}
+
+}
+
+/*
  * This function is based on probe() function in virtio_pci.c
  * It returns 0 on success.
  */
@@ -968,6 +996,10 @@ eth_virtio_dev_init(__rte_unused struct eth_driver *eth_drv,
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x",
 			eth_dev->data->port_id, pci_dev->id.vendor_id,
 			pci_dev->id.device_id);
+
+	/* Setup interrupt callback  */
+	rte_intr_callback_register(&pci_dev->intr_handle,
+				   virtio_interrupt_handler, eth_dev);
 	return 0;
 }
 
@@ -975,7 +1007,7 @@ static struct eth_driver rte_virtio_pmd = {
 	{
 		.name = "rte_virtio_pmd",
 		.id_table = pci_id_virtio_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 	},
 	.eth_dev_init = eth_virtio_dev_init,
 	.dev_private_size = sizeof(struct virtio_adapter),
@@ -1021,6 +1053,9 @@ static int
 virtio_dev_configure(struct rte_eth_dev *dev)
 {
 	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
+	struct virtio_hw *hw =
+		VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	int ret;
 
 	PMD_INIT_LOG(DEBUG, "configure");
 
@@ -1029,7 +1064,11 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 		return (-EINVAL);
 	}
 
-	return 0;
+	ret = vtpci_irq_config(hw, 0);
+	if (ret != 0)
+		PMD_DRV_LOG(ERR, "failed to set config vector");
+
+	return ret;
 }
 
 
@@ -1037,7 +1076,6 @@ static int
 virtio_dev_start(struct rte_eth_dev *dev)
 {
 	uint16_t nb_queues, i;
-	uint16_t status;
 	struct virtio_hw *hw =
 		VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
@@ -1052,18 +1090,22 @@ virtio_dev_start(struct rte_eth_dev *dev)
 	/* Do final configuration before rx/tx engine starts */
 	virtio_dev_rxtx_start(dev);
 
-	/* Check VIRTIO_NET_F_STATUS for link status*/
-	if (vtpci_with_feature(hw, VIRTIO_NET_F_STATUS)) {
-		vtpci_read_dev_config(hw,
-				offsetof(struct virtio_net_config, status),
-				&status, sizeof(status));
-		if ((status & VIRTIO_NET_S_LINK_UP) == 0)
-			PMD_INIT_LOG(ERR, "Port: %d Link is DOWN",
-				     dev->data->port_id);
-		else
-			PMD_INIT_LOG(DEBUG, "Port: %d Link is UP",
-				     dev->data->port_id);
+	/* check if lsc interrupt feature is enabled */
+	if (dev->data->dev_conf.intr_conf.lsc) {
+		if (!vtpci_with_feature(hw, VIRTIO_NET_F_STATUS)) {
+			PMD_DRV_LOG(ERR, "link status not supported by host");
+			return -ENOTSUP;
+		}
+
+		if (rte_intr_enable(&dev->pci_dev->intr_handle) < 0) {
+			PMD_DRV_LOG(ERR, "interrupt enable failed");
+			return -EIO;
+		}
 	}
+
+	/* Initialize Link state */
+	virtio_dev_link_update(dev, 0);
+
 	vtpci_reinit_complete(hw);
 
 	/*Notify the backend
@@ -1145,6 +1187,7 @@ virtio_dev_stop(struct rte_eth_dev *dev)
 		VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	/* reset the NIC */
+	vtpci_irq_config(hw, 0);
 	vtpci_reset(hw);
 	virtio_dev_free_mbufs(dev);
 }
@@ -1161,6 +1204,7 @@ virtio_dev_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complet
 	old = link;
 	link.link_duplex = FULL_DUPLEX;
 	link.link_speed  = SPEED_10G;
+
 	if (vtpci_with_feature(hw, VIRTIO_NET_F_STATUS)) {
 		PMD_INIT_LOG(DEBUG, "Get link status from hw");
 		vtpci_read_dev_config(hw,
@@ -1179,10 +1223,8 @@ virtio_dev_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complet
 		link.link_status = 1;   /* Link up */
 	}
 	virtio_dev_atomic_write_link_status(dev, &link);
-	if (old.link_status == link.link_status)
-		return -1;
-	/*changed*/
-	return 0;
+
+	return (old.link_status == link.link_status) ? -1 : 0;
 }
 
 static void
diff --git a/lib/librte_pmd_virtio/virtio_pci.c b/lib/librte_pmd_virtio/virtio_pci.c
index ca9c748..6d51032 100644
--- a/lib/librte_pmd_virtio/virtio_pci.c
+++ b/lib/librte_pmd_virtio/virtio_pci.c
@@ -127,3 +127,25 @@ vtpci_set_status(struct virtio_hw *hw, uint8_t status)
 
 	VIRTIO_WRITE_REG_1(hw, VIRTIO_PCI_STATUS, status);
 }
+
+uint8_t
+vtpci_isr(struct virtio_hw *hw)
+{
+
+	return VIRTIO_READ_REG_1(hw, VIRTIO_PCI_ISR);
+}
+
+
+/* Enable one vector (0) for Link State Intrerrupt */
+int
+vtpci_irq_config(struct virtio_hw *hw, uint16_t vec)
+{
+	VIRTIO_WRITE_REG_2(hw, VIRTIO_MSI_CONFIG_VECTOR, vec);
+	vec = VIRTIO_READ_REG_2(hw, VIRTIO_MSI_CONFIG_VECTOR);
+	if (vec == VIRTIO_MSI_NO_VECTOR) {
+		PMD_DRV_LOG(ERR, "failed to set config vector");
+		return -EBUSY;
+	}
+
+	return 0;
+}
diff --git a/lib/librte_pmd_virtio/virtio_pci.h b/lib/librte_pmd_virtio/virtio_pci.h
index 373f9dc..6998737 100644
--- a/lib/librte_pmd_virtio/virtio_pci.h
+++ b/lib/librte_pmd_virtio/virtio_pci.h
@@ -263,4 +263,8 @@ void vtpci_write_dev_config(struct virtio_hw *, uint64_t, void *, int);
 
 void vtpci_read_dev_config(struct virtio_hw *, uint64_t, void *, int);
 
+uint8_t vtpci_isr(struct virtio_hw *);
+
+int vtpci_irq_config(struct virtio_hw *, uint16_t);
+
 #endif /* _VIRTIO_PCI_H_ */
-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 05/17] ether: Add soft vlan encap/decap functions

[Ouyang Changchun]

It is helpful to allow device drivers that don't support hardware
VLAN stripping to emulate this in software. This allows application
to be device independent.

Avoid discarding shared mbufs. Make a copy in rte_vlan_insert() of any
packet to be tagged that has a reference count > 1.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_ether/rte_ether.h | 76 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/lib/librte_ether/rte_ether.h b/lib/librte_ether/rte_ether.h
index 187608d..3b6ab4b 100644
--- a/lib/librte_ether/rte_ether.h
+++ b/lib/librte_ether/rte_ether.h
@@ -49,6 +49,8 @@ extern "C" {
 
 #include <rte_memcpy.h>
 #include <rte_random.h>
+#include <rte_mbuf.h>
+#include <rte_byteorder.h>
 
 #define ETHER_ADDR_LEN  6 /**< Length of Ethernet address. */
 #define ETHER_TYPE_LEN  2 /**< Length of Ethernet type field. */
@@ -332,6 +334,80 @@ struct vxlan_hdr {
 #define ETHER_VXLAN_HLEN (sizeof(struct udp_hdr) + sizeof(struct vxlan_hdr))
 /**< VXLAN tunnel header length. */
 
+/**
+ * Extract VLAN tag information into mbuf
+ *
+ * Software version of VLAN stripping
+ *
+ * @param m
+ *   The packet mbuf.
+ * @return
+ *   - 0: Success
+ *   - 1: not a vlan packet
+ */
+static inline int rte_vlan_strip(struct rte_mbuf *m)
+{
+	struct ether_hdr *eh
+		 = rte_pktmbuf_mtod(m, struct ether_hdr *);
+
+	if (eh->ether_type != ETHER_TYPE_VLAN)
+		return -1;
+
+	struct vlan_hdr *vh = (struct vlan_hdr *)(eh + 1);
+	m->ol_flags |= PKT_RX_VLAN_PKT;
+	m->vlan_tci = rte_be_to_cpu_16(vh->vlan_tci);
+
+	/* Copy ether header over rather than moving whole packet */
+	memmove(rte_pktmbuf_adj(m, sizeof(struct vlan_hdr)),
+		eh, 2 * ETHER_ADDR_LEN);
+
+	return 0;
+}
+
+/**
+ * Insert VLAN tag into mbuf.
+ *
+ * Software version of VLAN unstripping
+ *
+ * @param m
+ *   The packet mbuf.
+ * @return
+ *   - 0: On success
+ *   -EPERM: mbuf is is shared overwriting would be unsafe
+ *   -ENOSPC: not enough headroom in mbuf
+ */
+static inline int rte_vlan_insert(struct rte_mbuf **m)
+{
+	struct ether_hdr *oh, *nh;
+	struct vlan_hdr *vh;
+
+#ifdef RTE_MBUF_REFCNT
+	/* Can't insert header if mbuf is shared */
+	if (rte_mbuf_refcnt_read(*m) > 1) {
+		struct rte_mbuf *copy;
+
+		copy = rte_pktmbuf_clone(*m, (*m)->pool);
+		if (unlikely(copy == NULL))
+			return -ENOMEM;
+		rte_pktmbuf_free(*m);
+		*m = copy;
+	}
+#endif
+	oh = rte_pktmbuf_mtod(*m, struct ether_hdr *);
+	nh = (struct ether_hdr *)
+		rte_pktmbuf_prepend(*m, sizeof(struct vlan_hdr));
+	if (nh == NULL)
+		return -ENOSPC;
+
+	memmove(nh, oh, 2 * ETHER_ADDR_LEN);
+	nh->ether_type = ETHER_TYPE_VLAN;
+
+	vh = (struct vlan_hdr *) (nh + 1);
+	vh->vlan_tci = rte_cpu_to_be_16((*m)->vlan_tci);
+
+	return 0;
+}
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 06/17] virtio: Use software vlan stripping

[Ouyang Changchun]

Implement VLAN stripping in software. This allows application
to be device independent.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_ether/rte_ethdev.h         |  3 +++
 lib/librte_pmd_virtio/virtio_ethdev.c |  2 ++
 lib/librte_pmd_virtio/virtio_pci.h    |  1 +
 lib/librte_pmd_virtio/virtio_rxtx.c   | 20 ++++++++++++++++++--
 4 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index f66805d..07d55b8 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -643,6 +643,9 @@ struct rte_eth_rxconf {
 #define ETH_TXQ_FLAGS_NOOFFLOADS \
 		(ETH_TXQ_FLAGS_NOVLANOFFL | ETH_TXQ_FLAGS_NOXSUMSCTP | \
 		 ETH_TXQ_FLAGS_NOXSUMUDP  | ETH_TXQ_FLAGS_NOXSUMTCP)
+#define ETH_TXQ_FLAGS_NOXSUMS \
+		(ETH_TXQ_FLAGS_NOXSUMSCTP | ETH_TXQ_FLAGS_NOXSUMUDP | \
+		 ETH_TXQ_FLAGS_NOXSUMTCP)
 /**
  * A structure used to configure a TX ring of an Ethernet port.
  */
diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index d37f2e9..829838c 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -1064,6 +1064,8 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 		return (-EINVAL);
 	}
 
+	hw->vlan_strip = rxmode->hw_vlan_strip;
+
 	ret = vtpci_irq_config(hw, 0);
 	if (ret != 0)
 		PMD_DRV_LOG(ERR, "failed to set config vector");
diff --git a/lib/librte_pmd_virtio/virtio_pci.h b/lib/librte_pmd_virtio/virtio_pci.h
index 6998737..6d93fac 100644
--- a/lib/librte_pmd_virtio/virtio_pci.h
+++ b/lib/librte_pmd_virtio/virtio_pci.h
@@ -168,6 +168,7 @@ struct virtio_hw {
 	uint32_t    max_tx_queues;
 	uint32_t    max_rx_queues;
 	uint16_t    vtnet_hdr_size;
+	uint8_t	    vlan_strip;
 	uint8_t	    use_msix;
 	uint8_t     mac_addr[ETHER_ADDR_LEN];
 };
diff --git a/lib/librte_pmd_virtio/virtio_rxtx.c b/lib/librte_pmd_virtio/virtio_rxtx.c
index f878c62..a5756e1 100644
--- a/lib/librte_pmd_virtio/virtio_rxtx.c
+++ b/lib/librte_pmd_virtio/virtio_rxtx.c
@@ -49,6 +49,7 @@
 #include <rte_prefetch.h>
 #include <rte_string_fns.h>
 #include <rte_errno.h>
+#include <rte_byteorder.h>
 
 #include "virtio_logs.h"
 #include "virtio_ethdev.h"
@@ -408,8 +409,8 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	if ((tx_conf->txq_flags & ETH_TXQ_FLAGS_NOOFFLOADS)
-	    != ETH_TXQ_FLAGS_NOOFFLOADS) {
+	if ((tx_conf->txq_flags & ETH_TXQ_FLAGS_NOXSUMS)
+	    != ETH_TXQ_FLAGS_NOXSUMS) {
 		PMD_INIT_LOG(ERR, "TX checksum offload not supported\n");
 		return -EINVAL;
 	}
@@ -446,6 +447,7 @@ uint16_t
 virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 {
 	struct virtqueue *rxvq = rx_queue;
+	struct virtio_hw *hw = rxvq->hw;
 	struct rte_mbuf *rxm, *new_mbuf;
 	uint16_t nb_used, num, nb_rx = 0;
 	uint32_t len[VIRTIO_MBUF_BURST_SZ];
@@ -489,6 +491,9 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		rxm->pkt_len = (uint32_t)(len[i] - hdr_size);
 		rxm->data_len = (uint16_t)(len[i] - hdr_size);
 
+		if (hw->vlan_strip)
+			rte_vlan_strip(rxm);
+
 		VIRTIO_DUMP_PACKET(rxm, rxm->data_len);
 
 		rx_pkts[nb_rx++] = rxm;
@@ -717,6 +722,17 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 		 */
 		if (likely(need <= 0)) {
 			txm = tx_pkts[nb_tx];
+
+			/* Do VLAN tag insertion */
+			if (txm->ol_flags & PKT_TX_VLAN_PKT) {
+				error = rte_vlan_insert(&txm);
+				if (unlikely(error)) {
+					rte_pktmbuf_free(txm);
+					++nb_tx;
+					continue;
+				}
+			}
+
 			/* Enqueue Packet buffers */
 			error = virtqueue_enqueue_xmit(txvq, txm);
 			if (unlikely(error)) {
-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 07/17] virtio: Remove unnecessary adapter structure

[Ouyang Changchun]

Cleanup virtio code by eliminating unnecessary nesting of
virtio hardware structure inside adapter structure.
Also allows removing unneeded macro, making code clearer.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_virtio/virtio_ethdev.c | 43 ++++++++++++-----------------------
 lib/librte_pmd_virtio/virtio_ethdev.h |  9 --------
 lib/librte_pmd_virtio/virtio_rxtx.c   |  3 +--
 3 files changed, 16 insertions(+), 39 deletions(-)

diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index 829838c..c89614d 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -207,8 +207,7 @@ virtio_send_command(struct virtqueue *vq, struct virtio_pmd_ctrl *ctrl,
 static int
 virtio_set_multiple_queues(struct rte_eth_dev *dev, uint16_t nb_queues)
 {
-	struct virtio_hw *hw
-		= VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct virtio_hw *hw = dev->data->dev_private;
 	struct virtio_pmd_ctrl ctrl;
 	int dlen[1];
 	int ret;
@@ -242,8 +241,7 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
 	const struct rte_memzone *mz;
 	uint16_t vq_size;
 	int size;
-	struct virtio_hw *hw =
-		VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct virtio_hw *hw = dev->data->dev_private;
 	struct virtqueue  *vq = NULL;
 
 	/* Write the virtqueue index to the Queue Select Field */
@@ -383,8 +381,7 @@ virtio_dev_cq_queue_setup(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx,
 	struct virtqueue *vq;
 	uint16_t nb_desc = 0;
 	int ret;
-	struct virtio_hw *hw =
-		VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct virtio_hw *hw = dev->data->dev_private;
 
 	PMD_INIT_FUNC_TRACE();
 	ret = virtio_dev_queue_setup(dev, VTNET_CQ, VTNET_SQ_CQ_QUEUE_IDX,
@@ -410,8 +407,7 @@ virtio_dev_close(struct rte_eth_dev *dev)
 static void
 virtio_dev_promiscuous_enable(struct rte_eth_dev *dev)
 {
-	struct virtio_hw *hw
-		= VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct virtio_hw *hw = dev->data->dev_private;
 	struct virtio_pmd_ctrl ctrl;
 	int dlen[1];
 	int ret;
@@ -430,8 +426,7 @@ virtio_dev_promiscuous_enable(struct rte_eth_dev *dev)
 static void
 virtio_dev_promiscuous_disable(struct rte_eth_dev *dev)
 {
-	struct virtio_hw *hw
-		= VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct virtio_hw *hw = dev->data->dev_private;
 	struct virtio_pmd_ctrl ctrl;
 	int dlen[1];
 	int ret;
@@ -450,8 +445,7 @@ virtio_dev_promiscuous_disable(struct rte_eth_dev *dev)
 static void
 virtio_dev_allmulticast_enable(struct rte_eth_dev *dev)
 {
-	struct virtio_hw *hw
-		= VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct virtio_hw *hw = dev->data->dev_private;
 	struct virtio_pmd_ctrl ctrl;
 	int dlen[1];
 	int ret;
@@ -470,8 +464,7 @@ virtio_dev_allmulticast_enable(struct rte_eth_dev *dev)
 static void
 virtio_dev_allmulticast_disable(struct rte_eth_dev *dev)
 {
-	struct virtio_hw *hw
-		= VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct virtio_hw *hw = dev->data->dev_private;
 	struct virtio_pmd_ctrl ctrl;
 	int dlen[1];
 	int ret;
@@ -853,8 +846,7 @@ virtio_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
 			 void *param)
 {
 	struct rte_eth_dev *dev = param;
-	struct virtio_hw *hw =
-		VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct virtio_hw *hw = dev->data->dev_private;
 	uint8_t isr;
 
 	/* Read interrupt status which clears interrupt */
@@ -880,12 +872,11 @@ static int
 eth_virtio_dev_init(__rte_unused struct eth_driver *eth_drv,
 		struct rte_eth_dev *eth_dev)
 {
+	struct virtio_hw *hw = eth_dev->data->dev_private;
 	struct virtio_net_config *config;
 	struct virtio_net_config local_config;
 	uint32_t offset_conf = sizeof(config->mac);
 	struct rte_pci_device *pci_dev;
-	struct virtio_hw *hw =
-		VIRTIO_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
 
 	if (RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr)) {
 		PMD_INIT_LOG(ERR,
@@ -1010,7 +1001,7 @@ static struct eth_driver rte_virtio_pmd = {
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 	},
 	.eth_dev_init = eth_virtio_dev_init,
-	.dev_private_size = sizeof(struct virtio_adapter),
+	.dev_private_size = sizeof(struct virtio_hw),
 };
 
 /*
@@ -1053,8 +1044,7 @@ static int
 virtio_dev_configure(struct rte_eth_dev *dev)
 {
 	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
-	struct virtio_hw *hw =
-		VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct virtio_hw *hw = dev->data->dev_private;
 	int ret;
 
 	PMD_INIT_LOG(DEBUG, "configure");
@@ -1078,8 +1068,7 @@ static int
 virtio_dev_start(struct rte_eth_dev *dev)
 {
 	uint16_t nb_queues, i;
-	struct virtio_hw *hw =
-		VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct virtio_hw *hw = dev->data->dev_private;
 
 	/* Tell the host we've noticed this device. */
 	vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_ACK);
@@ -1185,8 +1174,7 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev)
 static void
 virtio_dev_stop(struct rte_eth_dev *dev)
 {
-	struct virtio_hw *hw =
-		VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct virtio_hw *hw = dev->data->dev_private;
 
 	/* reset the NIC */
 	vtpci_irq_config(hw, 0);
@@ -1199,8 +1187,7 @@ virtio_dev_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complet
 {
 	struct rte_eth_link link, old;
 	uint16_t status;
-	struct virtio_hw *hw =
-		VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct virtio_hw *hw = dev->data->dev_private;
 	memset(&link, 0, sizeof(link));
 	virtio_dev_atomic_read_link_status(dev, &link);
 	old = link;
@@ -1232,7 +1219,7 @@ virtio_dev_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complet
 static void
 virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
-	struct virtio_hw *hw = VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct virtio_hw *hw = dev->data->dev_private;
 
 	dev_info->driver_name = dev->driver->pci_drv.name;
 	dev_info->max_rx_queues = (uint16_t)hw->max_rx_queues;
diff --git a/lib/librte_pmd_virtio/virtio_ethdev.h b/lib/librte_pmd_virtio/virtio_ethdev.h
index 1da3c62..55c9749 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.h
+++ b/lib/librte_pmd_virtio/virtio_ethdev.h
@@ -110,15 +110,6 @@ uint16_t virtio_recv_mergeable_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 uint16_t virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		uint16_t nb_pkts);
 
-/*
- * Structure to store private data for each driver instance (for each port).
- */
-struct virtio_adapter {
-	struct virtio_hw hw;
-};
-
-#define VIRTIO_DEV_PRIVATE_TO_HW(adapter)\
-	(&((struct virtio_adapter *)adapter)->hw)
 
 /*
  * The VIRTIO_NET_F_GUEST_TSO[46] features permit the host to send us
diff --git a/lib/librte_pmd_virtio/virtio_rxtx.c b/lib/librte_pmd_virtio/virtio_rxtx.c
index a5756e1..73ad3ac 100644
--- a/lib/librte_pmd_virtio/virtio_rxtx.c
+++ b/lib/librte_pmd_virtio/virtio_rxtx.c
@@ -326,8 +326,7 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 void
 virtio_dev_cq_start(struct rte_eth_dev *dev)
 {
-	struct virtio_hw *hw
-		= VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct virtio_hw *hw = dev->data->dev_private;
 
 	if (hw->cvq) {
 		virtio_dev_vring_start(hw->cvq, VTNET_CQ);
-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 08/17] virtio: Remove redundant vq_alignment

[Ouyang Changchun]

Since vq_alignment is constant (always 4K), it does not
need to be part of the vring struct.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_virtio/virtio_ethdev.c | 1 -
 lib/librte_pmd_virtio/virtio_rxtx.c   | 2 +-
 lib/librte_pmd_virtio/virtqueue.h     | 3 +--
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index c89614d..b7f65b9 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -294,7 +294,6 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
 	vq->port_id = dev->data->port_id;
 	vq->queue_id = queue_idx;
 	vq->vq_queue_index = vtpci_queue_idx;
-	vq->vq_alignment = VIRTIO_PCI_VRING_ALIGN;
 	vq->vq_nentries = vq_size;
 	vq->vq_free_cnt = vq_size;
 
diff --git a/lib/librte_pmd_virtio/virtio_rxtx.c b/lib/librte_pmd_virtio/virtio_rxtx.c
index 73ad3ac..b44f091 100644
--- a/lib/librte_pmd_virtio/virtio_rxtx.c
+++ b/lib/librte_pmd_virtio/virtio_rxtx.c
@@ -258,7 +258,7 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 	 * Reinitialise since virtio port might have been stopped and restarted
 	 */
 	memset(vq->vq_ring_virt_mem, 0, vq->vq_ring_size);
-	vring_init(vr, size, ring_mem, vq->vq_alignment);
+	vring_init(vr, size, ring_mem, VIRTIO_PCI_VRING_ALIGN);
 	vq->vq_used_cons_idx = 0;
 	vq->vq_desc_head_idx = 0;
 	vq->vq_avail_idx = 0;
diff --git a/lib/librte_pmd_virtio/virtqueue.h b/lib/librte_pmd_virtio/virtqueue.h
index f6ad98d..5b8a255 100644
--- a/lib/librte_pmd_virtio/virtqueue.h
+++ b/lib/librte_pmd_virtio/virtqueue.h
@@ -138,8 +138,7 @@ struct virtqueue {
 	uint8_t     port_id;              /**< Device port identifier. */
 
 	void        *vq_ring_virt_mem;    /**< linear address of vring*/
-	int         vq_alignment;
-	int         vq_ring_size;
+	unsigned int vq_ring_size;
 	phys_addr_t vq_ring_mem;          /**< physical address of vring */
 
 	struct vring vq_ring;    /**< vring keeping desc, used and avail */
-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 09/17] virtio: Fix how states are handled during initialization

[Ouyang Changchun]

Change order of initialiazation to match Linux kernel.
Don't blow away control queue by doing reset when stopped.

Calling dev_stop then dev_start would not work.
Dev_stop was calling virtio reset and that would clear all queues
and clear all feature negotiation.
Resolved by only doing reset on device removal.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_virtio/virtio_ethdev.c | 58 ++++++++++++++++++++---------------
 lib/librte_pmd_virtio/virtio_pci.c    | 10 ++----
 lib/librte_pmd_virtio/virtio_pci.h    |  3 +-
 3 files changed, 37 insertions(+), 34 deletions(-)

diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index b7f65b9..a07f4ca 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -398,9 +398,14 @@ virtio_dev_cq_queue_setup(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx,
 static void
 virtio_dev_close(struct rte_eth_dev *dev)
 {
+	struct virtio_hw *hw = dev->data->dev_private;
+
 	PMD_INIT_LOG(DEBUG, "virtio_dev_close");
 
-	virtio_dev_stop(dev);
+	/* reset the NIC */
+	vtpci_irq_config(hw, VIRTIO_MSI_NO_VECTOR);
+	vtpci_reset(hw);
+	virtio_dev_free_mbufs(dev);
 }
 
 static void
@@ -889,6 +894,9 @@ eth_virtio_dev_init(__rte_unused struct eth_driver *eth_drv,
 	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
 		return 0;
 
+	/* Tell the host we've noticed this device. */
+	vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_ACK);
+
 	pci_dev = eth_dev->pci_dev;
 	if (virtio_resource_init(pci_dev) < 0)
 		return -1;
@@ -899,9 +907,6 @@ eth_virtio_dev_init(__rte_unused struct eth_driver *eth_drv,
 	/* Reset the device although not necessary at startup */
 	vtpci_reset(hw);
 
-	/* Tell the host we've noticed this device. */
-	vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_ACK);
-
 	/* Tell the host we've known how to drive the device. */
 	vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_DRIVER);
 	virtio_negotiate_features(hw);
@@ -990,6 +995,9 @@ eth_virtio_dev_init(__rte_unused struct eth_driver *eth_drv,
 	/* Setup interrupt callback  */
 	rte_intr_callback_register(&pci_dev->intr_handle,
 				   virtio_interrupt_handler, eth_dev);
+
+	virtio_dev_cq_start(eth_dev);
+
 	return 0;
 }
 
@@ -1044,7 +1052,6 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 {
 	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
 	struct virtio_hw *hw = dev->data->dev_private;
-	int ret;
 
 	PMD_INIT_LOG(DEBUG, "configure");
 
@@ -1055,11 +1062,12 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 
 	hw->vlan_strip = rxmode->hw_vlan_strip;
 
-	ret = vtpci_irq_config(hw, 0);
-	if (ret != 0)
+	if (vtpci_irq_config(hw, 0) == VIRTIO_MSI_NO_VECTOR) {
 		PMD_DRV_LOG(ERR, "failed to set config vector");
+		return -EBUSY;
+	}
 
-	return ret;
+	return 0;
 }
 
 
@@ -1069,17 +1077,6 @@ virtio_dev_start(struct rte_eth_dev *dev)
 	uint16_t nb_queues, i;
 	struct virtio_hw *hw = dev->data->dev_private;
 
-	/* Tell the host we've noticed this device. */
-	vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_ACK);
-
-	/* Tell the host we've known how to drive the device. */
-	vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_DRIVER);
-
-	virtio_dev_cq_start(dev);
-
-	/* Do final configuration before rx/tx engine starts */
-	virtio_dev_rxtx_start(dev);
-
 	/* check if lsc interrupt feature is enabled */
 	if (dev->data->dev_conf.intr_conf.lsc) {
 		if (!vtpci_with_feature(hw, VIRTIO_NET_F_STATUS)) {
@@ -1096,8 +1093,16 @@ virtio_dev_start(struct rte_eth_dev *dev)
 	/* Initialize Link state */
 	virtio_dev_link_update(dev, 0);
 
+	/* On restart after stop do not touch queues */
+	if (hw->started)
+		return 0;
+
 	vtpci_reinit_complete(hw);
 
+	/* Do final configuration before rx/tx engine starts */
+	virtio_dev_rxtx_start(dev);
+	hw->started = 1;
+
 	/*Notify the backend
 	 *Otherwise the tap backend might already stop its queue due to fullness.
 	 *vhost backend will have no chance to be waked up
@@ -1168,17 +1173,20 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev)
 }
 
 /*
- * Stop device: disable rx and tx functions to allow for reconfiguring.
+ * Stop device: disable interrupt and mark link down
  */
 static void
 virtio_dev_stop(struct rte_eth_dev *dev)
 {
-	struct virtio_hw *hw = dev->data->dev_private;
+	struct rte_eth_link link;
 
-	/* reset the NIC */
-	vtpci_irq_config(hw, 0);
-	vtpci_reset(hw);
-	virtio_dev_free_mbufs(dev);
+	PMD_INIT_LOG(DEBUG, "stop");
+
+	if (dev->data->dev_conf.intr_conf.lsc)
+		rte_intr_disable(&dev->pci_dev->intr_handle);
+
+	memset(&link, 0, sizeof(link));
+	virtio_dev_atomic_write_link_status(dev, &link);
 }
 
 static int
diff --git a/lib/librte_pmd_virtio/virtio_pci.c b/lib/librte_pmd_virtio/virtio_pci.c
index 6d51032..b099e4f 100644
--- a/lib/librte_pmd_virtio/virtio_pci.c
+++ b/lib/librte_pmd_virtio/virtio_pci.c
@@ -137,15 +137,9 @@ vtpci_isr(struct virtio_hw *hw)
 
 
 /* Enable one vector (0) for Link State Intrerrupt */
-int
+uint16_t
 vtpci_irq_config(struct virtio_hw *hw, uint16_t vec)
 {
 	VIRTIO_WRITE_REG_2(hw, VIRTIO_MSI_CONFIG_VECTOR, vec);
-	vec = VIRTIO_READ_REG_2(hw, VIRTIO_MSI_CONFIG_VECTOR);
-	if (vec == VIRTIO_MSI_NO_VECTOR) {
-		PMD_DRV_LOG(ERR, "failed to set config vector");
-		return -EBUSY;
-	}
-
-	return 0;
+	return VIRTIO_READ_REG_2(hw, VIRTIO_MSI_CONFIG_VECTOR);
 }
diff --git a/lib/librte_pmd_virtio/virtio_pci.h b/lib/librte_pmd_virtio/virtio_pci.h
index 6d93fac..0a4b578 100644
--- a/lib/librte_pmd_virtio/virtio_pci.h
+++ b/lib/librte_pmd_virtio/virtio_pci.h
@@ -170,6 +170,7 @@ struct virtio_hw {
 	uint16_t    vtnet_hdr_size;
 	uint8_t	    vlan_strip;
 	uint8_t	    use_msix;
+	uint8_t     started;
 	uint8_t     mac_addr[ETHER_ADDR_LEN];
 };
 
@@ -266,6 +267,6 @@ void vtpci_read_dev_config(struct virtio_hw *, uint64_t, void *, int);
 
 uint8_t vtpci_isr(struct virtio_hw *);
 
-int vtpci_irq_config(struct virtio_hw *, uint16_t);
+uint16_t vtpci_irq_config(struct virtio_hw *, uint16_t);
 
 #endif /* _VIRTIO_PCI_H_ */
-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 10/17] virtio: Make vtpci_get_status local

[Ouyang Changchun]

Make vtpci_get_status a local function as it is used in one file.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_virtio/virtio_pci.c | 4 +++-
 lib/librte_pmd_virtio/virtio_pci.h | 2 --
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/librte_pmd_virtio/virtio_pci.c b/lib/librte_pmd_virtio/virtio_pci.c
index b099e4f..2245bec 100644
--- a/lib/librte_pmd_virtio/virtio_pci.c
+++ b/lib/librte_pmd_virtio/virtio_pci.c
@@ -35,6 +35,8 @@
 #include "virtio_pci.h"
 #include "virtio_logs.h"
 
+static uint8_t vtpci_get_status(struct virtio_hw *);
+
 void
 vtpci_read_dev_config(struct virtio_hw *hw, uint64_t offset,
 		void *dst, int length)
@@ -113,7 +115,7 @@ vtpci_reinit_complete(struct virtio_hw *hw)
 	vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_DRIVER_OK);
 }
 
-uint8_t
+static uint8_t
 vtpci_get_status(struct virtio_hw *hw)
 {
 	return VIRTIO_READ_REG_1(hw, VIRTIO_PCI_STATUS);
diff --git a/lib/librte_pmd_virtio/virtio_pci.h b/lib/librte_pmd_virtio/virtio_pci.h
index 0a4b578..64d9c34 100644
--- a/lib/librte_pmd_virtio/virtio_pci.h
+++ b/lib/librte_pmd_virtio/virtio_pci.h
@@ -255,8 +255,6 @@ void vtpci_reset(struct virtio_hw *);
 
 void vtpci_reinit_complete(struct virtio_hw *);
 
-uint8_t vtpci_get_status(struct virtio_hw *);
-
 void vtpci_set_status(struct virtio_hw *, uint8_t);
 
 uint32_t vtpci_negotiate_features(struct virtio_hw *, uint32_t);
-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 11/17] virtio: Check for packet headroom at compile time

[Ouyang Changchun]

Better to check at compile time than fail at runtime.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_virtio/virtio_ethdev.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index a07f4ca..c17cac8 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -882,11 +882,7 @@ eth_virtio_dev_init(__rte_unused struct eth_driver *eth_drv,
 	uint32_t offset_conf = sizeof(config->mac);
 	struct rte_pci_device *pci_dev;
 
-	if (RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr)) {
-		PMD_INIT_LOG(ERR,
-			"MBUF HEADROOM should be enough to hold virtio net hdr\n");
-		return -1;
-	}
+	RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr));
 
 	eth_dev->dev_ops = &virtio_eth_dev_ops;
 	eth_dev->tx_pkt_burst = &virtio_xmit_pkts;
-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 12/17] virtio: Move allocation before initialization

[Ouyang Changchun]

If allocation fails, don't want to leave virtio device stuck
in middle of initialization sequence.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_virtio/virtio_ethdev.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index c17cac8..13feda5 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -890,6 +890,15 @@ eth_virtio_dev_init(__rte_unused struct eth_driver *eth_drv,
 	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
 		return 0;
 
+	/* Allocate memory for storing MAC addresses */
+	eth_dev->data->mac_addrs = rte_zmalloc("virtio", ETHER_ADDR_LEN, 0);
+	if (eth_dev->data->mac_addrs == NULL) {
+		PMD_INIT_LOG(ERR,
+			"Failed to allocate %d bytes needed to store MAC addresses",
+			ETHER_ADDR_LEN);
+		return -ENOMEM;
+	}
+
 	/* Tell the host we've noticed this device. */
 	vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_ACK);
 
@@ -916,15 +925,6 @@ eth_virtio_dev_init(__rte_unused struct eth_driver *eth_drv,
 		hw->vtnet_hdr_size = sizeof(struct virtio_net_hdr);
 	}
 
-	/* Allocate memory for storing MAC addresses */
-	eth_dev->data->mac_addrs = rte_zmalloc("virtio", ETHER_ADDR_LEN, 0);
-	if (eth_dev->data->mac_addrs == NULL) {
-		PMD_INIT_LOG(ERR,
-			"Failed to allocate %d bytes needed to store MAC addresses",
-			ETHER_ADDR_LEN);
-		return -ENOMEM;
-	}
-
 	/* Copy the permanent MAC address to: virtio_hw */
 	virtio_get_hwaddr(hw);
 	ether_addr_copy((struct ether_addr *) hw->mac_addr,
-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 13/17] virtio: Add support for vlan filtering

[Ouyang Changchun]

Virtio supports vlan filtering.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_virtio/virtio_ethdev.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index 13feda5..ec5a51e 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -84,6 +84,8 @@ static void virtio_dev_tx_queue_release(__rte_unused void *txq);
 static void virtio_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats);
 static void virtio_dev_stats_reset(struct rte_eth_dev *dev);
 static void virtio_dev_free_mbufs(struct rte_eth_dev *dev);
+static int virtio_vlan_filter_set(struct rte_eth_dev *dev,
+				uint16_t vlan_id, int on);
 
 static int virtio_dev_queue_stats_mapping_set(
 	__rte_unused struct rte_eth_dev *eth_dev,
@@ -511,6 +513,7 @@ static struct eth_dev_ops virtio_eth_dev_ops = {
 	.tx_queue_release        = virtio_dev_tx_queue_release,
 	/* collect stats per queue */
 	.queue_stats_mapping_set = virtio_dev_queue_stats_mapping_set,
+	.vlan_filter_set         = virtio_vlan_filter_set,
 };
 
 static inline int
@@ -640,14 +643,31 @@ virtio_get_hwaddr(struct virtio_hw *hw)
 	}
 }
 
+static int
+virtio_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
+{
+	struct virtio_hw *hw = dev->data->dev_private;
+	struct virtio_pmd_ctrl ctrl;
+	int len;
+
+	if (!vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VLAN))
+		return -ENOTSUP;
+
+	ctrl.hdr.class = VIRTIO_NET_CTRL_VLAN;
+	ctrl.hdr.cmd = on ? VIRTIO_NET_CTRL_VLAN_ADD : VIRTIO_NET_CTRL_VLAN_DEL;
+	memcpy(ctrl.data, &vlan_id, sizeof(vlan_id));
+	len = sizeof(vlan_id);
+
+	return virtio_send_command(hw->cvq, &ctrl, &len, 1);
+}
 
 static void
 virtio_negotiate_features(struct virtio_hw *hw)
 {
 	uint32_t host_features, mask;
 
-	mask = VIRTIO_NET_F_CTRL_VLAN;
-	mask |= VIRTIO_NET_F_CSUM | VIRTIO_NET_F_GUEST_CSUM;
+	/* checksum offload not implemented */
+	mask = VIRTIO_NET_F_CSUM | VIRTIO_NET_F_GUEST_CSUM;
 
 	/* TSO and LRO are only available when their corresponding
 	 * checksum offload feature is also negotiated.
@@ -1058,6 +1078,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 
 	hw->vlan_strip = rxmode->hw_vlan_strip;
 
+	if (rxmode->hw_vlan_filter
+	    && !vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VLAN)) {
+		PMD_DRV_LOG(NOTICE,
+			    "vlan filtering not available on this host");
+		return -ENOTSUP;
+	}
+
 	if (vtpci_irq_config(hw, 0) == VIRTIO_MSI_NO_VECTOR) {
 		PMD_DRV_LOG(ERR, "failed to set config vector");
 		return -EBUSY;
-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 14/17] virtio: Add suport for multiple mac addresses

[Ouyang Changchun]

Virtio support multiple MAC addresses.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_virtio/virtio_ethdev.c | 94 ++++++++++++++++++++++++++++++++++-
 lib/librte_pmd_virtio/virtio_ethdev.h |  3 +-
 lib/librte_pmd_virtio/virtqueue.h     | 34 ++++++++++++-
 3 files changed, 127 insertions(+), 4 deletions(-)

diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index ec5a51e..e469ac2 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -86,6 +86,10 @@ static void virtio_dev_stats_reset(struct rte_eth_dev *dev);
 static void virtio_dev_free_mbufs(struct rte_eth_dev *dev);
 static int virtio_vlan_filter_set(struct rte_eth_dev *dev,
 				uint16_t vlan_id, int on);
+static void virtio_mac_addr_add(struct rte_eth_dev *dev,
+				struct ether_addr *mac_addr,
+				uint32_t index, uint32_t vmdq __rte_unused);
+static void virtio_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index);
 
 static int virtio_dev_queue_stats_mapping_set(
 	__rte_unused struct rte_eth_dev *eth_dev,
@@ -503,8 +507,6 @@ static struct eth_dev_ops virtio_eth_dev_ops = {
 	.stats_get               = virtio_dev_stats_get,
 	.stats_reset             = virtio_dev_stats_reset,
 	.link_update             = virtio_dev_link_update,
-	.mac_addr_add            = NULL,
-	.mac_addr_remove         = NULL,
 	.rx_queue_setup          = virtio_dev_rx_queue_setup,
 	/* meaningfull only to multiple queue */
 	.rx_queue_release        = virtio_dev_rx_queue_release,
@@ -514,6 +516,8 @@ static struct eth_dev_ops virtio_eth_dev_ops = {
 	/* collect stats per queue */
 	.queue_stats_mapping_set = virtio_dev_queue_stats_mapping_set,
 	.vlan_filter_set         = virtio_vlan_filter_set,
+	.mac_addr_add            = virtio_mac_addr_add,
+	.mac_addr_remove         = virtio_mac_addr_remove,
 };
 
 static inline int
@@ -644,6 +648,92 @@ virtio_get_hwaddr(struct virtio_hw *hw)
 }
 
 static int
+virtio_mac_table_set(struct virtio_hw *hw,
+		     const struct virtio_net_ctrl_mac *uc,
+		     const struct virtio_net_ctrl_mac *mc)
+{
+	struct virtio_pmd_ctrl ctrl;
+	int err, len[2];
+
+	ctrl.hdr.class = VIRTIO_NET_CTRL_MAC;
+	ctrl.hdr.cmd = VIRTIO_NET_CTRL_MAC_TABLE_SET;
+
+	len[0] = uc->entries * ETHER_ADDR_LEN + sizeof(uc->entries);
+	memcpy(ctrl.data, uc, len[0]);
+
+	len[1] = mc->entries * ETHER_ADDR_LEN + sizeof(mc->entries);
+	memcpy(ctrl.data + len[0], mc, len[1]);
+
+	err = virtio_send_command(hw->cvq, &ctrl, len, 2);
+	if (err != 0)
+		PMD_DRV_LOG(NOTICE, "mac table set failed: %d", err);
+
+	return err;
+}
+
+static void
+virtio_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
+		    uint32_t index, uint32_t vmdq __rte_unused)
+{
+	struct virtio_hw *hw = dev->data->dev_private;
+	const struct ether_addr *addrs = dev->data->mac_addrs;
+	unsigned int i;
+	struct virtio_net_ctrl_mac *uc, *mc;
+
+	if (index >= VIRTIO_MAX_MAC_ADDRS) {
+		PMD_DRV_LOG(ERR, "mac address index %u out of range", index);
+		return;
+	}
+
+	uc = alloca(VIRTIO_MAX_MAC_ADDRS * ETHER_ADDR_LEN + sizeof(uc->entries));
+	uc->entries = 0;
+	mc = alloca(VIRTIO_MAX_MAC_ADDRS * ETHER_ADDR_LEN + sizeof(mc->entries));
+	mc->entries = 0;
+
+	for (i = 0; i < VIRTIO_MAX_MAC_ADDRS; i++) {
+		const struct ether_addr *addr
+			= (i == index) ? mac_addr : addrs + i;
+		struct virtio_net_ctrl_mac *tbl
+			= is_multicast_ether_addr(addr) ? mc : uc;
+
+		memcpy(&tbl->macs[tbl->entries++], addr, ETHER_ADDR_LEN);
+	}
+
+	virtio_mac_table_set(hw, uc, mc);
+}
+
+static void
+virtio_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
+{
+	struct virtio_hw *hw = dev->data->dev_private;
+	struct ether_addr *addrs = dev->data->mac_addrs;
+	struct virtio_net_ctrl_mac *uc, *mc;
+	unsigned int i;
+
+	if (index >= VIRTIO_MAX_MAC_ADDRS) {
+		PMD_DRV_LOG(ERR, "mac address index %u out of range", index);
+		return;
+	}
+
+	uc = alloca(VIRTIO_MAX_MAC_ADDRS * ETHER_ADDR_LEN + sizeof(uc->entries));
+	uc->entries = 0;
+	mc = alloca(VIRTIO_MAX_MAC_ADDRS * ETHER_ADDR_LEN + sizeof(mc->entries));
+	mc->entries = 0;
+
+	for (i = 0; i < VIRTIO_MAX_MAC_ADDRS; i++) {
+		struct virtio_net_ctrl_mac *tbl;
+
+		if (i == index || is_zero_ether_addr(addrs + i))
+			continue;
+
+		tbl = is_multicast_ether_addr(addrs + i) ? mc : uc;
+		memcpy(&tbl->macs[tbl->entries++], addrs + i, ETHER_ADDR_LEN);
+	}
+
+	virtio_mac_table_set(hw, uc, mc);
+}
+
+static int
 virtio_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
 {
 	struct virtio_hw *hw = dev->data->dev_private;
diff --git a/lib/librte_pmd_virtio/virtio_ethdev.h b/lib/librte_pmd_virtio/virtio_ethdev.h
index 55c9749..74ac7e0 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.h
+++ b/lib/librte_pmd_virtio/virtio_ethdev.h
@@ -51,7 +51,7 @@
 
 #define VIRTIO_MAX_RX_QUEUES 128
 #define VIRTIO_MAX_TX_QUEUES 128
-#define VIRTIO_MAX_MAC_ADDRS 1
+#define VIRTIO_MAX_MAC_ADDRS 64
 #define VIRTIO_MIN_RX_BUFSIZE 64
 #define VIRTIO_MAX_RX_PKTLEN  9728
 
@@ -60,6 +60,7 @@
 	(VIRTIO_NET_F_MAC       | \
 	VIRTIO_NET_F_STATUS     | \
 	VIRTIO_NET_F_MQ         | \
+	VIRTIO_NET_F_CTRL_MAC_ADDR | \
 	VIRTIO_NET_F_CTRL_VQ    | \
 	VIRTIO_NET_F_CTRL_RX    | \
 	VIRTIO_NET_F_CTRL_VLAN  | \
diff --git a/lib/librte_pmd_virtio/virtqueue.h b/lib/librte_pmd_virtio/virtqueue.h
index 5b8a255..4cb82f9 100644
--- a/lib/librte_pmd_virtio/virtqueue.h
+++ b/lib/librte_pmd_virtio/virtqueue.h
@@ -99,6 +99,34 @@ enum { VTNET_RQ = 0, VTNET_TQ = 1, VTNET_CQ = 2 };
 #define VIRTIO_NET_CTRL_RX_NOBCAST      5
 
 /**
+ * Control the MAC
+ *
+ * The MAC filter table is managed by the hypervisor, the guest should
+ * assume the size is infinite.  Filtering should be considered
+ * non-perfect, ie. based on hypervisor resources, the guest may
+ * received packets from sources not specified in the filter list.
+ *
+ * In addition to the class/cmd header, the TABLE_SET command requires
+ * two out scatterlists.  Each contains a 4 byte count of entries followed
+ * by a concatenated byte stream of the ETH_ALEN MAC addresses.  The
+ * first sg list contains unicast addresses, the second is for multicast.
+ * This functionality is present if the VIRTIO_NET_F_CTRL_RX feature
+ * is available.
+ *
+ * The ADDR_SET command requests one out scatterlist, it contains a
+ * 6 bytes MAC address. This functionality is present if the
+ * VIRTIO_NET_F_CTRL_MAC_ADDR feature is available.
+ */
+struct virtio_net_ctrl_mac {
+	uint32_t entries;
+	uint8_t macs[][ETHER_ADDR_LEN];
+} __attribute__((packed));
+
+#define VIRTIO_NET_CTRL_MAC    1
+ #define VIRTIO_NET_CTRL_MAC_TABLE_SET        0
+ #define VIRTIO_NET_CTRL_MAC_ADDR_SET         1
+
+/**
  * Control VLAN filtering
  *
  * The VLAN filter table is controlled via a simple ADD/DEL interface.
@@ -121,7 +149,7 @@ typedef uint8_t virtio_net_ctrl_ack;
 #define VIRTIO_NET_OK     0
 #define VIRTIO_NET_ERR    1
 
-#define VIRTIO_MAX_CTRL_DATA 128
+#define VIRTIO_MAX_CTRL_DATA 2048
 
 struct virtio_pmd_ctrl {
 	struct virtio_net_ctrl_hdr hdr;
@@ -180,6 +208,10 @@ struct virtqueue {
 #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN        1
 #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX        0x8000
 #endif
+#ifndef VIRTIO_NET_F_CTRL_MAC_ADDR
+#define VIRTIO_NET_F_CTRL_MAC_ADDR 0x800000
+#define VIRTIO_NET_CTRL_MAC_ADDR_SET         1
+#endif
 
 /**
  * This is the first element of the scatter-gather list.  If you don't
-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 15/17] virtio: Add ability to set MAC address

[Ouyang Changchun]

Need to have do special things to set default mac address.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_ether/rte_ethdev.h         |  5 +++++
 lib/librte_pmd_virtio/virtio_ethdev.c | 24 ++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 07d55b8..cbe3fdf 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1249,6 +1249,10 @@ typedef void (*eth_mac_addr_add_t)(struct rte_eth_dev *dev,
 				  uint32_t vmdq);
 /**< @internal Set a MAC address into Receive Address Address Register */
 
+typedef void (*eth_mac_addr_set_t)(struct rte_eth_dev *dev,
+				  struct ether_addr *mac_addr);
+/**< @internal Set a MAC address into Receive Address Address Register */
+
 typedef int (*eth_uc_hash_table_set_t)(struct rte_eth_dev *dev,
 				  struct ether_addr *mac_addr,
 				  uint8_t on);
@@ -1482,6 +1486,7 @@ struct eth_dev_ops {
 	priority_flow_ctrl_set_t   priority_flow_ctrl_set; /**< Setup priority flow control.*/
 	eth_mac_addr_remove_t      mac_addr_remove; /**< Remove MAC address */
 	eth_mac_addr_add_t         mac_addr_add;  /**< Add a MAC address */
+	eth_mac_addr_set_t         mac_addr_set;  /**< Set a MAC address */
 	eth_uc_hash_table_set_t    uc_hash_table_set;  /**< Set Unicast Table Array */
 	eth_uc_all_hash_table_set_t uc_all_hash_table_set;  /**< Set Unicast hash bitmap */
 	eth_mirror_rule_set_t	   mirror_rule_set;  /**< Add a traffic mirror rule.*/
diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index e469ac2..c5f21c1 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -90,6 +90,8 @@ static void virtio_mac_addr_add(struct rte_eth_dev *dev,
 				struct ether_addr *mac_addr,
 				uint32_t index, uint32_t vmdq __rte_unused);
 static void virtio_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index);
+static void virtio_mac_addr_set(struct rte_eth_dev *dev,
+				struct ether_addr *mac_addr);
 
 static int virtio_dev_queue_stats_mapping_set(
 	__rte_unused struct rte_eth_dev *eth_dev,
@@ -518,6 +520,7 @@ static struct eth_dev_ops virtio_eth_dev_ops = {
 	.vlan_filter_set         = virtio_vlan_filter_set,
 	.mac_addr_add            = virtio_mac_addr_add,
 	.mac_addr_remove         = virtio_mac_addr_remove,
+	.mac_addr_set            = virtio_mac_addr_set,
 };
 
 static inline int
@@ -733,6 +736,27 @@ virtio_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
 	virtio_mac_table_set(hw, uc, mc);
 }
 
+static void
+virtio_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
+{
+	struct virtio_hw *hw = dev->data->dev_private;
+
+	memcpy(hw->mac_addr, mac_addr, ETHER_ADDR_LEN);
+
+	/* Use atomic update if available */
+	if (vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
+		struct virtio_pmd_ctrl ctrl;
+		int len = ETHER_ADDR_LEN;
+
+		ctrl.hdr.class = VIRTIO_NET_CTRL_MAC;
+		ctrl.hdr.cmd = VIRTIO_NET_CTRL_MAC_ADDR_SET;
+
+		memcpy(ctrl.data, mac_addr, ETHER_ADDR_LEN);
+		virtio_send_command(hw->cvq, &ctrl, &len, 1);
+	} else if (vtpci_with_feature(hw, VIRTIO_NET_F_MAC))
+		virtio_set_hwaddr(hw);
+}
+
 static int
 virtio_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
 {
-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 16/17] virtio: Free mbuf's with threshold

[Ouyang Changchun]

This makes virtio driver work like ixgbe. Transmit buffers are
held until a transmit threshold is reached. The previous behavior
was to hold mbuf's until the ring entry was reused which caused
more memory usage than needed.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_virtio/virtio_ethdev.c |  7 ++--
 lib/librte_pmd_virtio/virtio_rxtx.c   | 70 +++++++++++++++++++++++++++++------
 lib/librte_pmd_virtio/virtqueue.h     |  3 +-
 3 files changed, 64 insertions(+), 16 deletions(-)

diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index c5f21c1..1ec29e1 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -176,15 +176,16 @@ virtio_send_command(struct virtqueue *vq, struct virtio_pmd_ctrl *ctrl,
 
 	virtqueue_notify(vq);
 
-	while (vq->vq_used_cons_idx == vq->vq_ring.used->idx)
+	rte_rmb();
+	while (vq->vq_used_cons_idx == vq->vq_ring.used->idx) {
+		rte_rmb();
 		usleep(100);
+	}
 
 	while (vq->vq_used_cons_idx != vq->vq_ring.used->idx) {
 		uint32_t idx, desc_idx, used_idx;
 		struct vring_used_elem *uep;
 
-		virtio_rmb();
-
 		used_idx = (uint32_t)(vq->vq_used_cons_idx
 				& (vq->vq_nentries - 1));
 		uep = &vq->vq_ring.used->ring[used_idx];
diff --git a/lib/librte_pmd_virtio/virtio_rxtx.c b/lib/librte_pmd_virtio/virtio_rxtx.c
index b44f091..26c0a1d 100644
--- a/lib/librte_pmd_virtio/virtio_rxtx.c
+++ b/lib/librte_pmd_virtio/virtio_rxtx.c
@@ -129,9 +129,15 @@ virtqueue_dequeue_burst_rx(struct virtqueue *vq, struct rte_mbuf **rx_pkts,
 	return i;
 }
 
+#ifndef DEFAULT_TX_FREE_THRESH
+#define DEFAULT_TX_FREE_THRESH 32
+#endif
+
+/* Cleanup from completed transmits. */
 static void
-virtqueue_dequeue_pkt_tx(struct virtqueue *vq)
+virtio_xmit_cleanup(struct virtqueue *vq, uint16_t num)
 {
+#if 0
 	struct vring_used_elem *uep;
 	uint16_t used_idx, desc_idx;
 
@@ -140,6 +146,25 @@ virtqueue_dequeue_pkt_tx(struct virtqueue *vq)
 	desc_idx = (uint16_t) uep->id;
 	vq->vq_used_cons_idx++;
 	vq_ring_free_chain(vq, desc_idx);
+#endif
+	uint16_t i, used_idx, desc_idx;
+	for (i = 0; i < num ; i++) {
+		struct vring_used_elem *uep;
+		struct vq_desc_extra *dxp;
+
+		used_idx = (uint16_t)(vq->vq_used_cons_idx & (vq->vq_nentries - 1));
+		uep = &vq->vq_ring.used->ring[used_idx];
+		dxp = &vq->vq_descx[used_idx];
+
+		desc_idx = (uint16_t) uep->id;
+		vq->vq_used_cons_idx++;
+		vq_ring_free_chain(vq, desc_idx);
+
+		if (dxp->cookie != NULL) {
+			rte_pktmbuf_free(dxp->cookie);
+			dxp->cookie = NULL;
+		}
+	}
 }
 
 
@@ -203,8 +228,10 @@ virtqueue_enqueue_xmit(struct virtqueue *txvq, struct rte_mbuf *cookie)
 
 	idx = head_idx;
 	dxp = &txvq->vq_descx[idx];
+#if 0
 	if (dxp->cookie != NULL)
 		rte_pktmbuf_free(dxp->cookie);
+#endif
 	dxp->cookie = (void *)cookie;
 	dxp->ndescs = needed;
 
@@ -404,6 +431,7 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 {
 	uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
 	struct virtqueue *vq;
+	uint16_t tx_free_thresh;
 	int ret;
 
 	PMD_INIT_FUNC_TRACE();
@@ -421,6 +449,21 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 		return ret;
 	}
 
+	tx_free_thresh = tx_conf->tx_free_thresh;
+	if (tx_free_thresh == 0)
+		tx_free_thresh = RTE_MIN(vq->vq_nentries / 4, DEFAULT_TX_FREE_THRESH);
+
+	if (tx_free_thresh >= (vq->vq_nentries - 3)) {
+		RTE_LOG(ERR, PMD, "tx_free_thresh must be less than the "
+		        "number of TX entries minus 3 (%u)."
+		        " (tx_free_thresh=%u port=%u queue=%u)\n",
+		        vq->vq_nentries - 3,
+		        tx_free_thresh, dev->data->port_id, queue_idx);
+		return -EINVAL;
+	}
+
+	vq->vq_free_thresh = tx_free_thresh;
+
 	dev->data->tx_queues[queue_idx] = vq;
 	return 0;
 }
@@ -688,11 +731,9 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 {
 	struct virtqueue *txvq = tx_queue;
 	struct rte_mbuf *txm;
-	uint16_t nb_used, nb_tx, num;
+	uint16_t nb_used, nb_tx;
 	int error;
 
-	nb_tx = 0;
-
 	if (unlikely(nb_pkts < 1))
 		return nb_pkts;
 
@@ -700,21 +741,26 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 	nb_used = VIRTQUEUE_NUSED(txvq);
 
 	virtio_rmb();
+	if (likely(nb_used > txvq->vq_free_thresh))
+		virtio_xmit_cleanup(txvq, nb_used);
 
-	num = (uint16_t)(likely(nb_used < VIRTIO_MBUF_BURST_SZ) ? nb_used : VIRTIO_MBUF_BURST_SZ);
+	nb_tx = 0;
 
 	while (nb_tx < nb_pkts) {
 		/* Need one more descriptor for virtio header. */
 		int need = tx_pkts[nb_tx]->nb_segs - txvq->vq_free_cnt + 1;
-		int deq_cnt = RTE_MIN(need, (int)num);
 
-		num -= (deq_cnt > 0) ? deq_cnt : 0;
-		while (deq_cnt > 0) {
-			virtqueue_dequeue_pkt_tx(txvq);
-			deq_cnt--;
+		/*Positive value indicates it need free vring descriptors */
+		if (unlikely(need > 0)) {
+			nb_used = VIRTQUEUE_NUSED(txvq);
+			virtio_rmb();
+			need = RTE_MIN(need, (int)nb_used);
+
+			virtio_xmit_cleanup(txvq, need);
+			need = (int)tx_pkts[nb_tx]->nb_segs -
+				txvq->vq_free_cnt + 1;
 		}
 
-		need = (int)tx_pkts[nb_tx]->nb_segs - txvq->vq_free_cnt + 1;
 		/*
 		 * Zero or negative value indicates it has enough free
 		 * descriptors to use for transmitting.
@@ -723,7 +769,7 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 			txm = tx_pkts[nb_tx];
 
 			/* Do VLAN tag insertion */
-			if (txm->ol_flags & PKT_TX_VLAN_PKT) {
+			if (unlikely(txm->ol_flags & PKT_TX_VLAN_PKT)) {
 				error = rte_vlan_insert(&txm);
 				if (unlikely(error)) {
 					rte_pktmbuf_free(txm);
diff --git a/lib/librte_pmd_virtio/virtqueue.h b/lib/librte_pmd_virtio/virtqueue.h
index 4cb82f9..7672a12 100644
--- a/lib/librte_pmd_virtio/virtqueue.h
+++ b/lib/librte_pmd_virtio/virtqueue.h
@@ -164,6 +164,7 @@ struct virtqueue {
 	struct rte_mempool       *mpool;  /**< mempool for mbuf allocation */
 	uint16_t    queue_id;             /**< DPDK queue index. */
 	uint8_t     port_id;              /**< Device port identifier. */
+	uint16_t    vq_queue_index;       /**< PCI queue index */
 
 	void        *vq_ring_virt_mem;    /**< linear address of vring*/
 	unsigned int vq_ring_size;
@@ -172,7 +173,7 @@ struct virtqueue {
 	struct vring vq_ring;    /**< vring keeping desc, used and avail */
 	uint16_t    vq_free_cnt; /**< num of desc available */
 	uint16_t    vq_nentries; /**< vring desc numbers */
-	uint16_t    vq_queue_index;       /**< PCI queue index */
+	uint16_t    vq_free_thresh; /**< free threshold */
 	/**
 	 * Head of the free chain in the descriptor table. If
 	 * there are no free descriptors, this will be set to
-- 
1.8.4.2

[dpdk-dev] [RFC PATCH 17/17] virtio: Use port IO to get PCI resource.

[Ouyang Changchun]

Make virtio not require UIO for some security reasons, this is to match 6Wind's virtio-net-pmd.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
---
 lib/librte_eal/common/include/rte_pci.h |  2 +
 lib/librte_eal/linuxapp/eal/eal_pci.c   |  3 +-
 lib/librte_pmd_virtio/virtio_ethdev.c   | 75 ++++++++++++++++++++++++++++++++-
 3 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 66ed793..2021b3b 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -193,6 +193,8 @@ struct rte_pci_driver {
 
 /** Device needs PCI BAR mapping (done with either IGB_UIO or VFIO) */
 #define RTE_PCI_DRV_NEED_MAPPING 0x0001
+/** Device needs port IO(done with /proc/ioports) */
+#define RTE_PCI_DRV_IO_PORT 0x0002
 /** Device driver must be registered several times until failure - deprecated */
 #pragma GCC poison RTE_PCI_DRV_MULTIPLE
 /** Device needs to be unbound even if no module is provided */
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index b5f5410..dd60793 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -573,7 +573,8 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 #endif
 			/* map resources for devices that use igb_uio */
 			ret = pci_map_device(dev);
-			if (ret != 0)
+			if ((ret != 0) &&
+				((dr->drv_flags & RTE_PCI_DRV_IO_PORT) == 0))
 				return ret;
 		} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
 		           rte_eal_process_type() == RTE_PROC_PRIMARY) {
diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index 1ec29e1..4490a06 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -961,6 +961,69 @@ static int virtio_resource_init(struct rte_pci_device *pci_dev)
 		     start, size);
 	return 0;
 }
+
+/* Extract I/O port numbers from proc/ioports */
+static int virtio_resource_init_by_ioport(struct rte_pci_device *pci_dev)
+{
+	uint16_t start, end;
+	int size;
+	FILE* fp;
+	char* line = NULL;
+	char pci_id[16];
+	int found = 0;
+	size_t linesz;
+
+	snprintf(pci_id, sizeof(pci_id), PCI_PRI_FMT,
+		 pci_dev->addr.domain,
+		 pci_dev->addr.bus,
+		 pci_dev->addr.devid,
+		 pci_dev->addr.function);
+
+	fp = fopen("/proc/ioports", "r");
+	if (fp == NULL) {
+		PMD_INIT_LOG(ERR, "%s(): can't open ioports", __func__);
+		return -1;
+	}
+
+	while (getdelim(&line, &linesz, '\n', fp) > 0) {
+		char* ptr = line;
+		char* left;
+		int n;
+
+		n = strcspn(ptr, ":");
+		ptr[n]= 0;
+		left = &ptr[n+1];
+
+		while (*left && isspace(*left))
+			left++;
+
+		if (!strncmp(left, pci_id, strlen(pci_id))) {
+			found = 1;
+
+			while (*ptr && isspace(*ptr))
+				ptr++;
+
+			sscanf(ptr, "%04hx-%04hx", &start, &end);
+			size = end - start + 1;
+
+			break;
+		}
+	}
+
+	free(line);
+	fclose(fp);
+
+	if (!found)
+		return -1;
+
+	pci_dev->mem_resource[0].addr = (void *)(uintptr_t)(uint32_t)start;
+	pci_dev->mem_resource[0].len =  (uint64_t)size;
+	PMD_INIT_LOG(DEBUG,
+		     "PCI Port IO found start=0x%lx with size=0x%lx",
+		     start, size);
+	return 0;
+}
+
 #else
 static int
 virtio_has_msix(const struct rte_pci_addr *loc __rte_unused)
@@ -974,6 +1037,12 @@ static int virtio_resource_init(struct rte_pci_device *pci_dev __rte_unused)
 	/* no setup required */
 	return 0;
 }
+
+static int virtio_resource_init_by_ioport(struct rte_pci_device *pci_dev)
+{
+	/* no setup required */
+	return 0;
+}
 #endif
 
 /*
@@ -1039,7 +1108,8 @@ eth_virtio_dev_init(__rte_unused struct eth_driver *eth_drv,
 
 	pci_dev = eth_dev->pci_dev;
 	if (virtio_resource_init(pci_dev) < 0)
-		return -1;
+		if (virtio_resource_init_by_ioport(pci_dev) < 0)
+			return -1;
 
 	hw->use_msix = virtio_has_msix(&pci_dev->addr);
 	hw->io_base = (uint32_t)(uintptr_t)pci_dev->mem_resource[0].addr;
@@ -1136,7 +1206,8 @@ static struct eth_driver rte_virtio_pmd = {
 	{
 		.name = "rte_virtio_pmd",
 		.id_table = pci_id_virtio_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_IO_PORT |\
+			 RTE_PCI_DRV_INTR_LSC,
 	},
 	.eth_dev_init = eth_virtio_dev_init,
 	.dev_private_size = sizeof(struct virtio_hw),
-- 
1.8.4.2

Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation

[Thomas Monjalon]

Hi Changchun,

2014-12-08 14:21, Ouyang Changchun:
> This patch set bases on two original RFC patch sets from Stephen Hemminger[stephen@networkplumber.org]
> Refer to [http://dpdk.org/ml/archives/dev/2014-August/004845.html ] for the original one.
> This patch set also resolves some conflict with latest codes and removed duplicated codes.

As you sent the patches, you appear as the author.
But I guess Stephen should be the author for some of them.
Please check who has contributed the most in each patch to decide.

Thank you
-- 
Thomas

Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation

[Ouyang, Changchun]

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Monday, December 8, 2014 5:31 PM
> To: Ouyang, Changchun
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation
> 
> Hi Changchun,
> 
> 2014-12-08 14:21, Ouyang Changchun:
> > This patch set bases on two original RFC patch sets from Stephen
> Hemminger[stephen@networkplumber.org]
> > Refer to [http://dpdk.org/ml/archives/dev/2014-August/004845.html ] for
> the original one.
> > This patch set also resolves some conflict with latest codes and removed
> duplicated codes.
> 
> As you sent the patches, you appear as the author.
> But I guess Stephen should be the author for some of them.
> Please check who has contributed the most in each patch to decide.

You are right, most of patches originate from Stephen's patchset, except for the last one,
To be honest, I am ok whoever is the author of this patch set, :-),
We could co-own the feature of Single virtio if you all agree with it, and I think we couldn't finish
Such a feature without collaboration among us, this is why I tried to communicate with most of you 
to collect more feedback, suggestion and comments for this feature.
Very appreciate for all kinds of feedback, suggestion here, especially for patch set from Stephen. 

According to your request, how could we make this patch set looks more like Stephen as the author? 
Currently I add Stephen as Signed-off-by list in each patch(I got the agreement from Stephen before doing this :-)).
Need I send all patchset to Stephen and let Stephen send out them to dpdk.org?
Or any other better solution?
If you has better suggestion, I assume it works for all subsequent RFC and normal patch set.
 
Any other suggestions are welcome.

Thanks
Changchun

Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation

[Qiu, Michael]

On 12/9/2014 9:11 AM, Ouyang, Changchun wrote:
> Hi Thomas,
>
>> -----Original Message-----
>> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
>> Sent: Monday, December 8, 2014 5:31 PM
>> To: Ouyang, Changchun
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation
>>
>> Hi Changchun,
>>
>> 2014-12-08 14:21, Ouyang Changchun:
>>> This patch set bases on two original RFC patch sets from Stephen
>> Hemminger[stephen@networkplumber.org]
>>> Refer to [http://dpdk.org/ml/archives/dev/2014-August/004845.html ] for
>> the original one.
>>> This patch set also resolves some conflict with latest codes and removed
>> duplicated codes.
>>
>> As you sent the patches, you appear as the author.
>> But I guess Stephen should be the author for some of them.
>> Please check who has contributed the most in each patch to decide.
> You are right, most of patches originate from Stephen's patchset, except for the last one,
> To be honest, I am ok whoever is the author of this patch set, :-),
> We could co-own the feature of Single virtio if you all agree with it, and I think we couldn't finish
> Such a feature without collaboration among us, this is why I tried to communicate with most of you 
> to collect more feedback, suggestion and comments for this feature.
> Very appreciate for all kinds of feedback, suggestion here, especially for patch set from Stephen. 
>
> According to your request, how could we make this patch set looks more like Stephen as the author? 
> Currently I add Stephen as Signed-off-by list in each patch(I got the agreement from Stephen before doing this :-)).

Hi Ouyang,

"Signed-off-by" should be added by himself, because the one who in the Signed-off-by list should take responsibility for it(like potential bugs/issues).

Although, lots of patches are originate from Stephen, we still need himself add this line :)

If DPDK community's Signed-off-by" rule is different from linux(qemu, etc.), please ignore my comment :)

Thanks,
Michael 

> Need I send all patchset to Stephen and let Stephen send out them to dpdk.org?
> Or any other better solution?
> If you has better suggestion, I assume it works for all subsequent RFC and normal patch set.
>  
> Any other suggestions are welcome.
>
> Thanks
> Changchun
>
>
>
>

Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation

[Stephen Hemminger]

I sent the patches to Ouyang with my Signed-off.
He did the testing with current DPDK.

On Mon, Dec 8, 2014 at 7:23 PM, Qiu, Michael <michael.qiu@intel.com> wrote:

> On 12/9/2014 9:11 AM, Ouyang, Changchun wrote:
> > Hi Thomas,
> >
> >> -----Original Message-----
> >> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> >> Sent: Monday, December 8, 2014 5:31 PM
> >> To: Ouyang, Changchun
> >> Cc: dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation
> >>
> >> Hi Changchun,
> >>
> >> 2014-12-08 14:21, Ouyang Changchun:
> >>> This patch set bases on two original RFC patch sets from Stephen
> >> Hemminger[stephen@networkplumber.org]
> >>> Refer to [http://dpdk.org/ml/archives/dev/2014-August/004845.html ]
> for
> >> the original one.
> >>> This patch set also resolves some conflict with latest codes and
> removed
> >> duplicated codes.
> >>
> >> As you sent the patches, you appear as the author.
> >> But I guess Stephen should be the author for some of them.
> >> Please check who has contributed the most in each patch to decide.
> > You are right, most of patches originate from Stephen's patchset, except
> for the last one,
> > To be honest, I am ok whoever is the author of this patch set, :-),
> > We could co-own the feature of Single virtio if you all agree with it,
> and I think we couldn't finish
> > Such a feature without collaboration among us, this is why I tried to
> communicate with most of you
> > to collect more feedback, suggestion and comments for this feature.
> > Very appreciate for all kinds of feedback, suggestion here, especially
> for patch set from Stephen.
> >
> > According to your request, how could we make this patch set looks more
> like Stephen as the author?
> > Currently I add Stephen as Signed-off-by list in each patch(I got the
> agreement from Stephen before doing this :-)).
>
> Hi Ouyang,
>
> "Signed-off-by" should be added by himself, because the one who in the
> Signed-off-by list should take responsibility for it(like potential
> bugs/issues).
>
> Although, lots of patches are originate from Stephen, we still need
> himself add this line :)
>
> If DPDK community's Signed-off-by" rule is different from linux(qemu,
> etc.), please ignore my comment :)
>
> Thanks,
> Michael
>
> > Need I send all patchset to Stephen and let Stephen send out them to
> dpdk.org?
> > Or any other better solution?
> > If you has better suggestion, I assume it works for all subsequent RFC
> and normal patch set.
> >
> > Any other suggestions are welcome.
> >
> > Thanks
> > Changchun
> >
> >
> >
> >
>
>

Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation

[Ouyang, Changchun]

Hi

> -----Original Message-----
> From: Qiu, Michael
> Sent: Tuesday, December 9, 2014 11:23 AM
> To: Ouyang, Changchun; Thomas Monjalon; Stephen Hemminger
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation
> 
> On 12/9/2014 9:11 AM, Ouyang, Changchun wrote:
> > Hi Thomas,
> >
> >> -----Original Message-----
> >> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> >> Sent: Monday, December 8, 2014 5:31 PM
> >> To: Ouyang, Changchun
> >> Cc: dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
> >> implementation
> >>
> >> Hi Changchun,
> >>
> >> 2014-12-08 14:21, Ouyang Changchun:
> >>> This patch set bases on two original RFC patch sets from Stephen
> >> Hemminger[stephen@networkplumber.org]
> >>> Refer to [http://dpdk.org/ml/archives/dev/2014-August/004845.html ]
> >>> for
> >> the original one.
> >>> This patch set also resolves some conflict with latest codes and
> >>> removed
> >> duplicated codes.
> >>
> >> As you sent the patches, you appear as the author.
> >> But I guess Stephen should be the author for some of them.
> >> Please check who has contributed the most in each patch to decide.
> > You are right, most of patches originate from Stephen's patchset,
> > except for the last one, To be honest, I am ok whoever is the author
> > of this patch set, :-), We could co-own the feature of Single virtio
> > if you all agree with it, and I think we couldn't finish Such a
> > feature without collaboration among us, this is why I tried to communicate
> with most of you to collect more feedback, suggestion and comments for this
> feature.
> > Very appreciate for all kinds of feedback, suggestion here, especially for
> patch set from Stephen.
> >
> > According to your request, how could we make this patch set looks more
> like Stephen as the author?
> > Currently I add Stephen as Signed-off-by list in each patch(I got the
> agreement from Stephen before doing this :-)).
> 
> Hi Ouyang,
> 
> "Signed-off-by" should be added by himself, because the one who in the
> Signed-off-by list should take responsibility for it(like potential bugs/issues).
> 
> Although, lots of patches are originate from Stephen, we still need himself
> add this line :)

Hi Thomas, 
It that right? I can't add Stephen into Signed-off-by list even if I have gotten the agreement from Stephen,
What 's the strict rule here?

Thanks
Changchun

Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation

[Thomas Monjalon]

2014-12-09 05:41, Ouyang, Changchun:
> Hi
> 
> > -----Original Message-----
> > From: Qiu, Michael
> > Sent: Tuesday, December 9, 2014 11:23 AM
> > To: Ouyang, Changchun; Thomas Monjalon; Stephen Hemminger
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation
> > 
> > On 12/9/2014 9:11 AM, Ouyang, Changchun wrote:
> > > Hi Thomas,
> > >
> > >> -----Original Message-----
> > >> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > >> Sent: Monday, December 8, 2014 5:31 PM
> > >> To: Ouyang, Changchun
> > >> Cc: dev@dpdk.org
> > >> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
> > >> implementation
> > >>
> > >> Hi Changchun,
> > >>
> > >> 2014-12-08 14:21, Ouyang Changchun:
> > >>> This patch set bases on two original RFC patch sets from Stephen
> > >> Hemminger[stephen@networkplumber.org]
> > >>> Refer to [http://dpdk.org/ml/archives/dev/2014-August/004845.html ]
> > >>> for
> > >> the original one.
> > >>> This patch set also resolves some conflict with latest codes and
> > >>> removed
> > >> duplicated codes.
> > >>
> > >> As you sent the patches, you appear as the author.
> > >> But I guess Stephen should be the author for some of them.
> > >> Please check who has contributed the most in each patch to decide.
> > > You are right, most of patches originate from Stephen's patchset,
> > > except for the last one, To be honest, I am ok whoever is the author
> > > of this patch set, :-), We could co-own the feature of Single virtio
> > > if you all agree with it, and I think we couldn't finish Such a
> > > feature without collaboration among us, this is why I tried to communicate
> > with most of you to collect more feedback, suggestion and comments for this
> > feature.
> > > Very appreciate for all kinds of feedback, suggestion here, especially for
> > patch set from Stephen.
> > >
> > > According to your request, how could we make this patch set looks more
> > like Stephen as the author?
> > > Currently I add Stephen as Signed-off-by list in each patch(I got the
> > agreement from Stephen before doing this :-)).
> > 
> > Hi Ouyang,
> > 
> > "Signed-off-by" should be added by himself, because the one who in the
> > Signed-off-by list should take responsibility for it(like potential bugs/issues).
> > 
> > Although, lots of patches are originate from Stephen, we still need himself
> > add this line :)
> 
> Hi Thomas, 
> It that right? I can't add Stephen into Signed-off-by list even if I have gotten the agreement from Stephen,
> What 's the strict rule here?

Stephen sent the patches with his Signed-off, then you added yours.
This is OK.
Using git am, author would have been Stephen. To change author now, you can
edit each commit with interactive rebase and "git commit --amend --author=Stephen".
No need to resend now. Please check it for next version of the patchset.

-- 
Thomas

Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation

[Ouyang, Changchun]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, December 9, 2014 2:12 PM
> To: Ouyang, Changchun
> Cc: Qiu, Michael; Stephen Hemminger; dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation
> 
> 2014-12-09 05:41, Ouyang, Changchun:
> > Hi
> >
> > > -----Original Message-----
> > > From: Qiu, Michael
> > > Sent: Tuesday, December 9, 2014 11:23 AM
> > > To: Ouyang, Changchun; Thomas Monjalon; Stephen Hemminger
> > > Cc: dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
> > > implementation
> > >
> > > On 12/9/2014 9:11 AM, Ouyang, Changchun wrote:
> > > > Hi Thomas,
> > > >
> > > >> -----Original Message-----
> > > >> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > >> Sent: Monday, December 8, 2014 5:31 PM
> > > >> To: Ouyang, Changchun
> > > >> Cc: dev@dpdk.org
> > > >> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
> > > >> implementation
> > > >>
> > > >> Hi Changchun,
> > > >>
> > > >> 2014-12-08 14:21, Ouyang Changchun:
> > > >>> This patch set bases on two original RFC patch sets from Stephen
> > > >> Hemminger[stephen@networkplumber.org]
> > > >>> Refer to
> > > >>> [http://dpdk.org/ml/archives/dev/2014-August/004845.html ] for
> > > >> the original one.
> > > >>> This patch set also resolves some conflict with latest codes and
> > > >>> removed
> > > >> duplicated codes.
> > > >>
> > > >> As you sent the patches, you appear as the author.
> > > >> But I guess Stephen should be the author for some of them.
> > > >> Please check who has contributed the most in each patch to decide.
> > > > You are right, most of patches originate from Stephen's patchset,
> > > > except for the last one, To be honest, I am ok whoever is the
> > > > author of this patch set, :-), We could co-own the feature of
> > > > Single virtio if you all agree with it, and I think we couldn't
> > > > finish Such a feature without collaboration among us, this is why
> > > > I tried to communicate
> > > with most of you to collect more feedback, suggestion and comments
> > > for this feature.
> > > > Very appreciate for all kinds of feedback, suggestion here,
> > > > especially for
> > > patch set from Stephen.
> > > >
> > > > According to your request, how could we make this patch set looks
> > > > more
> > > like Stephen as the author?
> > > > Currently I add Stephen as Signed-off-by list in each patch(I got
> > > > the
> > > agreement from Stephen before doing this :-)).
> > >
> > > Hi Ouyang,
> > >
> > > "Signed-off-by" should be added by himself, because the one who in
> > > the Signed-off-by list should take responsibility for it(like potential
> bugs/issues).
> > >
> > > Although, lots of patches are originate from Stephen, we still need
> > > himself add this line :)
> >
> > Hi Thomas,
> > It that right? I can't add Stephen into Signed-off-by list even if I
> > have gotten the agreement from Stephen, What 's the strict rule here?
> 
> Stephen sent the patches with his Signed-off, then you added yours.
> This is OK.
> Using git am, author would have been Stephen. To change author now, you
> can edit each commit with interactive rebase and "git commit --amend --
> author=Stephen".
> No need to resend now. Please check it for next version of the patchset.
> 

So I understand correctly, Stephen need care for from patches from 1 to 16,
I need care for the 17th patch from next version.
What I mean "caring for" above is:  debug and validate them and send out patches

Thanks
Changchun

Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation

[Thomas Monjalon]

2014-12-09 06:40, Ouyang, Changchun:
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Tuesday, December 9, 2014 2:12 PM
> > To: Ouyang, Changchun
> > Cc: Qiu, Michael; Stephen Hemminger; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation
> > 
> > 2014-12-09 05:41, Ouyang, Changchun:
> > > Hi
> > >
> > > > -----Original Message-----
> > > > From: Qiu, Michael
> > > > Sent: Tuesday, December 9, 2014 11:23 AM
> > > > To: Ouyang, Changchun; Thomas Monjalon; Stephen Hemminger
> > > > Cc: dev@dpdk.org
> > > > Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
> > > > implementation
> > > >
> > > > On 12/9/2014 9:11 AM, Ouyang, Changchun wrote:
> > > > > Hi Thomas,
> > > > >
> > > > >> -----Original Message-----
> > > > >> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > >> Sent: Monday, December 8, 2014 5:31 PM
> > > > >> To: Ouyang, Changchun
> > > > >> Cc: dev@dpdk.org
> > > > >> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
> > > > >> implementation
> > > > >>
> > > > >> Hi Changchun,
> > > > >>
> > > > >> 2014-12-08 14:21, Ouyang Changchun:
> > > > >>> This patch set bases on two original RFC patch sets from Stephen
> > > > >> Hemminger[stephen@networkplumber.org]
> > > > >>> Refer to
> > > > >>> [http://dpdk.org/ml/archives/dev/2014-August/004845.html ] for
> > > > >> the original one.
> > > > >>> This patch set also resolves some conflict with latest codes and
> > > > >>> removed
> > > > >> duplicated codes.
> > > > >>
> > > > >> As you sent the patches, you appear as the author.
> > > > >> But I guess Stephen should be the author for some of them.
> > > > >> Please check who has contributed the most in each patch to decide.
> > > > > You are right, most of patches originate from Stephen's patchset,
> > > > > except for the last one, To be honest, I am ok whoever is the
> > > > > author of this patch set, :-), We could co-own the feature of
> > > > > Single virtio if you all agree with it, and I think we couldn't
> > > > > finish Such a feature without collaboration among us, this is why
> > > > > I tried to communicate
> > > > with most of you to collect more feedback, suggestion and comments
> > > > for this feature.
> > > > > Very appreciate for all kinds of feedback, suggestion here,
> > > > > especially for
> > > > patch set from Stephen.
> > > > >
> > > > > According to your request, how could we make this patch set looks
> > > > > more
> > > > like Stephen as the author?
> > > > > Currently I add Stephen as Signed-off-by list in each patch(I got
> > > > > the
> > > > agreement from Stephen before doing this :-)).
> > > >
> > > > Hi Ouyang,
> > > >
> > > > "Signed-off-by" should be added by himself, because the one who in
> > > > the Signed-off-by list should take responsibility for it(like potential
> > bugs/issues).
> > > >
> > > > Although, lots of patches are originate from Stephen, we still need
> > > > himself add this line :)
> > >
> > > Hi Thomas,
> > > It that right? I can't add Stephen into Signed-off-by list even if I
> > > have gotten the agreement from Stephen, What 's the strict rule here?
> > 
> > Stephen sent the patches with his Signed-off, then you added yours.
> > This is OK.
> > Using git am, author would have been Stephen. To change author now, you
> > can edit each commit with interactive rebase and "git commit --amend --
> > author=Stephen".
> > No need to resend now. Please check it for next version of the patchset.
> > 
> 
> So I understand correctly, Stephen need care for from patches from 1 to 16,
> I need care for the 17th patch from next version.
> What I mean "caring for" above is:  debug and validate them and send out patches

Just to be clear: you can send patches with Stephen's authorship.
That's not a problem. Please try to change --author and check how it looks when
you send them.

-- 
Thomas

Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation

[Bruce Richardson]

On Tue, Dec 09, 2014 at 06:40:23AM +0000, Ouyang, Changchun wrote:
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Tuesday, December 9, 2014 2:12 PM
> > To: Ouyang, Changchun
> > Cc: Qiu, Michael; Stephen Hemminger; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation
> > 
> > 2014-12-09 05:41, Ouyang, Changchun:
> > > Hi
> > >
> > > > -----Original Message-----
> > > > From: Qiu, Michael
> > > > Sent: Tuesday, December 9, 2014 11:23 AM
> > > > To: Ouyang, Changchun; Thomas Monjalon; Stephen Hemminger
> > > > Cc: dev@dpdk.org
> > > > Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
> > > > implementation
> > > >
> > > > On 12/9/2014 9:11 AM, Ouyang, Changchun wrote:
> > > > > Hi Thomas,
> > > > >
> > > > >> -----Original Message-----
> > > > >> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > >> Sent: Monday, December 8, 2014 5:31 PM
> > > > >> To: Ouyang, Changchun
> > > > >> Cc: dev@dpdk.org
> > > > >> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
> > > > >> implementation
> > > > >>
> > > > >> Hi Changchun,
> > > > >>
> > > > >> 2014-12-08 14:21, Ouyang Changchun:
> > > > >>> This patch set bases on two original RFC patch sets from Stephen
> > > > >> Hemminger[stephen@networkplumber.org]
> > > > >>> Refer to
> > > > >>> [http://dpdk.org/ml/archives/dev/2014-August/004845.html ] for
> > > > >> the original one.
> > > > >>> This patch set also resolves some conflict with latest codes and
> > > > >>> removed
> > > > >> duplicated codes.
> > > > >>
> > > > >> As you sent the patches, you appear as the author.
> > > > >> But I guess Stephen should be the author for some of them.
> > > > >> Please check who has contributed the most in each patch to decide.
> > > > > You are right, most of patches originate from Stephen's patchset,
> > > > > except for the last one, To be honest, I am ok whoever is the
> > > > > author of this patch set, :-), We could co-own the feature of
> > > > > Single virtio if you all agree with it, and I think we couldn't
> > > > > finish Such a feature without collaboration among us, this is why
> > > > > I tried to communicate
> > > > with most of you to collect more feedback, suggestion and comments
> > > > for this feature.
> > > > > Very appreciate for all kinds of feedback, suggestion here,
> > > > > especially for
> > > > patch set from Stephen.
> > > > >
> > > > > According to your request, how could we make this patch set looks
> > > > > more
> > > > like Stephen as the author?
> > > > > Currently I add Stephen as Signed-off-by list in each patch(I got
> > > > > the
> > > > agreement from Stephen before doing this :-)).
> > > >
> > > > Hi Ouyang,
> > > >
> > > > "Signed-off-by" should be added by himself, because the one who in
> > > > the Signed-off-by list should take responsibility for it(like potential
> > bugs/issues).
> > > >
> > > > Although, lots of patches are originate from Stephen, we still need
> > > > himself add this line :)
> > >
> > > Hi Thomas,
> > > It that right? I can't add Stephen into Signed-off-by list even if I
> > > have gotten the agreement from Stephen, What 's the strict rule here?
> > 
> > Stephen sent the patches with his Signed-off, then you added yours.
> > This is OK.
> > Using git am, author would have been Stephen. To change author now, you
> > can edit each commit with interactive rebase and "git commit --amend --
> > author=Stephen".
> > No need to resend now. Please check it for next version of the patchset.
> > 
> 
> So I understand correctly, Stephen need care for from patches from 1 to 16,
> I need care for the 17th patch from next version.
> What I mean "caring for" above is:  debug and validate them and send out patches
> 
> Thanks
> Changchun
> 
Just to clarify Thomas point here about use of "git am". If you get a patch
from someone to test or work on, use "git am" to apply it, rather than "git apply",
since "git am" generates a commit in your local repo and thereby maintains the
original authorship of the patch. If you do "git apply" and subsequently commit
yourself, you - rather than the original author - will appear as the "author" of
the patch, and you need to amend the commit as Thomas suggests to fix this.

So in short:
* git am == good
* git apply == bad

Regards,
/Bruce

Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation

[Ouyang, Changchun]

Hi Bruce,

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Tuesday, December 9, 2014 5:47 PM
> To: Ouyang, Changchun
> Cc: Thomas Monjalon; dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation
> 
> On Tue, Dec 09, 2014 at 06:40:23AM +0000, Ouyang, Changchun wrote:
> >
> >
> > > -----Original Message-----
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > Sent: Tuesday, December 9, 2014 2:12 PM
> > > To: Ouyang, Changchun
> > > Cc: Qiu, Michael; Stephen Hemminger; dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
> > > implementation
> > >
> > > 2014-12-09 05:41, Ouyang, Changchun:
> > > > Hi
> > > >
> > > > > -----Original Message-----
> > > > > From: Qiu, Michael
> > > > > Sent: Tuesday, December 9, 2014 11:23 AM
> > > > > To: Ouyang, Changchun; Thomas Monjalon; Stephen Hemminger
> > > > > Cc: dev@dpdk.org
> > > > > Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
> > > > > implementation
> > > > >
> > > > > On 12/9/2014 9:11 AM, Ouyang, Changchun wrote:
> > > > > > Hi Thomas,
> > > > > >
> > > > > >> -----Original Message-----
> > > > > >> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > >> Sent: Monday, December 8, 2014 5:31 PM
> > > > > >> To: Ouyang, Changchun
> > > > > >> Cc: dev@dpdk.org
> > > > > >> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
> > > > > >> implementation
> > > > > >>
> > > > > >> Hi Changchun,
> > > > > >>
> > > > > >> 2014-12-08 14:21, Ouyang Changchun:
> > > > > >>> This patch set bases on two original RFC patch sets from
> > > > > >>> Stephen
> > > > > >> Hemminger[stephen@networkplumber.org]
> > > > > >>> Refer to
> > > > > >>> [http://dpdk.org/ml/archives/dev/2014-August/004845.html ]
> > > > > >>> for
> > > > > >> the original one.
> > > > > >>> This patch set also resolves some conflict with latest codes
> > > > > >>> and removed
> > > > > >> duplicated codes.
> > > > > >>
> > > > > >> As you sent the patches, you appear as the author.
> > > > > >> But I guess Stephen should be the author for some of them.
> > > > > >> Please check who has contributed the most in each patch to
> decide.
> > > > > > You are right, most of patches originate from Stephen's
> > > > > > patchset, except for the last one, To be honest, I am ok
> > > > > > whoever is the author of this patch set, :-), We could co-own
> > > > > > the feature of Single virtio if you all agree with it, and I
> > > > > > think we couldn't finish Such a feature without collaboration
> > > > > > among us, this is why I tried to communicate
> > > > > with most of you to collect more feedback, suggestion and
> > > > > comments for this feature.
> > > > > > Very appreciate for all kinds of feedback, suggestion here,
> > > > > > especially for
> > > > > patch set from Stephen.
> > > > > >
> > > > > > According to your request, how could we make this patch set
> > > > > > looks more
> > > > > like Stephen as the author?
> > > > > > Currently I add Stephen as Signed-off-by list in each patch(I
> > > > > > got the
> > > > > agreement from Stephen before doing this :-)).
> > > > >
> > > > > Hi Ouyang,
> > > > >
> > > > > "Signed-off-by" should be added by himself, because the one who
> > > > > in the Signed-off-by list should take responsibility for it(like
> > > > > potential
> > > bugs/issues).
> > > > >
> > > > > Although, lots of patches are originate from Stephen, we still
> > > > > need himself add this line :)
> > > >
> > > > Hi Thomas,
> > > > It that right? I can't add Stephen into Signed-off-by list even if
> > > > I have gotten the agreement from Stephen, What 's the strict rule here?
> > >
> > > Stephen sent the patches with his Signed-off, then you added yours.
> > > This is OK.
> > > Using git am, author would have been Stephen. To change author now,
> > > you can edit each commit with interactive rebase and "git commit
> > > --amend -- author=Stephen".
> > > No need to resend now. Please check it for next version of the patchset.
> > >
> >
> > So I understand correctly, Stephen need care for from patches from 1
> > to 16, I need care for the 17th patch from next version.
> > What I mean "caring for" above is:  debug and validate them and send
> > out patches
> >
> > Thanks
> > Changchun
> >
> Just to clarify Thomas point here about use of "git am". If you get a patch
> from someone to test or work on, use "git am" to apply it, rather than "git
> apply", since "git am" generates a commit in your local repo and thereby
> maintains the original authorship of the patch. If you do "git apply" and
> subsequently commit yourself, you - rather than the original author - will
> appear as the "author" of the patch, and you need to amend the commit as
> Thomas suggests to fix this.
> 
> So in short:
> * git am == good
> * git apply == bad

Thanks very much for the clarification. I will use git am for next version.

Changchun

Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation

[Qiu, Michael]

On 2014/12/9 22:19, Ouyang, Changchun wrote:
> Hi Bruce,
>
>> -----Original Message-----
>> From: Richardson, Bruce
>> Sent: Tuesday, December 9, 2014 5:47 PM
>> To: Ouyang, Changchun
>> Cc: Thomas Monjalon; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation
>>
>> On Tue, Dec 09, 2014 at 06:40:23AM +0000, Ouyang, Changchun wrote:
>>>
>>>> -----Original Message-----
>>>> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
>>>> Sent: Tuesday, December 9, 2014 2:12 PM
>>>> To: Ouyang, Changchun
>>>> Cc: Qiu, Michael; Stephen Hemminger; dev@dpdk.org
>>>> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
>>>> implementation
>>>>
>>>> 2014-12-09 05:41, Ouyang, Changchun:
>>>>> Hi
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Qiu, Michael
>>>>>> Sent: Tuesday, December 9, 2014 11:23 AM
>>>>>> To: Ouyang, Changchun; Thomas Monjalon; Stephen Hemminger
>>>>>> Cc: dev@dpdk.org
>>>>>> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
>>>>>> implementation
>>>>>>
>>>>>> On 12/9/2014 9:11 AM, Ouyang, Changchun wrote:
>>>>>>> Hi Thomas,
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
>>>>>>>> Sent: Monday, December 8, 2014 5:31 PM
>>>>>>>> To: Ouyang, Changchun
>>>>>>>> Cc: dev@dpdk.org
>>>>>>>> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
>>>>>>>> implementation
>>>>>>>>
>>>>>>>> Hi Changchun,
>>>>>>>>
>>>>>>>> 2014-12-08 14:21, Ouyang Changchun:
>>>>>>>>> This patch set bases on two original RFC patch sets from
>>>>>>>>> Stephen
>>>>>>>> Hemminger[stephen@networkplumber.org]
>>>>>>>>> Refer to
>>>>>>>>> [http://dpdk.org/ml/archives/dev/2014-August/004845.html ]
>>>>>>>>> for
>>>>>>>> the original one.
>>>>>>>>> This patch set also resolves some conflict with latest codes
>>>>>>>>> and removed
>>>>>>>> duplicated codes.
>>>>>>>>
>>>>>>>> As you sent the patches, you appear as the author.
>>>>>>>> But I guess Stephen should be the author for some of them.
>>>>>>>> Please check who has contributed the most in each patch to
>> decide.
>>>>>>> You are right, most of patches originate from Stephen's
>>>>>>> patchset, except for the last one, To be honest, I am ok
>>>>>>> whoever is the author of this patch set, :-), We could co-own
>>>>>>> the feature of Single virtio if you all agree with it, and I
>>>>>>> think we couldn't finish Such a feature without collaboration
>>>>>>> among us, this is why I tried to communicate
>>>>>> with most of you to collect more feedback, suggestion and
>>>>>> comments for this feature.
>>>>>>> Very appreciate for all kinds of feedback, suggestion here,
>>>>>>> especially for
>>>>>> patch set from Stephen.
>>>>>>> According to your request, how could we make this patch set
>>>>>>> looks more
>>>>>> like Stephen as the author?
>>>>>>> Currently I add Stephen as Signed-off-by list in each patch(I
>>>>>>> got the
>>>>>> agreement from Stephen before doing this :-)).
>>>>>>
>>>>>> Hi Ouyang,
>>>>>>
>>>>>> "Signed-off-by" should be added by himself, because the one who
>>>>>> in the Signed-off-by list should take responsibility for it(like
>>>>>> potential
>>>> bugs/issues).
>>>>>> Although, lots of patches are originate from Stephen, we still
>>>>>> need himself add this line :)
>>>>> Hi Thomas,
>>>>> It that right? I can't add Stephen into Signed-off-by list even if
>>>>> I have gotten the agreement from Stephen, What 's the strict rule here?
>>>> Stephen sent the patches with his Signed-off, then you added yours.
>>>> This is OK.
>>>> Using git am, author would have been Stephen. To change author now,
>>>> you can edit each commit with interactive rebase and "git commit
>>>> --amend -- author=Stephen".
>>>> No need to resend now. Please check it for next version of the patchset.
>>>>
>>> So I understand correctly, Stephen need care for from patches from 1
>>> to 16, I need care for the 17th patch from next version.
>>> What I mean "caring for" above is:  debug and validate them and send
>>> out patches
>>>
>>> Thanks
>>> Changchun
>>>
>> Just to clarify Thomas point here about use of "git am". If you get a patch
>> from someone to test or work on, use "git am" to apply it, rather than "git
>> apply", since "git am" generates a commit in your local repo and thereby
>> maintains the original authorship of the patch. If you do "git apply" and
>> subsequently commit yourself, you - rather than the original author - will
>> appear as the "author" of the patch, and you need to amend the commit as
>> Thomas suggests to fix this.
>>
>> So in short:
>> * git am == good
>> * git apply == bad
> Thanks very much for the clarification. I will use git am for next version.

BTW, you also can use "git am ./xx/*" to patch a series patch set to
your local git tree.

Thanks,
Michael
> Changchun
>
>

Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation

[Ouyang, Changchun]

Hi Michael,

> -----Original Message-----
> From: Qiu, Michael
> Sent: Wednesday, December 10, 2014 12:03 AM
> To: Ouyang, Changchun; Richardson, Bruce
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio implementation
> 
> On 2014/12/9 22:19, Ouyang, Changchun wrote:
> > Hi Bruce,
> >
> >> -----Original Message-----
> >> From: Richardson, Bruce
> >> Sent: Tuesday, December 9, 2014 5:47 PM
> >> To: Ouyang, Changchun
> >> Cc: Thomas Monjalon; dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
> >> implementation
> >>
> >> On Tue, Dec 09, 2014 at 06:40:23AM +0000, Ouyang, Changchun wrote:
> >>>
> >>>> -----Original Message-----
> >>>> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> >>>> Sent: Tuesday, December 9, 2014 2:12 PM
> >>>> To: Ouyang, Changchun
> >>>> Cc: Qiu, Michael; Stephen Hemminger; dev@dpdk.org
> >>>> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
> >>>> implementation
> >>>>
> >>>> 2014-12-09 05:41, Ouyang, Changchun:
> >>>>> Hi
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Qiu, Michael
> >>>>>> Sent: Tuesday, December 9, 2014 11:23 AM
> >>>>>> To: Ouyang, Changchun; Thomas Monjalon; Stephen Hemminger
> >>>>>> Cc: dev@dpdk.org
> >>>>>> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
> >>>>>> implementation
> >>>>>>
> >>>>>> On 12/9/2014 9:11 AM, Ouyang, Changchun wrote:
> >>>>>>> Hi Thomas,
> >>>>>>>
> >>>>>>>> -----Original Message-----
> >>>>>>>> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> >>>>>>>> Sent: Monday, December 8, 2014 5:31 PM
> >>>>>>>> To: Ouyang, Changchun
> >>>>>>>> Cc: dev@dpdk.org
> >>>>>>>> Subject: Re: [dpdk-dev] [RFC PATCH 00/17] Single virtio
> >>>>>>>> implementation
> >>>>>>>>
> >>>>>>>> Hi Changchun,
> >>>>>>>>
> >>>>>>>> 2014-12-08 14:21, Ouyang Changchun:
> >>>>>>>>> This patch set bases on two original RFC patch sets from
> >>>>>>>>> Stephen
> >>>>>>>> Hemminger[stephen@networkplumber.org]
> >>>>>>>>> Refer to
> >>>>>>>>> [http://dpdk.org/ml/archives/dev/2014-August/004845.html ]
> for
> >>>>>>>> the original one.
> >>>>>>>>> This patch set also resolves some conflict with latest codes
> >>>>>>>>> and removed
> >>>>>>>> duplicated codes.
> >>>>>>>>
> >>>>>>>> As you sent the patches, you appear as the author.
> >>>>>>>> But I guess Stephen should be the author for some of them.
> >>>>>>>> Please check who has contributed the most in each patch to
> >> decide.
> >>>>>>> You are right, most of patches originate from Stephen's
> >>>>>>> patchset, except for the last one, To be honest, I am ok whoever
> >>>>>>> is the author of this patch set, :-), We could co-own the
> >>>>>>> feature of Single virtio if you all agree with it, and I think
> >>>>>>> we couldn't finish Such a feature without collaboration among
> >>>>>>> us, this is why I tried to communicate
> >>>>>> with most of you to collect more feedback, suggestion and
> >>>>>> comments for this feature.
> >>>>>>> Very appreciate for all kinds of feedback, suggestion here,
> >>>>>>> especially for
> >>>>>> patch set from Stephen.
> >>>>>>> According to your request, how could we make this patch set
> >>>>>>> looks more
> >>>>>> like Stephen as the author?
> >>>>>>> Currently I add Stephen as Signed-off-by list in each patch(I
> >>>>>>> got the
> >>>>>> agreement from Stephen before doing this :-)).
> >>>>>>
> >>>>>> Hi Ouyang,
> >>>>>>
> >>>>>> "Signed-off-by" should be added by himself, because the one who
> >>>>>> in the Signed-off-by list should take responsibility for it(like
> >>>>>> potential
> >>>> bugs/issues).
> >>>>>> Although, lots of patches are originate from Stephen, we still
> >>>>>> need himself add this line :)
> >>>>> Hi Thomas,
> >>>>> It that right? I can't add Stephen into Signed-off-by list even if
> >>>>> I have gotten the agreement from Stephen, What 's the strict rule
> here?
> >>>> Stephen sent the patches with his Signed-off, then you added yours.
> >>>> This is OK.
> >>>> Using git am, author would have been Stephen. To change author now,
> >>>> you can edit each commit with interactive rebase and "git commit
> >>>> --amend -- author=Stephen".
> >>>> No need to resend now. Please check it for next version of the
> patchset.
> >>>>
> >>> So I understand correctly, Stephen need care for from patches from 1
> >>> to 16, I need care for the 17th patch from next version.
> >>> What I mean "caring for" above is:  debug and validate them and send
> >>> out patches
> >>>
> >>> Thanks
> >>> Changchun
> >>>
> >> Just to clarify Thomas point here about use of "git am". If you get a
> >> patch from someone to test or work on, use "git am" to apply it,
> >> rather than "git apply", since "git am" generates a commit in your
> >> local repo and thereby maintains the original authorship of the
> >> patch. If you do "git apply" and subsequently commit yourself, you -
> >> rather than the original author - will appear as the "author" of the
> >> patch, and you need to amend the commit as Thomas suggests to fix this.
> >>
> >> So in short:
> >> * git am == good
> >> * git apply == bad
> > Thanks very much for the clarification. I will use git am for next version.
> 
> BTW, you also can use "git am ./xx/*" to patch a series patch set to your local
> git tree.

Ok,  thanks for sharing.
 
Thanks and regards,,
Changchun