[PATCH] net/iavf: fix error of virtchnl command

[PATCH] net/iavf: fix error of virtchnl command

[Yiding Zhou]

When the device is bonded, bond pmd will register callback for LSC event.
This callback will execute some virtchnl commands in eal-intr-thread to
reinitialize the device with interrupts disabled. In this case, responses
to all commands not be received.

This commit starts a thread to handle all events to fix this issue.

Fixes: 48de41ca11f0 ("net/avf: enable link status update")
CC: stable@dpdk.org

Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
---
 drivers/net/iavf/iavf.h        |   2 +
 drivers/net/iavf/iavf_ethdev.c |   5 ++
 drivers/net/iavf/iavf_vchnl.c  | 130 ++++++++++++++++++++++++++++++++-
 3 files changed, 136 insertions(+), 1 deletion(-)

diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index 025ab3ff60..7c215e1797 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -417,6 +417,8 @@ _atomic_set_async_response_cmd(struct iavf_info *vf, enum virtchnl_ops ops)
 }
 int iavf_check_api_version(struct iavf_adapter *adapter);
 int iavf_get_vf_resource(struct iavf_adapter *adapter);
+void iavf_dev_event_handler_fini(void);
+int iavf_dev_event_handler_init(void);
 void iavf_handle_virtchnl_msg(struct rte_eth_dev *dev);
 int iavf_enable_vlan_strip(struct iavf_adapter *adapter);
 int iavf_disable_vlan_strip(struct iavf_adapter *adapter);
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 506fcff6e3..a584b8918d 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -2629,6 +2629,9 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_ether_addr_copy((struct rte_ether_addr *)hw->mac.addr,
 			&eth_dev->data->mac_addrs[0]);
 
+	if (iavf_dev_event_handler_init())
+		goto init_vf_err;
+
 	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) {
 		/* register callback func to eal lib */
 		rte_intr_callback_register(pci_dev->intr_handle,
@@ -2783,6 +2786,8 @@ iavf_dev_uninit(struct rte_eth_dev *dev)
 
 	iavf_dev_close(dev);
 
+	iavf_dev_event_handler_fini();
+
 	return 0;
 }
 
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 21bd1e2193..5f3dd8779e 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -2,6 +2,8 @@
  * Copyright(c) 2017 Intel Corporation
  */
 
+#include <unistd.h>
+#include <fcntl.h>
 #include <stdio.h>
 #include <errno.h>
 #include <stdint.h>
@@ -27,6 +29,132 @@
 #define MAX_TRY_TIMES 2000
 #define ASQ_DELAY_MS  1
 
+struct iavf_arq_event_element {
+	TAILQ_ENTRY(iavf_arq_event_element) next;
+	struct rte_eth_dev *dev;
+	enum rte_eth_event_type event;
+	void *param;
+};
+
+struct iavf_event_handler {
+	rte_atomic32_t ndev;
+	pthread_t tid;
+	int fd[2];
+	pthread_mutex_t lock;
+	TAILQ_HEAD(event_lsit, iavf_arq_event_element) pending;
+};
+
+static struct iavf_event_handler event_handler = {
+	.fd = {-1, -1},
+};
+
+#ifndef TAILQ_FOREACH_SAFE
+#define TAILQ_FOREACH_SAFE(var, head, field, tvar) \
+	for ((var) = TAILQ_FIRST((head)); \
+		(var) && ((tvar) = TAILQ_NEXT((var), field), 1); \
+	(var) = (tvar))
+#endif
+
+static void *
+iavf_dev_event_handle(void *param __rte_unused)
+{
+	struct iavf_event_handler *handler = &event_handler;
+	TAILQ_HEAD(event_list, iavf_arq_event_element) pending;
+
+	while (true) {
+		char unused;
+		int nr = read(handler->fd[0], &unused, 1);
+		if (nr <= 0)
+			break;
+
+		TAILQ_INIT(&pending);
+		pthread_mutex_lock(&handler->lock);
+		TAILQ_CONCAT(&pending, &handler->pending, next);
+		pthread_mutex_unlock(&handler->lock);
+
+		struct iavf_arq_event_element *pos, *save_next;
+		TAILQ_FOREACH_SAFE(pos, &pending, next, save_next) {
+			TAILQ_REMOVE(&pending, pos, next);
+			rte_eth_dev_callback_process(pos->dev, pos->event, pos->param);
+			rte_free(pos);
+		}
+	}
+	return NULL;
+}
+
+static void
+iavf_dev_event_post(struct rte_eth_dev *dev,
+		enum rte_eth_event_type event,
+		void *param)
+{
+	struct iavf_event_handler *handler = &event_handler;
+	char notify_byte;
+	struct iavf_arq_event_element *elem = rte_malloc(NULL, sizeof(*elem), 0);
+	if (!elem)
+		return;
+
+	elem->dev = dev;
+	elem->event = event;
+	elem->param = param;
+
+	pthread_mutex_lock(&handler->lock);
+	TAILQ_INSERT_TAIL(&handler->pending, elem, next);
+	pthread_mutex_unlock(&handler->lock);
+
+	write(handler->fd[1], &notify_byte, 1);
+}
+
+int
+iavf_dev_event_handler_init(void)
+{
+	struct iavf_event_handler *handler = &event_handler;
+
+	if (rte_atomic32_add_return(&handler->ndev, 1) != 1)
+		return 0;
+
+	if (pipe(handler->fd)) {
+		rte_atomic32_dec(&handler->ndev);
+		return -1;
+	}
+
+	TAILQ_INIT(&handler->pending);
+	pthread_mutex_init(&handler->lock, NULL);
+
+	int flags = fcntl(handler->fd[1], F_GETFL);
+	fcntl(handler->fd[1], F_SETFL, flags | O_NONBLOCK);
+	if (rte_ctrl_thread_create(&handler->tid, "iavf-event-thread",
+				NULL, iavf_dev_event_handle, NULL)) {
+		rte_atomic32_dec(&handler->ndev);
+		return -1;
+	}
+
+	return 0;
+}
+
+void
+iavf_dev_event_handler_fini(void)
+{
+	struct iavf_event_handler *handler = &event_handler;
+
+	if (!rte_atomic32_dec_and_test(&handler->ndev))
+		return;
+
+	pthread_cancel(handler->tid);
+	close(handler->fd[0]);
+	close(handler->fd[1]);
+	handler->fd[0] = -1;
+	handler->fd[1] = -1;
+
+	pthread_join(handler->tid, NULL);
+	pthread_mutex_destroy(&handler->lock);
+
+	struct iavf_arq_event_element *pos, *save_next;
+	TAILQ_FOREACH_SAFE(pos, &handler->pending, next, save_next) {
+		TAILQ_REMOVE(&handler->pending, pos, next);
+		rte_free(pos);
+	}
+}
+
 static uint32_t
 iavf_convert_link_speed(enum virtchnl_link_speed virt_link_speed)
 {
@@ -293,7 +421,7 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,
 			vf->link_speed = iavf_convert_link_speed(speed);
 		}
 		iavf_dev_link_update(dev, 0);
-		rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
+		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 		break;
 	case VIRTCHNL_EVENT_PF_DRIVER_CLOSE:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_PF_DRIVER_CLOSE event");
-- 
2.34.1

[PATCH v2] net/iavf: fix error of virtchnl command

[Yiding Zhou]

When the device is bonded, bond pmd will register callback for LSC event.
This callback will execute some virtchnl commands in eal-intr-thread to
reinitialize the device with interrupts disabled. In this case, responses
to all commands not be received.

This commit starts a thread to handle all events to fix this issue.

Fixes: 48de41ca11f0 ("net/avf: enable link status update")
CC: stable@dpdk.org

Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
---
 drivers/net/iavf/iavf.h        |   2 +
 drivers/net/iavf/iavf_ethdev.c |   5 ++
 drivers/net/iavf/iavf_vchnl.c  | 131 ++++++++++++++++++++++++++++++++-
 3 files changed, 137 insertions(+), 1 deletion(-)

diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index 025ab3ff60..7c215e1797 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -417,6 +417,8 @@ _atomic_set_async_response_cmd(struct iavf_info *vf, enum virtchnl_ops ops)
 }
 int iavf_check_api_version(struct iavf_adapter *adapter);
 int iavf_get_vf_resource(struct iavf_adapter *adapter);
+void iavf_dev_event_handler_fini(void);
+int iavf_dev_event_handler_init(void);
 void iavf_handle_virtchnl_msg(struct rte_eth_dev *dev);
 int iavf_enable_vlan_strip(struct iavf_adapter *adapter);
 int iavf_disable_vlan_strip(struct iavf_adapter *adapter);
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 506fcff6e3..a584b8918d 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -2629,6 +2629,9 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_ether_addr_copy((struct rte_ether_addr *)hw->mac.addr,
 			&eth_dev->data->mac_addrs[0]);
 
+	if (iavf_dev_event_handler_init())
+		goto init_vf_err;
+
 	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) {
 		/* register callback func to eal lib */
 		rte_intr_callback_register(pci_dev->intr_handle,
@@ -2783,6 +2786,8 @@ iavf_dev_uninit(struct rte_eth_dev *dev)
 
 	iavf_dev_close(dev);
 
+	iavf_dev_event_handler_fini();
+
 	return 0;
 }
 
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 21bd1e2193..e357211c68 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -2,6 +2,8 @@
  * Copyright(c) 2017 Intel Corporation
  */
 
+#include <unistd.h>
+#include <fcntl.h>
 #include <stdio.h>
 #include <errno.h>
 #include <stdint.h>
@@ -27,6 +29,133 @@
 #define MAX_TRY_TIMES 2000
 #define ASQ_DELAY_MS  1
 
+struct iavf_arq_event_element {
+	TAILQ_ENTRY(iavf_arq_event_element) next;
+	struct rte_eth_dev *dev;
+	enum rte_eth_event_type event;
+	void *param;
+};
+
+struct iavf_event_handler {
+	rte_atomic32_t ndev;
+	pthread_t tid;
+	int fd[2];
+	pthread_mutex_t lock;
+	TAILQ_HEAD(event_lsit, iavf_arq_event_element) pending;
+};
+
+static struct iavf_event_handler event_handler = {
+	.fd = {-1, -1},
+};
+
+#ifndef TAILQ_FOREACH_SAFE
+#define TAILQ_FOREACH_SAFE(var, head, field, tvar) \
+	for ((var) = TAILQ_FIRST((head)); \
+		(var) && ((tvar) = TAILQ_NEXT((var), field), 1); \
+	(var) = (tvar))
+#endif
+
+static void *
+iavf_dev_event_handle(void *param __rte_unused)
+{
+	struct iavf_event_handler *handler = &event_handler;
+	TAILQ_HEAD(event_list, iavf_arq_event_element) pending;
+
+	while (true) {
+		char unused[8];
+		ssize_t nr = read(handler->fd[0], &unused, sizeof(unused));
+		if (nr <= 0)
+			break;
+
+		TAILQ_INIT(&pending);
+		pthread_mutex_lock(&handler->lock);
+		TAILQ_CONCAT(&pending, &handler->pending, next);
+		pthread_mutex_unlock(&handler->lock);
+
+		struct iavf_arq_event_element *pos, *save_next;
+		TAILQ_FOREACH_SAFE(pos, &pending, next, save_next) {
+			TAILQ_REMOVE(&pending, pos, next);
+			rte_eth_dev_callback_process(pos->dev, pos->event, pos->param);
+			rte_free(pos);
+		}
+	}
+	return NULL;
+}
+
+static void
+iavf_dev_event_post(struct rte_eth_dev *dev,
+		enum rte_eth_event_type event,
+		void *param)
+{
+	struct iavf_event_handler *handler = &event_handler;
+	char notify_byte;
+	struct iavf_arq_event_element *elem = rte_malloc(NULL, sizeof(*elem), 0);
+	if (!elem)
+		return;
+
+	elem->dev = dev;
+	elem->event = event;
+	elem->param = param;
+
+	pthread_mutex_lock(&handler->lock);
+	TAILQ_INSERT_TAIL(&handler->pending, elem, next);
+	pthread_mutex_unlock(&handler->lock);
+
+	ssize_t nw = write(handler->fd[1], &notify_byte, 1);
+	RTE_SET_USED(nw);
+}
+
+int
+iavf_dev_event_handler_init(void)
+{
+	struct iavf_event_handler *handler = &event_handler;
+
+	if (rte_atomic32_add_return(&handler->ndev, 1) != 1)
+		return 0;
+
+	if (pipe(handler->fd)) {
+		rte_atomic32_dec(&handler->ndev);
+		return -1;
+	}
+
+	TAILQ_INIT(&handler->pending);
+	pthread_mutex_init(&handler->lock, NULL);
+
+	int flags = fcntl(handler->fd[1], F_GETFL);
+	fcntl(handler->fd[1], F_SETFL, flags | O_NONBLOCK);
+	if (rte_ctrl_thread_create(&handler->tid, "iavf-event-thread",
+				NULL, iavf_dev_event_handle, NULL)) {
+		rte_atomic32_dec(&handler->ndev);
+		return -1;
+	}
+
+	return 0;
+}
+
+void
+iavf_dev_event_handler_fini(void)
+{
+	struct iavf_event_handler *handler = &event_handler;
+
+	if (!rte_atomic32_dec_and_test(&handler->ndev))
+		return;
+
+	pthread_cancel(handler->tid);
+	close(handler->fd[0]);
+	close(handler->fd[1]);
+	handler->fd[0] = -1;
+	handler->fd[1] = -1;
+
+	pthread_join(handler->tid, NULL);
+	pthread_mutex_destroy(&handler->lock);
+
+	struct iavf_arq_event_element *pos, *save_next;
+	TAILQ_FOREACH_SAFE(pos, &handler->pending, next, save_next) {
+		TAILQ_REMOVE(&handler->pending, pos, next);
+		rte_free(pos);
+	}
+}
+
 static uint32_t
 iavf_convert_link_speed(enum virtchnl_link_speed virt_link_speed)
 {
@@ -293,7 +422,7 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,
 			vf->link_speed = iavf_convert_link_speed(speed);
 		}
 		iavf_dev_link_update(dev, 0);
-		rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
+		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 		break;
 	case VIRTCHNL_EVENT_PF_DRIVER_CLOSE:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_PF_DRIVER_CLOSE event");
-- 
2.34.1

[PATCH v3] net/iavf: fix error of virtchnl command

[Yiding Zhou]

When the device is bonded, bond pmd will register callback for LSC event.
This callback will execute some virtchnl commands in eal-intr-thread to
reinitialize the device with interrupts disabled. In this case, responses
to all commands not be received.

This commit starts a thread to handle all events to fix this issue.

Fixes: 48de41ca11f0 ("net/avf: enable link status update")
CC: stable@dpdk.org

Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
---
v3: fix CI errors
---
 drivers/net/iavf/iavf.h        |   2 +
 drivers/net/iavf/iavf_ethdev.c |   5 ++
 drivers/net/iavf/iavf_vchnl.c  | 136 ++++++++++++++++++++++++++++++++-
 3 files changed, 142 insertions(+), 1 deletion(-)

diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index 26b858f6f0..1edebab8dc 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -424,6 +424,8 @@ _atomic_set_async_response_cmd(struct iavf_info *vf, enum virtchnl_ops ops)
 }
 int iavf_check_api_version(struct iavf_adapter *adapter);
 int iavf_get_vf_resource(struct iavf_adapter *adapter);
+void iavf_dev_event_handler_fini(void);
+int iavf_dev_event_handler_init(void);
 void iavf_handle_virtchnl_msg(struct rte_eth_dev *dev);
 int iavf_enable_vlan_strip(struct iavf_adapter *adapter);
 int iavf_disable_vlan_strip(struct iavf_adapter *adapter);
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 782be82c7f..633d684804 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -2631,6 +2631,9 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_ether_addr_copy((struct rte_ether_addr *)hw->mac.addr,
 			&eth_dev->data->mac_addrs[0]);
 
+	if (iavf_dev_event_handler_init())
+		goto init_vf_err;
+
 	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) {
 		/* register callback func to eal lib */
 		rte_intr_callback_register(pci_dev->intr_handle,
@@ -2785,6 +2788,8 @@ iavf_dev_uninit(struct rte_eth_dev *dev)
 
 	iavf_dev_close(dev);
 
+	iavf_dev_event_handler_fini();
+
 	return 0;
 }
 
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 0fa2617cd2..35ceccaedd 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -2,6 +2,7 @@
  * Copyright(c) 2017 Intel Corporation
  */
 
+#include <unistd.h>
 #include <stdio.h>
 #include <errno.h>
 #include <stdint.h>
@@ -11,6 +12,7 @@
 #include <inttypes.h>
 #include <rte_byteorder.h>
 #include <rte_common.h>
+#include <rte_os_shim.h>
 
 #include <rte_debug.h>
 #include <rte_alarm.h>
@@ -27,6 +29,138 @@
 #define MAX_TRY_TIMES 2000
 #define ASQ_DELAY_MS  1
 
+#define MAX_EVENT_PENDING 16
+
+struct iavf_arq_event_element {
+	TAILQ_ENTRY(iavf_arq_event_element) next;
+	struct rte_eth_dev *dev;
+	enum rte_eth_event_type event;
+	void *param;
+};
+
+struct iavf_event_handler {
+	rte_atomic32_t ndev;
+	pthread_t tid;
+	int fd[2];
+	pthread_mutex_t lock;
+	TAILQ_HEAD(event_lsit, iavf_arq_event_element) pending;
+};
+
+static struct iavf_event_handler event_handler = {
+	.fd = {-1, -1},
+};
+
+#ifndef TAILQ_FOREACH_SAFE
+#define TAILQ_FOREACH_SAFE(var, head, field, tvar) \
+	for ((var) = TAILQ_FIRST((head)); \
+		(var) && ((tvar) = TAILQ_NEXT((var), field), 1); \
+	(var) = (tvar))
+#endif
+
+static void *
+iavf_dev_event_handle(void *param __rte_unused)
+{
+	struct iavf_event_handler *handler = &event_handler;
+	TAILQ_HEAD(event_list, iavf_arq_event_element) pending;
+
+	while (true) {
+		char unused[MAX_EVENT_PENDING];
+		ssize_t nr = read(handler->fd[0], &unused, sizeof(unused));
+		if (nr <= 0)
+			break;
+
+		TAILQ_INIT(&pending);
+		pthread_mutex_lock(&handler->lock);
+		TAILQ_CONCAT(&pending, &handler->pending, next);
+		pthread_mutex_unlock(&handler->lock);
+
+		struct iavf_arq_event_element *pos, *save_next;
+		TAILQ_FOREACH_SAFE(pos, &pending, next, save_next) {
+			TAILQ_REMOVE(&pending, pos, next);
+			rte_eth_dev_callback_process(pos->dev, pos->event, pos->param);
+			rte_free(pos);
+		}
+	}
+	return NULL;
+}
+
+static void
+iavf_dev_event_post(struct rte_eth_dev *dev,
+		enum rte_eth_event_type event,
+		void *param)
+{
+	struct iavf_event_handler *handler = &event_handler;
+	char notify_byte;
+	struct iavf_arq_event_element *elem = rte_malloc(NULL, sizeof(*elem), 0);
+	if (!elem)
+		return;
+
+	elem->dev = dev;
+	elem->event = event;
+	elem->param = param;
+
+	pthread_mutex_lock(&handler->lock);
+	TAILQ_INSERT_TAIL(&handler->pending, elem, next);
+	pthread_mutex_unlock(&handler->lock);
+
+	ssize_t nw = write(handler->fd[1], &notify_byte, 1);
+	RTE_SET_USED(nw);
+}
+
+int
+iavf_dev_event_handler_init(void)
+{
+	struct iavf_event_handler *handler = &event_handler;
+
+	if (rte_atomic32_add_return(&handler->ndev, 1) != 1)
+		return 0;
+#if defined(RTE_EXEC_ENV_IS_WINDOWS) && RTE_EXEC_ENV_IS_WINDOWS != 0
+	int err = _pipe(handler->fd, MAX_EVENT_PENDING, _O_BINARY);
+#else
+	int err = pipe(handler->fd);
+#endif
+	if (err != 0) {
+		rte_atomic32_dec(&handler->ndev);
+		return -1;
+	}
+
+	TAILQ_INIT(&handler->pending);
+	pthread_mutex_init(&handler->lock, NULL);
+
+	if (rte_ctrl_thread_create(&handler->tid, "iavf-event-thread",
+				NULL, iavf_dev_event_handle, NULL)) {
+		rte_atomic32_dec(&handler->ndev);
+		return -1;
+	}
+
+	return 0;
+}
+
+void
+iavf_dev_event_handler_fini(void)
+{
+	struct iavf_event_handler *handler = &event_handler;
+
+	if (!rte_atomic32_dec_and_test(&handler->ndev))
+		return;
+
+	int unused = pthread_cancel(handler->tid);
+	RTE_SET_USED(unused);
+	close(handler->fd[0]);
+	close(handler->fd[1]);
+	handler->fd[0] = -1;
+	handler->fd[1] = -1;
+
+	pthread_join(handler->tid, NULL);
+	pthread_mutex_destroy(&handler->lock);
+
+	struct iavf_arq_event_element *pos, *save_next;
+	TAILQ_FOREACH_SAFE(pos, &handler->pending, next, save_next) {
+		TAILQ_REMOVE(&handler->pending, pos, next);
+		rte_free(pos);
+	}
+}
+
 static uint32_t
 iavf_convert_link_speed(enum virtchnl_link_speed virt_link_speed)
 {
@@ -293,7 +427,7 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,
 			vf->link_speed = iavf_convert_link_speed(speed);
 		}
 		iavf_dev_link_update(dev, 0);
-		rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
+		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 		break;
 	case VIRTCHNL_EVENT_PF_DRIVER_CLOSE:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_PF_DRIVER_CLOSE event");
-- 
2.34.1

RE: [PATCH v3] net/iavf: fix error of virtchnl command

[Zhang, Qi Z]

> -----Original Message-----
> From: Yiding Zhou <yidingx.zhou@intel.com>
> Sent: Saturday, October 8, 2022 4:49 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Zhou, YidingX <yidingx.zhou@intel.com>;
> stable@dpdk.org
> Subject: [PATCH v3] net/iavf: fix error of virtchnl command


... 

> When the device is bonded, bond pmd will register callback for LSC event.
> This callback will execute some virtchnl commands in eal-intr-thread to
> reinitialize the device with interrupts disabled. In this case, responses to all
> commands not be received.
> 
> This commit starts a thread to handle all events to fix this issue.
> 
> Fixes: 48de41ca11f0 ("net/avf: enable link status update")
> CC: stable@dpdk.org
> 
> Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
> ---
> +
>  static uint32_t
>  iavf_convert_link_speed(enum virtchnl_link_speed virt_link_speed)  { @@ -
> 293,7 +427,7 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev,
> uint8_t *msg,
>  			vf->link_speed = iavf_convert_link_speed(speed);
>  		}
>  		iavf_dev_link_update(dev, 0);
> -		rte_eth_dev_callback_process(dev,
> RTE_ETH_EVENT_INTR_LSC, NULL);
> +		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL);

If we decide to raise an etherdev event from a separate thread, is it better to do this for all events ?

RE: [PATCH v3] net/iavf: fix error of virtchnl command

[Zhou, YidingX]

> > This commit starts a thread to handle all events to fix this issue.
> >
> > Fixes: 48de41ca11f0 ("net/avf: enable link status update")
> > CC: stable@dpdk.org
> >
> > Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
> > ---
> > +
> >  static uint32_t
> >  iavf_convert_link_speed(enum virtchnl_link_speed virt_link_speed)  {
> > @@ -
> > 293,7 +427,7 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev,
> > uint8_t *msg,
> >  			vf->link_speed = iavf_convert_link_speed(speed);
> >  		}
> >  		iavf_dev_link_update(dev, 0);
> > -		rte_eth_dev_callback_process(dev,
> > RTE_ETH_EVENT_INTR_LSC, NULL);
> > +		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
> 
> If we decide to raise an etherdev event from a separate thread, is it better to
> do this for all events ?

Since there are no bug reports for other events, I have not handle them in event thread, I will follow your comments in v4.
Thanks~

[PATCH v4] net/iavf: fix error of virtchnl command

[Yiding Zhou]

When the device is bonded, bond pmd will register callback for LSC event.
This callback will execute some virtchnl commands in eal-intr-thread to
reinitialize the device with interrupts disabled. In this case, responses
to all commands not be received.

This commit starts a thread to handle all events to fix this issue.

Fixes: 48de41ca11f0 ("net/avf: enable link status update")
CC: stable@dpdk.org

Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
---
v4: add 'reset' and 'ipsec' event handling
v3: fix CI errors
---
 drivers/net/iavf/iavf.h        |   2 +
 drivers/net/iavf/iavf_ethdev.c |   5 ++
 drivers/net/iavf/iavf_vchnl.c  | 152 +++++++++++++++++++++++++++++++--
 3 files changed, 153 insertions(+), 6 deletions(-)

diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index 26b858f6f0..1edebab8dc 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -424,6 +424,8 @@ _atomic_set_async_response_cmd(struct iavf_info *vf, enum virtchnl_ops ops)
 }
 int iavf_check_api_version(struct iavf_adapter *adapter);
 int iavf_get_vf_resource(struct iavf_adapter *adapter);
+void iavf_dev_event_handler_fini(void);
+int iavf_dev_event_handler_init(void);
 void iavf_handle_virtchnl_msg(struct rte_eth_dev *dev);
 int iavf_enable_vlan_strip(struct iavf_adapter *adapter);
 int iavf_disable_vlan_strip(struct iavf_adapter *adapter);
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 782be82c7f..633d684804 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -2631,6 +2631,9 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_ether_addr_copy((struct rte_ether_addr *)hw->mac.addr,
 			&eth_dev->data->mac_addrs[0]);
 
+	if (iavf_dev_event_handler_init())
+		goto init_vf_err;
+
 	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) {
 		/* register callback func to eal lib */
 		rte_intr_callback_register(pci_dev->intr_handle,
@@ -2785,6 +2788,8 @@ iavf_dev_uninit(struct rte_eth_dev *dev)
 
 	iavf_dev_close(dev);
 
+	iavf_dev_event_handler_fini();
+
 	return 0;
 }
 
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 4327c5a786..43e18ca5f7 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -2,6 +2,7 @@
  * Copyright(c) 2017 Intel Corporation
  */
 
+#include <unistd.h>
 #include <stdio.h>
 #include <errno.h>
 #include <stdint.h>
@@ -11,6 +12,7 @@
 #include <inttypes.h>
 #include <rte_byteorder.h>
 #include <rte_common.h>
+#include <rte_os_shim.h>
 
 #include <rte_debug.h>
 #include <rte_alarm.h>
@@ -27,6 +29,145 @@
 #define MAX_TRY_TIMES 2000
 #define ASQ_DELAY_MS  1
 
+#define MAX_EVENT_PENDING 16
+
+struct iavf_event_element {
+	TAILQ_ENTRY(iavf_event_element) next;
+	struct rte_eth_dev *dev;
+	enum rte_eth_event_type event;
+	void *param;
+	size_t param_alloc_size;
+	uint8_t param_alloc_data[0];
+};
+
+struct iavf_event_handler {
+	uint32_t ndev;
+	pthread_t tid;
+	int fd[2];
+	pthread_mutex_t lock;
+	TAILQ_HEAD(event_lsit, iavf_event_element) pending;
+};
+
+static struct iavf_event_handler event_handler = {
+	.fd = {-1, -1},
+};
+
+#ifndef TAILQ_FOREACH_SAFE
+#define TAILQ_FOREACH_SAFE(var, head, field, tvar) \
+	for ((var) = TAILQ_FIRST((head)); \
+		(var) && ((tvar) = TAILQ_NEXT((var), field), 1); \
+	(var) = (tvar))
+#endif
+
+static void *
+iavf_dev_event_handle(void *param __rte_unused)
+{
+	struct iavf_event_handler *handler = &event_handler;
+	TAILQ_HEAD(event_list, iavf_event_element) pending;
+
+	while (true) {
+		char unused[MAX_EVENT_PENDING];
+		ssize_t nr = read(handler->fd[0], &unused, sizeof(unused));
+		if (nr <= 0)
+			break;
+
+		TAILQ_INIT(&pending);
+		pthread_mutex_lock(&handler->lock);
+		TAILQ_CONCAT(&pending, &handler->pending, next);
+		pthread_mutex_unlock(&handler->lock);
+
+		struct iavf_event_element *pos, *save_next;
+		TAILQ_FOREACH_SAFE(pos, &pending, next, save_next) {
+			TAILQ_REMOVE(&pending, pos, next);
+			rte_eth_dev_callback_process(pos->dev, pos->event, pos->param);
+			rte_free(pos);
+		}
+	}
+	return NULL;
+}
+
+static void
+iavf_dev_event_post(struct rte_eth_dev *dev,
+		enum rte_eth_event_type event,
+		void *param, size_t param_alloc_size)
+{
+	struct iavf_event_handler *handler = &event_handler;
+	char notify_byte;
+	struct iavf_event_element *elem = rte_malloc(NULL, sizeof(*elem) + param_alloc_size, 0);
+	if (!elem)
+		return;
+
+	elem->dev = dev;
+	elem->event = event;
+	elem->param = param;
+	elem->param_alloc_size = param_alloc_size;
+	if (param && param_alloc_size) {
+		rte_memcpy(elem->param_alloc_data, param, param_alloc_size);
+		elem->param = elem->param_alloc_data;
+	}
+
+	pthread_mutex_lock(&handler->lock);
+	TAILQ_INSERT_TAIL(&handler->pending, elem, next);
+	pthread_mutex_unlock(&handler->lock);
+
+	ssize_t nw = write(handler->fd[1], &notify_byte, 1);
+	RTE_SET_USED(nw);
+}
+
+int
+iavf_dev_event_handler_init(void)
+{
+	struct iavf_event_handler *handler = &event_handler;
+
+	if (__atomic_add_fetch(&handler->ndev, 1, __ATOMIC_RELAXED) != 1)
+		return 0;
+#if defined(RTE_EXEC_ENV_IS_WINDOWS) && RTE_EXEC_ENV_IS_WINDOWS != 0
+	int err = _pipe(handler->fd, MAX_EVENT_PENDING, O_BINARY);
+#else
+	int err = pipe(handler->fd);
+#endif
+	if (err != 0) {
+		__atomic_sub_fetch(&handler->ndev, 1, __ATOMIC_RELAXED);
+		return -1;
+	}
+
+	TAILQ_INIT(&handler->pending);
+	pthread_mutex_init(&handler->lock, NULL);
+
+	if (rte_ctrl_thread_create(&handler->tid, "iavf-event-thread",
+				NULL, iavf_dev_event_handle, NULL)) {
+		__atomic_sub_fetch(&handler->ndev, 1, __ATOMIC_RELAXED);
+		return -1;
+	}
+
+	return 0;
+}
+
+void
+iavf_dev_event_handler_fini(void)
+{
+	struct iavf_event_handler *handler = &event_handler;
+
+	if (__atomic_sub_fetch(&handler->ndev, 1, __ATOMIC_RELAXED) != 0)
+		return;
+
+	int unused = pthread_cancel(handler->tid);
+	RTE_SET_USED(unused);
+	close(handler->fd[0]);
+	close(handler->fd[1]);
+	handler->fd[0] = -1;
+	handler->fd[1] = -1;
+
+	pthread_join(handler->tid, NULL);
+	pthread_mutex_destroy(&handler->lock);
+
+	struct iavf_event_element *pos, *save_next;
+	TAILQ_FOREACH_SAFE(pos, &handler->pending, next, save_next) {
+		TAILQ_REMOVE(&handler->pending, pos, next);
+		rte_free(pos);
+	}
+}
+
 static uint32_t
 iavf_convert_link_speed(enum virtchnl_link_speed virt_link_speed)
 {
@@ -278,8 +419,8 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,
 	case VIRTCHNL_EVENT_RESET_IMPENDING:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING event");
 		vf->vf_reset = true;
-		rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET,
-					      NULL);
+		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
+					      NULL, 0);
 		break;
 	case VIRTCHNL_EVENT_LINK_CHANGE:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event");
@@ -293,7 +434,7 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,
 			vf->link_speed = iavf_convert_link_speed(speed);
 		}
 		iavf_dev_link_update(dev, 0);
-		rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
+		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL, 0);
 		break;
 	case VIRTCHNL_EVENT_PF_DRIVER_CLOSE:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_PF_DRIVER_CLOSE event");
@@ -359,9 +500,8 @@ iavf_handle_virtchnl_msg(struct rte_eth_dev *dev)
 					desc.subtype =
 						RTE_ETH_EVENT_IPSEC_UNKNOWN;
 					desc.metadata = ev->ipsec_event_data;
-					rte_eth_dev_callback_process(dev,
-							RTE_ETH_EVENT_IPSEC,
-							&desc);
+					iavf_dev_event_post(dev, RTE_ETH_EVENT_IPSEC,
+							&desc, sizeof(desc));
 					return;
 				}
 
-- 
2.34.1

[PATCH] net/ice/base: fix duplicate flow rules

[Yiding Zhou]

When a vsi that already exists in the created vsi_list subscribes to the
same filter again, the return value ICE_SUCCESS results in duplicate flow
rules to be stored, which will cause 'flush' and 'destroy' errors.

Fixes: fed0c5ca5f19 ("net/ice/base: support programming a new switch recipe")
Cc: stable@dpdk.org

Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
---
 drivers/net/ice/base/ice_switch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ice/base/ice_switch.c b/drivers/net/ice/base/ice_switch.c
index 4b115ce660..a2581f404d 100644
--- a/drivers/net/ice/base/ice_switch.c
+++ b/drivers/net/ice/base/ice_switch.c
@@ -8786,7 +8786,7 @@ ice_adv_add_update_vsi_list(struct ice_hw *hw,
 
 		/* A rule already exists with the new VSI being added */
 		if (ice_is_bit_set(m_entry->vsi_list_info->vsi_map, vsi_handle))
-			return ICE_SUCCESS;
+			return ICE_ERR_ALREADY_EXISTS;
 
 		/* Update the previously created VSI list set with
 		 * the new VSI ID passed in
-- 
2.34.1

RE: [PATCH v4] net/iavf: fix error of virtchnl command

[Zhang, Qi Z]

> -----Original Message-----
> From: Yiding Zhou <yidingx.zhou@intel.com>
> Sent: Thursday, October 13, 2022 2:20 PM
> To: dev@dpdk.org
> Cc: Zhou, YidingX <yidingx.zhou@intel.com>; stable@dpdk.org
> Subject: [PATCH v4] net/iavf: fix error of virtchnl command
> 
> When the device is bonded, bond pmd will register callback for LSC event.
> This callback will execute some virtchnl commands in eal-intr-thread to
> reinitialize the device with interrupts disabled. In this case, responses to all
> commands not be received.

Can we reword this commit log a little bit?

I think first of all, the patch is about code refactor, it separate all etherdev event notification from interrupt handler thread to a separated thread. 

Secondary, it also fixed the problem in bond.

> 
> This commit starts a thread to handle all events to fix this issue.
> 
> Fixes: 48de41ca11f0 ("net/avf: enable link status update")
> CC: stable@dpdk.org
> 
> Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
> ---
> v4: add 'reset' and 'ipsec' event handling
> v3: fix CI errors
> ---
>  drivers/net/iavf/iavf.h        |   2 +
>  drivers/net/iavf/iavf_ethdev.c |   5 ++
>  drivers/net/iavf/iavf_vchnl.c  | 152 +++++++++++++++++++++++++++++++--
>  3 files changed, 153 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h index
> 26b858f6f0..1edebab8dc 100644
> --- a/drivers/net/iavf/iavf.h
> +++ b/drivers/net/iavf/iavf.h
> @@ -424,6 +424,8 @@ _atomic_set_async_response_cmd(struct iavf_info
> *vf, enum virtchnl_ops ops)  }  int iavf_check_api_version(struct iavf_adapter
> *adapter);  int iavf_get_vf_resource(struct iavf_adapter *adapter);
> +void iavf_dev_event_handler_fini(void);
> +int iavf_dev_event_handler_init(void);
>  void iavf_handle_virtchnl_msg(struct rte_eth_dev *dev);  int
> iavf_enable_vlan_strip(struct iavf_adapter *adapter);  int
> iavf_disable_vlan_strip(struct iavf_adapter *adapter); diff --git
> a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c index
> 782be82c7f..633d684804 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -2631,6 +2631,9 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
>  	rte_ether_addr_copy((struct rte_ether_addr *)hw->mac.addr,
>  			&eth_dev->data->mac_addrs[0]);
> 
> +	if (iavf_dev_event_handler_init())
> +		goto init_vf_err;
> +
>  	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR)
> {
>  		/* register callback func to eal lib */
>  		rte_intr_callback_register(pci_dev->intr_handle,
> @@ -2785,6 +2788,8 @@ iavf_dev_uninit(struct rte_eth_dev *dev)
> 
>  	iavf_dev_close(dev);
> 
> +	iavf_dev_event_handler_fini();
> +
>  	return 0;
>  }
> 
> diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c index
> 4327c5a786..43e18ca5f7 100644
> --- a/drivers/net/iavf/iavf_vchnl.c
> +++ b/drivers/net/iavf/iavf_vchnl.c
> @@ -2,6 +2,7 @@
>   * Copyright(c) 2017 Intel Corporation
>   */
> 
> +#include <unistd.h>
>  #include <stdio.h>
>  #include <errno.h>
>  #include <stdint.h>
> @@ -11,6 +12,7 @@
>  #include <inttypes.h>
>  #include <rte_byteorder.h>
>  #include <rte_common.h>
> +#include <rte_os_shim.h>
> 
>  #include <rte_debug.h>
>  #include <rte_alarm.h>
> @@ -27,6 +29,145 @@
>  #define MAX_TRY_TIMES 2000
>  #define ASQ_DELAY_MS  1
> 
> +#define MAX_EVENT_PENDING 16
> +
> +struct iavf_event_element {
> +	TAILQ_ENTRY(iavf_event_element) next;
> +	struct rte_eth_dev *dev;
> +	enum rte_eth_event_type event;
> +	void *param;
> +	size_t param_alloc_size;
> +	uint8_t param_alloc_data[0];
> +};
> +
> +struct iavf_event_handler {
> +	uint32_t ndev;
> +	pthread_t tid;
> +	int fd[2];
> +	pthread_mutex_t lock;
> +	TAILQ_HEAD(event_lsit, iavf_event_element) pending; };
> +
> +static struct iavf_event_handler event_handler = {
> +	.fd = {-1, -1},
> +};
> +
> +#ifndef TAILQ_FOREACH_SAFE
> +#define TAILQ_FOREACH_SAFE(var, head, field, tvar) \
> +	for ((var) = TAILQ_FIRST((head)); \
> +		(var) && ((tvar) = TAILQ_NEXT((var), field), 1); \
> +	(var) = (tvar))
> +#endif
> +
> +static void *
> +iavf_dev_event_handle(void *param __rte_unused) {
> +	struct iavf_event_handler *handler = &event_handler;
> +	TAILQ_HEAD(event_list, iavf_event_element) pending;
> +
> +	while (true) {
> +		char unused[MAX_EVENT_PENDING];
> +		ssize_t nr = read(handler->fd[0], &unused, sizeof(unused));
> +		if (nr <= 0)
> +			break;
> +
> +		TAILQ_INIT(&pending);
> +		pthread_mutex_lock(&handler->lock);
> +		TAILQ_CONCAT(&pending, &handler->pending, next);
> +		pthread_mutex_unlock(&handler->lock);
> +
> +		struct iavf_event_element *pos, *save_next;
> +		TAILQ_FOREACH_SAFE(pos, &pending, next, save_next) {
> +			TAILQ_REMOVE(&pending, pos, next);
> +			rte_eth_dev_callback_process(pos->dev, pos->event,
> pos->param);
> +			rte_free(pos);
> +		}
> +	}
> +	return NULL;
> +}
> +
> +static void
> +iavf_dev_event_post(struct rte_eth_dev *dev,
> +		enum rte_eth_event_type event,
> +		void *param, size_t param_alloc_size) {
> +	struct iavf_event_handler *handler = &event_handler;
> +	char notify_byte;
> +	struct iavf_event_element *elem = rte_malloc(NULL, sizeof(*elem) +
> param_alloc_size, 0);
> +	if (!elem)
> +		return;
> +
> +	elem->dev = dev;
> +	elem->event = event;
> +	elem->param = param;
> +	elem->param_alloc_size = param_alloc_size;
> +	if (param && param_alloc_size) {
> +		rte_memcpy(elem->param_alloc_data, param,
> param_alloc_size);
> +		elem->param = elem->param_alloc_data;
> +	}
> +
> +	pthread_mutex_lock(&handler->lock);
> +	TAILQ_INSERT_TAIL(&handler->pending, elem, next);
> +	pthread_mutex_unlock(&handler->lock);
> +
> +	ssize_t nw = write(handler->fd[1], &notify_byte, 1);
> +	RTE_SET_USED(nw);
> +}
> +
> +int
> +iavf_dev_event_handler_init(void)
> +{
> +	struct iavf_event_handler *handler = &event_handler;
> +
> +	if (__atomic_add_fetch(&handler->ndev, 1, __ATOMIC_RELAXED) !=
> 1)
> +		return 0;
> +#if defined(RTE_EXEC_ENV_IS_WINDOWS) &&
> RTE_EXEC_ENV_IS_WINDOWS != 0
> +	int err = _pipe(handler->fd, MAX_EVENT_PENDING, O_BINARY); #else
> +	int err = pipe(handler->fd);
> +#endif
> +	if (err != 0) {
> +		__atomic_sub_fetch(&handler->ndev, 1,
> __ATOMIC_RELAXED);
> +		return -1;
> +	}
> +
> +	TAILQ_INIT(&handler->pending);
> +	pthread_mutex_init(&handler->lock, NULL);
> +
> +	if (rte_ctrl_thread_create(&handler->tid, "iavf-event-thread",
> +				NULL, iavf_dev_event_handle, NULL)) {
> +		__atomic_sub_fetch(&handler->ndev, 1,
> __ATOMIC_RELAXED);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +void
> +iavf_dev_event_handler_fini(void)
> +{
> +	struct iavf_event_handler *handler = &event_handler;
> +
> +	if (__atomic_sub_fetch(&handler->ndev, 1, __ATOMIC_RELAXED) !=
> 0)
> +		return;
> +
> +	int unused = pthread_cancel(handler->tid);
> +	RTE_SET_USED(unused);
> +	close(handler->fd[0]);
> +	close(handler->fd[1]);
> +	handler->fd[0] = -1;
> +	handler->fd[1] = -1;
> +
> +	pthread_join(handler->tid, NULL);
> +	pthread_mutex_destroy(&handler->lock);
> +
> +	struct iavf_event_element *pos, *save_next;
> +	TAILQ_FOREACH_SAFE(pos, &handler->pending, next, save_next) {
> +		TAILQ_REMOVE(&handler->pending, pos, next);
> +		rte_free(pos);
> +	}
> +}
> +
>  static uint32_t
>  iavf_convert_link_speed(enum virtchnl_link_speed virt_link_speed)  { @@ -
> 278,8 +419,8 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev,
> uint8_t *msg,
>  	case VIRTCHNL_EVENT_RESET_IMPENDING:
>  		PMD_DRV_LOG(DEBUG,
> "VIRTCHNL_EVENT_RESET_IMPENDING event");
>  		vf->vf_reset = true;
> -		rte_eth_dev_callback_process(dev,
> RTE_ETH_EVENT_INTR_RESET,
> -					      NULL);
> +		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
> +					      NULL, 0);
>  		break;
>  	case VIRTCHNL_EVENT_LINK_CHANGE:
>  		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE
> event"); @@ -293,7 +434,7 @@ iavf_handle_pf_event_msg(struct
> rte_eth_dev *dev, uint8_t *msg,
>  			vf->link_speed = iavf_convert_link_speed(speed);
>  		}
>  		iavf_dev_link_update(dev, 0);
> -		rte_eth_dev_callback_process(dev,
> RTE_ETH_EVENT_INTR_LSC, NULL);
> +		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL,
> 0);
>  		break;
>  	case VIRTCHNL_EVENT_PF_DRIVER_CLOSE:
>  		PMD_DRV_LOG(DEBUG,
> "VIRTCHNL_EVENT_PF_DRIVER_CLOSE event"); @@ -359,9 +500,8 @@
> iavf_handle_virtchnl_msg(struct rte_eth_dev *dev)
>  					desc.subtype =
> 
> 	RTE_ETH_EVENT_IPSEC_UNKNOWN;
>  					desc.metadata = ev-
> >ipsec_event_data;
> -					rte_eth_dev_callback_process(dev,
> -
> 	RTE_ETH_EVENT_IPSEC,
> -							&desc);
> +					iavf_dev_event_post(dev,
> RTE_ETH_EVENT_IPSEC,
> +							&desc, sizeof(desc));
>  					return;
>  				}
> 
> --
> 2.34.1

RE: [PATCH] net/ice/base: fix duplicate flow rules

[Xu, Ke1]

> -----Original Message-----
> From: Yiding Zhou <yidingx.zhou@intel.com>
> Sent: Thursday, October 13, 2022 2:21 PM
> To: dev@dpdk.org
> Cc: Zhou, YidingX <yidingx.zhou@intel.com>; stable@dpdk.org
> Subject: [PATCH] net/ice/base: fix duplicate flow rules
> 
> When a vsi that already exists in the created vsi_list subscribes to the same
> filter again, the return value ICE_SUCCESS results in duplicate flow rules to be
> stored, which will cause 'flush' and 'destroy' errors.
> 
> Fixes: fed0c5ca5f19 ("net/ice/base: support programming a new switch
> recipe")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>

Validated and passed on DPDK 22.11 rc1.
Also managed applying and passed validation on DPDK 20.11.

Tested-by: Ke Xu <ke1.xu@intel.com>

> ---
>  drivers/net/ice/base/ice_switch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 2.34.1

[PATCH v5] net/iavf: add thread for event callbacks

[Yiding Zhou]

All callbacks registered for ethdev events are called in eal-intr-thread,
and some of them execute virtchnl commands. Because interrupts are disabled
in the intr thread, there will be no response received for these commands.
So all callbacks should be called in a new context.

When the device is bonded, the bond pmd registers callback for LSC event to
execute virtchnl commands to reinitialize the device, it would also raise
the above issue.

This commit add a new thread to call all event callbacks.

Fixes: 48de41ca11f0 ("net/avf: enable link status update")
Fixes: 84108425054a ("net/iavf: support asynchronous virtual channel message")
Cc: stable@dpdk.org

Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
---
v5: reword the title and log
v4: add 'reset' and 'ipsec' handling
v3: fix CI errors
---
 drivers/net/iavf/iavf.h        |   2 +
 drivers/net/iavf/iavf_ethdev.c |   5 ++
 drivers/net/iavf/iavf_vchnl.c  | 153 +++++++++++++++++++++++++++++++--
 3 files changed, 154 insertions(+), 6 deletions(-)

diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index 26b858f6f0..1edebab8dc 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -424,6 +424,8 @@ _atomic_set_async_response_cmd(struct iavf_info *vf, enum virtchnl_ops ops)
 }
 int iavf_check_api_version(struct iavf_adapter *adapter);
 int iavf_get_vf_resource(struct iavf_adapter *adapter);
+void iavf_dev_event_handler_fini(void);
+int iavf_dev_event_handler_init(void);
 void iavf_handle_virtchnl_msg(struct rte_eth_dev *dev);
 int iavf_enable_vlan_strip(struct iavf_adapter *adapter);
 int iavf_disable_vlan_strip(struct iavf_adapter *adapter);
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 782be82c7f..633d684804 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -2631,6 +2631,9 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_ether_addr_copy((struct rte_ether_addr *)hw->mac.addr,
 			&eth_dev->data->mac_addrs[0]);
 
+	if (iavf_dev_event_handler_init())
+		goto init_vf_err;
+
 	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) {
 		/* register callback func to eal lib */
 		rte_intr_callback_register(pci_dev->intr_handle,
@@ -2785,6 +2788,8 @@ iavf_dev_uninit(struct rte_eth_dev *dev)
 
 	iavf_dev_close(dev);
 
+	iavf_dev_event_handler_fini();
+
 	return 0;
 }
 
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 4327c5a786..02ec1ae008 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -2,6 +2,7 @@
  * Copyright(c) 2017 Intel Corporation
  */
 
+#include <fcntl.h>
 #include <stdio.h>
 #include <errno.h>
 #include <stdint.h>
@@ -11,6 +12,7 @@
 #include <inttypes.h>
 #include <rte_byteorder.h>
 #include <rte_common.h>
+#include <rte_os_shim.h>
 
 #include <rte_debug.h>
 #include <rte_alarm.h>
@@ -27,6 +29,146 @@
 #define MAX_TRY_TIMES 2000
 #define ASQ_DELAY_MS  1
 
+#define MAX_EVENT_PENDING 16
+
+struct iavf_event_element {
+	TAILQ_ENTRY(iavf_event_element) next;
+	struct rte_eth_dev *dev;
+	enum rte_eth_event_type event;
+	void *param;
+	size_t param_alloc_size;
+	uint8_t param_alloc_data[0];
+};
+
+struct iavf_event_handler {
+	uint32_t ndev;
+	pthread_t tid;
+	int fd[2];
+	pthread_mutex_t lock;
+	TAILQ_HEAD(event_lsit, iavf_event_element) pending;
+};
+
+static struct iavf_event_handler event_handler = {
+	.fd = {-1, -1},
+};
+
+#ifndef TAILQ_FOREACH_SAFE
+#define TAILQ_FOREACH_SAFE(var, head, field, tvar) \
+	for ((var) = TAILQ_FIRST((head)); \
+		(var) && ((tvar) = TAILQ_NEXT((var), field), 1); \
+	(var) = (tvar))
+#endif
+
+static void *
+iavf_dev_event_handle(void *param __rte_unused)
+{
+	struct iavf_event_handler *handler = &event_handler;
+	TAILQ_HEAD(event_list, iavf_event_element) pending;
+
+	while (true) {
+		char unused[MAX_EVENT_PENDING];
+		ssize_t nr = read(handler->fd[0], &unused, sizeof(unused));
+		if (nr <= 0)
+			break;
+
+		TAILQ_INIT(&pending);
+		pthread_mutex_lock(&handler->lock);
+		TAILQ_CONCAT(&pending, &handler->pending, next);
+		pthread_mutex_unlock(&handler->lock);
+
+		struct iavf_event_element *pos, *save_next;
+		TAILQ_FOREACH_SAFE(pos, &pending, next, save_next) {
+			TAILQ_REMOVE(&pending, pos, next);
+			rte_eth_dev_callback_process(pos->dev, pos->event, pos->param);
+			rte_free(pos);
+		}
+	}
+
+	return NULL;
+}
+
+static void
+iavf_dev_event_post(struct rte_eth_dev *dev,
+		enum rte_eth_event_type event,
+		void *param, size_t param_alloc_size)
+{
+	struct iavf_event_handler *handler = &event_handler;
+	char notify_byte;
+	struct iavf_event_element *elem = rte_malloc(NULL, sizeof(*elem) + param_alloc_size, 0);
+	if (!elem)
+		return;
+
+	elem->dev = dev;
+	elem->event = event;
+	elem->param = param;
+	elem->param_alloc_size = param_alloc_size;
+	if (param && param_alloc_size) {
+		rte_memcpy(elem->param_alloc_data, param, param_alloc_size);
+		elem->param = elem->param_alloc_data;
+	}
+
+	pthread_mutex_lock(&handler->lock);
+	TAILQ_INSERT_TAIL(&handler->pending, elem, next);
+	pthread_mutex_unlock(&handler->lock);
+
+	ssize_t nw = write(handler->fd[1], &notify_byte, 1);
+	RTE_SET_USED(nw);
+}
+
+int
+iavf_dev_event_handler_init(void)
+{
+	struct iavf_event_handler *handler = &event_handler;
+
+	if (__atomic_add_fetch(&handler->ndev, 1, __ATOMIC_RELAXED) != 1)
+		return 0;
+#if defined(RTE_EXEC_ENV_IS_WINDOWS) && RTE_EXEC_ENV_IS_WINDOWS != 0
+	int err = _pipe(handler->fd, MAX_EVENT_PENDING, O_BINARY);
+#else
+	int err = pipe(handler->fd);
+#endif
+	if (err != 0) {
+		__atomic_sub_fetch(&handler->ndev, 1, __ATOMIC_RELAXED);
+		return -1;
+	}
+
+	TAILQ_INIT(&handler->pending);
+	pthread_mutex_init(&handler->lock, NULL);
+
+	if (rte_ctrl_thread_create(&handler->tid, "iavf-event-thread",
+				NULL, iavf_dev_event_handle, NULL)) {
+		__atomic_sub_fetch(&handler->ndev, 1, __ATOMIC_RELAXED);
+		return -1;
+	}
+
+	return 0;
+}
+
+void
+iavf_dev_event_handler_fini(void)
+{
+	struct iavf_event_handler *handler = &event_handler;
+
+	if (__atomic_sub_fetch(&handler->ndev, 1, __ATOMIC_RELAXED) != 0)
+		return;
+
+	int unused = pthread_cancel(handler->tid);
+	RTE_SET_USED(unused);
+	close(handler->fd[0]);
+	close(handler->fd[1]);
+	handler->fd[0] = -1;
+	handler->fd[1] = -1;
+
+	pthread_join(handler->tid, NULL);
+	pthread_mutex_destroy(&handler->lock);
+
+	struct iavf_event_element *pos, *save_next;
+	TAILQ_FOREACH_SAFE(pos, &handler->pending, next, save_next) {
+		TAILQ_REMOVE(&handler->pending, pos, next);
+		rte_free(pos);
+	}
+}
+
 static uint32_t
 iavf_convert_link_speed(enum virtchnl_link_speed virt_link_speed)
 {
@@ -278,8 +420,8 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,
 	case VIRTCHNL_EVENT_RESET_IMPENDING:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING event");
 		vf->vf_reset = true;
-		rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET,
-					      NULL);
+		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
+					      NULL, 0);
 		break;
 	case VIRTCHNL_EVENT_LINK_CHANGE:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event");
@@ -293,7 +435,7 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,
 			vf->link_speed = iavf_convert_link_speed(speed);
 		}
 		iavf_dev_link_update(dev, 0);
-		rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
+		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL, 0);
 		break;
 	case VIRTCHNL_EVENT_PF_DRIVER_CLOSE:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_PF_DRIVER_CLOSE event");
@@ -359,9 +501,8 @@ iavf_handle_virtchnl_msg(struct rte_eth_dev *dev)
 					desc.subtype =
 						RTE_ETH_EVENT_IPSEC_UNKNOWN;
 					desc.metadata = ev->ipsec_event_data;
-					rte_eth_dev_callback_process(dev,
-							RTE_ETH_EVENT_IPSEC,
-							&desc);
+					iavf_dev_event_post(dev, RTE_ETH_EVENT_IPSEC,
+							&desc, sizeof(desc));
 					return;
 				}
 
-- 
2.34.1

RE: [PATCH v5] net/iavf: add thread for event callbacks

[Zhang, Qi Z]

> -----Original Message-----
> From: Yiding Zhou <yidingx.zhou@intel.com>
> Sent: Thursday, October 20, 2022 1:00 PM
> To: dev@dpdk.org
> Cc: Zhou, YidingX <yidingx.zhou@intel.com>; stable@dpdk.org
> Subject: [PATCH v5] net/iavf: add thread for event callbacks
> 
> All callbacks registered for ethdev events are called in eal-intr-thread, and
> some of them execute virtchnl commands. Because interrupts are disabled in
> the intr thread, there will be no response received for these commands.
> So all callbacks should be called in a new context.
> 
> When the device is bonded, the bond pmd registers callback for LSC event to
> execute virtchnl commands to reinitialize the device, it would also raise the
> above issue.
> 
> This commit add a new thread to call all event callbacks.
> 
> Fixes: 48de41ca11f0 ("net/avf: enable link status update")
> Fixes: 84108425054a ("net/iavf: support asynchronous virtual channel
> message")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel after fix some typo

Thanks
Qi

Re: [PATCH v5] net/iavf: add thread for event callbacks

[Thomas Monjalon]

20/10/2022 07:00, Yiding Zhou:
> This commit add a new thread to call all event callbacks.

You may be interested to look at the new API rte_thread_*
instead of calling pthread directly.

RE: [PATCH] net/ice/base: fix duplicate flow rules

[Zhou, YidingX]

Hi, Qi

This  patch has been merged to kernel driver.
Should  it be merged to dpdk now? 

> -----Original Message-----
> From: Zhou, YidingX <yidingx.zhou@intel.com>
> Sent: Thursday, October 13, 2022 2:21 PM
> To: dev@dpdk.org
> Cc: Zhou, YidingX <yidingx.zhou@intel.com>; stable@dpdk.org
> Subject: [PATCH] net/ice/base: fix duplicate flow rules
> 
> When a vsi that already exists in the created vsi_list subscribes to the same
> filter again, the return value ICE_SUCCESS results in duplicate flow rules to be
> stored, which will cause 'flush' and 'destroy' errors.
> 
> Fixes: fed0c5ca5f19 ("net/ice/base: support programming a new switch recipe")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
> ---
>  drivers/net/ice/base/ice_switch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ice/base/ice_switch.c
> b/drivers/net/ice/base/ice_switch.c
> index 4b115ce660..a2581f404d 100644
> --- a/drivers/net/ice/base/ice_switch.c
> +++ b/drivers/net/ice/base/ice_switch.c
> @@ -8786,7 +8786,7 @@ ice_adv_add_update_vsi_list(struct ice_hw *hw,
> 
>  		/* A rule already exists with the new VSI being added */
>  		if (ice_is_bit_set(m_entry->vsi_list_info->vsi_map, vsi_handle))
> -			return ICE_SUCCESS;
> +			return ICE_ERR_ALREADY_EXISTS;
> 
>  		/* Update the previously created VSI list set with
>  		 * the new VSI ID passed in
> --
> 2.34.1

RE: [PATCH] net/ice/base: fix duplicate flow rules

[Zhang, Qi Z]

> -----Original Message-----
> From: Xu, Ke1 <ke1.xu@intel.com>
> Sent: Wednesday, October 19, 2022 8:20 PM
> To: Zhou, YidingX <yidingx.zhou@intel.com>; dev@dpdk.org
> Cc: Zhou, YidingX <yidingx.zhou@intel.com>; stable@dpdk.org
> Subject: RE: [PATCH] net/ice/base: fix duplicate flow rules
> 
> 
> > -----Original Message-----
> > From: Yiding Zhou <yidingx.zhou@intel.com>
> > Sent: Thursday, October 13, 2022 2:21 PM
> > To: dev@dpdk.org
> > Cc: Zhou, YidingX <yidingx.zhou@intel.com>; stable@dpdk.org
> > Subject: [PATCH] net/ice/base: fix duplicate flow rules
> >
> > When a vsi that already exists in the created vsi_list subscribes to
> > the same filter again, the return value ICE_SUCCESS results in
> > duplicate flow rules to be stored, which will cause 'flush' and 'destroy'
> errors.
> >
> > Fixes: fed0c5ca5f19 ("net/ice/base: support programming a new switch
> > recipe")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
> 
> Validated and passed on DPDK 22.11 rc1.
> Also managed applying and passed validation on DPDK 20.11.
> 
> Tested-by: Ke Xu <ke1.xu@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi
> 
> > ---
> >  drivers/net/ice/base/ice_switch.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > 2.34.1