[PATCH 0/5] add LSC and Rxq interrupt for virtio-user

[PATCH 0/5] add LSC and Rxq interrupt for virtio-user

[Jianfeng Tan]

This is an attempt to add LSC and Rxq interrupt for a virtual device,
virtio-user (with the backend of both vhost-user and vhost-kernel).

HOW TO TEST:

Step 1: start testpmd with a virtual vhost device:
  $ testpmd -c 0x3 -n 4 --socket-mem 1024 --no-pci \
	  --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i

Step 2: start l3fwd-power with a virtio-user device:
  $ l3fwd-power -c 0xc -n 4 --socket-mem 1024 --no-pci \
	  --file-prefix=l3fwd-pwd \
	  --vdev=virtio_user0,path=/tmp/sock0 \
	  -- -p 1 -P --config="(0,0,1)" \
	  --no-numa --parse-ptype

Step 3: start burst in testpmd
  (testpmd)> start tx_first

  Packets will be received and forwarded back by l3fwd-power.
  And l3fwd-power will keep in polling mode.

Step 4: stop burst in testpmd
  (testpmd)> stop

  l3fwd-power will change to interrupt mode with few CPU
  consumption.

Step 5: kill testpmd
  $ kill -p `pidof testpmd`

  The link status will be changed to down in l3fwd-power.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>

Jianfeng Tan (5):
  eal/linux: add interrupt type for vdev
  net/virtio-user: add rxq interrupt mode support
  net/virtio-user: support to report net status
  net/virtio-user: add lsc support with vhost-user adapter
  net/virtio-user: add lsc support with vhost-kernel adapter

 drivers/net/virtio/virtio_ethdev.c                 | 46 +++++++++----
 drivers/net/virtio/virtio_ethdev.h                 |  2 +
 drivers/net/virtio/virtio_user/virtio_user_dev.c   | 52 ++++++++++++++-
 drivers/net/virtio/virtio_user/virtio_user_dev.h   |  4 +-
 drivers/net/virtio/virtio_user_ethdev.c            | 76 +++++++++++++++++++---
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       | 32 ++++++++-
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c         |  2 +
 .../linuxapp/eal/include/exec-env/rte_interrupts.h |  5 +-
 8 files changed, 192 insertions(+), 27 deletions(-)

-- 
2.7.4

[PATCH 1/5] eal/linux: add interrupt type for vdev

[Jianfeng Tan]

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       | 32 ++++++++++++++++++++--
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c         |  2 ++
 .../linuxapp/eal/include/exec-env/rte_interrupts.h |  5 ++--
 3 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index b5b3f2b..afe6ee6 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -46,6 +46,7 @@
 #include <sys/ioctl.h>
 #include <sys/eventfd.h>
 #include <assert.h>
+#include <stdbool.h>
 
 #include <rte_common.h>
 #include <rte_interrupts.h>
@@ -581,6 +582,9 @@ rte_intr_callback_unregister(const struct rte_intr_handle *intr_handle,
 int
 rte_intr_enable(const struct rte_intr_handle *intr_handle)
 {
+	if (intr_handle && intr_handle->type == RTE_INTR_HANDLE_VDEV)
+		return 0;
+
 	if (!intr_handle || intr_handle->fd < 0 || intr_handle->uio_cfg_fd < 0)
 		return -1;
 
@@ -625,6 +629,9 @@ rte_intr_enable(const struct rte_intr_handle *intr_handle)
 int
 rte_intr_disable(const struct rte_intr_handle *intr_handle)
 {
+	if (intr_handle && intr_handle->type == RTE_INTR_HANDLE_VDEV)
+		return 0;
+
 	if (!intr_handle || intr_handle->fd < 0 || intr_handle->uio_cfg_fd < 0)
 		return -1;
 
@@ -669,6 +676,7 @@ rte_intr_disable(const struct rte_intr_handle *intr_handle)
 static int
 eal_intr_process_interrupts(struct epoll_event *events, int nfds)
 {
+	bool call;
 	int n, bytes_read;
 	struct rte_intr_source *src;
 	struct rte_intr_callback *cb;
@@ -701,6 +709,8 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds)
 		src->active = 1;
 		rte_spinlock_unlock(&intr_lock);
 
+		call = false;
+
 		/* set the length to be read dor different handle type */
 		switch (src->intr_handle.type) {
 		case RTE_INTR_HANDLE_UIO:
@@ -717,13 +727,18 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds)
 			bytes_read = sizeof(buf.vfio_intr_count);
 			break;
 #endif
+		case RTE_INTR_HANDLE_VDEV:
 		case RTE_INTR_HANDLE_EXT:
+			bytes_read = 0;
+			call = true;
+			break;
+
 		default:
 			bytes_read = 1;
 			break;
 		}
 
-		if (src->intr_handle.type != RTE_INTR_HANDLE_EXT) {
+		if (bytes_read > 0) {
 			/**
 			 * read out to clear the ready-to-be-read flag
 			 * for epoll_wait.
@@ -740,12 +755,14 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds)
 			} else if (bytes_read == 0)
 				RTE_LOG(ERR, EAL, "Read nothing from file "
 					"descriptor %d\n", events[n].data.fd);
+			else
+				call = true;
 		}
 
 		/* grab a lock, again to call callbacks and update status. */
 		rte_spinlock_lock(&intr_lock);
 
-		if (bytes_read > 0) {
+		if (call) {
 
 			/* Finally, call all callbacks. */
 			TAILQ_FOREACH(cb, &src->callbacks, next) {
@@ -856,7 +873,7 @@ eal_intr_thread_main(__rte_unused void *arg)
 		TAILQ_FOREACH(src, &intr_sources, next) {
 			if (src->callbacks.tqh_first == NULL)
 				continue; /* skip those with no callbacks */
-			ev.events = EPOLLIN | EPOLLPRI;
+			ev.events = EPOLLIN | EPOLLPRI | EPOLLRDHUP | EPOLLHUP;
 			ev.data.fd = src->intr_handle.fd;
 
 			/**
@@ -937,6 +954,12 @@ eal_intr_proc_rxtx_intr(int fd, const struct rte_intr_handle *intr_handle)
 		bytes_read = sizeof(buf.vfio_intr_count);
 		break;
 #endif
+	case RTE_INTR_HANDLE_VDEV:
+		/* for vdev, fd points to:
+		 * a. eventfd which does not need to read out;
+		 * b. datapath fd which needs PMD to read out.
+		 */
+		return;
 	default:
 		bytes_read = 1;
 		RTE_LOG(INFO, EAL, "unexpected intr type\n");
@@ -1242,5 +1265,8 @@ rte_intr_cap_multiple(struct rte_intr_handle *intr_handle)
 	if (intr_handle->type == RTE_INTR_HANDLE_VFIO_MSIX)
 		return 1;
 
+	if (intr_handle->type == RTE_INTR_HANDLE_VDEV)
+		return 1;
+
 	return 0;
 }
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 5f478c5..5486346 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -268,6 +268,8 @@ pci_vfio_setup_interrupts(struct rte_pci_device *dev, int vfio_dev_fd)
 			return -1;
 		}
 
+		printf("intr_handle.fd = %d\n", fd);
+
 		dev->intr_handle.fd = fd;
 		dev->intr_handle.vfio_dev_fd = vfio_dev_fd;
 
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
index d459bf4..b8ee1de 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
@@ -49,8 +49,9 @@ enum rte_intr_handle_type {
 	RTE_INTR_HANDLE_VFIO_LEGACY,  /**< vfio device handle (legacy) */
 	RTE_INTR_HANDLE_VFIO_MSI,     /**< vfio device handle (MSI) */
 	RTE_INTR_HANDLE_VFIO_MSIX,    /**< vfio device handle (MSIX) */
-	RTE_INTR_HANDLE_ALARM,    /**< alarm handle */
-	RTE_INTR_HANDLE_EXT, /**< external handler */
+	RTE_INTR_HANDLE_ALARM,        /**< alarm handle */
+	RTE_INTR_HANDLE_EXT,          /**< external handler */
+	RTE_INTR_HANDLE_VDEV,         /**< virtual device */
 	RTE_INTR_HANDLE_MAX
 };
 
-- 
2.7.4

[PATCH 2/5] net/virtio-user: add rxq interrupt mode support

[Jianfeng Tan]

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_ethdev.c               | 17 ++++++++++++++--
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 25 +++++++++++++++++++++++-
 drivers/net/virtio/virtio_user/virtio_user_dev.h |  2 +-
 drivers/net/virtio/virtio_user_ethdev.c          | 12 +++++++++++-
 4 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 4dc03b9..5d80d1a 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1264,6 +1264,10 @@ virtio_configure_intr(struct rte_eth_dev *dev)
 {
 	struct virtio_hw *hw = dev->data->dev_private;
 
+
+#ifdef RTE_VIRTIO_USER
+        if (!hw->virtio_user_dev) {
+#endif
 	if (!rte_intr_cap_multiple(dev->intr_handle)) {
 		PMD_INIT_LOG(ERR, "Multiple intr vector not supported");
 		return -ENOTSUP;
@@ -1273,6 +1277,9 @@ virtio_configure_intr(struct rte_eth_dev *dev)
 		PMD_INIT_LOG(ERR, "Fail to create eventfd");
 		return -1;
 	}
+#ifdef RTE_VIRTIO_USER
+	}
+#endif
 
 	if (!dev->intr_handle->intr_vec) {
 		dev->intr_handle->intr_vec =
@@ -1293,6 +1300,9 @@ virtio_configure_intr(struct rte_eth_dev *dev)
 				   virtio_interrupt_handler,
 				   dev);
 
+#ifdef RTE_VIRTIO_USER
+        if (!hw->virtio_user_dev) {
+#endif
 	/* DO NOT try to remove this! This function will enable msix, or QEMU
 	 * will encounter SIGSEGV when DRIVER_OK is sent.
 	 * And for legacy devices, this should be done before queue/vec binding
@@ -1303,6 +1313,9 @@ virtio_configure_intr(struct rte_eth_dev *dev)
 		PMD_DRV_LOG(ERR, "interrupt enable failed");
 		return -1;
 	}
+#ifdef RTE_VIRTIO_USER
+	}
+#endif
 
 	if (virtio_queues_bind_intr(dev) < 0) {
 		PMD_INIT_LOG(ERR, "Failed to bind queue/interrupt");
@@ -1409,6 +1422,8 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
 	if (ret < 0)
 		return ret;
 
+	vtpci_reinit_complete(hw);
+
 	if (eth_dev->data->dev_conf.intr_conf.rxq) {
 		if (virtio_configure_intr(eth_dev) < 0) {
 			PMD_INIT_LOG(ERR, "failed to configure interrupt");
@@ -1416,8 +1431,6 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
 		}
 	}
 
-	vtpci_reinit_complete(hw);
-
 	if (pci_dev)
 		PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x",
 			eth_dev->data->port_id, pci_dev->id.vendor_id,
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 21ed00d..9777d6b 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -140,8 +140,28 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
 	return 0;
 }
 
+static void
+virtio_user_fill_intr_handle(struct virtio_user_dev *dev, uint8_t portid)
+{
+	uint32_t i;
+	struct rte_eth_dev *eth_dev = &rte_eth_devices[portid];
+
+	if (!eth_dev->intr_handle) {
+		eth_dev->intr_handle = malloc(sizeof(*eth_dev->intr_handle));
+		if (!eth_dev->intr_handle)
+			return;
+		memset(eth_dev->intr_handle, 0, sizeof(*eth_dev->intr_handle));
+	}
+
+	for (i = 0; i < dev->max_queue_pairs; ++i)
+		eth_dev->intr_handle->efds[i] = dev->callfds[i];
+	eth_dev->intr_handle->nb_efd = dev->max_queue_pairs;
+	eth_dev->intr_handle->max_intr = dev->max_queue_pairs + 1;
+	eth_dev->intr_handle->type = RTE_INTR_HANDLE_VDEV;
+}
+
 int
-virtio_user_start_device(struct virtio_user_dev *dev)
+virtio_user_start_device(struct virtio_user_dev *dev, uint8_t portid)
 {
 	uint64_t features;
 	int ret;
@@ -175,6 +195,9 @@ virtio_user_start_device(struct virtio_user_dev *dev)
 	 */
 	dev->ops->enable_qp(dev, 0, 1);
 
+	/* Step 5: prepare for interrupt mode */
+	virtio_user_fill_intr_handle(dev, portid);
+
 	return 0;
 error:
 	/* TODO: free resource here or caller to check */
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 0d39f40..3b529f0 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -66,7 +66,7 @@ struct virtio_user_dev {
 	struct virtio_user_backend_ops *ops;
 };
 
-int virtio_user_start_device(struct virtio_user_dev *dev);
+int virtio_user_start_device(struct virtio_user_dev *dev, uint8_t portid);
 int virtio_user_stop_device(struct virtio_user_dev *dev);
 int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 			 int cq, int queue_size, const char *mac);
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 0b226ac..fa79419 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -101,7 +101,7 @@ virtio_user_set_status(struct virtio_hw *hw, uint8_t status)
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
 
 	if (status & VIRTIO_CONFIG_STATUS_DRIVER_OK)
-		virtio_user_start_device(dev);
+		virtio_user_start_device(dev, hw->port_id);
 	else if (status == VIRTIO_CONFIG_STATUS_RESET)
 		virtio_user_reset(hw);
 	dev->status = status;
@@ -148,6 +148,15 @@ virtio_user_set_config_irq(struct virtio_hw *hw __rte_unused,
 	return VIRTIO_MSI_NO_VECTOR;
 }
 
+static uint16_t
+virtio_user_set_queue_irq(struct virtio_hw *hw __rte_unused,
+			  struct virtqueue *vq __rte_unused,
+			  uint16_t vec)
+{
+	/* pretend we have done that */
+	return vec;
+}
+
 /* This function is to get the queue size, aka, number of descs, of a specified
  * queue. Different with the VHOST_USER_GET_QUEUE_NUM, which is used to get the
  * max supported queues.
@@ -226,6 +235,7 @@ const struct virtio_pci_ops virtio_user_ops = {
 	.set_features	= virtio_user_set_features,
 	.get_isr	= virtio_user_get_isr,
 	.set_config_irq	= virtio_user_set_config_irq,
+	.set_queue_irq	= virtio_user_set_queue_irq,
 	.get_queue_num	= virtio_user_get_queue_num,
 	.setup_queue	= virtio_user_setup_queue,
 	.del_queue	= virtio_user_del_queue,
-- 
2.7.4

Re: [PATCH 2/5] net/virtio-user: add rxq interrupt mode support

[Yuanhan Liu]

On Fri, Mar 03, 2017 at 05:56:40PM +0000, Jianfeng Tan wrote:
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>

I don't see a single word to explain how this patch works :/

> ---
>  drivers/net/virtio/virtio_ethdev.c               | 17 ++++++++++++++--
>  drivers/net/virtio/virtio_user/virtio_user_dev.c | 25 +++++++++++++++++++++++-
>  drivers/net/virtio/virtio_user/virtio_user_dev.h |  2 +-
>  drivers/net/virtio/virtio_user_ethdev.c          | 12 +++++++++++-
>  4 files changed, 51 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 4dc03b9..5d80d1a 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1264,6 +1264,10 @@ virtio_configure_intr(struct rte_eth_dev *dev)
>  {
>  	struct virtio_hw *hw = dev->data->dev_private;
>  
> +
> +#ifdef RTE_VIRTIO_USER
> +        if (!hw->virtio_user_dev) {
> +#endif

No need to put the #ifdef block here. virtio_user_dev is defined even
when RTE_VIRTIO_USER is not configured.

...

> +	vtpci_reinit_complete(hw);
> +
>  	if (eth_dev->data->dev_conf.intr_conf.rxq) {
>  		if (virtio_configure_intr(eth_dev) < 0) {
>  			PMD_INIT_LOG(ERR, "failed to configure interrupt");
> @@ -1416,8 +1431,6 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
>  		}
>  	}
>  
> -	vtpci_reinit_complete(hw);


Hmm, why ...? Such stealthy change definitely need an explanation.
And it's more likely it needs a single patch.

	--yliu

Re: [PATCH 2/5] net/virtio-user: add rxq interrupt mode support

[Tan, Jianfeng]

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Friday, March 17, 2017 2:48 PM
> To: Tan, Jianfeng
> Cc: dev@dpdk.org; david.marchand@6wind.com
> Subject: Re: [PATCH 2/5] net/virtio-user: add rxq interrupt mode support
> 
> On Fri, Mar 03, 2017 at 05:56:40PM +0000, Jianfeng Tan wrote:
> > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> 
> I don't see a single word to explain how this patch works :/

Sorry, will add that in next version.

> 
> > ---
> >  drivers/net/virtio/virtio_ethdev.c               | 17 ++++++++++++++--
> >  drivers/net/virtio/virtio_user/virtio_user_dev.c | 25
> +++++++++++++++++++++++-
> >  drivers/net/virtio/virtio_user/virtio_user_dev.h |  2 +-
> >  drivers/net/virtio/virtio_user_ethdev.c          | 12 +++++++++++-
> >  4 files changed, 51 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
> > index 4dc03b9..5d80d1a 100644
> > --- a/drivers/net/virtio/virtio_ethdev.c
> > +++ b/drivers/net/virtio/virtio_ethdev.c
> > @@ -1264,6 +1264,10 @@ virtio_configure_intr(struct rte_eth_dev *dev)
> >  {
> >  	struct virtio_hw *hw = dev->data->dev_private;
> >
> > +
> > +#ifdef RTE_VIRTIO_USER
> > +        if (!hw->virtio_user_dev) {
> > +#endif
> 
> No need to put the #ifdef block here. virtio_user_dev is defined even
> when RTE_VIRTIO_USER is not configured.

Correct. I'll remove that.

> 
> ...
> 
> > +	vtpci_reinit_complete(hw);
> > +
> >  	if (eth_dev->data->dev_conf.intr_conf.rxq) {
> >  		if (virtio_configure_intr(eth_dev) < 0) {
> >  			PMD_INIT_LOG(ERR, "failed to configure interrupt");
> > @@ -1416,8 +1431,6 @@ virtio_init_device(struct rte_eth_dev *eth_dev,
> uint64_t req_features)
> >  		}
> >  	}
> >
> > -	vtpci_reinit_complete(hw);
> 
> 
> Hmm, why ...? Such stealthy change definitely need an explanation.
> And it's more likely it needs a single patch.

I should have described it more. The reason of above change is to make sure intr_handle is filled in virtio_user_start_device(). But reconsidering this, this could have effect on virtio pci device as it requires interrupts are set up before setting DRIVER_OK. So how about:

1. configure intr for PCI devices;
2. vtpci_reinit_complete();
3. configure intr for vdev;

Thanks,
Jianfeng

[PATCH 3/5] net/virtio-user: support to report net status

[Jianfeng Tan]

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_user/virtio_user_dev.c |  1 +
 drivers/net/virtio/virtio_user_ethdev.c          | 13 +++++++------
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 9777d6b..cc6f557 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -176,6 +176,7 @@ virtio_user_start_device(struct virtio_user_dev *dev, uint8_t portid)
 	features &= ~(1ull << VIRTIO_NET_F_MAC);
 	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to know */
 	features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
+	features &= ~(1ull << VIRTIO_NET_F_STATUS);
 	ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES, &features);
 	if (ret < 0)
 		goto error;
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index fa79419..fbdd0a8 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -121,7 +121,8 @@ virtio_user_get_features(struct virtio_hw *hw)
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
 
 	/* unmask feature bits defined in vhost user protocol */
-	return dev->device_features & VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
+	return (dev->device_features | (1 << VIRTIO_NET_F_STATUS))
+		& VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
 }
 
 static void
@@ -129,23 +130,23 @@ virtio_user_set_features(struct virtio_hw *hw, uint64_t features)
 {
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
 
-	dev->features = features & dev->device_features;
+	dev->features = features;
 }
 
 static uint8_t
 virtio_user_get_isr(struct virtio_hw *hw __rte_unused)
 {
-	/* When config interrupt happens, driver calls this function to query
-	 * what kinds of change happen. Interrupt mode not supported for now.
+	/* rxq interrupts and config interrupt are separated in virtio-user,
+	 * here we only report config change.
 	 */
-	return 0;
+	return VIRTIO_PCI_ISR_CONFIG;
 }
 
 static uint16_t
 virtio_user_set_config_irq(struct virtio_hw *hw __rte_unused,
 		    uint16_t vec __rte_unused)
 {
-	return VIRTIO_MSI_NO_VECTOR;
+	return 0;
 }
 
 static uint16_t
-- 
2.7.4

Re: [PATCH 3/5] net/virtio-user: support to report net status

[Yuanhan Liu]

On Fri, Mar 03, 2017 at 05:56:41PM +0000, Jianfeng Tan wrote:
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>

Yet again, not a single work on explanation.

> ---
>  drivers/net/virtio/virtio_user/virtio_user_dev.c |  1 +
>  drivers/net/virtio/virtio_user_ethdev.c          | 13 +++++++------
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 9777d6b..cc6f557 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -176,6 +176,7 @@ virtio_user_start_device(struct virtio_user_dev *dev, uint8_t portid)
>  	features &= ~(1ull << VIRTIO_NET_F_MAC);
>  	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to know */
>  	features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
> +	features &= ~(1ull << VIRTIO_NET_F_STATUS);
>  	ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES, &features);
>  	if (ret < 0)
>  		goto error;
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
> index fa79419..fbdd0a8 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -121,7 +121,8 @@ virtio_user_get_features(struct virtio_hw *hw)
>  	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
>  
>  	/* unmask feature bits defined in vhost user protocol */
> -	return dev->device_features & VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
> +	return (dev->device_features | (1 << VIRTIO_NET_F_STATUS))
> +		& VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;

Why not handle the features at virtio_user_dev_init()?

>  }
>  
>  static void
> @@ -129,23 +130,23 @@ virtio_user_set_features(struct virtio_hw *hw, uint64_t features)
>  {
>  	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
>  
> -	dev->features = features & dev->device_features;
> +	dev->features = features;
>  }
>  
>  static uint8_t
>  virtio_user_get_isr(struct virtio_hw *hw __rte_unused)
>  {
> -	/* When config interrupt happens, driver calls this function to query
> -	 * what kinds of change happen. Interrupt mode not supported for now.
> +	/* rxq interrupts and config interrupt are separated in virtio-user,
> +	 * here we only report config change.
>  	 */
> -	return 0;
> +	return VIRTIO_PCI_ISR_CONFIG;
>  }
>  
>  static uint16_t
>  virtio_user_set_config_irq(struct virtio_hw *hw __rte_unused,
>  		    uint16_t vec __rte_unused)
>  {
> -	return VIRTIO_MSI_NO_VECTOR;
> +	return 0;

And the above two changes have something to do with this patch (support
to report net statu)?

	--yliu

Re: [PATCH 3/5] net/virtio-user: support to report net status

[Tan, Jianfeng]

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Friday, March 17, 2017 2:55 PM
> To: Tan, Jianfeng
> Cc: dev@dpdk.org; david.marchand@6wind.com
> Subject: Re: [PATCH 3/5] net/virtio-user: support to report net status
> 
> On Fri, Mar 03, 2017 at 05:56:41PM +0000, Jianfeng Tan wrote:
> > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> 
> Yet again, not a single work on explanation.

Sorry, will add in next version.

> 
> > ---
> >  drivers/net/virtio/virtio_user/virtio_user_dev.c |  1 +
> >  drivers/net/virtio/virtio_user_ethdev.c          | 13 +++++++------
> >  2 files changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > index 9777d6b..cc6f557 100644
> > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > @@ -176,6 +176,7 @@ virtio_user_start_device(struct virtio_user_dev
> *dev, uint8_t portid)
> >  	features &= ~(1ull << VIRTIO_NET_F_MAC);
> >  	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to
> know */
> >  	features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
> > +	features &= ~(1ull << VIRTIO_NET_F_STATUS);
> >  	ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES,
> &features);
> >  	if (ret < 0)
> >  		goto error;
> > diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> b/drivers/net/virtio/virtio_user_ethdev.c
> > index fa79419..fbdd0a8 100644
> > --- a/drivers/net/virtio/virtio_user_ethdev.c
> > +++ b/drivers/net/virtio/virtio_user_ethdev.c
> > @@ -121,7 +121,8 @@ virtio_user_get_features(struct virtio_hw *hw)
> >  	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
> >
> >  	/* unmask feature bits defined in vhost user protocol */
> > -	return dev->device_features &
> VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
> > +	return (dev->device_features | (1 << VIRTIO_NET_F_STATUS))
> > +		& VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
> 
> Why not handle the features at virtio_user_dev_init()?

You mean add VIRTIO_NET_F_STATUS when get_features from device? Yes, we could do that there. But we originally add device_features to only record features supported by device.

> 
> >  }
> >
> >  static void
> > @@ -129,23 +130,23 @@ virtio_user_set_features(struct virtio_hw *hw,
> uint64_t features)
> >  {
> >  	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
> >
> > -	dev->features = features & dev->device_features;
> > +	dev->features = features;
> >  }
> >
> >  static uint8_t
> >  virtio_user_get_isr(struct virtio_hw *hw __rte_unused)
> >  {
> > -	/* When config interrupt happens, driver calls this function to query
> > -	 * what kinds of change happen. Interrupt mode not supported for
> now.
> > +	/* rxq interrupts and config interrupt are separated in virtio-user,
> > +	 * here we only report config change.
> >  	 */
> > -	return 0;
> > +	return VIRTIO_PCI_ISR_CONFIG;
> >  }
> >
> >  static uint16_t
> >  virtio_user_set_config_irq(struct virtio_hw *hw __rte_unused,
> >  		    uint16_t vec __rte_unused)
> >  {
> > -	return VIRTIO_MSI_NO_VECTOR;
> > +	return 0;
> 
> And the above two changes have something to do with this patch (support
> to report net statu)?

Yes, just pretend that irq/vec binding is configured successfully. When setting up config irq, the result is checked.

Thanks,
Jianfeng

Re: [PATCH 3/5] net/virtio-user: support to report net status

[Yuanhan Liu]

On Mon, Mar 27, 2017 at 07:46:32AM +0000, Tan, Jianfeng wrote:
> > > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > > index 9777d6b..cc6f557 100644
> > > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > > @@ -176,6 +176,7 @@ virtio_user_start_device(struct virtio_user_dev
> > *dev, uint8_t portid)
> > >  	features &= ~(1ull << VIRTIO_NET_F_MAC);
> > >  	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to
> > know */
> > >  	features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
> > > +	features &= ~(1ull << VIRTIO_NET_F_STATUS);
> > >  	ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES,
> > &features);
> > >  	if (ret < 0)
> > >  		goto error;
> > > diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> > b/drivers/net/virtio/virtio_user_ethdev.c
> > > index fa79419..fbdd0a8 100644
> > > --- a/drivers/net/virtio/virtio_user_ethdev.c
> > > +++ b/drivers/net/virtio/virtio_user_ethdev.c
> > > @@ -121,7 +121,8 @@ virtio_user_get_features(struct virtio_hw *hw)
> > >  	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
> > >
> > >  	/* unmask feature bits defined in vhost user protocol */
> > > -	return dev->device_features &
> > VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
> > > +	return (dev->device_features | (1 << VIRTIO_NET_F_STATUS))
> > > +		& VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
> > 
> > Why not handle the features at virtio_user_dev_init()?
> 
> You mean add VIRTIO_NET_F_STATUS when get_features from device? Yes, we could do that there. But we originally add device_features to only record features supported by device.
> 

Aren't you adding the F_STATUS features to this device?

	--yliu

Re: [PATCH 3/5] net/virtio-user: support to report net status

[Tan, Jianfeng]

On 3/29/2017 2:33 PM, Yuanhan Liu wrote:
> On Mon, Mar 27, 2017 at 07:46:32AM +0000, Tan, Jianfeng wrote:
>>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>> index 9777d6b..cc6f557 100644
>>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>> @@ -176,6 +176,7 @@ virtio_user_start_device(struct virtio_user_dev
>>> *dev, uint8_t portid)
>>>>   	features &= ~(1ull << VIRTIO_NET_F_MAC);
>>>>   	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to
>>> know */
>>>>   	features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
>>>> +	features &= ~(1ull << VIRTIO_NET_F_STATUS);
>>>>   	ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES,
>>> &features);
>>>>   	if (ret < 0)
>>>>   		goto error;
>>>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
>>> b/drivers/net/virtio/virtio_user_ethdev.c
>>>> index fa79419..fbdd0a8 100644
>>>> --- a/drivers/net/virtio/virtio_user_ethdev.c
>>>> +++ b/drivers/net/virtio/virtio_user_ethdev.c
>>>> @@ -121,7 +121,8 @@ virtio_user_get_features(struct virtio_hw *hw)
>>>>   	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
>>>>
>>>>   	/* unmask feature bits defined in vhost user protocol */
>>>> -	return dev->device_features &
>>> VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
>>>> +	return (dev->device_features | (1 << VIRTIO_NET_F_STATUS))
>>>> +		& VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
>>> Why not handle the features at virtio_user_dev_init()?
>> You mean add VIRTIO_NET_F_STATUS when get_features from device? Yes, we could do that there. But we originally add device_features to only record features supported by device.
>>
> Aren't you adding the F_STATUS features to this device?
>

For virtio driver, yes, we are adding F_STATUS feature so the it sees a 
device supporting LSC. But for backend driver, we need hide this 
feature, it happens when we set_features to the backend driver.

Thanks,
Jianfeng

Re: [PATCH 3/5] net/virtio-user: support to report net status

[Yuanhan Liu]

On Wed, Mar 29, 2017 at 03:07:28PM +0800, Tan, Jianfeng wrote:
> 
> 
> On 3/29/2017 2:33 PM, Yuanhan Liu wrote:
> >On Mon, Mar 27, 2017 at 07:46:32AM +0000, Tan, Jianfeng wrote:
> >>>>diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>>index 9777d6b..cc6f557 100644
> >>>>--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>>+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>>@@ -176,6 +176,7 @@ virtio_user_start_device(struct virtio_user_dev
> >>>*dev, uint8_t portid)
> >>>>  	features &= ~(1ull << VIRTIO_NET_F_MAC);
> >>>>  	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to
> >>>know */
> >>>>  	features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
> >>>>+	features &= ~(1ull << VIRTIO_NET_F_STATUS);
> >>>>  	ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES,
> >>>&features);
> >>>>  	if (ret < 0)
> >>>>  		goto error;
> >>>>diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> >>>b/drivers/net/virtio/virtio_user_ethdev.c
> >>>>index fa79419..fbdd0a8 100644
> >>>>--- a/drivers/net/virtio/virtio_user_ethdev.c
> >>>>+++ b/drivers/net/virtio/virtio_user_ethdev.c
> >>>>@@ -121,7 +121,8 @@ virtio_user_get_features(struct virtio_hw *hw)
> >>>>  	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
> >>>>
> >>>>  	/* unmask feature bits defined in vhost user protocol */
> >>>>-	return dev->device_features &
> >>>VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
> >>>>+	return (dev->device_features | (1 << VIRTIO_NET_F_STATUS))
> >>>>+		& VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
> >>>Why not handle the features at virtio_user_dev_init()?
> >>You mean add VIRTIO_NET_F_STATUS when get_features from device? Yes, we could do that there. But we originally add device_features to only record features supported by device.
> >>
> >Aren't you adding the F_STATUS features to this device?
> >
> 
> For virtio driver, yes, we are adding F_STATUS feature so the it sees a
> device supporting LSC.

That means you are adding a device feature (F_STATUS) and reporting it to
the driver that this feature is always on, no matter whether the device
actually supports it or not? This looks wrong to me.

> But for backend driver, we need hide this feature, it
> happens when we set_features to the backend driver.

The feature negotion of virtio-user seems broken to me.

	--yliu

Re: [PATCH 3/5] net/virtio-user: support to report net status

[Tan, Jianfeng]

On 3/29/2017 3:14 PM, Yuanhan Liu wrote:
> On Wed, Mar 29, 2017 at 03:07:28PM +0800, Tan, Jianfeng wrote:
>>
>> On 3/29/2017 2:33 PM, Yuanhan Liu wrote:
>>> On Mon, Mar 27, 2017 at 07:46:32AM +0000, Tan, Jianfeng wrote:
>>>>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>>>> index 9777d6b..cc6f557 100644
>>>>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>>>> @@ -176,6 +176,7 @@ virtio_user_start_device(struct virtio_user_dev
>>>>> *dev, uint8_t portid)
>>>>>>   	features &= ~(1ull << VIRTIO_NET_F_MAC);
>>>>>>   	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to
>>>>> know */
>>>>>>   	features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
>>>>>> +	features &= ~(1ull << VIRTIO_NET_F_STATUS);
>>>>>>   	ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES,
>>>>> &features);
>>>>>>   	if (ret < 0)
>>>>>>   		goto error;
>>>>>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
>>>>> b/drivers/net/virtio/virtio_user_ethdev.c
>>>>>> index fa79419..fbdd0a8 100644
>>>>>> --- a/drivers/net/virtio/virtio_user_ethdev.c
>>>>>> +++ b/drivers/net/virtio/virtio_user_ethdev.c
>>>>>> @@ -121,7 +121,8 @@ virtio_user_get_features(struct virtio_hw *hw)
>>>>>>   	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
>>>>>>
>>>>>>   	/* unmask feature bits defined in vhost user protocol */
>>>>>> -	return dev->device_features &
>>>>> VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
>>>>>> +	return (dev->device_features | (1 << VIRTIO_NET_F_STATUS))
>>>>>> +		& VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
>>>>> Why not handle the features at virtio_user_dev_init()?
>>>> You mean add VIRTIO_NET_F_STATUS when get_features from device? Yes, we could do that there. But we originally add device_features to only record features supported by device.
>>>>
>>> Aren't you adding the F_STATUS features to this device?
>>>
>> For virtio driver, yes, we are adding F_STATUS feature so the it sees a
>> device supporting LSC.
> That means you are adding a device feature (F_STATUS) and reporting it to
> the driver that this feature is always on, no matter whether the device
> actually supports it or not? This looks wrong to me.

Why? IMO, device is not necessary to know this feature. For vhost-user, 
except explicitly cutting down the unix connection, I don't see a 
message of vhost-user protocol to report that.

>
>> But for backend driver, we need hide this feature, it
>> happens when we set_features to the backend driver.
> The feature negotion of virtio-user seems broken to me.
>
>
Why? vhost does not claim to support this feature, VHOST_SUPPORTED_FEATURES.

Thanks,
Jianfeng

Re: [PATCH 3/5] net/virtio-user: support to report net status

[Yuanhan Liu]

On Wed, Mar 29, 2017 at 03:48:04PM +0800, Tan, Jianfeng wrote:
> 
> 
> On 3/29/2017 3:14 PM, Yuanhan Liu wrote:
> >On Wed, Mar 29, 2017 at 03:07:28PM +0800, Tan, Jianfeng wrote:
> >>
> >>On 3/29/2017 2:33 PM, Yuanhan Liu wrote:
> >>>On Mon, Mar 27, 2017 at 07:46:32AM +0000, Tan, Jianfeng wrote:
> >>>>>>diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>>>b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>>>>index 9777d6b..cc6f557 100644
> >>>>>>--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>>>>+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>>>>@@ -176,6 +176,7 @@ virtio_user_start_device(struct virtio_user_dev
> >>>>>*dev, uint8_t portid)
> >>>>>>  	features &= ~(1ull << VIRTIO_NET_F_MAC);
> >>>>>>  	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to
> >>>>>know */
> >>>>>>  	features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
> >>>>>>+	features &= ~(1ull << VIRTIO_NET_F_STATUS);
> >>>>>>  	ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES,
> >>>>>&features);
> >>>>>>  	if (ret < 0)
> >>>>>>  		goto error;
> >>>>>>diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> >>>>>b/drivers/net/virtio/virtio_user_ethdev.c
> >>>>>>index fa79419..fbdd0a8 100644
> >>>>>>--- a/drivers/net/virtio/virtio_user_ethdev.c
> >>>>>>+++ b/drivers/net/virtio/virtio_user_ethdev.c
> >>>>>>@@ -121,7 +121,8 @@ virtio_user_get_features(struct virtio_hw *hw)
> >>>>>>  	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
> >>>>>>
> >>>>>>  	/* unmask feature bits defined in vhost user protocol */
> >>>>>>-	return dev->device_features &
> >>>>>VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
> >>>>>>+	return (dev->device_features | (1 << VIRTIO_NET_F_STATUS))
> >>>>>>+		& VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
> >>>>>Why not handle the features at virtio_user_dev_init()?
> >>>>You mean add VIRTIO_NET_F_STATUS when get_features from device? Yes, we could do that there. But we originally add device_features to only record features supported by device.
> >>>>
> >>>Aren't you adding the F_STATUS features to this device?
> >>>
> >>For virtio driver, yes, we are adding F_STATUS feature so the it sees a
> >>device supporting LSC.
> >That means you are adding a device feature (F_STATUS) and reporting it to
> >the driver that this feature is always on, no matter whether the device
> >actually supports it or not? This looks wrong to me.
> 
> Why?

Because you were doing hack to make virtio-user work, while there is no
such hack in the QEMU virtio-pci implementation.

> IMO, device is not necessary to know this feature.

F_STATUS is a device feature. If such feature is not claimed to be
supported by the device, the driver should ignore it. But you are
unconditionally letting the driver handle it, even though  the
virito-user device does not claim to support it.

Note that F_STATUS is set in the QEMU virtio-net PCI device.

	--yliu

> For vhost-user,
> except explicitly cutting down the unix connection, I don't see a message of
> vhost-user protocol to report that.
> 
> >
> >>But for backend driver, we need hide this feature, it
> >>happens when we set_features to the backend driver.
> >The feature negotion of virtio-user seems broken to me.
> >
> >
> Why?
> vhost does not claim to support this feature, VHOST_SUPPORTED_FEATURES.
> 
> Thanks,
> Jianfeng

Re: [PATCH 3/5] net/virtio-user: support to report net status

[Tan, Jianfeng]

On 3/29/2017 4:00 PM, Yuanhan Liu wrote:
> On Wed, Mar 29, 2017 at 03:48:04PM +0800, Tan, Jianfeng wrote:
>>
>> On 3/29/2017 3:14 PM, Yuanhan Liu wrote:
>>> On Wed, Mar 29, 2017 at 03:07:28PM +0800, Tan, Jianfeng wrote:
>>>> On 3/29/2017 2:33 PM, Yuanhan Liu wrote:
>>>>> On Mon, Mar 27, 2017 at 07:46:32AM +0000, Tan, Jianfeng wrote:
>>>>>>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>>>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>>>>>> index 9777d6b..cc6f557 100644
>>>>>>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>>>>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>>>>>> @@ -176,6 +176,7 @@ virtio_user_start_device(struct virtio_user_dev
>>>>>>> *dev, uint8_t portid)
>>>>>>>>   	features &= ~(1ull << VIRTIO_NET_F_MAC);
>>>>>>>>   	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to
>>>>>>> know */
>>>>>>>>   	features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
>>>>>>>> +	features &= ~(1ull << VIRTIO_NET_F_STATUS);
>>>>>>>>   	ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES,
>>>>>>> &features);
>>>>>>>>   	if (ret < 0)
>>>>>>>>   		goto error;
>>>>>>>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
>>>>>>> b/drivers/net/virtio/virtio_user_ethdev.c
>>>>>>>> index fa79419..fbdd0a8 100644
>>>>>>>> --- a/drivers/net/virtio/virtio_user_ethdev.c
>>>>>>>> +++ b/drivers/net/virtio/virtio_user_ethdev.c
>>>>>>>> @@ -121,7 +121,8 @@ virtio_user_get_features(struct virtio_hw *hw)
>>>>>>>>   	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
>>>>>>>>
>>>>>>>>   	/* unmask feature bits defined in vhost user protocol */
>>>>>>>> -	return dev->device_features &
>>>>>>> VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
>>>>>>>> +	return (dev->device_features | (1 << VIRTIO_NET_F_STATUS))
>>>>>>>> +		& VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
>>>>>>> Why not handle the features at virtio_user_dev_init()?
>>>>>> You mean add VIRTIO_NET_F_STATUS when get_features from device? Yes, we could do that there. But we originally add device_features to only record features supported by device.
>>>>>>
>>>>> Aren't you adding the F_STATUS features to this device?
>>>>>
>>>> For virtio driver, yes, we are adding F_STATUS feature so the it sees a
>>>> device supporting LSC.
>>> That means you are adding a device feature (F_STATUS) and reporting it to
>>> the driver that this feature is always on, no matter whether the device
>>> actually supports it or not? This looks wrong to me.
>> Why?
> Because you were doing hack to make virtio-user work, while there is no
> such hack in the QEMU virtio-pci implementation.
>
>> IMO, device is not necessary to know this feature.
> F_STATUS is a device feature. If such feature is not claimed to be
> supported by the device, the driver should ignore it. But you are
> unconditionally letting the driver handle it, even though  the
> virito-user device does not claim to support it.
>
> Note that F_STATUS is set in the QEMU virtio-net PCI device.

In QEMU virtio-net PCI device, the device includes QEMU device emulation 
+ vhost backend driver. This feature only shows at QEMU device 
emulation, instead of vhost backend driver. This is why we did not see 
this feature at any vhost backends.
And the embedded virtio-user actually substitutes QEMU device emulation 
part, so this feature should end at this layer.

Let's look this in this way, if we let this feature go through to vhost 
user, it will be treated as an error in vhost_user_set_features().

Thanks,
Jianfeng

>
> 	--yliu
>
>> For vhost-user,
>> except explicitly cutting down the unix connection, I don't see a message of
>> vhost-user protocol to report that.
>>
>>>> But for backend driver, we need hide this feature, it
>>>> happens when we set_features to the backend driver.
>>> The feature negotion of virtio-user seems broken to me.
>>>
>>>
>> Why?
>> vhost does not claim to support this feature, VHOST_SUPPORTED_FEATURES.
>>
>> Thanks,
>> Jianfeng

Re: [PATCH 3/5] net/virtio-user: support to report net status

[Yuanhan Liu]

On Wed, Mar 29, 2017 at 04:33:20PM +0800, Tan, Jianfeng wrote:
> 
> 
> On 3/29/2017 4:00 PM, Yuanhan Liu wrote:
> >On Wed, Mar 29, 2017 at 03:48:04PM +0800, Tan, Jianfeng wrote:
> >>
> >>On 3/29/2017 3:14 PM, Yuanhan Liu wrote:
> >>>On Wed, Mar 29, 2017 at 03:07:28PM +0800, Tan, Jianfeng wrote:
> >>>>On 3/29/2017 2:33 PM, Yuanhan Liu wrote:
> >>>>>On Mon, Mar 27, 2017 at 07:46:32AM +0000, Tan, Jianfeng wrote:
> >>>>>>>>diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>>>>>b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>>>>>>index 9777d6b..cc6f557 100644
> >>>>>>>>--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>>>>>>+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>>>>>>@@ -176,6 +176,7 @@ virtio_user_start_device(struct virtio_user_dev
> >>>>>>>*dev, uint8_t portid)
> >>>>>>>>  	features &= ~(1ull << VIRTIO_NET_F_MAC);
> >>>>>>>>  	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to
> >>>>>>>know */
> >>>>>>>>  	features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
> >>>>>>>>+	features &= ~(1ull << VIRTIO_NET_F_STATUS);
> >>>>>>>>  	ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES,
> >>>>>>>&features);
> >>>>>>>>  	if (ret < 0)
> >>>>>>>>  		goto error;
> >>>>>>>>diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> >>>>>>>b/drivers/net/virtio/virtio_user_ethdev.c
> >>>>>>>>index fa79419..fbdd0a8 100644
> >>>>>>>>--- a/drivers/net/virtio/virtio_user_ethdev.c
> >>>>>>>>+++ b/drivers/net/virtio/virtio_user_ethdev.c
> >>>>>>>>@@ -121,7 +121,8 @@ virtio_user_get_features(struct virtio_hw *hw)
> >>>>>>>>  	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
> >>>>>>>>
> >>>>>>>>  	/* unmask feature bits defined in vhost user protocol */
> >>>>>>>>-	return dev->device_features &
> >>>>>>>VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
> >>>>>>>>+	return (dev->device_features | (1 << VIRTIO_NET_F_STATUS))
> >>>>>>>>+		& VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
> >>>>>>>Why not handle the features at virtio_user_dev_init()?
> >>>>>>You mean add VIRTIO_NET_F_STATUS when get_features from device? Yes, we could do that there. But we originally add device_features to only record features supported by device.
> >>>>>>
> >>>>>Aren't you adding the F_STATUS features to this device?
> >>>>>
> >>>>For virtio driver, yes, we are adding F_STATUS feature so the it sees a
> >>>>device supporting LSC.
> >>>That means you are adding a device feature (F_STATUS) and reporting it to
> >>>the driver that this feature is always on, no matter whether the device
> >>>actually supports it or not? This looks wrong to me.
> >>Why?
> >Because you were doing hack to make virtio-user work, while there is no
> >such hack in the QEMU virtio-pci implementation.
> >
> >>IMO, device is not necessary to know this feature.
> >F_STATUS is a device feature. If such feature is not claimed to be
> >supported by the device, the driver should ignore it. But you are
> >unconditionally letting the driver handle it, even though  the
> >virito-user device does not claim to support it.
> >
> >Note that F_STATUS is set in the QEMU virtio-net PCI device.
> 
> In QEMU virtio-net PCI device, the device includes QEMU device emulation +
> vhost backend driver. This feature only shows at QEMU device emulation,
> instead of vhost backend driver. This is why we did not see this feature at
> any vhost backends.
> And the embedded virtio-user actually substitutes QEMU device emulation
> part, so this feature should end at this layer.
> 
> Let's look this in this way, if we let this feature go through to vhost
> user, it will be treated as an error in vhost_user_set_features().

Again, why can't we (virtio-user) follow QEMU?

	--yliu

Re: [PATCH 3/5] net/virtio-user: support to report net status

[Tan, Jianfeng]

On 3/29/2017 4:36 PM, Yuanhan Liu wrote:
> On Wed, Mar 29, 2017 at 04:33:20PM +0800, Tan, Jianfeng wrote:
>>
>> On 3/29/2017 4:00 PM, Yuanhan Liu wrote:
>>> On Wed, Mar 29, 2017 at 03:48:04PM +0800, Tan, Jianfeng wrote:
>>>> On 3/29/2017 3:14 PM, Yuanhan Liu wrote:
>>>>> On Wed, Mar 29, 2017 at 03:07:28PM +0800, Tan, Jianfeng wrote:
>>>>>> On 3/29/2017 2:33 PM, Yuanhan Liu wrote:
>>>>>>> On Mon, Mar 27, 2017 at 07:46:32AM +0000, Tan, Jianfeng wrote:
>>>>>>>>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>>>>>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>>>>>>>> index 9777d6b..cc6f557 100644
>>>>>>>>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>>>>>>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>>>>>>>> @@ -176,6 +176,7 @@ virtio_user_start_device(struct virtio_user_dev
>>>>>>>>> *dev, uint8_t portid)
>>>>>>>>>>   	features &= ~(1ull << VIRTIO_NET_F_MAC);
>>>>>>>>>>   	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to
>>>>>>>>> know */
>>>>>>>>>>   	features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
>>>>>>>>>> +	features &= ~(1ull << VIRTIO_NET_F_STATUS);
>>>>>>>>>>   	ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES,
>>>>>>>>> &features);
>>>>>>>>>>   	if (ret < 0)
>>>>>>>>>>   		goto error;
>>>>>>>>>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
>>>>>>>>> b/drivers/net/virtio/virtio_user_ethdev.c
>>>>>>>>>> index fa79419..fbdd0a8 100644
>>>>>>>>>> --- a/drivers/net/virtio/virtio_user_ethdev.c
>>>>>>>>>> +++ b/drivers/net/virtio/virtio_user_ethdev.c
>>>>>>>>>> @@ -121,7 +121,8 @@ virtio_user_get_features(struct virtio_hw *hw)
>>>>>>>>>>   	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
>>>>>>>>>>
>>>>>>>>>>   	/* unmask feature bits defined in vhost user protocol */
>>>>>>>>>> -	return dev->device_features &
>>>>>>>>> VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
>>>>>>>>>> +	return (dev->device_features | (1 << VIRTIO_NET_F_STATUS))
>>>>>>>>>> +		& VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
>>>>>>>>> Why not handle the features at virtio_user_dev_init()?
>>>>>>>> You mean add VIRTIO_NET_F_STATUS when get_features from device? Yes, we could do that there. But we originally add device_features to only record features supported by device.
>>>>>>>>
>>>>>>> Aren't you adding the F_STATUS features to this device?
>>>>>>>
>>>>>> For virtio driver, yes, we are adding F_STATUS feature so the it sees a
>>>>>> device supporting LSC.
>>>>> That means you are adding a device feature (F_STATUS) and reporting it to
>>>>> the driver that this feature is always on, no matter whether the device
>>>>> actually supports it or not? This looks wrong to me.
>>>> Why?
>>> Because you were doing hack to make virtio-user work, while there is no
>>> such hack in the QEMU virtio-pci implementation.
>>>
>>>> IMO, device is not necessary to know this feature.
>>> F_STATUS is a device feature. If such feature is not claimed to be
>>> supported by the device, the driver should ignore it. But you are
>>> unconditionally letting the driver handle it, even though  the
>>> virito-user device does not claim to support it.
>>>
>>> Note that F_STATUS is set in the QEMU virtio-net PCI device.
>> In QEMU virtio-net PCI device, the device includes QEMU device emulation +
>> vhost backend driver. This feature only shows at QEMU device emulation,
>> instead of vhost backend driver. This is why we did not see this feature at
>> any vhost backends.
>> And the embedded virtio-user actually substitutes QEMU device emulation
>> part, so this feature should end at this layer.
>>
>> Let's look this in this way, if we let this feature go through to vhost
>> user, it will be treated as an error in vhost_user_set_features().
> Again, why can't we (virtio-user) follow QEMU?

Through F2F discussion, I got to know that you treat 
virtio_user_ethdev.c as front driver part. That makes sense. I'll move 
this change into vhost-user directory.

Thanks,
Jianfeng

[PATCH 4/5] net/virtio-user: add lsc support with vhost-user adapter

[Jianfeng Tan]

So far, virtio-user with vhost-user as the backend can only support
client mode. So when vhost user backend is down, i.e., unix socket
connection is broken and cannot be re-connected. We will force the
link state to be down.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_ethdev.c               |  2 +-
 drivers/net/virtio/virtio_ethdev.h               |  2 ++
 drivers/net/virtio/virtio_user/virtio_user_dev.c |  2 ++
 drivers/net/virtio/virtio_user_ethdev.c          | 35 +++++++++++++++++++++++-
 4 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 5d80d1a..58b20eb 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1190,7 +1190,7 @@ virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features)
  * Process Virtio Config changed interrupt and call the callback
  * if link state changed.
  */
-static void
+void
 virtio_interrupt_handler(struct rte_intr_handle *handle,
 			 void *param)
 {
diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index 777a14b..3f13763 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -112,4 +112,6 @@ uint16_t virtio_xmit_pkts_simple(void *tx_queue, struct rte_mbuf **tx_pkts,
 
 int eth_virtio_dev_init(struct rte_eth_dev *eth_dev);
 
+void virtio_interrupt_handler(struct rte_intr_handle *handle, void *param);
+
 #endif /* _VIRTIO_ETHDEV_H_ */
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index cc6f557..2d79818 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -158,6 +158,8 @@ virtio_user_fill_intr_handle(struct virtio_user_dev *dev, uint8_t portid)
 	eth_dev->intr_handle->nb_efd = dev->max_queue_pairs;
 	eth_dev->intr_handle->max_intr = dev->max_queue_pairs + 1;
 	eth_dev->intr_handle->type = RTE_INTR_HANDLE_VDEV;
+	if (dev->vhostfd >= 0)
+		eth_dev->intr_handle->fd = dev->vhostfd;
 }
 
 int
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index fbdd0a8..14f4a5c 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -34,10 +34,14 @@
 #include <stdint.h>
 #include <sys/types.h>
 #include <unistd.h>
+#include <fcntl.h>
+#include <sys/types.h>
+#include <sys/socket.h>
 
 #include <rte_malloc.h>
 #include <rte_kvargs.h>
 #include <rte_vdev.h>
+#include <rte_alarm.h>
 
 #include "virtio_ethdev.h"
 #include "virtio_logs.h"
@@ -49,6 +53,16 @@
 #define virtio_user_get_dev(hw) \
 	((struct virtio_user_dev *)(hw)->virtio_user_dev)
 
+static void virtio_user_delayed_handler(void *param)
+{
+	struct virtio_hw *hw = (struct virtio_hw *)param;
+	struct rte_eth_dev *dev = &rte_eth_devices[hw->port_id];
+
+	rte_intr_callback_unregister(dev->intr_handle,
+				     virtio_interrupt_handler,
+				     dev);
+}
+
 static void
 virtio_user_read_dev_config(struct virtio_hw *hw, size_t offset,
 		     void *dst, int length)
@@ -63,8 +77,27 @@ virtio_user_read_dev_config(struct virtio_hw *hw, size_t offset,
 		return;
 	}
 
-	if (offset == offsetof(struct virtio_net_config, status))
+	if (offset == offsetof(struct virtio_net_config, status)) {
+		char buf[128];
+
+		if (dev->vhostfd >= 0) {
+			int flags;
+			int r;
+
+			flags = fcntl(dev->vhostfd, F_GETFL);
+			fcntl(dev->vhostfd, F_SETFL, flags | O_NONBLOCK);
+			r = recv(dev->vhostfd, buf, 128, 0);
+			if (r == 0 || (r < 0 && errno != EAGAIN)) {
+				dev->status &= (~VIRTIO_NET_S_LINK_UP);
+				/* link can never be up again */
+				rte_eal_alarm_set(1, virtio_user_delayed_handler, (void *)hw);
+			} else {
+				dev->status |= VIRTIO_NET_S_LINK_UP;
+			}
+			fcntl(dev->vhostfd, F_SETFL, flags & (~O_NONBLOCK));
+		}
 		*(uint16_t *)dst = dev->status;
+	}
 
 	if (offset == offsetof(struct virtio_net_config, max_virtqueue_pairs))
 		*(uint16_t *)dst = dev->max_queue_pairs;
-- 
2.7.4

Re: [PATCH 4/5] net/virtio-user: add lsc support with vhost-user adapter

[Yuanhan Liu]

On Fri, Mar 03, 2017 at 05:56:42PM +0000, Jianfeng Tan wrote:
> So far, virtio-user with vhost-user as the backend can only support
> client mode. So when vhost user backend is down, i.e., unix socket
> connection is broken and cannot be re-connected. We will force the
> link state to be down.
> 
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>  static void
>  virtio_user_read_dev_config(struct virtio_hw *hw, size_t offset,
>  		     void *dst, int length)
> @@ -63,8 +77,27 @@ virtio_user_read_dev_config(struct virtio_hw *hw, size_t offset,
>  		return;
>  	}
>  
> -	if (offset == offsetof(struct virtio_net_config, status))
> +	if (offset == offsetof(struct virtio_net_config, status)) {
> +		char buf[128];
> +
> +		if (dev->vhostfd >= 0) {
> +			int flags;
> +			int r;
> +
> +			flags = fcntl(dev->vhostfd, F_GETFL);
> +			fcntl(dev->vhostfd, F_SETFL, flags | O_NONBLOCK);
> +			r = recv(dev->vhostfd, buf, 128, 0);

I think you need specify flag MSG_PEEK, to not consume the data if
there any?

	--yliu
> +			if (r == 0 || (r < 0 && errno != EAGAIN)) {
> +				dev->status &= (~VIRTIO_NET_S_LINK_UP);
> +				/* link can never be up again */
> +				rte_eal_alarm_set(1, virtio_user_delayed_handler, (void *)hw);
> +			} else {
> +				dev->status |= VIRTIO_NET_S_LINK_UP;
> +			}
> +			fcntl(dev->vhostfd, F_SETFL, flags & (~O_NONBLOCK));
> +		}
>  		*(uint16_t *)dst = dev->status;
> +	}
>  
>  	if (offset == offsetof(struct virtio_net_config, max_virtqueue_pairs))
>  		*(uint16_t *)dst = dev->max_queue_pairs;
> -- 
> 2.7.4

Re: [PATCH 4/5] net/virtio-user: add lsc support with vhost-user adapter

[Tan, Jianfeng]

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Friday, March 17, 2017 4:29 PM
> To: Tan, Jianfeng
> Cc: dev@dpdk.org; david.marchand@6wind.com
> Subject: Re: [PATCH 4/5] net/virtio-user: add lsc support with vhost-user
> adapter
> 
> On Fri, Mar 03, 2017 at 05:56:42PM +0000, Jianfeng Tan wrote:
> > So far, virtio-user with vhost-user as the backend can only support
> > client mode. So when vhost user backend is down, i.e., unix socket
> > connection is broken and cannot be re-connected. We will force the
> > link state to be down.
> >
> > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> >  static void
> >  virtio_user_read_dev_config(struct virtio_hw *hw, size_t offset,
> >  		     void *dst, int length)
> > @@ -63,8 +77,27 @@ virtio_user_read_dev_config(struct virtio_hw *hw,
> size_t offset,
> >  		return;
> >  	}
> >
> > -	if (offset == offsetof(struct virtio_net_config, status))
> > +	if (offset == offsetof(struct virtio_net_config, status)) {
> > +		char buf[128];
> > +
> > +		if (dev->vhostfd >= 0) {
> > +			int flags;
> > +			int r;
> > +
> > +			flags = fcntl(dev->vhostfd, F_GETFL);
> > +			fcntl(dev->vhostfd, F_SETFL, flags | O_NONBLOCK);
> > +			r = recv(dev->vhostfd, buf, 128, 0);
> 
> I think you need specify flag MSG_PEEK, to not consume the data if
> there any?

Thanks for catching this issue, I'll fix it in next version.

Thanks,
Jianfeng

[PATCH 5/5] net/virtio-user: add lsc support with vhost-kernel adapter

[Jianfeng Tan]

To check the link status, we use inotify to check if the file
/sys/class/net/tapX/flags is modified.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_ethdev.c               | 27 ++++++++++++++++--------
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 24 +++++++++++++++++++++
 drivers/net/virtio/virtio_user/virtio_user_dev.h |  2 ++
 drivers/net/virtio/virtio_user_ethdev.c          | 16 ++++++++++++++
 4 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 58b20eb..1069219 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1292,10 +1292,9 @@ virtio_configure_intr(struct rte_eth_dev *dev)
 		}
 	}
 
-	/* Re-register callback to update max_intr */
-	rte_intr_callback_unregister(dev->intr_handle,
-				     virtio_interrupt_handler,
-				     dev);
+	/* Temporarily register callback as rte_intr_enable will use max_intr of
+	 * intr_handle on intr_sources.
+	 */
 	rte_intr_callback_register(dev->intr_handle,
 				   virtio_interrupt_handler,
 				   dev);
@@ -1322,6 +1321,9 @@ virtio_configure_intr(struct rte_eth_dev *dev)
 		return -1;
 	}
 
+	rte_intr_callback_unregister(dev->intr_handle,
+				     virtio_interrupt_handler,
+				     dev);
 	return 0;
 }
 
@@ -1335,6 +1337,13 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
 	struct rte_pci_device *pci_dev = NULL;
 	int ret;
 
+	/* Callback could have been registered by last initialization,
+	 * unregister it.
+	 */
+	rte_intr_callback_unregister(eth_dev->intr_handle,
+				     virtio_interrupt_handler,
+				     eth_dev);
+
 	/* Reset the device although not necessary at startup */
 	vtpci_reset(hw);
 
@@ -1436,6 +1445,11 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
 			eth_dev->data->port_id, pci_dev->id.vendor_id,
 			pci_dev->id.device_id);
 
+	/* Setup interrupt callback */
+	if (eth_dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC)
+		rte_intr_callback_register(eth_dev->intr_handle,
+			virtio_interrupt_handler, eth_dev);
+
 	return 0;
 }
 
@@ -1546,11 +1560,6 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 	if (ret < 0)
 		return ret;
 
-	/* Setup interrupt callback  */
-	if (eth_dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC)
-		rte_intr_callback_register(eth_dev->intr_handle,
-			virtio_interrupt_handler, eth_dev);
-
 	return 0;
 }
 
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 2d79818..a331c37 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -42,6 +42,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#include <sys/inotify.h>
 
 #include "vhost.h"
 #include "virtio_user_dev.h"
@@ -160,6 +161,24 @@ virtio_user_fill_intr_handle(struct virtio_user_dev *dev, uint8_t portid)
 	eth_dev->intr_handle->type = RTE_INTR_HANDLE_VDEV;
 	if (dev->vhostfd >= 0)
 		eth_dev->intr_handle->fd = dev->vhostfd;
+	else {
+		char path[PATH_MAX];
+
+		snprintf(path, PATH_MAX, "/sys/class/net/%s/flags", dev->ifname);
+
+		dev->fd_inotify = inotify_init1(IN_NONBLOCK);
+		if (dev->fd_inotify == -1)
+			return;
+
+		if (inotify_add_watch(dev->fd_inotify, path, IN_MODIFY) == -1) {
+			close(dev->fd_inotify);
+			dev->fd_inotify = -1;
+			return;
+		}
+
+		dev->fd_tap_flags = open(path, O_RDONLY);
+		eth_dev->intr_handle->fd = dev->fd_inotify;
+	}
 }
 
 int
@@ -219,6 +238,11 @@ int virtio_user_stop_device(struct virtio_user_dev *dev)
 	for (i = 0; i < dev->max_queue_pairs; ++i)
 		dev->ops->enable_qp(dev, i, 0);
 
+	if (dev->fd_inotify >= 0)
+		close(dev->fd_inotify);
+	if (dev->fd_tap_flags >= 0)
+		close(dev->fd_tap_flags);
+
 	free(dev->ifname);
 	dev->ifname = NULL;
 
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 3b529f0..dba8254 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -47,6 +47,8 @@ struct virtio_user_dev {
 	char		*ifname;
 	int		*vhostfds;
 	int		*tapfds;
+	int		fd_inotify;
+	int		fd_tap_flags;
 
 	/* for both vhost_user and vhost_kernel */
 	int		callfds[VIRTIO_MAX_VIRTQUEUES * 2 + 1];
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 14f4a5c..f2dd505 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -95,6 +95,22 @@ virtio_user_read_dev_config(struct virtio_hw *hw, size_t offset,
 				dev->status |= VIRTIO_NET_S_LINK_UP;
 			}
 			fcntl(dev->vhostfd, F_SETFL, flags & (~O_NONBLOCK));
+		} else {
+			uint64_t flags = 0;
+
+			while (read(dev->fd_inotify, buf, sizeof buf) > 0) {};
+
+			lseek(dev->fd_tap_flags, 0, SEEK_SET);
+			if (read(dev->fd_tap_flags, buf, 128) > 0) {
+				char *end;
+
+				flags = strtoll(buf, &end, 16);
+			}
+
+			if (flags & 1)
+				dev->status |= VIRTIO_NET_S_LINK_UP;
+			else
+				dev->status &= (~VIRTIO_NET_S_LINK_UP);
 		}
 		*(uint16_t *)dst = dev->status;
 	}
-- 
2.7.4

[PATCH v2 0/5] add LSC and Rxq interrupt for virtio-user

[Jianfeng Tan]

v2:
  - Drop the support of LSC for virtio-user + vhost-kernel as the backend.
  - Rearrange the sequence of intr config and DRIVER_OK.
  - Address comments from yuanhan.
  - Update doc.

This is an attempt to add LSC and Rxq interrupt for a virtual device,
virtio-user (with the backend of both vhost-user and vhost-kernel).

We need to support:
Case 1. Rxq interrupt for virtio-user + vhost-user as the backend.
Case 2. LSC interrupt for virtio-user + vhost-user as the backend.
Case 3. Rxq interrupt for virtio-user + vhost-kernel as the backend.

Another case is not supported now as we did not find a way to monitor
tap device up/down events:
  - LSC interrupt for virtio-user + vhost-kernel as the backend.

HOW TO TEST:

Step 1: start testpmd with a virtual vhost device:
  $ testpmd -c 0x3 -n 4 --socket-mem 1024 --no-pci \
	  --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i

Step 2: start l3fwd-power with a virtio-user device:
  $ l3fwd-power -c 0xc -n 4 --socket-mem 1024 --no-pci \
	  --file-prefix=l3fwd-pwd \
	  --vdev=virtio_user0,path=/tmp/sock0 \
	  -- -p 1 -P --config="(0,0,1)" \
	  --no-numa --parse-ptype

Step 3: start burst in testpmd
  (testpmd)> start tx_first

  Packets will be received and forwarded back by l3fwd-power.
  And l3fwd-power will keep in polling mode.

Step 4: stop burst in testpmd
  (testpmd)> stop

  l3fwd-power will change to interrupt mode with few CPU
  consumption.

Step 5: kill testpmd
  $ kill -p `pidof testpmd`

  The link status will be changed to down in l3fwd-power.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>


Jianfeng Tan (5):
  eal/linux: add interrupt type for vdev
  net/virtio: add interrupt configure for vdev
  net/virtio-user: add rxq interrupt mode support
  net/virtio-user: support to report net status
  net/virtio-user: add lsc support

 doc/guides/rel_notes/release_17_05.rst             | 12 ++++
 drivers/net/virtio/virtio_ethdev.c                 | 59 +++++++++++++++---
 drivers/net/virtio/virtio_ethdev.h                 |  2 +
 drivers/net/virtio/virtio_user/virtio_user_dev.c   | 28 ++++++++-
 drivers/net/virtio/virtio_user/virtio_user_dev.h   |  2 +-
 drivers/net/virtio/virtio_user_ethdev.c            | 70 +++++++++++++++++++---
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       | 30 +++++++++-
 .../linuxapp/eal/include/exec-env/rte_interrupts.h |  5 +-
 8 files changed, 186 insertions(+), 22 deletions(-)

-- 
2.7.4

[PATCH v2 1/5] eal/linux: add interrupt type for vdev

[Jianfeng Tan]

A new interrupt type, RTE_INTR_HANDLE_VDEV, is added to support lsc and rxq
interrupt for vdev.

For lsc interrupt, except from original EPOLLIN events, we also listen for
socket peer closed connection event (EPOLLRDHUP and EPOLLHUP).

For rxq interrupt, add a precondition to avoid invoking any vfio and uio
code.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       | 30 +++++++++++++++++++---
 .../linuxapp/eal/include/exec-env/rte_interrupts.h |  5 ++--
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 92a19cb..5cf7bbc 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -46,6 +46,7 @@
 #include <sys/ioctl.h>
 #include <sys/eventfd.h>
 #include <assert.h>
+#include <stdbool.h>
 
 #include <rte_common.h>
 #include <rte_interrupts.h>
@@ -583,6 +584,9 @@ rte_intr_callback_unregister(const struct rte_intr_handle *intr_handle,
 int
 rte_intr_enable(const struct rte_intr_handle *intr_handle)
 {
+	if (intr_handle && intr_handle->type == RTE_INTR_HANDLE_VDEV)
+		return 0;
+
 	if (!intr_handle || intr_handle->fd < 0 || intr_handle->uio_cfg_fd < 0)
 		return -1;
 
@@ -627,6 +631,9 @@ rte_intr_enable(const struct rte_intr_handle *intr_handle)
 int
 rte_intr_disable(const struct rte_intr_handle *intr_handle)
 {
+	if (intr_handle && intr_handle->type == RTE_INTR_HANDLE_VDEV)
+		return 0;
+
 	if (!intr_handle || intr_handle->fd < 0 || intr_handle->uio_cfg_fd < 0)
 		return -1;
 
@@ -671,6 +678,7 @@ rte_intr_disable(const struct rte_intr_handle *intr_handle)
 static int
 eal_intr_process_interrupts(struct epoll_event *events, int nfds)
 {
+	bool call = false;
 	int n, bytes_read;
 	struct rte_intr_source *src;
 	struct rte_intr_callback *cb;
@@ -719,13 +727,18 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds)
 			bytes_read = sizeof(buf.vfio_intr_count);
 			break;
 #endif
+		case RTE_INTR_HANDLE_VDEV:
 		case RTE_INTR_HANDLE_EXT:
+			bytes_read = 0;
+			call = true;
+			break;
+
 		default:
 			bytes_read = 1;
 			break;
 		}
 
-		if (src->intr_handle.type != RTE_INTR_HANDLE_EXT) {
+		if (bytes_read > 0) {
 			/**
 			 * read out to clear the ready-to-be-read flag
 			 * for epoll_wait.
@@ -742,12 +755,14 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds)
 			} else if (bytes_read == 0)
 				RTE_LOG(ERR, EAL, "Read nothing from file "
 					"descriptor %d\n", events[n].data.fd);
+			else
+				call = true;
 		}
 
 		/* grab a lock, again to call callbacks and update status. */
 		rte_spinlock_lock(&intr_lock);
 
-		if (bytes_read > 0) {
+		if (call) {
 
 			/* Finally, call all callbacks. */
 			TAILQ_FOREACH(cb, &src->callbacks, next) {
@@ -858,7 +873,7 @@ eal_intr_thread_main(__rte_unused void *arg)
 		TAILQ_FOREACH(src, &intr_sources, next) {
 			if (src->callbacks.tqh_first == NULL)
 				continue; /* skip those with no callbacks */
-			ev.events = EPOLLIN | EPOLLPRI;
+			ev.events = EPOLLIN | EPOLLPRI | EPOLLRDHUP | EPOLLHUP;
 			ev.data.fd = src->intr_handle.fd;
 
 			/**
@@ -939,6 +954,12 @@ eal_intr_proc_rxtx_intr(int fd, const struct rte_intr_handle *intr_handle)
 		bytes_read = sizeof(buf.vfio_intr_count);
 		break;
 #endif
+	case RTE_INTR_HANDLE_VDEV:
+		/* for vdev, fd points to:
+		 * a. eventfd which does not need to read out;
+		 * b. datapath fd which needs PMD to read out.
+		 */
+		return;
 	default:
 		bytes_read = 1;
 		RTE_LOG(INFO, EAL, "unexpected intr type\n");
@@ -1244,5 +1265,8 @@ rte_intr_cap_multiple(struct rte_intr_handle *intr_handle)
 	if (intr_handle->type == RTE_INTR_HANDLE_VFIO_MSIX)
 		return 1;
 
+	if (intr_handle->type == RTE_INTR_HANDLE_VDEV)
+		return 1;
+
 	return 0;
 }
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
index d459bf4..b8ee1de 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
@@ -49,8 +49,9 @@ enum rte_intr_handle_type {
 	RTE_INTR_HANDLE_VFIO_LEGACY,  /**< vfio device handle (legacy) */
 	RTE_INTR_HANDLE_VFIO_MSI,     /**< vfio device handle (MSI) */
 	RTE_INTR_HANDLE_VFIO_MSIX,    /**< vfio device handle (MSIX) */
-	RTE_INTR_HANDLE_ALARM,    /**< alarm handle */
-	RTE_INTR_HANDLE_EXT, /**< external handler */
+	RTE_INTR_HANDLE_ALARM,        /**< alarm handle */
+	RTE_INTR_HANDLE_EXT,          /**< external handler */
+	RTE_INTR_HANDLE_VDEV,         /**< virtual device */
 	RTE_INTR_HANDLE_MAX
 };
 
-- 
2.7.4

[PATCH v2 2/5] net/virtio: add interrupt configure for vdev

[Jianfeng Tan]

For virtio PCI devices, interrupt should be configured before setting
VIRTIO_CONFIG_STATUS_DRIVER_OK so that QEMU can properly set eventfds
in the host.

For virtio virtual devices, VIRTIO_CONFIG_STATUS_DRIVER_OK should be
set firstly, so that intr_handle is initialized in
virtio_user_start_device().

To accommodate both requirements, we rearrange the sequence like this:
  a. set interrupt configure for PCI devices.
  b. set VIRTIO_CONFIG_STATUS_DRIVER_OK.
  c. set interrupt configure for virtual devices.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_ethdev.c | 57 ++++++++++++++++++++++++++++++++++----
 1 file changed, 51 insertions(+), 6 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d9986ab..f0213ba 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1275,7 +1275,7 @@ virtio_queues_unbind_intr(struct rte_eth_dev *dev)
 }
 
 static int
-virtio_configure_intr(struct rte_eth_dev *dev)
+virtio_configure_intr_pci(struct rte_eth_dev *dev)
 {
 	struct virtio_hw *hw = dev->data->dev_private;
 
@@ -1327,6 +1327,37 @@ virtio_configure_intr(struct rte_eth_dev *dev)
 	return 0;
 }
 
+static int
+virtio_configure_intr_vdev(struct rte_eth_dev *dev)
+{
+	struct virtio_hw *hw = dev->data->dev_private;
+
+	if (!dev->intr_handle->intr_vec) {
+		dev->intr_handle->intr_vec =
+			rte_zmalloc("intr_vec",
+				    hw->max_queue_pairs * sizeof(int), 0);
+		if (!dev->intr_handle->intr_vec) {
+			PMD_INIT_LOG(ERR, "Failed to allocate %u rxq vectors",
+				     hw->max_queue_pairs);
+			return -ENOMEM;
+		}
+	}
+
+	/* Re-register callback to update max_intr */
+	rte_intr_callback_unregister(dev->intr_handle,
+				     virtio_interrupt_handler,
+				     dev);
+	rte_intr_callback_register(dev->intr_handle,
+				   virtio_interrupt_handler,
+				   dev);
+
+	if (virtio_queues_bind_intr(dev) < 0) {
+		PMD_INIT_LOG(ERR, "Failed to bind queue/interrupt");
+		return -1;
+	}
+
+	return 0;
+}
 /* reset device and renegotiate features if needed */
 static int
 virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
@@ -1450,15 +1481,29 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
 	if (ret < 0)
 		return ret;
 
-	if (eth_dev->data->dev_conf.intr_conf.rxq) {
-		if (virtio_configure_intr(eth_dev) < 0) {
-			PMD_INIT_LOG(ERR, "failed to configure interrupt");
-			return -1;
-		}
+	/* For virtio PCI devices, setup interrupt configuration before
+	 * setting VIRTIO_CONFIG_STATUS_DRIVER_OK, required by QEMU.
+	 */
+	if (pci_dev &&
+	    eth_dev->data->dev_conf.intr_conf.rxq &&
+	    virtio_configure_intr_pci(eth_dev) < 0) {
+		PMD_INIT_LOG(ERR, "failed to configure interrupt");
+		return -1;
 	}
 
 	vtpci_reinit_complete(hw);
 
+	/* For virtio vdev, setup interrupt configuration after
+	 * setting VIRTIO_CONFIG_STATUS_DRIVER_OK, so that intr_handle
+	 * is initialized in virtio_user_start_device().
+	 */
+	if (!pci_dev &&
+	    eth_dev->data->dev_conf.intr_conf.rxq &&
+	    virtio_configure_intr_vdev(eth_dev) < 0) {
+		PMD_INIT_LOG(ERR, "failed to configure interrupt");
+		return -1;
+	}
+
 	if (pci_dev)
 		PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x",
 			eth_dev->data->port_id, pci_dev->id.vendor_id,
-- 
2.7.4

Re: [PATCH v2 2/5] net/virtio: add interrupt configure for vdev

[Yuanhan Liu]

On Tue, Mar 28, 2017 at 08:21:53AM +0000, Jianfeng Tan wrote:
> For virtio PCI devices, interrupt should be configured before setting
> VIRTIO_CONFIG_STATUS_DRIVER_OK so that QEMU can properly set eventfds
> in the host.
> 
> For virtio virtual devices, VIRTIO_CONFIG_STATUS_DRIVER_OK should be
> set firstly, so that intr_handle is initialized in
> virtio_user_start_device().

I'm wondering why can't you let virtio-user follow virtio-pci?

	--yliu
> 
> To accommodate both requirements, we rearrange the sequence like this:
>   a. set interrupt configure for PCI devices.
>   b. set VIRTIO_CONFIG_STATUS_DRIVER_OK.
>   c. set interrupt configure for virtual devices.
> 
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>

Re: [PATCH v2 2/5] net/virtio: add interrupt configure for vdev

[Tan, Jianfeng]

On 3/29/2017 2:27 PM, Yuanhan Liu wrote:
> On Tue, Mar 28, 2017 at 08:21:53AM +0000, Jianfeng Tan wrote:
>> For virtio PCI devices, interrupt should be configured before setting
>> VIRTIO_CONFIG_STATUS_DRIVER_OK so that QEMU can properly set eventfds
>> in the host.
>>
>> For virtio virtual devices, VIRTIO_CONFIG_STATUS_DRIVER_OK should be
>> set firstly, so that intr_handle is initialized in
>> virtio_user_start_device().
> I'm wondering why can't you let virtio-user follow virtio-pci?

It's because that virtio-user not only counts on 
virtio_user_start_device() to allocate intr_handle, it also needs the 
information in this function to initialize this struct. For virtio-pci, 
similar information is from rte_intr_enable/rte_intr_efd_enable.

Or do you mean we can move calling virtio_user_start_device() ahead? I 
can hardly find other place instead of DRIVER_OK.

Thanks,
Jianfeng

Re: [PATCH v2 2/5] net/virtio: add interrupt configure for vdev

[Yuanhan Liu]

On Wed, Mar 29, 2017 at 03:03:16PM +0800, Tan, Jianfeng wrote:
> 
> 
> On 3/29/2017 2:27 PM, Yuanhan Liu wrote:
> >On Tue, Mar 28, 2017 at 08:21:53AM +0000, Jianfeng Tan wrote:
> >>For virtio PCI devices, interrupt should be configured before setting
> >>VIRTIO_CONFIG_STATUS_DRIVER_OK so that QEMU can properly set eventfds
> >>in the host.
> >>
> >>For virtio virtual devices, VIRTIO_CONFIG_STATUS_DRIVER_OK should be
> >>set firstly, so that intr_handle is initialized in
> >>virtio_user_start_device().
> >I'm wondering why can't you let virtio-user follow virtio-pci?
> 
> It's because that virtio-user not only counts on virtio_user_start_device()
> to allocate intr_handle, it also needs the information in this function to
> initialize this struct. For virtio-pci, similar information is from
> rte_intr_enable/rte_intr_efd_enable.
> 
> Or do you mean we can move calling virtio_user_start_device() ahead? I can
> hardly find other place instead of DRIVER_OK.

For example, virtio_user_dev_init()?

	--yliu

Re: [PATCH v2 2/5] net/virtio: add interrupt configure for vdev

[Tan, Jianfeng]

On 3/29/2017 3:09 PM, Yuanhan Liu wrote:
> On Wed, Mar 29, 2017 at 03:03:16PM +0800, Tan, Jianfeng wrote:
>>
>> On 3/29/2017 2:27 PM, Yuanhan Liu wrote:
>>> On Tue, Mar 28, 2017 at 08:21:53AM +0000, Jianfeng Tan wrote:
>>>> For virtio PCI devices, interrupt should be configured before setting
>>>> VIRTIO_CONFIG_STATUS_DRIVER_OK so that QEMU can properly set eventfds
>>>> in the host.
>>>>
>>>> For virtio virtual devices, VIRTIO_CONFIG_STATUS_DRIVER_OK should be
>>>> set firstly, so that intr_handle is initialized in
>>>> virtio_user_start_device().
>>> I'm wondering why can't you let virtio-user follow virtio-pci?
>> It's because that virtio-user not only counts on virtio_user_start_device()
>> to allocate intr_handle, it also needs the information in this function to
>> initialize this struct. For virtio-pci, similar information is from
>> rte_intr_enable/rte_intr_efd_enable.
>>
>> Or do you mean we can move calling virtio_user_start_device() ahead? I can
>> hardly find other place instead of DRIVER_OK.
> For example, virtio_user_dev_init()?
>
>
But in that way, there is only one chance to negotiate features with the 
backend. What if we change the configuration through 
rte_eth_dev_configure()?

Yes, we can just put callfd/kickfd creation and intr_handle 
initialization into virtio_user_dev_init(), and its destructor into 
virtio_user_dev_uninit(). It sounds like a feasible way to go.

Thanks,
Jianfeng

Re: [PATCH v2 2/5] net/virtio: add interrupt configure for vdev

[Yuanhan Liu]

On Wed, Mar 29, 2017 at 03:27:28PM +0800, Tan, Jianfeng wrote:
> 
> 
> On 3/29/2017 3:09 PM, Yuanhan Liu wrote:
> >On Wed, Mar 29, 2017 at 03:03:16PM +0800, Tan, Jianfeng wrote:
> >>
> >>On 3/29/2017 2:27 PM, Yuanhan Liu wrote:
> >>>On Tue, Mar 28, 2017 at 08:21:53AM +0000, Jianfeng Tan wrote:
> >>>>For virtio PCI devices, interrupt should be configured before setting
> >>>>VIRTIO_CONFIG_STATUS_DRIVER_OK so that QEMU can properly set eventfds
> >>>>in the host.
> >>>>
> >>>>For virtio virtual devices, VIRTIO_CONFIG_STATUS_DRIVER_OK should be
> >>>>set firstly, so that intr_handle is initialized in
> >>>>virtio_user_start_device().
> >>>I'm wondering why can't you let virtio-user follow virtio-pci?
> >>It's because that virtio-user not only counts on virtio_user_start_device()
> >>to allocate intr_handle, it also needs the information in this function to
> >>initialize this struct. For virtio-pci, similar information is from
> >>rte_intr_enable/rte_intr_efd_enable.
> >>
> >>Or do you mean we can move calling virtio_user_start_device() ahead? I can
> >>hardly find other place instead of DRIVER_OK.
> >For example, virtio_user_dev_init()?
> >
> >
> But in that way, there is only one chance to negotiate features with the
> backend. What if we change the configuration through
> rte_eth_dev_configure()?

Then there will be a reset; everything should be reset properly in the
device.

	--yliu
> 
> Yes, we can just put callfd/kickfd creation and intr_handle initialization
> into virtio_user_dev_init(), and its destructor into
> virtio_user_dev_uninit(). It sounds like a feasible way to go.
> 
> Thanks,
> Jianfeng

[PATCH v2 3/5] net/virtio-user: add rxq interrupt mode support

[Jianfeng Tan]

For rxq interrupt, the device (backend driver) will notify driver
through callfd. Each virtqueue has a callfd. To keep compatible
with the existing framework, we will give these callfds to
interrupt thread for listening for interrupts.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 doc/guides/rel_notes/release_17_05.rst           |  7 +++++++
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 25 +++++++++++++++++++++++-
 drivers/net/virtio/virtio_user/virtio_user_dev.h |  2 +-
 drivers/net/virtio/virtio_user_ethdev.c          | 12 +++++++++++-
 4 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/doc/guides/rel_notes/release_17_05.rst b/doc/guides/rel_notes/release_17_05.rst
index bb64428..2b5eaab 100644
--- a/doc/guides/rel_notes/release_17_05.rst
+++ b/doc/guides/rel_notes/release_17_05.rst
@@ -57,6 +57,13 @@ New Features
   * Enable Vhost PMD's MTU get feature.
   * Get max MTU value from host in Virtio PMD
 
+* **Added interrupt mode support for virtio-user.**
+
+  Implemented Rxq interrupt mode support for virtio-user as a virtual
+  device. Supported cases:
+
+  * Rxq interrupt for virtio-user + vhost-user as the backend.
+  * Rxq interrupt for virtio-user + vhost-kernel as the backend.
 
 Resolved Issues
 ---------------
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 9dcdac8..e269ad1 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -140,8 +140,28 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
 	return 0;
 }
 
+static void
+virtio_user_fill_intr_handle(struct virtio_user_dev *dev, uint8_t portid)
+{
+	uint32_t i;
+	struct rte_eth_dev *eth_dev = &rte_eth_devices[portid];
+
+	if (!eth_dev->intr_handle) {
+		eth_dev->intr_handle = malloc(sizeof(*eth_dev->intr_handle));
+		if (!eth_dev->intr_handle)
+			return;
+		memset(eth_dev->intr_handle, 0, sizeof(*eth_dev->intr_handle));
+	}
+
+	for (i = 0; i < dev->max_queue_pairs; ++i)
+		eth_dev->intr_handle->efds[i] = dev->callfds[i];
+	eth_dev->intr_handle->nb_efd = dev->max_queue_pairs;
+	eth_dev->intr_handle->max_intr = dev->max_queue_pairs + 1;
+	eth_dev->intr_handle->type = RTE_INTR_HANDLE_VDEV;
+}
+
 int
-virtio_user_start_device(struct virtio_user_dev *dev)
+virtio_user_start_device(struct virtio_user_dev *dev, uint8_t portid)
 {
 	uint64_t features;
 	int ret;
@@ -175,6 +195,9 @@ virtio_user_start_device(struct virtio_user_dev *dev)
 	 */
 	dev->ops->enable_qp(dev, 0, 1);
 
+	/* Step 5: prepare for interrupt mode */
+	virtio_user_fill_intr_handle(dev, portid);
+
 	return 0;
 error:
 	/* TODO: free resource here or caller to check */
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index bd2e4ca..de4bb5c 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -66,7 +66,7 @@ struct virtio_user_dev {
 	struct virtio_user_backend_ops *ops;
 };
 
-int virtio_user_start_device(struct virtio_user_dev *dev);
+int virtio_user_start_device(struct virtio_user_dev *dev, uint8_t portid);
 int virtio_user_stop_device(struct virtio_user_dev *dev);
 int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 			 int cq, int queue_size, const char *mac);
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 2961e6b..e4d4c03 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -101,7 +101,7 @@ virtio_user_set_status(struct virtio_hw *hw, uint8_t status)
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
 
 	if (status & VIRTIO_CONFIG_STATUS_DRIVER_OK)
-		virtio_user_start_device(dev);
+		virtio_user_start_device(dev, hw->port_id);
 	else if (status == VIRTIO_CONFIG_STATUS_RESET)
 		virtio_user_reset(hw);
 	dev->status = status;
@@ -148,6 +148,15 @@ virtio_user_set_config_irq(struct virtio_hw *hw __rte_unused,
 	return VIRTIO_MSI_NO_VECTOR;
 }
 
+static uint16_t
+virtio_user_set_queue_irq(struct virtio_hw *hw __rte_unused,
+			  struct virtqueue *vq __rte_unused,
+			  uint16_t vec)
+{
+	/* pretend we have done that */
+	return vec;
+}
+
 /* This function is to get the queue size, aka, number of descs, of a specified
  * queue. Different with the VHOST_USER_GET_QUEUE_NUM, which is used to get the
  * max supported queues.
@@ -226,6 +235,7 @@ const struct virtio_pci_ops virtio_user_ops = {
 	.set_features	= virtio_user_set_features,
 	.get_isr	= virtio_user_get_isr,
 	.set_config_irq	= virtio_user_set_config_irq,
+	.set_queue_irq	= virtio_user_set_queue_irq,
 	.get_queue_num	= virtio_user_get_queue_num,
 	.setup_queue	= virtio_user_setup_queue,
 	.del_queue	= virtio_user_del_queue,
-- 
2.7.4

[PATCH v2 4/5] net/virtio-user: support to report net status

[Jianfeng Tan]

Originally, we did not report support of VIRTIO_NET_F_STATUS.
This feature is not reported by vhost backend, instead, it
is added/removed by QEMU in virtio PCI case.

We report the support of this feature so that following patch
will depend on this feature to enable LSC interrupt.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_user/virtio_user_dev.c |  1 +
 drivers/net/virtio/virtio_user_ethdev.c          | 13 +++++++------
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index e269ad1..5637ccf 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -176,6 +176,7 @@ virtio_user_start_device(struct virtio_user_dev *dev, uint8_t portid)
 	features &= ~(1ull << VIRTIO_NET_F_MAC);
 	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to know */
 	features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
+	features &= ~(1ull << VIRTIO_NET_F_STATUS);
 	ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES, &features);
 	if (ret < 0)
 		goto error;
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index e4d4c03..5190029 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -121,7 +121,8 @@ virtio_user_get_features(struct virtio_hw *hw)
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
 
 	/* unmask feature bits defined in vhost user protocol */
-	return dev->device_features & VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
+	return (dev->device_features | (1 << VIRTIO_NET_F_STATUS))
+		& VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
 }
 
 static void
@@ -129,23 +130,23 @@ virtio_user_set_features(struct virtio_hw *hw, uint64_t features)
 {
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
 
-	dev->features = features & dev->device_features;
+	dev->features = features;
 }
 
 static uint8_t
 virtio_user_get_isr(struct virtio_hw *hw __rte_unused)
 {
-	/* When config interrupt happens, driver calls this function to query
-	 * what kinds of change happen. Interrupt mode not supported for now.
+	/* rxq interrupts and config interrupt are separated in virtio-user,
+	 * here we only report config change.
 	 */
-	return 0;
+	return VIRTIO_PCI_ISR_CONFIG;
 }
 
 static uint16_t
 virtio_user_set_config_irq(struct virtio_hw *hw __rte_unused,
 		    uint16_t vec __rte_unused)
 {
-	return VIRTIO_MSI_NO_VECTOR;
+	return 0;
 }
 
 static uint16_t
-- 
2.7.4

[PATCH v2 5/5] net/virtio-user: add lsc support

[Jianfeng Tan]

So far, virtio-user with vhost-user as the backend can only support
client mode. So when vhost user backend is down, i.e., unix socket
connection is broken, the connection cannot be re-connected. We will
forcely set the link state to be down.

Note: virtio-user with vhost-kernel as the backend still cannot
support lsc now as we fail to find a way to monitor the backend, tap
device, up/down events.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 doc/guides/rel_notes/release_17_05.rst           |  7 +++-
 drivers/net/virtio/virtio_ethdev.c               |  2 +-
 drivers/net/virtio/virtio_ethdev.h               |  2 ++
 drivers/net/virtio/virtio_user/virtio_user_dev.c |  2 ++
 drivers/net/virtio/virtio_user_ethdev.c          | 45 +++++++++++++++++++++++-
 5 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/doc/guides/rel_notes/release_17_05.rst b/doc/guides/rel_notes/release_17_05.rst
index 2b5eaab..42ba190 100644
--- a/doc/guides/rel_notes/release_17_05.rst
+++ b/doc/guides/rel_notes/release_17_05.rst
@@ -59,11 +59,12 @@ New Features
 
 * **Added interrupt mode support for virtio-user.**
 
-  Implemented Rxq interrupt mode support for virtio-user as a virtual
+  Implemented Rxq interrupt mode and LSC support for virtio-user as a virtual
   device. Supported cases:
 
   * Rxq interrupt for virtio-user + vhost-user as the backend.
   * Rxq interrupt for virtio-user + vhost-kernel as the backend.
+  * LSC interrupt for virtio-user + vhost-user as the backend.
 
 Resolved Issues
 ---------------
@@ -119,6 +120,10 @@ Known Issues
    Also, make sure to start the actual text at the margin.
    =========================================================
 
+   * **LSC interrupt cannot work for virtio-user + vhost-kernel.**
+
+     LSC interrupt cannot be detected when setting the backend, tap device,
+     up/down as we fail to find a way to monitor such event.
 
 API Changes
 -----------
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index f0213ba..952cf70 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1205,7 +1205,7 @@ virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features)
  * Process Virtio Config changed interrupt and call the callback
  * if link state changed.
  */
-static void
+void
 virtio_interrupt_handler(struct rte_intr_handle *handle,
 			 void *param)
 {
diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index aa78adc..4009c4d 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -113,4 +113,6 @@ uint16_t virtio_xmit_pkts_simple(void *tx_queue, struct rte_mbuf **tx_pkts,
 
 int eth_virtio_dev_init(struct rte_eth_dev *eth_dev);
 
+void virtio_interrupt_handler(struct rte_intr_handle *handle, void *param);
+
 #endif /* _VIRTIO_ETHDEV_H_ */
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 5637ccf..d19623d 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -158,6 +158,8 @@ virtio_user_fill_intr_handle(struct virtio_user_dev *dev, uint8_t portid)
 	eth_dev->intr_handle->nb_efd = dev->max_queue_pairs;
 	eth_dev->intr_handle->max_intr = dev->max_queue_pairs + 1;
 	eth_dev->intr_handle->type = RTE_INTR_HANDLE_VDEV;
+	if (dev->vhostfd >= 0)
+		eth_dev->intr_handle->fd = dev->vhostfd;
 }
 
 int
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 5190029..2c698c8 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -34,10 +34,14 @@
 #include <stdint.h>
 #include <sys/types.h>
 #include <unistd.h>
+#include <fcntl.h>
+#include <sys/types.h>
+#include <sys/socket.h>
 
 #include <rte_malloc.h>
 #include <rte_kvargs.h>
 #include <rte_vdev.h>
+#include <rte_alarm.h>
 
 #include "virtio_ethdev.h"
 #include "virtio_logs.h"
@@ -49,6 +53,16 @@
 #define virtio_user_get_dev(hw) \
 	((struct virtio_user_dev *)(hw)->virtio_user_dev)
 
+static void virtio_user_delayed_handler(void *param)
+{
+	struct virtio_hw *hw = (struct virtio_hw *)param;
+	struct rte_eth_dev *dev = &rte_eth_devices[hw->port_id];
+
+	rte_intr_callback_unregister(dev->intr_handle,
+				     virtio_interrupt_handler,
+				     dev);
+}
+
 static void
 virtio_user_read_dev_config(struct virtio_hw *hw, size_t offset,
 		     void *dst, int length)
@@ -63,8 +77,37 @@ virtio_user_read_dev_config(struct virtio_hw *hw, size_t offset,
 		return;
 	}
 
-	if (offset == offsetof(struct virtio_net_config, status))
+	if (offset == offsetof(struct virtio_net_config, status)) {
+		char buf[128];
+
+		if (dev->vhostfd >= 0) {
+			int r;
+			int flags;
+
+			flags = fcntl(dev->vhostfd, F_GETFL);
+			fcntl(dev->vhostfd, F_SETFL, flags | O_NONBLOCK);
+			r = recv(dev->vhostfd, buf, 128, MSG_PEEK);
+			if (r == 0 || (r < 0 && errno != EAGAIN)) {
+				dev->status &= (~VIRTIO_NET_S_LINK_UP);
+				PMD_DRV_LOG(ERR, "virtio-user port %u is down",
+					    hw->port_id);
+				/* Only client mode is available now. Once the
+				 * connection is broken, it can never be up
+				 * again. Besides, this function could be called
+				 * in the process of interrupt handling,
+				 * callback cannot be unregistered here, set an
+				 * alarm to do it.
+				 */
+				rte_eal_alarm_set(1,
+						  virtio_user_delayed_handler,
+						  (void *)hw);
+			} else {
+				dev->status |= VIRTIO_NET_S_LINK_UP;
+			}
+			fcntl(dev->vhostfd, F_SETFL, flags & (~O_NONBLOCK));
+		}
 		*(uint16_t *)dst = dev->status;
+	}
 
 	if (offset == offsetof(struct virtio_net_config, max_virtqueue_pairs))
 		*(uint16_t *)dst = dev->max_queue_pairs;
-- 
2.7.4

[PATCH v3 0/5] add LSC and Rxq interrupt for virtio-user

[Jianfeng Tan]

v3:
  - Move NET_STATUS addition into device layer.
  - Drop original 2nd patch.
  - Add a patch to move open/close of eventfds to init/uninit.
  - As we initialize intr_handle in each vdev driver, in
    rte_intr_efd_enable(), skip vdev when initialize intr_handle.
  - After commit 26bbd3e7dc51("net/virtio: disable LSC interrupt if
    MSIX not enabled"), we requier msix to support LSC.

v2:
  - Drop the support of LSC for virtio-user + vhost-kernel as the backend.
  - Rearrange the sequence of intr config and DRIVER_OK.
  - Address comments from yuanhan.
  - Update doc.

This is an attempt to add LSC and Rxq interrupt for a virtual device,
virtio-user (with the backend of both vhost-user and vhost-kernel).

We need to support:
Case 1. Rxq interrupt for virtio-user + vhost-user as the backend.
Case 2. LSC interrupt for virtio-user + vhost-user as the backend.
Case 3. Rxq interrupt for virtio-user + vhost-kernel as the backend.

Another case is not supported now as we did not find a way to monitor
tap device up/down events:
  - LSC interrupt for virtio-user + vhost-kernel as the backend.

HOW TO TEST:

Step 1: start testpmd with a virtual vhost device:
  $ testpmd -c 0x3 -n 4 --socket-mem 1024 --no-pci \
	  --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i

Step 2: start l3fwd-power with a virtio-user device:
  $ l3fwd-power -c 0xc -n 4 --socket-mem 1024 --no-pci \
	  --file-prefix=l3fwd-pwd \
	  --vdev=virtio_user0,path=/tmp/sock0 \
	  -- -p 1 -P --config="(0,0,1)" \
	  --no-numa --parse-ptype

Step 3: start burst in testpmd
  (testpmd)> start tx_first

  Packets will be received and forwarded back by l3fwd-power.
  And l3fwd-power will keep in polling mode.

Step 4: stop burst in testpmd
  (testpmd)> stop

  l3fwd-power will change to interrupt mode with few CPU
  consumption.

Step 5: kill testpmd
  $ kill -p `pidof testpmd`

  The link status will be changed to down in l3fwd-power.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>

Jianfeng Tan (5):
  eal/linux: add interrupt type for vdev
  net/virtio-user: move eventfd open/close into init/uninit
  net/virtio-user: add rxq interrupt mode support
  net/virtio-user: support to report net status
  net/virtio-user: add lsc support

 doc/guides/rel_notes/release_17_05.rst             |  12 +++
 drivers/net/virtio/virtio_ethdev.c                 |   2 +-
 drivers/net/virtio/virtio_ethdev.h                 |   2 +
 drivers/net/virtio/virtio_user/virtio_user_dev.c   | 114 +++++++++++++++------
 drivers/net/virtio/virtio_user/virtio_user_dev.h   |   1 +
 drivers/net/virtio/virtio_user_ethdev.c            |  71 +++++++++++--
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       |  32 +++++-
 .../linuxapp/eal/include/exec-env/rte_interrupts.h |   5 +-
 8 files changed, 197 insertions(+), 42 deletions(-)

-- 
2.7.4

[PATCH v3 1/5] eal/linux: add interrupt type for vdev

[Jianfeng Tan]

A new interrupt type, RTE_INTR_HANDLE_VDEV, is added to support lsc and rxq
interrupt for vdev.

For lsc interrupt, except from original EPOLLIN events, we also listen for
socket peer closed connection event (EPOLLRDHUP and EPOLLHUP).

For rxq interrupt, add a precondition to avoid invoking any vfio and uio
code.

For intr_handle initialization, let each vdev driver to do that.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       | 32 ++++++++++++++++++++--
 .../linuxapp/eal/include/exec-env/rte_interrupts.h |  5 ++--
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 92a19cb..9cc2843 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -46,6 +46,7 @@
 #include <sys/ioctl.h>
 #include <sys/eventfd.h>
 #include <assert.h>
+#include <stdbool.h>
 
 #include <rte_common.h>
 #include <rte_interrupts.h>
@@ -583,6 +584,9 @@ rte_intr_callback_unregister(const struct rte_intr_handle *intr_handle,
 int
 rte_intr_enable(const struct rte_intr_handle *intr_handle)
 {
+	if (intr_handle && intr_handle->type == RTE_INTR_HANDLE_VDEV)
+		return 0;
+
 	if (!intr_handle || intr_handle->fd < 0 || intr_handle->uio_cfg_fd < 0)
 		return -1;
 
@@ -627,6 +631,9 @@ rte_intr_enable(const struct rte_intr_handle *intr_handle)
 int
 rte_intr_disable(const struct rte_intr_handle *intr_handle)
 {
+	if (intr_handle && intr_handle->type == RTE_INTR_HANDLE_VDEV)
+		return 0;
+
 	if (!intr_handle || intr_handle->fd < 0 || intr_handle->uio_cfg_fd < 0)
 		return -1;
 
@@ -671,6 +678,7 @@ rte_intr_disable(const struct rte_intr_handle *intr_handle)
 static int
 eal_intr_process_interrupts(struct epoll_event *events, int nfds)
 {
+	bool call = false;
 	int n, bytes_read;
 	struct rte_intr_source *src;
 	struct rte_intr_callback *cb;
@@ -719,13 +727,18 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds)
 			bytes_read = sizeof(buf.vfio_intr_count);
 			break;
 #endif
+		case RTE_INTR_HANDLE_VDEV:
 		case RTE_INTR_HANDLE_EXT:
+			bytes_read = 0;
+			call = true;
+			break;
+
 		default:
 			bytes_read = 1;
 			break;
 		}
 
-		if (src->intr_handle.type != RTE_INTR_HANDLE_EXT) {
+		if (bytes_read > 0) {
 			/**
 			 * read out to clear the ready-to-be-read flag
 			 * for epoll_wait.
@@ -742,12 +755,14 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds)
 			} else if (bytes_read == 0)
 				RTE_LOG(ERR, EAL, "Read nothing from file "
 					"descriptor %d\n", events[n].data.fd);
+			else
+				call = true;
 		}
 
 		/* grab a lock, again to call callbacks and update status. */
 		rte_spinlock_lock(&intr_lock);
 
-		if (bytes_read > 0) {
+		if (call) {
 
 			/* Finally, call all callbacks. */
 			TAILQ_FOREACH(cb, &src->callbacks, next) {
@@ -858,7 +873,7 @@ eal_intr_thread_main(__rte_unused void *arg)
 		TAILQ_FOREACH(src, &intr_sources, next) {
 			if (src->callbacks.tqh_first == NULL)
 				continue; /* skip those with no callbacks */
-			ev.events = EPOLLIN | EPOLLPRI;
+			ev.events = EPOLLIN | EPOLLPRI | EPOLLRDHUP | EPOLLHUP;
 			ev.data.fd = src->intr_handle.fd;
 
 			/**
@@ -939,6 +954,12 @@ eal_intr_proc_rxtx_intr(int fd, const struct rte_intr_handle *intr_handle)
 		bytes_read = sizeof(buf.vfio_intr_count);
 		break;
 #endif
+	case RTE_INTR_HANDLE_VDEV:
+		/* for vdev, fd points to:
+		 * a. eventfd which does not need to read out;
+		 * b. datapath fd which needs PMD to read out.
+		 */
+		return;
 	default:
 		bytes_read = 1;
 		RTE_LOG(INFO, EAL, "unexpected intr type\n");
@@ -1189,6 +1210,8 @@ rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd)
 		}
 		intr_handle->nb_efd   = n;
 		intr_handle->max_intr = NB_OTHER_INTR + n;
+	} else if (intr_handle->type == RTE_INTR_HANDLE_VDEV) {
+		/* do nothing, and let vdev driver to initialize this struct */
 	} else {
 		intr_handle->efds[0]  = intr_handle->fd;
 		intr_handle->nb_efd   = RTE_MIN(nb_efd, 1U);
@@ -1244,5 +1267,8 @@ rte_intr_cap_multiple(struct rte_intr_handle *intr_handle)
 	if (intr_handle->type == RTE_INTR_HANDLE_VFIO_MSIX)
 		return 1;
 
+	if (intr_handle->type == RTE_INTR_HANDLE_VDEV)
+		return 1;
+
 	return 0;
 }
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
index d459bf4..b8ee1de 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
@@ -49,8 +49,9 @@ enum rte_intr_handle_type {
 	RTE_INTR_HANDLE_VFIO_LEGACY,  /**< vfio device handle (legacy) */
 	RTE_INTR_HANDLE_VFIO_MSI,     /**< vfio device handle (MSI) */
 	RTE_INTR_HANDLE_VFIO_MSIX,    /**< vfio device handle (MSIX) */
-	RTE_INTR_HANDLE_ALARM,    /**< alarm handle */
-	RTE_INTR_HANDLE_EXT, /**< external handler */
+	RTE_INTR_HANDLE_ALARM,        /**< alarm handle */
+	RTE_INTR_HANDLE_EXT,          /**< external handler */
+	RTE_INTR_HANDLE_VDEV,         /**< virtual device */
 	RTE_INTR_HANDLE_MAX
 };
 
-- 
2.7.4

[PATCH v3 2/5] net/virtio-user: move eventfd open/close into init/uninit

[Jianfeng Tan]

Originally, eventfd is opened when initializing each vq; and gets closded
in virtio_user_stop_device().

To make it possible to initialize intr_handle struct in init() in following
patch, we put the open() of all eventfds into init(); and put the close()
into uninit().

Suggested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 83 +++++++++++++++---------
 1 file changed, 54 insertions(+), 29 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 9dcdac8..8ff23c5 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -54,21 +54,11 @@ virtio_user_create_queue(struct virtio_user_dev *dev, uint32_t queue_sel)
 	 * firstly because vhost depends on this msg to allocate virtqueue
 	 * pair.
 	 */
-	int callfd;
 	struct vhost_vring_file file;
 
-	/* May use invalid flag, but some backend leverages kickfd and callfd as
-	 * criteria to judge if dev is alive. so finally we use real event_fd.
-	 */
-	callfd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);
-	if (callfd < 0) {
-		PMD_DRV_LOG(ERR, "callfd error, %s", strerror(errno));
-		return -1;
-	}
 	file.index = queue_sel;
-	file.fd = callfd;
+	file.fd = dev->callfds[queue_sel];
 	dev->ops->send_request(dev, VHOST_USER_SET_VRING_CALL, &file);
-	dev->callfds[queue_sel] = callfd;
 
 	return 0;
 }
@@ -76,7 +66,6 @@ virtio_user_create_queue(struct virtio_user_dev *dev, uint32_t queue_sel)
 static int
 virtio_user_kick_queue(struct virtio_user_dev *dev, uint32_t queue_sel)
 {
-	int kickfd;
 	struct vhost_vring_file file;
 	struct vhost_vring_state state;
 	struct vring *vring = &dev->vrings[queue_sel];
@@ -103,15 +92,9 @@ virtio_user_kick_queue(struct virtio_user_dev *dev, uint32_t queue_sel)
 	 * lastly because vhost depends on this msg to judge if
 	 * virtio is ready.
 	 */
-	kickfd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);
-	if (kickfd < 0) {
-		PMD_DRV_LOG(ERR, "kickfd error, %s", strerror(errno));
-		return -1;
-	}
 	file.index = queue_sel;
-	file.fd = kickfd;
+	file.fd = dev->kickfds[queue_sel];
 	dev->ops->send_request(dev, VHOST_USER_SET_VRING_KICK, &file);
-	dev->kickfds[queue_sel] = kickfd;
 
 	return 0;
 }
@@ -185,11 +168,6 @@ int virtio_user_stop_device(struct virtio_user_dev *dev)
 {
 	uint32_t i;
 
-	for (i = 0; i < dev->max_queue_pairs * 2; ++i) {
-		close(dev->callfds[i]);
-		close(dev->kickfds[i]);
-	}
-
 	for (i = 0; i < dev->max_queue_pairs; ++i)
 		dev->ops->enable_qp(dev, i, 0);
 
@@ -232,19 +210,61 @@ is_vhost_user_by_type(const char *path)
 }
 
 static int
-virtio_user_dev_setup(struct virtio_user_dev *dev)
+virtio_user_dev_init_notify(struct virtio_user_dev *dev)
 {
-	uint32_t i, q;
+	uint32_t i, j;
+	int callfd;
+	int kickfd;
 
-	dev->vhostfd = -1;
 	for (i = 0; i < VIRTIO_MAX_VIRTQUEUES; ++i) {
-		dev->kickfds[i] = -1;
-		dev->callfds[i] = -1;
+		if (i >= dev->max_queue_pairs * 2) {
+			dev->kickfds[i] = -1;
+			dev->callfds[i] = -1;
+			continue;
+		}
+
+		/* May use invalid flag, but some backend uses kickfd and
+		 * callfd as criteria to judge if dev is alive. so finally we
+		 * use real event_fd.
+		 */
+		callfd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);
+		if (callfd < 0) {
+			PMD_DRV_LOG(ERR, "callfd error, %s", strerror(errno));
+			break;
+		}
+		kickfd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);
+		if (kickfd < 0) {
+			PMD_DRV_LOG(ERR, "kickfd error, %s", strerror(errno));
+			break;
+		}
+		dev->callfds[i] = callfd;
+		dev->kickfds[i] = kickfd;
 	}
 
+	if (i < VIRTIO_MAX_VIRTQUEUES) {
+		for (j = 0; j <= i; ++j) {
+			close(dev->callfds[j]);
+			close(dev->kickfds[j]);
+		}
+
+		return -1;
+	}
+
+	return 0;
+}
+
+static int
+virtio_user_dev_setup(struct virtio_user_dev *dev)
+{
+	uint32_t q;
+
+	dev->vhostfd = -1;
 	dev->vhostfds = NULL;
 	dev->tapfds = NULL;
 
+	if (virtio_user_dev_init_notify(dev) < 0)
+		return -1;
+
 	if (is_vhost_user_by_type(dev->path)) {
 		dev->ops = &ops_user;
 	} else {
@@ -319,6 +339,11 @@ virtio_user_dev_uninit(struct virtio_user_dev *dev)
 
 	virtio_user_stop_device(dev);
 
+	for (i = 0; i < dev->max_queue_pairs * 2; ++i) {
+		close(dev->callfds[i]);
+		close(dev->kickfds[i]);
+	}
+
 	close(dev->vhostfd);
 
 	if (dev->vhostfds) {
-- 
2.7.4

[PATCH v3 3/5] net/virtio-user: add rxq interrupt mode support

[Jianfeng Tan]

For rxq interrupt, the device (backend driver) will notify driver
through callfd. Each virtqueue has a callfd. To keep compatible
with the existing framework, we will give these callfds to
interrupt thread for listening for interrupts.

Before that, we need to allocate intr_handle, and fill callfds
into it so that driver can use it to set up rxq interrupt mode.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 doc/guides/rel_notes/release_17_05.rst           |  7 ++++++
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 27 ++++++++++++++++++++++++
 drivers/net/virtio/virtio_user/virtio_user_dev.h |  1 +
 drivers/net/virtio/virtio_user_ethdev.c          | 12 ++++++++++-
 4 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/doc/guides/rel_notes/release_17_05.rst b/doc/guides/rel_notes/release_17_05.rst
index bb64428..2b5eaab 100644
--- a/doc/guides/rel_notes/release_17_05.rst
+++ b/doc/guides/rel_notes/release_17_05.rst
@@ -57,6 +57,13 @@ New Features
   * Enable Vhost PMD's MTU get feature.
   * Get max MTU value from host in Virtio PMD
 
+* **Added interrupt mode support for virtio-user.**
+
+  Implemented Rxq interrupt mode support for virtio-user as a virtual
+  device. Supported cases:
+
+  * Rxq interrupt for virtio-user + vhost-user as the backend.
+  * Rxq interrupt for virtio-user + vhost-kernel as the backend.
 
 Resolved Issues
 ---------------
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 8ff23c5..ce4ead0 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -254,6 +254,30 @@ virtio_user_dev_init_notify(struct virtio_user_dev *dev)
 }
 
 static int
+virtio_user_fill_intr_handle(struct virtio_user_dev *dev)
+{
+	uint32_t i;
+	struct rte_eth_dev *eth_dev = &rte_eth_devices[dev->portid];
+
+	if (!eth_dev->intr_handle) {
+		eth_dev->intr_handle = malloc(sizeof(*eth_dev->intr_handle));
+		if (!eth_dev->intr_handle) {
+			PMD_DRV_LOG(ERR, "fail to allocate intr_handle");
+			return -1;
+		}
+		memset(eth_dev->intr_handle, 0, sizeof(*eth_dev->intr_handle));
+	}
+
+	for (i = 0; i < dev->max_queue_pairs; ++i)
+		eth_dev->intr_handle->efds[i] = dev->callfds[i];
+	eth_dev->intr_handle->nb_efd = dev->max_queue_pairs;
+	eth_dev->intr_handle->max_intr = dev->max_queue_pairs + 1;
+	eth_dev->intr_handle->type = RTE_INTR_HANDLE_VDEV;
+
+	return 0;
+}
+
+static int
 virtio_user_dev_setup(struct virtio_user_dev *dev)
 {
 	uint32_t q;
@@ -265,6 +289,9 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
 	if (virtio_user_dev_init_notify(dev) < 0)
 		return -1;
 
+	if (virtio_user_fill_intr_handle(dev) < 0)
+		return -1;
+
 	if (is_vhost_user_by_type(dev->path)) {
 		dev->ops = &ops_user;
 	} else {
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index bd2e4ca..44c4e68 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -60,6 +60,7 @@ struct virtio_user_dev {
 				   */
 	uint64_t	device_features; /* supported features by device */
 	uint8_t		status;
+	uint8_t		portid;
 	uint8_t		mac_addr[ETHER_ADDR_LEN];
 	char		path[PATH_MAX];
 	struct vring	vrings[VIRTIO_MAX_VIRTQUEUES];
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 2961e6b..335d70d 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -148,6 +148,15 @@ virtio_user_set_config_irq(struct virtio_hw *hw __rte_unused,
 	return VIRTIO_MSI_NO_VECTOR;
 }
 
+static uint16_t
+virtio_user_set_queue_irq(struct virtio_hw *hw __rte_unused,
+			  struct virtqueue *vq __rte_unused,
+			  uint16_t vec)
+{
+	/* pretend we have done that */
+	return vec;
+}
+
 /* This function is to get the queue size, aka, number of descs, of a specified
  * queue. Different with the VHOST_USER_GET_QUEUE_NUM, which is used to get the
  * max supported queues.
@@ -226,6 +235,7 @@ const struct virtio_pci_ops virtio_user_ops = {
 	.set_features	= virtio_user_set_features,
 	.get_isr	= virtio_user_get_isr,
 	.set_config_irq	= virtio_user_set_config_irq,
+	.set_queue_irq	= virtio_user_set_queue_irq,
 	.get_queue_num	= virtio_user_get_queue_num,
 	.setup_queue	= virtio_user_setup_queue,
 	.del_queue	= virtio_user_del_queue,
@@ -307,7 +317,7 @@ virtio_user_eth_dev_alloc(const char *name)
 		return NULL;
 	}
 
-	hw->port_id = data->port_id;
+	dev->portid = hw->port_id = data->port_id;
 	virtio_hw_internal[hw->port_id].vtpci_ops = &virtio_user_ops;
 	hw->use_msix = 0;
 	hw->modern   = 0;
-- 
2.7.4

[PATCH v3 4/5] net/virtio-user: support to report net status

[Jianfeng Tan]

Originally, we did not report support of VIRTIO_NET_F_STATUS.
This feature is not reported by vhost backend, instead, it
is added/removed by QEMU in virtio PCI case.

We report the support of this feature so that following patch
will depend on this feature to enable LSC interrupt.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 4 ++++
 drivers/net/virtio/virtio_user_ethdev.c          | 8 ++++----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index ce4ead0..6e26df0 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -139,6 +139,7 @@ virtio_user_start_device(struct virtio_user_dev *dev)
 	features &= ~(1ull << VIRTIO_NET_F_MAC);
 	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to know */
 	features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
+	features &= ~(1ull << VIRTIO_NET_F_STATUS);
 	ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES, &features);
 	if (ret < 0)
 		goto error;
@@ -356,6 +357,9 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		dev->device_features &= ~(1ull << VIRTIO_NET_F_CTRL_MAC_ADDR);
 	}
 
+	/* The backend will not report this feature, we add it explicitly */
+	dev->device_features |= (1ull << VIRTIO_NET_F_STATUS);
+
 	return 0;
 }
 
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 335d70d..e8b14e5 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -135,17 +135,17 @@ virtio_user_set_features(struct virtio_hw *hw, uint64_t features)
 static uint8_t
 virtio_user_get_isr(struct virtio_hw *hw __rte_unused)
 {
-	/* When config interrupt happens, driver calls this function to query
-	 * what kinds of change happen. Interrupt mode not supported for now.
+	/* rxq interrupts and config interrupt are separated in virtio-user,
+	 * here we only report config change.
 	 */
-	return 0;
+	return VIRTIO_PCI_ISR_CONFIG;
 }
 
 static uint16_t
 virtio_user_set_config_irq(struct virtio_hw *hw __rte_unused,
 		    uint16_t vec __rte_unused)
 {
-	return VIRTIO_MSI_NO_VECTOR;
+	return 0;
 }
 
 static uint16_t
-- 
2.7.4

[PATCH v3 5/5] net/virtio-user: add lsc support

[Jianfeng Tan]

So far, virtio-user with vhost-user as the backend can only support
client mode. So when vhost user backend is down, i.e., unix socket
connection is broken, the connection cannot be re-connected. We will
forcely set the link state to be down.

Note: virtio-user with vhost-kernel as the backend still cannot
support lsc now as we fail to find a way to monitor the backend, tap
device, up/down events.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 doc/guides/rel_notes/release_17_05.rst  |  7 ++++-
 drivers/net/virtio/virtio_ethdev.c      |  2 +-
 drivers/net/virtio/virtio_ethdev.h      |  2 ++
 drivers/net/virtio/virtio_user_ethdev.c | 51 +++++++++++++++++++++++++++++++--
 4 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/doc/guides/rel_notes/release_17_05.rst b/doc/guides/rel_notes/release_17_05.rst
index 2b5eaab..42ba190 100644
--- a/doc/guides/rel_notes/release_17_05.rst
+++ b/doc/guides/rel_notes/release_17_05.rst
@@ -59,11 +59,12 @@ New Features
 
 * **Added interrupt mode support for virtio-user.**
 
-  Implemented Rxq interrupt mode support for virtio-user as a virtual
+  Implemented Rxq interrupt mode and LSC support for virtio-user as a virtual
   device. Supported cases:
 
   * Rxq interrupt for virtio-user + vhost-user as the backend.
   * Rxq interrupt for virtio-user + vhost-kernel as the backend.
+  * LSC interrupt for virtio-user + vhost-user as the backend.
 
 Resolved Issues
 ---------------
@@ -119,6 +120,10 @@ Known Issues
    Also, make sure to start the actual text at the margin.
    =========================================================
 
+   * **LSC interrupt cannot work for virtio-user + vhost-kernel.**
+
+     LSC interrupt cannot be detected when setting the backend, tap device,
+     up/down as we fail to find a way to monitor such event.
 
 API Changes
 -----------
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d9986ab..6a9568f 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1205,7 +1205,7 @@ virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features)
  * Process Virtio Config changed interrupt and call the callback
  * if link state changed.
  */
-static void
+void
 virtio_interrupt_handler(struct rte_intr_handle *handle,
 			 void *param)
 {
diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index aa78adc..4009c4d 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -113,4 +113,6 @@ uint16_t virtio_xmit_pkts_simple(void *tx_queue, struct rte_mbuf **tx_pkts,
 
 int eth_virtio_dev_init(struct rte_eth_dev *eth_dev);
 
+void virtio_interrupt_handler(struct rte_intr_handle *handle, void *param);
+
 #endif /* _VIRTIO_ETHDEV_H_ */
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index e8b14e5..d8cea20 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -34,10 +34,14 @@
 #include <stdint.h>
 #include <sys/types.h>
 #include <unistd.h>
+#include <fcntl.h>
+#include <sys/types.h>
+#include <sys/socket.h>
 
 #include <rte_malloc.h>
 #include <rte_kvargs.h>
 #include <rte_vdev.h>
+#include <rte_alarm.h>
 
 #include "virtio_ethdev.h"
 #include "virtio_logs.h"
@@ -49,6 +53,16 @@
 #define virtio_user_get_dev(hw) \
 	((struct virtio_user_dev *)(hw)->virtio_user_dev)
 
+static void virtio_user_delayed_handler(void *param)
+{
+	struct virtio_hw *hw = (struct virtio_hw *)param;
+	struct rte_eth_dev *dev = &rte_eth_devices[hw->port_id];
+
+	rte_intr_callback_unregister(dev->intr_handle,
+				     virtio_interrupt_handler,
+				     dev);
+}
+
 static void
 virtio_user_read_dev_config(struct virtio_hw *hw, size_t offset,
 		     void *dst, int length)
@@ -63,8 +77,37 @@ virtio_user_read_dev_config(struct virtio_hw *hw, size_t offset,
 		return;
 	}
 
-	if (offset == offsetof(struct virtio_net_config, status))
+	if (offset == offsetof(struct virtio_net_config, status)) {
+		char buf[128];
+
+		if (dev->vhostfd >= 0) {
+			int r;
+			int flags;
+
+			flags = fcntl(dev->vhostfd, F_GETFL);
+			fcntl(dev->vhostfd, F_SETFL, flags | O_NONBLOCK);
+			r = recv(dev->vhostfd, buf, 128, MSG_PEEK);
+			if (r == 0 || (r < 0 && errno != EAGAIN)) {
+				dev->status &= (~VIRTIO_NET_S_LINK_UP);
+				PMD_DRV_LOG(ERR, "virtio-user port %u is down",
+					    hw->port_id);
+				/* Only client mode is available now. Once the
+				 * connection is broken, it can never be up
+				 * again. Besides, this function could be called
+				 * in the process of interrupt handling,
+				 * callback cannot be unregistered here, set an
+				 * alarm to do it.
+				 */
+				rte_eal_alarm_set(1,
+						  virtio_user_delayed_handler,
+						  (void *)hw);
+			} else {
+				dev->status |= VIRTIO_NET_S_LINK_UP;
+			}
+			fcntl(dev->vhostfd, F_SETFL, flags & (~O_NONBLOCK));
+		}
 		*(uint16_t *)dst = dev->status;
+	}
 
 	if (offset == offsetof(struct virtio_net_config, max_virtqueue_pairs))
 		*(uint16_t *)dst = dev->max_queue_pairs;
@@ -319,7 +362,11 @@ virtio_user_eth_dev_alloc(const char *name)
 
 	dev->portid = hw->port_id = data->port_id;
 	virtio_hw_internal[hw->port_id].vtpci_ops = &virtio_user_ops;
-	hw->use_msix = 0;
+	/* After commit 26bbd3e7dc51("net/virtio: disable LSC interrupt if MSIX
+	 * not enabled"), we require this to enable LSC. Just pretend that we
+	 * support msix.
+	 */
+	hw->use_msix = 1;
 	hw->modern   = 0;
 	hw->use_simple_rxtx = 0;
 	hw->virtio_user_dev = dev;
-- 
2.7.4

Re: [PATCH v3 5/5] net/virtio-user: add lsc support

[Yuanhan Liu]

On Fri, Mar 31, 2017 at 07:44:58PM +0000, Jianfeng Tan wrote:
> +static void virtio_user_delayed_handler(void *param)

Coding style issue.

...
>  
>  	if (offset == offsetof(struct virtio_net_config, max_virtqueue_pairs))
>  		*(uint16_t *)dst = dev->max_queue_pairs;
> @@ -319,7 +362,11 @@ virtio_user_eth_dev_alloc(const char *name)
>  
>  	dev->portid = hw->port_id = data->port_id;
>  	virtio_hw_internal[hw->port_id].vtpci_ops = &virtio_user_ops;
> -	hw->use_msix = 0;
> +	/* After commit 26bbd3e7dc51("net/virtio: disable LSC interrupt if MSIX
> +	 * not enabled"), we require this to enable LSC. Just pretend that we
> +	 * support msix.
> +	 */

We normally do not reference a commit in comments. Instead, you could
simply say "this is required to enable LSC (see __the_function__), ...".

Series applied to dpdk-next-virtio, with above minor fixes.

	--yliu

> +	hw->use_msix = 1;
>  	hw->modern   = 0;
>  	hw->use_simple_rxtx = 0;
>  	hw->virtio_user_dev = dev;
> -- 
> 2.7.4