[4/5] net/netvsc: implement link state change callback

Message ID 20180830223512.21297-5-stephen@networkplumber.org (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series netvsc changes for 18.11 |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Stephen Hemminger Aug. 30, 2018, 10:35 p.m. UTC
  From: Stephen Hemminger <sthemmin@microsoft.com>

Implement callback functionality on link state changes.
This is not really driven off of interrupt file descriptor like most other
PMD's. Instead, it happens when a link state change message arrives
in the common ring buffer.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 doc/guides/nics/netvsc.rst     |  3 ++-
 drivers/net/netvsc/hn_ethdev.c | 12 +++++-------
 drivers/net/netvsc/hn_rndis.c  | 11 ++++++++---
 drivers/net/netvsc/hn_rndis.h  |  2 +-
 drivers/net/netvsc/hn_rxtx.c   |  4 ++--
 drivers/net/netvsc/hn_var.h    |  1 +
 6 files changed, 19 insertions(+), 14 deletions(-)
  

Comments

Gaëtan Rivet Aug. 31, 2018, 8:25 a.m. UTC | #1
Hi Stephen,

On Thu, Aug 30, 2018 at 03:35:11PM -0700, Stephen Hemminger wrote:
> From: Stephen Hemminger <sthemmin@microsoft.com>
> 
> Implement callback functionality on link state changes.
> This is not really driven off of interrupt file descriptor like most other
> PMD's. Instead, it happens when a link state change message arrives
> in the common ring buffer.
> 

Does this mean that the lsc event will be processed in a dataplane
thread? Looking at the _rte_eth_dev_callback_process() call, it seems
so.

Shouldn't this be executed in the context of the eal-intr-thread
instead? This thread is marked control and should be configured with the
proper afinity, unless dataplane threads.

Maybe I missed something, it's just to double-check that this will
behave nicely with applications relying on the eal-intr-thread afinity.
  
Stephen Hemminger Aug. 31, 2018, 3:13 p.m. UTC | #2
On Fri, 31 Aug 2018 10:25:47 +0200
Gaëtan Rivet <gaetan.rivet@6wind.com> wrote:

> Hi Stephen,
> 
> On Thu, Aug 30, 2018 at 03:35:11PM -0700, Stephen Hemminger wrote:
> > From: Stephen Hemminger <sthemmin@microsoft.com>
> > 
> > Implement callback functionality on link state changes.
> > This is not really driven off of interrupt file descriptor like most other
> > PMD's. Instead, it happens when a link state change message arrives
> > in the common ring buffer.
> >   
> 
> Does this mean that the lsc event will be processed in a dataplane
> thread? Looking at the _rte_eth_dev_callback_process() call, it seems
> so.
> 
> Shouldn't this be executed in the context of the eal-intr-thread
> instead? This thread is marked control and should be configured with the
> proper afinity, unless dataplane threads.
> 
> Maybe I missed something, it's just to double-check that this will
> behave nicely with applications relying on the eal-intr-thread afinity.
> 
> -- 
> Gaëtan Rivet
> 6WIND

There is no EAL API to take the event and propogate it over to the eal interrupt
thread. The interrupt thread is buried in the internals of EAL.
  

Patch

diff --git a/doc/guides/nics/netvsc.rst b/doc/guides/nics/netvsc.rst
index cc07ce46f885..c5f9b7c6fa1e 100644
--- a/doc/guides/nics/netvsc.rst
+++ b/doc/guides/nics/netvsc.rst
@@ -28,7 +28,8 @@  In this release, the hyper PMD driver provides the basic functionality of packet
 
 *   VLAN tags are always stripped and presented in mbuf tci field.
 
-*   The Hyper-V driver does not use or support Link State or Rx interrupt.
+*   The Hyper-V driver does not use or support interrupts. Link state change
+    callback is done via change events in the packet ring.
 
 *   The maximum number of queues is limited by the host (currently 64).
     When used with 4.16 kernel only a single queue is available.
diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index 8643e0b3c057..63e04a453546 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -107,6 +107,10 @@  eth_dev_vmbus_allocate(struct rte_vmbus_device *dev, size_t private_data_size)
 	}
 
 	eth_dev->device = &dev->device;
+
+	/* interrupt is simulated */
+	dev->intr_handle.type = RTE_INTR_HANDLE_EXT;
+	eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
 	eth_dev->intr_handle = &dev->intr_handle;
 
 	return eth_dev;
@@ -187,7 +191,7 @@  static int hn_parse_args(const struct rte_eth_dev *dev)
  *   means block this call until link is up.
  *   which is not worth supporting.
  */
-static int
+int
 hn_dev_link_update(struct rte_eth_dev *dev,
 		   __rte_unused int wait_to_complete)
 {
@@ -554,12 +558,6 @@  hn_dev_start(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	/* check if lsc interrupt feature is enabled */
-	if (dev->data->dev_conf.intr_conf.lsc) {
-		PMD_DRV_LOG(ERR, "link status not supported yet");
-		return -ENOTSUP;
-	}
-
 	return hn_rndis_set_rxfilter(hv,
 				     NDIS_PACKET_TYPE_BROADCAST |
 				     NDIS_PACKET_TYPE_ALL_MULTICAST |
diff --git a/drivers/net/netvsc/hn_rndis.c b/drivers/net/netvsc/hn_rndis.c
index f44add726b91..a5d205ce2636 100644
--- a/drivers/net/netvsc/hn_rndis.c
+++ b/drivers/net/netvsc/hn_rndis.c
@@ -11,6 +11,7 @@ 
 #include <errno.h>
 #include <unistd.h>
 
+#include <rte_ethdev_driver.h>
 #include <rte_ethdev.h>
 #include <rte_string_fns.h>
 #include <rte_memzone.h>
@@ -281,7 +282,7 @@  static int hn_nvs_send_rndis_ctrl(struct vmbus_channel *chan,
 				  &nvs_rndis, sizeof(nvs_rndis), 0U, NULL);
 }
 
-void hn_rndis_link_status(struct hn_data *hv __rte_unused, const void *msg)
+void hn_rndis_link_status(struct rte_eth_dev *dev, const void *msg)
 {
 	const struct rndis_status_msg *indicate = msg;
 
@@ -290,15 +291,19 @@  void hn_rndis_link_status(struct hn_data *hv __rte_unused, const void *msg)
 	PMD_DRV_LOG(DEBUG, "link status %#x", indicate->status);
 
 	switch (indicate->status) {
-	case RNDIS_STATUS_LINK_SPEED_CHANGE:
 	case RNDIS_STATUS_NETWORK_CHANGE:
 	case RNDIS_STATUS_TASK_OFFLOAD_CURRENT_CONFIG:
 		/* ignore not in DPDK API */
 		break;
 
+	case RNDIS_STATUS_LINK_SPEED_CHANGE:
 	case RNDIS_STATUS_MEDIA_CONNECT:
 	case RNDIS_STATUS_MEDIA_DISCONNECT:
-		/* TODO handle as LSC interrupt  */
+		if (dev->data->dev_conf.intr_conf.lsc &&
+		    hn_dev_link_update(dev, 0) == 0)
+			_rte_eth_dev_callback_process(dev,
+						      RTE_ETH_EVENT_INTR_LSC,
+						      NULL);
 		break;
 	default:
 		PMD_DRV_LOG(NOTICE, "unknown RNDIS indication: %#x",
diff --git a/drivers/net/netvsc/hn_rndis.h b/drivers/net/netvsc/hn_rndis.h
index 89e2e6ba0fcb..01b5120631b1 100644
--- a/drivers/net/netvsc/hn_rndis.h
+++ b/drivers/net/netvsc/hn_rndis.h
@@ -6,7 +6,7 @@  struct hn_data;
 
 void hn_rndis_receive_response(struct hn_data *hv,
 			      const void *data, uint32_t len);
-void	hn_rndis_link_status(struct hn_data *hv, const void *data);
+void	hn_rndis_link_status(struct rte_eth_dev *dev, const void *msg);
 int	hn_rndis_attach(struct hn_data *hv);
 void	hn_rndis_detach(struct hn_data *hv);
 int	hn_rndis_get_eaddr(struct hn_data *hv, uint8_t *eaddr);
diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c
index 92de5d09e442..ad22a95f6c27 100644
--- a/drivers/net/netvsc/hn_rxtx.c
+++ b/drivers/net/netvsc/hn_rxtx.c
@@ -601,7 +601,7 @@  static void hn_rndis_rx_data(struct hn_rx_queue *rxq,
 }
 
 static void
-hn_rndis_receive(const struct rte_eth_dev *dev, struct hn_rx_queue *rxq,
+hn_rndis_receive(struct rte_eth_dev *dev, struct hn_rx_queue *rxq,
 		 struct hn_rx_bufinfo *rxb, void *buf, uint32_t len)
 {
 	const struct rndis_msghdr *hdr = buf;
@@ -613,7 +613,7 @@  hn_rndis_receive(const struct rte_eth_dev *dev, struct hn_rx_queue *rxq,
 		break;
 
 	case RNDIS_INDICATE_STATUS_MSG:
-		hn_rndis_link_status(rxq->hv, buf);
+		hn_rndis_link_status(dev, buf);
 		break;
 
 	case RNDIS_INITIALIZE_CMPLT:
diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h
index ff722560d9d9..17b67941dc83 100644
--- a/drivers/net/netvsc/hn_var.h
+++ b/drivers/net/netvsc/hn_var.h
@@ -142,6 +142,7 @@  uint16_t hn_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		      uint16_t nb_pkts);
 
 int	hn_tx_pool_init(struct rte_eth_dev *dev);
+int	hn_dev_link_update(struct rte_eth_dev *dev, int wait);
 int	hn_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 			      uint16_t nb_desc, unsigned int socket_id,
 			      const struct rte_eth_txconf *tx_conf);