[dpdk-dev] [PATCH 0/7] vmbus/netvsc: fix multi-process support

[dpdk-dev] [PATCH 0/7] vmbus/netvsc: fix multi-process support

[Stephen Hemminger]

From: Stephen Hemminger <sthemmin@microsoft.com>

These fix the primary/secondary process model support in the
vmbus and netvsc PMD.  They do NOT fix the failsafe PMD.

The last two are not bug fixes but address unnecessary
code found while debugging the mp support.

Stephen Hemminger (7):
  bus/vmbus: fix secondary process setup
  net/netvsc: fix VF support with secondary process
  bus/vmbus: fix check for mmap failure
  bus/vmbus: stop mapping if empty resource found
  bus/vmbus: map ring in secondary
  bus/vmbus: refactor secondary mapping
  net/netvsc: remove unnecessary format of ether address

 drivers/bus/vmbus/linux/vmbus_uio.c  |  54 +++++++++++-
 drivers/bus/vmbus/private.h          |   3 +
 drivers/bus/vmbus/vmbus_channel.c    |  20 ++++-
 drivers/bus/vmbus/vmbus_common_uio.c | 122 +++++++++++++--------------
 drivers/net/netvsc/hn_ethdev.c       |   3 +-
 drivers/net/netvsc/hn_rxtx.c         |   8 +-
 drivers/net/netvsc/hn_var.h          |  30 ++++++-
 drivers/net/netvsc/hn_vf.c           |  85 +++++++++----------
 8 files changed, 206 insertions(+), 119 deletions(-)

-- 
2.20.1

[dpdk-dev] [PATCH 1/7] bus/vmbus: fix secondary process setup

[Stephen Hemminger]

From: Stephen Hemminger <sthemmin@microsoft.com>

The secondary process doesn't correctly map the second
and later resources because it doesn't change the offset.

Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/bus/vmbus/vmbus_common_uio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/vmbus/vmbus_common_uio.c b/drivers/bus/vmbus/vmbus_common_uio.c
index 5ddd36ab62d2..46e233d9fac3 100644
--- a/drivers/bus/vmbus/vmbus_common_uio.c
+++ b/drivers/bus/vmbus/vmbus_common_uio.c
@@ -47,9 +47,10 @@ vmbus_uio_map_secondary(struct rte_vmbus_device *dev)
 
 		for (i = 0; i != uio_res->nb_maps; i++) {
 			void *mapaddr;
+			off_t offset = i * PAGE_SIZE;
 
 			mapaddr = vmbus_map_resource(uio_res->maps[i].addr,
-						     fd, 0,
+						     fd, offset,
 						     uio_res->maps[i].size, 0);
 
 			if (mapaddr == uio_res->maps[i].addr)
-- 
2.20.1

[dpdk-dev] [PATCH 2/7] net/netvsc: fix VF support with secondary process

[Stephen Hemminger]

From: Stephen Hemminger <sthemmin@microsoft.com>

The VF device management in netvsc was using a pointer to the
rte_eth_devices. But the actual rte_eth_devices array is likely to
be place in the secondary process; which causes a crash.

The solution is to record the port of the VF (instead of a pointer)
and find the device in the per process array as needed.

Fixes: dc7680e8597c ("net/netvsc: support integrated VF")
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/netvsc/hn_ethdev.c |  3 +-
 drivers/net/netvsc/hn_rxtx.c   |  8 ++--
 drivers/net/netvsc/hn_var.h    | 30 ++++++++++++-
 drivers/net/netvsc/hn_vf.c     | 82 +++++++++++++++++-----------------
 4 files changed, 75 insertions(+), 48 deletions(-)

diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index 49b7ca7b2244..407ee484935a 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -735,6 +735,7 @@ eth_hn_dev_init(struct rte_eth_dev *eth_dev)
 	hv->port_id = eth_dev->data->port_id;
 	hv->latency = HN_CHAN_LATENCY_NS;
 	hv->max_queues = 1;
+	hv->vf_port = HN_INVALID_PORT;
 
 	err = hn_parse_args(eth_dev);
 	if (err)
@@ -788,7 +789,7 @@ eth_hn_dev_init(struct rte_eth_dev *eth_dev)
 	hv->max_queues = RTE_MIN(rxr_cnt, (unsigned int)max_chan);
 
 	/* If VF was reported but not added, do it now */
-	if (hv->vf_present && !hv->vf_dev) {
+	if (hv->vf_present && !hn_vf_attached(hv)) {
 		PMD_INIT_LOG(DEBUG, "Adding VF device");
 
 		err = hn_vf_add(eth_dev, hv);
diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c
index 6197118b01ee..fecd69887e2b 100644
--- a/drivers/net/netvsc/hn_rxtx.c
+++ b/drivers/net/netvsc/hn_rxtx.c
@@ -1313,8 +1313,8 @@ hn_xmit_pkts(void *ptxq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 		return 0;
 
 	/* Transmit over VF if present and up */
-	vf_dev = hv->vf_dev;
-	rte_compiler_barrier();
+	vf_dev = hn_get_vf_dev(hv);
+
 	if (vf_dev && vf_dev->data->dev_started) {
 		void *sub_q = vf_dev->data->tx_queues[queue_id];
 
@@ -1404,8 +1404,8 @@ hn_recv_pkts(void *prxq, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	if (unlikely(hv->closed))
 		return 0;
 
-	vf_dev = hv->vf_dev;
-	rte_compiler_barrier();
+	/* Transmit over VF if present and up */
+	vf_dev = hn_get_vf_dev(hv);
 
 	if (vf_dev && vf_dev->data->dev_started) {
 		/* Normally, with SR-IOV the ring buffer will be empty */
diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h
index 7f3266c451fb..8383f3246ca4 100644
--- a/drivers/net/netvsc/hn_var.h
+++ b/drivers/net/netvsc/hn_var.h
@@ -91,15 +91,19 @@ struct hn_rx_bufinfo {
 	struct rte_mbuf_ext_shared_info shinfo;
 } __rte_cache_aligned;
 
+#define HN_INVALID_PORT	UINT16_MAX
+
 struct hn_data {
 	struct rte_vmbus_device *vmbus;
 	struct hn_rx_queue *primary;
-	struct rte_eth_dev *vf_dev;		/* Subordinate device */
 	rte_spinlock_t  vf_lock;
 	uint16_t	port_id;
-	uint8_t		closed;
+	uint16_t	vf_port;
+
 	uint8_t		vf_present;
+	uint8_t		closed;
 	uint8_t		vlan_strip;
+
 	uint32_t	link_status;
 	uint32_t	link_speed;
 
@@ -170,6 +174,28 @@ int	hn_dev_rx_queue_setup(struct rte_eth_dev *dev,
 			      struct rte_mempool *mp);
 void	hn_dev_rx_queue_release(void *arg);
 
+/* Check if VF is attached */
+static inline bool
+hn_vf_attached(const struct hn_data *hv)
+{
+	return hv->vf_port != HN_INVALID_PORT;
+}
+
+/* Get VF device for existing netvsc device */
+static inline struct rte_eth_dev *
+hn_get_vf_dev(const struct hn_data *hv)
+{
+	uint16_t vf_port = hv->vf_port;
+
+	/* make sure vf_port is loaded */
+	rte_smp_rmb();
+
+	if (vf_port == HN_INVALID_PORT)
+		return NULL;
+	else
+		return &rte_eth_devices[vf_port];
+}
+
 void	hn_vf_info_get(struct hn_data *hv,
 		       struct rte_eth_dev_info *info);
 int	hn_vf_add(struct rte_eth_dev *dev, struct hn_data *hv);
diff --git a/drivers/net/netvsc/hn_vf.c b/drivers/net/netvsc/hn_vf.c
index 3f714ec99029..de278eb7b403 100644
--- a/drivers/net/netvsc/hn_vf.c
+++ b/drivers/net/netvsc/hn_vf.c
@@ -51,15 +51,20 @@ static int hn_vf_match(const struct rte_eth_dev *dev)
 	return -ENOENT;
 }
 
+
 /*
  * Attach new PCI VF device and return the port_id
  */
-static int hn_vf_attach(struct hn_data *hv, uint16_t port_id,
-			struct rte_eth_dev **vf_dev)
+static int hn_vf_attach(struct hn_data *hv, uint16_t port_id)
 {
 	struct rte_eth_dev_owner owner = { .id = RTE_ETH_DEV_NO_OWNER };
 	int ret;
 
+	if (hn_vf_attached(hv)) {
+		PMD_DRV_LOG(ERR, "VF already attached");
+		return -EEXIST;
+	}
+
 	ret = rte_eth_dev_owner_get(port_id, &owner);
 	if (ret < 0) {
 		PMD_DRV_LOG(ERR, "Can not find owner for port %d", port_id);
@@ -79,8 +84,9 @@ static int hn_vf_attach(struct hn_data *hv, uint16_t port_id,
 	}
 
 	PMD_DRV_LOG(DEBUG, "Attach VF device %u", port_id);
+	hv->vf_port = port_id;
 	rte_smp_wmb();
-	*vf_dev = &rte_eth_devices[port_id];
+
 	return 0;
 }
 
@@ -96,12 +102,7 @@ int hn_vf_add(struct rte_eth_dev *dev, struct hn_data *hv)
 	}
 
 	rte_spinlock_lock(&hv->vf_lock);
-	if (hv->vf_dev) {
-		PMD_DRV_LOG(ERR, "VF already attached");
-		err = -EBUSY;
-	} else {
-		err = hn_vf_attach(hv, port, &hv->vf_dev);
-	}
+	err = hn_vf_attach(hv, port);
 
 	if (err == 0) {
 		dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
@@ -120,22 +121,22 @@ int hn_vf_add(struct rte_eth_dev *dev, struct hn_data *hv)
 /* Remove new VF device */
 static void hn_vf_remove(struct hn_data *hv)
 {
-	struct rte_eth_dev *vf_dev;
 
 	rte_spinlock_lock(&hv->vf_lock);
-	vf_dev = hv->vf_dev;
-	if (!vf_dev) {
+
+	if (!hn_vf_attached(hv)) {
 		PMD_DRV_LOG(ERR, "VF path not active");
-		rte_spinlock_unlock(&hv->vf_lock);
-		return;
-	}
+	} else {
+		/* Stop incoming packets from arriving on VF */
+		hn_nvs_set_datapath(hv, NVS_DATAPATH_SYNTHETIC);
 
-	/* Stop incoming packets from arriving on VF */
-	hn_nvs_set_datapath(hv, NVS_DATAPATH_SYNTHETIC);
-	hv->vf_dev = NULL;
+		/* Stop transmission over VF */
+		hv->vf_port = HN_INVALID_PORT;
+		rte_smp_wmb();
 
-	/* Give back ownership */
-	rte_eth_dev_owner_unset(vf_dev->data->port_id, hv->owner.id);
+		/* Give back ownership */
+		rte_eth_dev_owner_unset(hv->vf_port, hv->owner.id);
+	}
 	rte_spinlock_unlock(&hv->vf_lock);
 }
 
@@ -207,7 +208,7 @@ void hn_vf_info_get(struct hn_data *hv, struct rte_eth_dev_info *info)
 	struct rte_eth_dev *vf_dev;
 
 	rte_spinlock_lock(&hv->vf_lock);
-	vf_dev = hv->vf_dev;
+	vf_dev = hn_get_vf_dev(hv);
 	if (vf_dev)
 		hn_vf_info_merge(vf_dev, info);
 	rte_spinlock_unlock(&hv->vf_lock);
@@ -221,7 +222,7 @@ int hn_vf_link_update(struct rte_eth_dev *dev,
 	int ret = 0;
 
 	rte_spinlock_lock(&hv->vf_lock);
-	vf_dev = hv->vf_dev;
+	vf_dev = hn_get_vf_dev(hv);
 	if (vf_dev && vf_dev->dev_ops->link_update)
 		ret = (*vf_dev->dev_ops->link_update)(vf_dev, wait_to_complete);
 	rte_spinlock_unlock(&hv->vf_lock);
@@ -249,13 +250,14 @@ static int hn_vf_lsc_event(uint16_t port_id __rte_unused,
 }
 
 static int _hn_vf_configure(struct rte_eth_dev *dev,
-			    struct rte_eth_dev *vf_dev,
+			    uint16_t vf_port,
 			    const struct rte_eth_conf *dev_conf)
 {
 	struct rte_eth_conf vf_conf = *dev_conf;
-	uint16_t vf_port = vf_dev->data->port_id;
+	struct rte_eth_dev *vf_dev;
 	int ret;
 
+	vf_dev = &rte_eth_devices[vf_port];
 	if (dev_conf->intr_conf.lsc &&
 	    (vf_dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC)) {
 		PMD_DRV_LOG(DEBUG, "enabling LSC for VF %u",
@@ -294,13 +296,11 @@ int hn_vf_configure(struct rte_eth_dev *dev,
 		    const struct rte_eth_conf *dev_conf)
 {
 	struct hn_data *hv = dev->data->dev_private;
-	struct rte_eth_dev *vf_dev;
 	int ret = 0;
 
 	rte_spinlock_lock(&hv->vf_lock);
-	vf_dev = hv->vf_dev;
-	if (vf_dev)
-		ret = _hn_vf_configure(dev, vf_dev, dev_conf);
+	if (hv->vf_port != HN_INVALID_PORT)
+		ret = _hn_vf_configure(dev, hv->vf_port, dev_conf);
 	rte_spinlock_unlock(&hv->vf_lock);
 	return ret;
 }
@@ -312,7 +312,7 @@ const uint32_t *hn_vf_supported_ptypes(struct rte_eth_dev *dev)
 	const uint32_t *ptypes = NULL;
 
 	rte_spinlock_lock(&hv->vf_lock);
-	vf_dev = hv->vf_dev;
+	vf_dev = hn_get_vf_dev(hv);
 	if (vf_dev && vf_dev->dev_ops->dev_supported_ptypes_get)
 		ptypes = (*vf_dev->dev_ops->dev_supported_ptypes_get)(vf_dev);
 	rte_spinlock_unlock(&hv->vf_lock);
@@ -327,7 +327,7 @@ int hn_vf_start(struct rte_eth_dev *dev)
 	int ret = 0;
 
 	rte_spinlock_lock(&hv->vf_lock);
-	vf_dev = hv->vf_dev;
+	vf_dev = hn_get_vf_dev(hv);
 	if (vf_dev)
 		ret = rte_eth_dev_start(vf_dev->data->port_id);
 	rte_spinlock_unlock(&hv->vf_lock);
@@ -340,7 +340,7 @@ void hn_vf_stop(struct rte_eth_dev *dev)
 	struct rte_eth_dev *vf_dev;
 
 	rte_spinlock_lock(&hv->vf_lock);
-	vf_dev = hv->vf_dev;
+	vf_dev = hn_get_vf_dev(hv);
 	if (vf_dev)
 		rte_eth_dev_stop(vf_dev->data->port_id);
 	rte_spinlock_unlock(&hv->vf_lock);
@@ -352,7 +352,7 @@ void hn_vf_stop(struct rte_eth_dev *dev)
 		struct hn_data *hv = (dev)->data->dev_private;	\
 		struct rte_eth_dev *vf_dev;			\
 		rte_spinlock_lock(&hv->vf_lock);		\
-		vf_dev = hv->vf_dev;				\
+		vf_dev = hn_get_vf_dev(hv);			\
 		if (vf_dev)					\
 			func(vf_dev->data->port_id);		\
 		rte_spinlock_unlock(&hv->vf_lock);		\
@@ -402,7 +402,7 @@ int hn_vf_mc_addr_list(struct rte_eth_dev *dev,
 	int ret = 0;
 
 	rte_spinlock_lock(&hv->vf_lock);
-	vf_dev = hv->vf_dev;
+	vf_dev = hn_get_vf_dev(hv);
 	if (vf_dev)
 		ret = rte_eth_dev_set_mc_addr_list(vf_dev->data->port_id,
 						   mc_addr_set, nb_mc_addr);
@@ -420,7 +420,7 @@ int hn_vf_tx_queue_setup(struct rte_eth_dev *dev,
 	int ret = 0;
 
 	rte_spinlock_lock(&hv->vf_lock);
-	vf_dev = hv->vf_dev;
+	vf_dev = hn_get_vf_dev(hv);
 	if (vf_dev)
 		ret = rte_eth_tx_queue_setup(vf_dev->data->port_id,
 					     queue_idx, nb_desc,
@@ -434,7 +434,7 @@ void hn_vf_tx_queue_release(struct hn_data *hv, uint16_t queue_id)
 	struct rte_eth_dev *vf_dev;
 
 	rte_spinlock_lock(&hv->vf_lock);
-	vf_dev = hv->vf_dev;
+	vf_dev = hn_get_vf_dev(hv);
 	if (vf_dev && vf_dev->dev_ops->tx_queue_release) {
 		void *subq = vf_dev->data->tx_queues[queue_id];
 
@@ -455,7 +455,7 @@ int hn_vf_rx_queue_setup(struct rte_eth_dev *dev,
 	int ret = 0;
 
 	rte_spinlock_lock(&hv->vf_lock);
-	vf_dev = hv->vf_dev;
+	vf_dev = hn_get_vf_dev(hv);
 	if (vf_dev)
 		ret = rte_eth_rx_queue_setup(vf_dev->data->port_id,
 					     queue_idx, nb_desc,
@@ -469,7 +469,7 @@ void hn_vf_rx_queue_release(struct hn_data *hv, uint16_t queue_id)
 	struct rte_eth_dev *vf_dev;
 
 	rte_spinlock_lock(&hv->vf_lock);
-	vf_dev = hv->vf_dev;
+	vf_dev = hn_get_vf_dev(hv);
 	if (vf_dev && vf_dev->dev_ops->rx_queue_release) {
 		void *subq = vf_dev->data->rx_queues[queue_id];
 
@@ -486,7 +486,7 @@ int hn_vf_stats_get(struct rte_eth_dev *dev,
 	int ret = 0;
 
 	rte_spinlock_lock(&hv->vf_lock);
-	vf_dev = hv->vf_dev;
+	vf_dev = hn_get_vf_dev(hv);
 	if (vf_dev)
 		ret = rte_eth_stats_get(vf_dev->data->port_id, stats);
 	rte_spinlock_unlock(&hv->vf_lock);
@@ -503,7 +503,7 @@ int hn_vf_xstats_get_names(struct rte_eth_dev *dev,
 	char tmp[RTE_ETH_XSTATS_NAME_SIZE];
 
 	rte_spinlock_lock(&hv->vf_lock);
-	vf_dev = hv->vf_dev;
+	vf_dev = hn_get_vf_dev(hv);
 	if (vf_dev && vf_dev->dev_ops->xstats_get_names)
 		count = vf_dev->dev_ops->xstats_get_names(vf_dev, names, n);
 	rte_spinlock_unlock(&hv->vf_lock);
@@ -528,7 +528,7 @@ int hn_vf_xstats_get(struct rte_eth_dev *dev,
 	int count = 0;
 
 	rte_spinlock_lock(&hv->vf_lock);
-	vf_dev = hv->vf_dev;
+	vf_dev = hn_get_vf_dev(hv);
 	if (vf_dev && vf_dev->dev_ops->xstats_get)
 		count = vf_dev->dev_ops->xstats_get(vf_dev, xstats, n);
 	rte_spinlock_unlock(&hv->vf_lock);
@@ -542,7 +542,7 @@ void hn_vf_xstats_reset(struct rte_eth_dev *dev)
 	struct rte_eth_dev *vf_dev;
 
 	rte_spinlock_lock(&hv->vf_lock);
-	vf_dev = hv->vf_dev;
+	vf_dev = hn_get_vf_dev(hv);
 	if (vf_dev && vf_dev->dev_ops->xstats_reset)
 		vf_dev->dev_ops->xstats_reset(vf_dev);
 	rte_spinlock_unlock(&hv->vf_lock);
-- 
2.20.1

[dpdk-dev] [PATCH 3/7] bus/vmbus: fix check for mmap failure

[Stephen Hemminger]

From: Stephen Hemminger <sthemmin@microsoft.com>

The code was testing the result of mmap incorrectly.
I.e the test that a local pointer is not MAP_FAILED would
always succeed and therefore hid any potential problems.

Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/bus/vmbus/linux/vmbus_uio.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/vmbus/linux/vmbus_uio.c b/drivers/bus/vmbus/linux/vmbus_uio.c
index 09f7efdca286..8c6bc52fd475 100644
--- a/drivers/bus/vmbus/linux/vmbus_uio.c
+++ b/drivers/bus/vmbus/linux/vmbus_uio.c
@@ -202,6 +202,7 @@ static int vmbus_uio_map_subchan(const struct rte_vmbus_device *dev,
 	char ring_path[PATH_MAX];
 	size_t file_size;
 	struct stat sb;
+	void *mapaddr;
 	int fd;
 
 	snprintf(ring_path, sizeof(ring_path),
@@ -232,14 +233,16 @@ static int vmbus_uio_map_subchan(const struct rte_vmbus_device *dev,
 		return -EINVAL;
 	}
 
-	*ring_size = file_size / 2;
-	*ring_buf = vmbus_map_resource(vmbus_map_addr, fd,
-				       0, sb.st_size, 0);
+	mapaddr = vmbus_map_resource(vmbus_map_addr, fd,
+				     0, file_size, 0);
 	close(fd);
 
-	if (ring_buf == MAP_FAILED)
+	if (mapaddr == MAP_FAILED)
 		return -EIO;
 
+	*ring_size = file_size / 2;
+	*ring_buf = mapaddr;
+
 	vmbus_map_addr = RTE_PTR_ADD(ring_buf, file_size);
 	return 0;
 }
-- 
2.20.1

[dpdk-dev] [PATCH 4/7] bus/vmbus: stop mapping if empty resource found

[Stephen Hemminger]

From: Stephen Hemminger <sthemmin@microsoft.com>

If vmbus is run on older kernel (without all the uio mappings),
then the bus driver should stop when it hits the missing mappings
rather than recording the empty values.

Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/bus/vmbus/vmbus_common_uio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/vmbus/vmbus_common_uio.c b/drivers/bus/vmbus/vmbus_common_uio.c
index 46e233d9fac3..a6545b758e36 100644
--- a/drivers/bus/vmbus/vmbus_common_uio.c
+++ b/drivers/bus/vmbus/vmbus_common_uio.c
@@ -98,9 +98,9 @@ vmbus_uio_map_primary(struct rte_vmbus_device *dev)
 
 	/* Map the resources */
 	for (i = 0; i < VMBUS_MAX_RESOURCE; i++) {
-		/* skip empty BAR */
+		/* stop at empty BAR */
 		if (dev->resource[i].len == 0)
-			continue;
+			break;
 
 		ret = vmbus_uio_map_resource_by_index(dev, i, uio_res, 0);
 		if (ret)
-- 
2.20.1

[dpdk-dev] [PATCH 5/7] bus/vmbus: map ring in secondary

[Stephen Hemminger]

From: Stephen Hemminger <sthemmin@microsoft.com>

Need to remember primary channel in secondary process.
Then use it to iterate over subchannels in secondary
process mapping setup.

Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/bus/vmbus/linux/vmbus_uio.c  | 43 ++++++++++++++++++++++++++++
 drivers/bus/vmbus/private.h          |  3 ++
 drivers/bus/vmbus/vmbus_channel.c    | 20 +++++++++++--
 drivers/bus/vmbus/vmbus_common_uio.c | 15 ++++++++++
 4 files changed, 78 insertions(+), 3 deletions(-)

diff --git a/drivers/bus/vmbus/linux/vmbus_uio.c b/drivers/bus/vmbus/linux/vmbus_uio.c
index 8c6bc52fd475..fb60ee126d9b 100644
--- a/drivers/bus/vmbus/linux/vmbus_uio.c
+++ b/drivers/bus/vmbus/linux/vmbus_uio.c
@@ -247,6 +247,49 @@ static int vmbus_uio_map_subchan(const struct rte_vmbus_device *dev,
 	return 0;
 }
 
+int
+vmbus_uio_map_secondary_subchan(const struct rte_vmbus_device *dev,
+				const struct vmbus_channel *chan)
+{
+	const struct vmbus_br *br = &chan->txbr;
+	char ring_path[PATH_MAX];
+	void *mapaddr, *ring_buf;
+	uint32_t ring_size;
+	int fd;
+
+	snprintf(ring_path, sizeof(ring_path),
+		 "%s/%s/channels/%u/ring",
+		 SYSFS_VMBUS_DEVICES, dev->device.name,
+		 chan->relid);
+
+	ring_buf = br->vbr;
+	ring_size = br->dsize + sizeof(struct vmbus_bufring);
+	VMBUS_LOG(INFO, "secondary ring_buf %p size %u",
+		  ring_buf, ring_size);
+
+	fd = open(ring_path, O_RDWR);
+	if (fd < 0) {
+		VMBUS_LOG(ERR, "Cannot open %s: %s",
+			  ring_path, strerror(errno));
+		return -errno;
+	}
+
+	mapaddr = vmbus_map_resource(ring_buf, fd, 0, 2 * ring_size, 0);
+	close(fd);
+
+	if (mapaddr == ring_buf)
+		return 0;
+
+	if (mapaddr == MAP_FAILED)
+		VMBUS_LOG(ERR,
+			  "mmap subchan %u in secondary failed", chan->relid);
+	else
+		VMBUS_LOG(ERR,
+			  "mmap subchan %u in secondary address mismatch",
+			  chan->relid);
+	return -1;
+}
+
 int vmbus_uio_map_rings(struct vmbus_channel *chan)
 {
 	const struct rte_vmbus_device *dev = chan->device;
diff --git a/drivers/bus/vmbus/private.h b/drivers/bus/vmbus/private.h
index 211127dd8db5..f19b14e4a657 100644
--- a/drivers/bus/vmbus/private.h
+++ b/drivers/bus/vmbus/private.h
@@ -45,6 +45,7 @@ struct mapped_vmbus_resource {
 
 	rte_uuid_t id;
 	int nb_maps;
+	struct vmbus_channel *primary;
 	struct vmbus_map maps[VMBUS_MAX_RESOURCE];
 	char path[PATH_MAX];
 };
@@ -107,6 +108,8 @@ bool vmbus_uio_subchannels_supported(const struct rte_vmbus_device *dev,
 int vmbus_uio_get_subchan(struct vmbus_channel *primary,
 			  struct vmbus_channel **subchan);
 int vmbus_uio_map_rings(struct vmbus_channel *chan);
+int vmbus_uio_map_secondary_subchan(const struct rte_vmbus_device *dev,
+				    const struct vmbus_channel *chan);
 
 void vmbus_br_setup(struct vmbus_br *br, void *buf, unsigned int blen);
 
diff --git a/drivers/bus/vmbus/vmbus_channel.c b/drivers/bus/vmbus/vmbus_channel.c
index bd14c0662b46..46b3ba3f9f9e 100644
--- a/drivers/bus/vmbus/vmbus_channel.c
+++ b/drivers/bus/vmbus/vmbus_channel.c
@@ -352,12 +352,21 @@ int vmbus_chan_create(const struct rte_vmbus_device *device,
 int rte_vmbus_chan_open(struct rte_vmbus_device *device,
 			struct vmbus_channel **new_chan)
 {
+	struct mapped_vmbus_resource *uio_res;
 	int err;
 
+	uio_res = vmbus_uio_find_resource(device);
+	if (!uio_res) {
+		VMBUS_LOG(ERR, "can't find uio resource");
+		return -EINVAL;
+	}
+
 	err = vmbus_chan_create(device, device->relid, 0,
 				device->monitor_id, new_chan);
-	if (!err)
+	if (!err) {
 		device->primary = *new_chan;
+		uio_res->primary = *new_chan;
+	}
 
 	return err;
 }
@@ -396,11 +405,16 @@ void rte_vmbus_chan_close(struct vmbus_channel *chan)
 	const struct rte_vmbus_device *device = chan->device;
 	struct vmbus_channel *primary = device->primary;
 
-	if (chan != primary)
+	/*
+	 * intentionally leak primary channel because
+	 * secondary may still reference it
+	 */
+	if (chan != primary) {
 		STAILQ_REMOVE(&primary->subchannel_list, chan,
 			      vmbus_channel, next);
+		rte_free(chan);
+	}
 
-	rte_free(chan);
 }
 
 static void vmbus_dump_ring(FILE *f, const char *id, const struct vmbus_br *br)
diff --git a/drivers/bus/vmbus/vmbus_common_uio.c b/drivers/bus/vmbus/vmbus_common_uio.c
index a6545b758e36..9947f82ab194 100644
--- a/drivers/bus/vmbus/vmbus_common_uio.c
+++ b/drivers/bus/vmbus/vmbus_common_uio.c
@@ -27,6 +27,7 @@ static int
 vmbus_uio_map_secondary(struct rte_vmbus_device *dev)
 {
 	int fd, i;
+	struct vmbus_channel *chan;
 	struct mapped_vmbus_resource *uio_res;
 	struct mapped_vmbus_res_list *uio_res_list
 		= RTE_TAILQ_CAST(vmbus_tailq.head, mapped_vmbus_res_list);
@@ -76,6 +77,20 @@ vmbus_uio_map_secondary(struct rte_vmbus_device *dev)
 
 		/* fd is not needed in slave process, close it */
 		close(fd);
+
+		dev->primary = uio_res->primary;
+		if (!dev->primary) {
+			VMBUS_LOG(ERR, "missing primary channel");
+			return -1;
+		}
+
+		STAILQ_FOREACH(chan, &dev->primary->subchannel_list, next) {
+			if (vmbus_uio_map_secondary_subchan(dev, chan) != 0) {
+				VMBUS_LOG(ERR, "cannot map secondary subchan");
+				return -1;
+			}
+		}
+
 		return 0;
 	}
 
-- 
2.20.1

[dpdk-dev] [PATCH 6/7] bus/vmbus: refactor secondary mapping

[Stephen Hemminger]

From: Stephen Hemminger <sthemmin@microsoft.com>

The secondary mapping function was duplicating the code
used to search the uio_resource list.

Skip the unwinding since map failure already makes device
unusable.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/bus/vmbus/vmbus_common_uio.c | 132 ++++++++++++---------------
 1 file changed, 58 insertions(+), 74 deletions(-)

diff --git a/drivers/bus/vmbus/vmbus_common_uio.c b/drivers/bus/vmbus/vmbus_common_uio.c
index 9947f82ab194..1aa5cb2e4b92 100644
--- a/drivers/bus/vmbus/vmbus_common_uio.c
+++ b/drivers/bus/vmbus/vmbus_common_uio.c
@@ -23,79 +23,82 @@ static struct rte_tailq_elem vmbus_tailq = {
 };
 EAL_REGISTER_TAILQ(vmbus_tailq)
 
-static int
-vmbus_uio_map_secondary(struct rte_vmbus_device *dev)
+struct mapped_vmbus_resource *
+vmbus_uio_find_resource(const struct rte_vmbus_device *dev)
 {
-	int fd, i;
-	struct vmbus_channel *chan;
 	struct mapped_vmbus_resource *uio_res;
-	struct mapped_vmbus_res_list *uio_res_list
-		= RTE_TAILQ_CAST(vmbus_tailq.head, mapped_vmbus_res_list);
+	struct mapped_vmbus_res_list *uio_res_list =
+			RTE_TAILQ_CAST(vmbus_tailq.head, mapped_vmbus_res_list);
+
+	if (dev == NULL)
+		return NULL;
 
 	TAILQ_FOREACH(uio_res, uio_res_list, next) {
+		if (rte_uuid_compare(uio_res->id, dev->device_id) == 0)
+			return uio_res;
+	}
+	return NULL;
+}
 
-		/* skip this element if it doesn't match our UUID */
-		if (rte_uuid_compare(uio_res->id, dev->device_id) != 0)
-			continue;
+static int
+vmbus_uio_map_secondary(struct rte_vmbus_device *dev)
+{
+	struct mapped_vmbus_resource *uio_res;
+	struct vmbus_channel *chan;
+	int fd, i;
 
-		/* open /dev/uioX */
-		fd = open(uio_res->path, O_RDWR);
-		if (fd < 0) {
-			VMBUS_LOG(ERR, "Cannot open %s: %s",
-				  uio_res->path, strerror(errno));
-			return -1;
-		}
+	uio_res = vmbus_uio_find_resource(dev);
+	if (!uio_res) {
+		VMBUS_LOG(ERR,  "Cannot find resource for device");
+		return -1;
+	}
 
-		for (i = 0; i != uio_res->nb_maps; i++) {
-			void *mapaddr;
-			off_t offset = i * PAGE_SIZE;
+	/* open /dev/uioX */
+	fd = open(uio_res->path, O_RDWR);
+	if (fd < 0) {
+		VMBUS_LOG(ERR, "Cannot open %s: %s",
+			  uio_res->path, strerror(errno));
+		return -1;
+	}
 
-			mapaddr = vmbus_map_resource(uio_res->maps[i].addr,
-						     fd, offset,
-						     uio_res->maps[i].size, 0);
+	for (i = 0; i != uio_res->nb_maps; i++) {
+		void *mapaddr;
+		off_t offset = i * PAGE_SIZE;
 
-			if (mapaddr == uio_res->maps[i].addr)
-				continue;
+		mapaddr = vmbus_map_resource(uio_res->maps[i].addr,
+					     fd, offset,
+					     uio_res->maps[i].size, 0);
 
-			VMBUS_LOG(ERR,
-				  "Cannot mmap device resource file %s to address: %p",
-				  uio_res->path, uio_res->maps[i].addr);
+		if (mapaddr == uio_res->maps[i].addr)
+			continue;	/* successful map */
 
-			if (mapaddr != MAP_FAILED)
-				/* unmap addr wrongly mapped */
-				vmbus_unmap_resource(mapaddr,
-						     (size_t)uio_res->maps[i].size);
+		if (mapaddr == MAP_FAILED)
+			VMBUS_LOG(ERR,
+				  "mmap resource %d in secondary failed", i);
+		else
+			VMBUS_LOG(ERR,
+				  "mmap resource %d address mismatch", i);
 
-			/* unmap addrs correctly mapped */
-			while (--i >= 0)
-				vmbus_unmap_resource(uio_res->maps[i].addr,
-						     (size_t)uio_res->maps[i].size);
+		close(fd);
+		return -1;
+	}
 
-			close(fd);
-			return -1;
-		}
+	/* fd is not needed in slave process, close it */
+	close(fd);
 
-		/* fd is not needed in slave process, close it */
-		close(fd);
+	dev->primary = uio_res->primary;
+	if (!dev->primary) {
+		VMBUS_LOG(ERR, "missing primary channel");
+		return -1;
+	}
 
-		dev->primary = uio_res->primary;
-		if (!dev->primary) {
-			VMBUS_LOG(ERR, "missing primary channel");
+	STAILQ_FOREACH(chan, &dev->primary->subchannel_list, next) {
+		if (vmbus_uio_map_secondary_subchan(dev, chan) != 0) {
+			VMBUS_LOG(ERR, "cannot map secondary subchan");
 			return -1;
 		}
-
-		STAILQ_FOREACH(chan, &dev->primary->subchannel_list, next) {
-			if (vmbus_uio_map_secondary_subchan(dev, chan) != 0) {
-				VMBUS_LOG(ERR, "cannot map secondary subchan");
-				return -1;
-			}
-		}
-
-		return 0;
 	}
-
-	VMBUS_LOG(ERR,  "Cannot find resource for device");
-	return 1;
+	return 0;
 }
 
 static int
@@ -136,25 +139,6 @@ vmbus_uio_map_primary(struct rte_vmbus_device *dev)
 	return -1;
 }
 
-
-struct mapped_vmbus_resource *
-vmbus_uio_find_resource(const struct rte_vmbus_device *dev)
-{
-	struct mapped_vmbus_resource *uio_res;
-	struct mapped_vmbus_res_list *uio_res_list =
-			RTE_TAILQ_CAST(vmbus_tailq.head, mapped_vmbus_res_list);
-
-	if (dev == NULL)
-		return NULL;
-
-	TAILQ_FOREACH(uio_res, uio_res_list, next) {
-		/* skip this element if it doesn't match our VMBUS address */
-		if (rte_uuid_compare(uio_res->id, dev->device_id) == 0)
-			return uio_res;
-	}
-	return NULL;
-}
-
 /* map the VMBUS resource of a VMBUS device in virtual memory */
 int
 vmbus_uio_map_resource(struct rte_vmbus_device *dev)
-- 
2.20.1

[dpdk-dev] [PATCH 7/7] net/netvsc: remove unnecessary format of ether address

[Stephen Hemminger]

From: Stephen Hemminger <sthemmin@microsoft.com>

The ethernet address was being converted to a string but
the code using that is no longer present.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/netvsc/hn_vf.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/netvsc/hn_vf.c b/drivers/net/netvsc/hn_vf.c
index de278eb7b403..f68e1f9c5473 100644
--- a/drivers/net/netvsc/hn_vf.c
+++ b/drivers/net/netvsc/hn_vf.c
@@ -33,10 +33,8 @@
 static int hn_vf_match(const struct rte_eth_dev *dev)
 {
 	const struct ether_addr *mac = dev->data->mac_addrs;
-	char buf[32];
 	int i;
 
-	ether_format_addr(buf, sizeof(buf), mac);
 	RTE_ETH_FOREACH_DEV(i) {
 		const struct rte_eth_dev *vf_dev = &rte_eth_devices[i];
 		const struct ether_addr *vf_mac = vf_dev->data->mac_addrs;
@@ -44,7 +42,6 @@ static int hn_vf_match(const struct rte_eth_dev *dev)
 		if (vf_dev == dev)
 			continue;
 
-		ether_format_addr(buf, sizeof(buf), vf_mac);
 		if (is_same_ether_addr(mac, vf_mac))
 			return i;
 	}
-- 
2.20.1

Re: [dpdk-dev] [dpdk-stable] [PATCH 0/7] vmbus/netvsc: fix multi-process support

[Thomas Monjalon]

08/02/2019 04:44, Stephen Hemminger:
> These fix the primary/secondary process model support in the
> vmbus and netvsc PMD.  They do NOT fix the failsafe PMD.
> 
> The last two are not bug fixes but address unnecessary
> code found while debugging the mp support.
> 
> Stephen Hemminger (7):
>   bus/vmbus: fix secondary process setup
>   net/netvsc: fix VF support with secondary process
>   bus/vmbus: fix check for mmap failure
>   bus/vmbus: stop mapping if empty resource found
>   bus/vmbus: map ring in secondary
>   bus/vmbus: refactor secondary mapping
>   net/netvsc: remove unnecessary format of ether address

Adding Cc: stable@dpdk.org for fixes.
If you do not want to backport, please check with LTS maintainers.

Applied, thanks

Re: [dpdk-dev] [dpdk-stable] [PATCH 0/7] vmbus/netvsc: fix multi-process support

[Thomas Monjalon]

08/02/2019 04:44, Stephen Hemminger:
> These fix the primary/secondary process model support in the
> vmbus and netvsc PMD.  They do NOT fix the failsafe PMD.
> 
> The last two are not bug fixes but address unnecessary
> code found while debugging the mp support.
> 
> Stephen Hemminger (7):
>   bus/vmbus: fix secondary process setup
>   net/netvsc: fix VF support with secondary process
>   bus/vmbus: fix check for mmap failure
>   bus/vmbus: stop mapping if empty resource found
>   bus/vmbus: map ring in secondary
>   bus/vmbus: refactor secondary mapping
>   net/netvsc: remove unnecessary format of ether address

Adding Cc: stable@dpdk.org for fixes.
If you do not want to backport, please check with LTS maintainers.

Applied, thanks