[dpdk-dev] [PATCH] kni: Fix request overwritten

[Elad Nachman]

Fix lack of multiple KNI requests handling support by introducing a 
rotating ring mechanism for the sync buffer.

Prevent kni_net_change_rx_flags() from calling kni_net_process_request()
with a malformed request.

Bugzilla ID: 809

Signed-off-by: Elad Nachman <eladv6 at gmail.com>
---
 kernel/linux/kni/kni_dev.h  |  2 ++
 kernel/linux/kni/kni_misc.c |  2 ++
 kernel/linux/kni/kni_net.c  | 25 +++++++++++++++++--------
 lib/kni/rte_kni.c           | 14 +++++++-------
 lib/kni/rte_kni_common.h    |  1 +
 5 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index c15da311ba..b9e8b3d10d 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -74,6 +74,8 @@ struct kni_dev {
 
 	void *sync_kva;
 	void *sync_va;
+	unsigned int sync_ring_size;
+	atomic_t sync_ring_idx;
 
 	void *mbuf_kva;
 	void *mbuf_va;
diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index 2b464c4381..f3cee97818 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -345,6 +345,8 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 
 	kni->net_dev = net_dev;
 	kni->core_id = dev_info.core_id;
+	kni->sync_ring_size = dev_info.sync_ring_size;
+	kni->sync_ring_idx.counter = 0;
 	strncpy(kni->name, dev_info.name, RTE_KNI_NAMESIZE);
 
 	/* Translate user space info into kernel space info */
diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index 611719b5ee..dc066cdd98 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -110,6 +110,8 @@ kni_net_process_request(struct net_device *dev, struct rte_kni_request *req)
 	void *resp_va;
 	uint32_t num;
 	int ret_val;
+	unsigned int idx;
+	char *k_reqptr, *v_reqptr;
 
 	ASSERT_RTNL();
 
@@ -122,10 +124,17 @@ kni_net_process_request(struct net_device *dev, struct rte_kni_request *req)
 	}
 
 	mutex_lock(&kni->sync_lock);
-
+	idx = atomic_read(&kni->sync_ring_idx);
+	atomic_cmpxchg(&kni->sync_ring_idx, idx,
+		(idx + 1) % kni->sync_ring_size);
 	/* Construct data */
-	memcpy(kni->sync_kva, req, sizeof(struct rte_kni_request));
-	num = kni_fifo_put(kni->req_q, &kni->sync_va, 1);
+	k_reqptr = (char *)((uintptr_t)kni->sync_kva +
+			sizeof(struct rte_kni_request) * idx);
+	v_reqptr = (char *)((uintptr_t)kni->sync_va +
+			sizeof(struct rte_kni_request) * idx);
+
+	memcpy(k_reqptr, req, sizeof(struct rte_kni_request));
+	num = kni_fifo_put(kni->req_q, (void **)&v_reqptr, 1);
 	if (num < 1) {
 		pr_err("Cannot send to req_q\n");
 		ret = -EBUSY;
@@ -147,14 +156,14 @@ kni_net_process_request(struct net_device *dev, struct rte_kni_request *req)
 		goto fail;
 	}
 	num = kni_fifo_get(kni->resp_q, (void **)&resp_va, 1);
-	if (num != 1 || resp_va != kni->sync_va) {
+	if (num != 1) {
 		/* This should never happen */
 		pr_err("No data in resp_q\n");
 		ret = -ENODATA;
 		goto fail;
 	}
 
-	memcpy(req, kni->sync_kva, sizeof(struct rte_kni_request));
+	memcpy(req, k_reqptr, sizeof(struct rte_kni_request));
 async:
 	ret = 0;
 
@@ -681,13 +690,12 @@ kni_net_change_mtu(struct net_device *dev, int new_mtu)
 static void
 kni_net_change_rx_flags(struct net_device *netdev, int flags)
 {
-	struct rte_kni_request req;
+	struct rte_kni_request req = { 0 };
 
 	memset(&req, 0, sizeof(req));
 
 	if (flags & IFF_ALLMULTI) {
 		req.req_id = RTE_KNI_REQ_CHANGE_ALLMULTI;
-
 		if (netdev->flags & IFF_ALLMULTI)
 			req.allmulti = 1;
 		else
@@ -703,7 +711,8 @@ kni_net_change_rx_flags(struct net_device *netdev, int flags)
 			req.promiscusity = 0;
 	}
 
-	kni_net_process_request(netdev, &req);
+	if (req.req_id)
+		kni_net_process_request(netdev, &req);
 }
 
 /*
diff --git a/lib/kni/rte_kni.c b/lib/kni/rte_kni.c
index d3e236005e..e921d41ce8 100644
--- a/lib/kni/rte_kni.c
+++ b/lib/kni/rte_kni.c
@@ -31,6 +31,9 @@
 #define KNI_FIFO_COUNT_MAX     1024
 #define KNI_FIFO_SIZE          (KNI_FIFO_COUNT_MAX * sizeof(void *) + \
 					sizeof(struct rte_kni_fifo))
+#define KNI_REQS_SIZE          (KNI_FIFO_COUNT_MAX * \
+					sizeof(struct rte_kni_request) + \
+					sizeof(struct rte_kni_fifo))
 
 #define KNI_REQUEST_MBUF_NUM_MAX      32
 
@@ -175,8 +178,8 @@ kni_reserve_mz(struct rte_kni *kni)
 	KNI_MEM_CHECK(kni->m_resp_q == NULL, resp_q_fail);
 
 	snprintf(mz_name, RTE_MEMZONE_NAMESIZE, KNI_SYNC_ADDR_MZ_NAME_FMT, kni->name);
-	kni->m_sync_addr = rte_memzone_reserve(mz_name, KNI_FIFO_SIZE, SOCKET_ID_ANY,
-			RTE_MEMZONE_IOVA_CONTIG);
+	kni->m_sync_addr = rte_memzone_reserve(mz_name, KNI_REQS_SIZE,
+			SOCKET_ID_ANY, RTE_MEMZONE_IOVA_CONTIG);
 	KNI_MEM_CHECK(kni->m_sync_addr == NULL, sync_addr_fail);
 
 	return 0;
@@ -307,6 +310,7 @@ rte_kni_alloc(struct rte_mempool *pktmbuf_pool,
 	kni->sync_addr = kni->m_sync_addr->addr;
 	dev_info.sync_va = kni->m_sync_addr->addr;
 	dev_info.sync_phys = kni->m_sync_addr->iova;
+	dev_info.sync_ring_size = KNI_FIFO_COUNT_MAX;
 
 	kni->pktmbuf_pool = pktmbuf_pool;
 	kni->group_id = conf->group_id;
@@ -544,11 +548,7 @@ rte_kni_handle_request(struct rte_kni *kni)
 	if (ret != 1)
 		return 0; /* It is OK of can not getting the request mbuf */
 
-	if (req != kni->sync_addr) {
-		RTE_LOG(ERR, KNI, "Wrong req pointer %p\n", req);
-		return -1;
-	}
-
+	printf("%s: req %p id %u\n", __func__, req, req->req_id);
 	/* Analyze the request and call the relevant actions for it */
 	switch (req->req_id) {
 	case RTE_KNI_REQ_CHANGE_MTU: /* Change MTU */
diff --git a/lib/kni/rte_kni_common.h b/lib/kni/rte_kni_common.h
index b547ea5501..a78b9bafa0 100644
--- a/lib/kni/rte_kni_common.h
+++ b/lib/kni/rte_kni_common.h
@@ -110,6 +110,7 @@ struct rte_kni_device_info {
 	phys_addr_t resp_phys;
 	phys_addr_t sync_phys;
 	void * sync_va;
+	unsigned int sync_ring_size;
 
 	/* mbuf mempool */
 	void * mbuf_va;
-- 
2.17.1