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

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

[Yiding Zhou]

[upstream commit cb5c1b91f76f436724cd09f26c7432b2775b519c]

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")

Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
---
 drivers/net/iavf/iavf.h        |   2 +
 drivers/net/iavf/iavf_ethdev.c |   4 +
 drivers/net/iavf/iavf_vchnl.c  | 148 ++++++++++++++++++++++++++++++++-
 3 files changed, 150 insertions(+), 4 deletions(-)

diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index 297c69775b..356ff532af 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -214,6 +214,8 @@ _atomic_set_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 3cb02bd1fb..28bc1ca2da 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -1413,6 +1413,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())
+		return -1;
+
 	/* register callback func to eal lib */
 	rte_intr_callback_register(&pci_dev->intr_handle,
 				   iavf_dev_interrupt_handler,
@@ -1460,6 +1463,7 @@ iavf_dev_uninit(struct rte_eth_dev *dev)
 	dev->rx_pkt_burst = NULL;
 	dev->tx_pkt_burst = NULL;
 	iavf_dev_close(dev);
+	iavf_dev_event_handler_fini();
 
 	rte_free(vf->vf_res);
 	vf->vsi_res = NULL;
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index ba627f1103..1f5f43a7a5 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>
@@ -30,6 +31,146 @@
 #define MAX_TRY_TIMES 200
 #define ASQ_DELAY_MS  10
 
+#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);
+	}
+}
+
 /* Read data in admin queue to get msg from pf driver */
 static enum iavf_status_code
 iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
@@ -189,8 +330,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");
@@ -204,8 +345,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");
-- 
2.34.1

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

[Christian Ehrhardt]

On Wed, Nov 16, 2022 at 3:35 AM Yiding Zhou <yidingx.zhou@intel.com> wrote:
>
> [upstream commit cb5c1b91f76f436724cd09f26c7432b2775b519c]

Hi,
I tried to apply this but it causes build errors:

[  151s] iavf_vchnl.o: In function `iavf_dev_event_handler_fini':
[  151s] iavf_vchnl.c:(.text+0x5d9): undefined reference to `pthread_cancel'
[  151s] iavf_vchnl.c:(.text+0x611): undefined reference to `pthread_join'
[  151s] collect2: error: ld returned 1 exit status
[  151s] make[4]: *** [librte_pmd_iavf.so.20.0] Error 1
[  151s] make[3]: *** [iavf] Error 2

On all redhat, fedora, suse builds.
Interestingly the Ubuntu builds worked fine, but only them.

Please have a look and resubmit.

> 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")
>
> Signed-off-by: Yiding Zhou <yidingx.zhou@intel.com>
> ---
>  drivers/net/iavf/iavf.h        |   2 +
>  drivers/net/iavf/iavf_ethdev.c |   4 +
>  drivers/net/iavf/iavf_vchnl.c  | 148 ++++++++++++++++++++++++++++++++-
>  3 files changed, 150 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
> index 297c69775b..356ff532af 100644
> --- a/drivers/net/iavf/iavf.h
> +++ b/drivers/net/iavf/iavf.h
> @@ -214,6 +214,8 @@ _atomic_set_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 3cb02bd1fb..28bc1ca2da 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -1413,6 +1413,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())
> +               return -1;
> +
>         /* register callback func to eal lib */
>         rte_intr_callback_register(&pci_dev->intr_handle,
>                                    iavf_dev_interrupt_handler,
> @@ -1460,6 +1463,7 @@ iavf_dev_uninit(struct rte_eth_dev *dev)
>         dev->rx_pkt_burst = NULL;
>         dev->tx_pkt_burst = NULL;
>         iavf_dev_close(dev);
> +       iavf_dev_event_handler_fini();
>
>         rte_free(vf->vf_res);
>         vf->vsi_res = NULL;
> diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
> index ba627f1103..1f5f43a7a5 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>
> @@ -30,6 +31,146 @@
>  #define MAX_TRY_TIMES 200
>  #define ASQ_DELAY_MS  10
>
> +#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);
> +       }
> +}
> +
>  /* Read data in admin queue to get msg from pf driver */
>  static enum iavf_status_code
>  iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
> @@ -189,8 +330,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");
> @@ -204,8 +345,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");
> --
> 2.34.1
>


-- 
Christian Ehrhardt
Senior Staff Engineer, Ubuntu Server
Canonical Ltd

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

[Zhou, YidingX]

> > -----Original Message-----
> > From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> > Sent: Wednesday, November 16, 2022 4:18 PM
> > To: Zhou, YidingX <yidingx.zhou@intel.com>
> > Cc: stable@dpdk.org
> > Subject: Re: [PATCH 19.11] net/iavf: add thread for event callbacks
> >
> > On Wed, Nov 16, 2022 at 3:35 AM Yiding Zhou <yidingx.zhou@intel.com>
> wrote:
> > >
> > > [upstream commit cb5c1b91f76f436724cd09f26c7432b2775b519c]
> >
> > Hi,
> > I tried to apply this but it causes build errors:
> >
> > [  151s] iavf_vchnl.o: In function `iavf_dev_event_handler_fini':
> > [  151s] iavf_vchnl.c:(.text+0x5d9): undefined reference to `pthread_cancel'
> > [  151s] iavf_vchnl.c:(.text+0x611): undefined reference to `pthread_join'
> > [  151s] collect2: error: ld returned 1 exit status [  151s] make[4]:
> > *** [librte_pmd_iavf.so.20.0] Error 1 [  151s] make[3]: *** [iavf]
> > Error 2
> >
> > On all redhat, fedora, suse builds.
> > Interestingly the Ubuntu builds worked fine, but only them.
> >
> > Please have a look and resubmit.
> >
> Ok, I will check this on other systems.

Hi
I have built DPDK successfully on the following systems.

OS: Fedora release 36 (Thirty Six)
meson: 0.63.1
ninja: 1.10.2.git.kitware.jobserver-1
gcc: gcc (GCC) 12.2.1 20220819 (Red Hat 12.2.1-1)
----------------
OS: SUSE Linux Enterprise Server 15 SP4  (x86_64) 
meson: n0.61.5
ninja: 1.10.2.git.kitware.jobserver-1
gcc: gcc (SUSE Linux) 7.5.0
----------------
OS: Red Hat Enterprise Linux release 9.0 (Plow)
meson: 0.63.2
ninja: 1.10.2.git.kitware.jobserver-1
gcc: gcc (GCC) 11.2.1 20220127 (Red Hat 11.2.1-9)

What is your build environment?

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

[Christian Ehrhardt]

On Wed, Nov 16, 2022 at 10:58 AM Zhou, YidingX <yidingx.zhou@intel.com> wrote:
>
> > > -----Original Message-----
> > > From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> > > Sent: Wednesday, November 16, 2022 4:18 PM
> > > To: Zhou, YidingX <yidingx.zhou@intel.com>
> > > Cc: stable@dpdk.org
> > > Subject: Re: [PATCH 19.11] net/iavf: add thread for event callbacks
> > >
> > > On Wed, Nov 16, 2022 at 3:35 AM Yiding Zhou <yidingx.zhou@intel.com>
> > wrote:
> > > >
> > > > [upstream commit cb5c1b91f76f436724cd09f26c7432b2775b519c]
> > >
> > > Hi,
> > > I tried to apply this but it causes build errors:
> > >
> > > [  151s] iavf_vchnl.o: In function `iavf_dev_event_handler_fini':
> > > [  151s] iavf_vchnl.c:(.text+0x5d9): undefined reference to `pthread_cancel'
> > > [  151s] iavf_vchnl.c:(.text+0x611): undefined reference to `pthread_join'
> > > [  151s] collect2: error: ld returned 1 exit status [  151s] make[4]:
> > > *** [librte_pmd_iavf.so.20.0] Error 1 [  151s] make[3]: *** [iavf]
> > > Error 2
> > >
> > > On all redhat, fedora, suse builds.
> > > Interestingly the Ubuntu builds worked fine, but only them.
> > >
> > > Please have a look and resubmit.
> > >
> > Ok, I will check this on other systems.
>
> Hi
> I have built DPDK successfully on the following systems.
>
> OS: Fedora release 36 (Thirty Six)
> meson: 0.63.1
> ninja: 1.10.2.git.kitware.jobserver-1
> gcc: gcc (GCC) 12.2.1 20220819 (Red Hat 12.2.1-1)
> ----------------
> OS: SUSE Linux Enterprise Server 15 SP4  (x86_64)
> meson: n0.61.5
> ninja: 1.10.2.git.kitware.jobserver-1
> gcc: gcc (SUSE Linux) 7.5.0
> ----------------
> OS: Red Hat Enterprise Linux release 9.0 (Plow)
> meson: 0.63.2
> ninja: 1.10.2.git.kitware.jobserver-1
> gcc: gcc (GCC) 11.2.1 20220127 (Red Hat 11.2.1-9)
>
> What is your build environment?

Hi,
We use OBS (Thanks Lucas to set this up, I'm only re-using what he
did) to do some cross release/distro build checks based on our WIP
branches on github.
You can dive into details of logs and configuration here:
https://build.opensuse.org/package/show/home:cpaelzer:branches:home:bluca:dpdk/dpdk-19.11

Not all builds work every time, but between before & after applying a
patch for 19.11.x I at least want to see no regressions.
Due to that, please ignore suse_factory; suse_tumbleweed, debian_next
and fedora_rawhide - those have been broken before.
But most others start to fail when applying your patch while being ok before.

I'm done with DPDK 19.11 for the rest of today, so to make more sense
for you I have applied your patch again and restarted the OBS service.
In a few minutes you'll see the artifacts of a build with your patch
applied so you can have a look.

-- 
Christian Ehrhardt
Senior Staff Engineer, Ubuntu Server
Canonical Ltd

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

[Zhou, YidingX]

> > > > -----Original Message-----
> > > > From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> > > > Sent: Wednesday, November 16, 2022 4:18 PM
> > > > To: Zhou, YidingX <yidingx.zhou@intel.com>
> > > > Cc: stable@dpdk.org
> > > > Subject: Re: [PATCH 19.11] net/iavf: add thread for event
> > > > callbacks
> > > >
> > > > On Wed, Nov 16, 2022 at 3:35 AM Yiding Zhou
> > > > <yidingx.zhou@intel.com>
> > > wrote:
> > > > >
> > > > > [upstream commit cb5c1b91f76f436724cd09f26c7432b2775b519c]
> > > >
> > > > Hi,
> > > > I tried to apply this but it causes build errors:
> > > >
> > > > [  151s] iavf_vchnl.o: In function `iavf_dev_event_handler_fini':
> > > > [  151s] iavf_vchnl.c:(.text+0x5d9): undefined reference to
> `pthread_cancel'
> > > > [  151s] iavf_vchnl.c:(.text+0x611): undefined reference to `pthread_join'
> > > > [  151s] collect2: error: ld returned 1 exit status [  151s] make[4]:
> > > > *** [librte_pmd_iavf.so.20.0] Error 1 [  151s] make[3]: *** [iavf]
> > > > Error 2
> > > >
> > > > On all redhat, fedora, suse builds.
> > > > Interestingly the Ubuntu builds worked fine, but only them.
> > > >
> > > > Please have a look and resubmit.
> > > >
> > > Ok, I will check this on other systems.
> >
> > Hi
> > I have built DPDK successfully on the following systems.
> >
> > OS: Fedora release 36 (Thirty Six)
> > meson: 0.63.1
> > ninja: 1.10.2.git.kitware.jobserver-1
> > gcc: gcc (GCC) 12.2.1 20220819 (Red Hat 12.2.1-1)
> > ----------------
> > OS: SUSE Linux Enterprise Server 15 SP4  (x86_64)
> > meson: n0.61.5
> > ninja: 1.10.2.git.kitware.jobserver-1
> > gcc: gcc (SUSE Linux) 7.5.0
> > ----------------
> > OS: Red Hat Enterprise Linux release 9.0 (Plow)
> > meson: 0.63.2
> > ninja: 1.10.2.git.kitware.jobserver-1
> > gcc: gcc (GCC) 11.2.1 20220127 (Red Hat 11.2.1-9)
> >
> > What is your build environment?
> 
> Hi,
> We use OBS (Thanks Lucas to set this up, I'm only re-using what he
> did) to do some cross release/distro build checks based on our WIP branches on
> github.
> You can dive into details of logs and configuration here:
> https://build.opensuse.org/package/show/home:cpaelzer:branches:home:bluca:
> dpdk/dpdk-19.11
> 
> Not all builds work every time, but between before & after applying a patch for
> 19.11.x I at least want to see no regressions.
> Due to that, please ignore suse_factory; suse_tumbleweed, debian_next and
> fedora_rawhide - those have been broken before.
> But most others start to fail when applying your patch while being ok before.
> 
> I'm done with DPDK 19.11 for the rest of today, so to make more sense for you I
> have applied your patch again and restarted the OBS service.
> In a few minutes you'll see the artifacts of a build with your patch applied so
> you can have a look.
> 

Hi, I have checked the log of OBS, It seems that '- lpthread' is missing from the linker's parameter,
but other files that also called 'pthread_join' can be correctly linked (e.g. lib/librte_eal/common/eal_common_thread.c). 
This is very confusing. 
I may not be able to solve this issue in a short time, so this patch should not be applied to 19.11.
Thanks.
> --
> Christian Ehrhardt
> Senior Staff Engineer, Ubuntu Server
> Canonical Ltd

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

[Christian Ehrhardt]

On Thu, Nov 17, 2022 at 10:05 AM Zhou, YidingX <yidingx.zhou@intel.com> wrote:
>
> > > > > -----Original Message-----
> > > > > From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> > > > > Sent: Wednesday, November 16, 2022 4:18 PM
> > > > > To: Zhou, YidingX <yidingx.zhou@intel.com>
> > > > > Cc: stable@dpdk.org
> > > > > Subject: Re: [PATCH 19.11] net/iavf: add thread for event
> > > > > callbacks
> > > > >
> > > > > On Wed, Nov 16, 2022 at 3:35 AM Yiding Zhou
> > > > > <yidingx.zhou@intel.com>
> > > > wrote:
> > > > > >
> > > > > > [upstream commit cb5c1b91f76f436724cd09f26c7432b2775b519c]
> > > > >
> > > > > Hi,
> > > > > I tried to apply this but it causes build errors:
> > > > >
> > > > > [  151s] iavf_vchnl.o: In function `iavf_dev_event_handler_fini':
> > > > > [  151s] iavf_vchnl.c:(.text+0x5d9): undefined reference to
> > `pthread_cancel'
> > > > > [  151s] iavf_vchnl.c:(.text+0x611): undefined reference to `pthread_join'
> > > > > [  151s] collect2: error: ld returned 1 exit status [  151s] make[4]:
> > > > > *** [librte_pmd_iavf.so.20.0] Error 1 [  151s] make[3]: *** [iavf]
> > > > > Error 2
> > > > >
> > > > > On all redhat, fedora, suse builds.
> > > > > Interestingly the Ubuntu builds worked fine, but only them.
> > > > >
> > > > > Please have a look and resubmit.
> > > > >
> > > > Ok, I will check this on other systems.
> > >
> > > Hi
> > > I have built DPDK successfully on the following systems.
> > >
> > > OS: Fedora release 36 (Thirty Six)
> > > meson: 0.63.1
> > > ninja: 1.10.2.git.kitware.jobserver-1
> > > gcc: gcc (GCC) 12.2.1 20220819 (Red Hat 12.2.1-1)
> > > ----------------
> > > OS: SUSE Linux Enterprise Server 15 SP4  (x86_64)
> > > meson: n0.61.5
> > > ninja: 1.10.2.git.kitware.jobserver-1
> > > gcc: gcc (SUSE Linux) 7.5.0
> > > ----------------
> > > OS: Red Hat Enterprise Linux release 9.0 (Plow)
> > > meson: 0.63.2
> > > ninja: 1.10.2.git.kitware.jobserver-1
> > > gcc: gcc (GCC) 11.2.1 20220127 (Red Hat 11.2.1-9)
> > >
> > > What is your build environment?
> >
> > Hi,
> > We use OBS (Thanks Lucas to set this up, I'm only re-using what he
> > did) to do some cross release/distro build checks based on our WIP branches on
> > github.
> > You can dive into details of logs and configuration here:
> > https://build.opensuse.org/package/show/home:cpaelzer:branches:home:bluca:
> > dpdk/dpdk-19.11
> >
> > Not all builds work every time, but between before & after applying a patch for
> > 19.11.x I at least want to see no regressions.
> > Due to that, please ignore suse_factory; suse_tumbleweed, debian_next and
> > fedora_rawhide - those have been broken before.
> > But most others start to fail when applying your patch while being ok before.
> >
> > I'm done with DPDK 19.11 for the rest of today, so to make more sense for you I
> > have applied your patch again and restarted the OBS service.
> > In a few minutes you'll see the artifacts of a build with your patch applied so
> > you can have a look.
> >
>
> Hi, I have checked the log of OBS, It seems that '- lpthread' is missing from the linker's parameter,
> but other files that also called 'pthread_join' can be correctly linked (e.g. lib/librte_eal/common/eal_common_thread.c).
> This is very confusing.
> I may not be able to solve this issue in a short time, so this patch should not be applied to 19.11.
> Thanks.

Ok, please come back once you have sorted it out.
This pattern reminds me that 19.11 still had make + meson based build systems.
The builds that keep working are meson based (Ubuntu/Debian) and the
builds failing IIRC back in the 19.11.x days used make.
If you tested your change on RH/Suse/Fedora using meson that might
explain why you could not reproduce it.

> > --
> > Christian Ehrhardt
> > Senior Staff Engineer, Ubuntu Server
> > Canonical Ltd



-- 
Christian Ehrhardt
Senior Staff Engineer, Ubuntu Server
Canonical Ltd