From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D50B6A054D for ; Wed, 16 Nov 2022 09:18:05 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id ADFB340DFB; Wed, 16 Nov 2022 09:18:05 +0100 (CET) Received: from smtp-relay-internal-0.canonical.com (smtp-relay-internal-0.canonical.com [185.125.188.122]) by mails.dpdk.org (Postfix) with ESMTP id 336FC400EF for ; Wed, 16 Nov 2022 09:18:05 +0100 (CET) Received: from mail-oa1-f69.google.com (mail-oa1-f69.google.com [209.85.160.69]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-0.canonical.com (Postfix) with ESMTPS id 7A11E3F07F for ; Wed, 16 Nov 2022 08:18:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1668586684; bh=cEE5g/ma+yZ0kERTYo09QRUHO4o/Kbjak76lScBoW/o=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=NaEjRdb2EQJpnMWlCFejF7gi1zWMSFxzuu3hh48/vnxqDCk3Cyrkf8Sr/KBXBprY5 vKTJjj0xZkwlFRJBaAIh8zOv0oYN4YC7/TOdfQjHR++LwPk4wKVFofmdwEHKGm+KPm HOG9Z/PIZ8QzhCN/9EqPjPyPl2W/DDD9nE2Vn5USmNDjyBkTy+nKQ23UYZ6h3PTswO vQZ+mKdDXQLwxCHPIF8NpnWSqoWBxLHHoP6Z5s8C8SeQEksf7+r4Z3DMa12GWWmnw/ l49cudNcYv8ulwtv1n+U3cSPvjtzv6Rx2h4qdT+CPpa9yzhoAuwEmhYxY3mgn7D3fE dIO4H44J/7+wg== Received: by mail-oa1-f69.google.com with SMTP id 586e51a60fabf-13bc77c87f6so7854932fac.19 for ; Wed, 16 Nov 2022 00:18:04 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=cEE5g/ma+yZ0kERTYo09QRUHO4o/Kbjak76lScBoW/o=; b=47BdtOyaazTl/7jt2M3IV6GfFvL8sg49/em6AnmZ6QOgCznOFdIesDWT3qBl41+DFy V9OEjrbs0sSB5U/nGquPCE5EIlcLz3bXZtpG5VDyo2rZJ8HmHM67LVeA0cflIf3s+M+l QIjNJvPXlVk7IYMbVOxBDt1WjzlQjobr30rxyKWvb9IqbBWWHBMkfD6V2OaSmBSpUPPE Ylb9lkPre126eg4RPgCJs/tKSA8M4sRbGwWy0jJrL+570LWEkC/fuflVVY6OjkmmgVhM 4TVxuYfHviR5GsH+9wTrP6p1LQJHEpUVHUErT/Vsu31q97lav6kxluamvmkZEwQjt6ht FYHA== X-Gm-Message-State: ANoB5plb11AvBmvE95iTIE7B2dJRhnBvLq05CPm66QzhlMQjl8VVgrZT H9IX10lpWof88QLEf0tfi8MbXrwPQPLtaWe9KHnijz0Z7s0qqDdmaQa2sS7y3v7EsMih9D7L/wF HyitNb+TPUtTpD4WQ9fzn7byJAwmO89/fHd2891J5 X-Received: by 2002:a05:6808:3011:b0:354:74a1:d0a with SMTP id ay17-20020a056808301100b0035474a10d0amr1027684oib.204.1668586683372; Wed, 16 Nov 2022 00:18:03 -0800 (PST) X-Google-Smtp-Source: AA0mqf7+LiNLEx+y+jgkDQE8UZ6EcAdTzUoRPe4T/5RqfvpNv3dRuHLdR5VFzalxx51L5dyT3f2GAymlagHrltLUHpM= X-Received: by 2002:a05:6808:3011:b0:354:74a1:d0a with SMTP id ay17-20020a056808301100b0035474a10d0amr1027680oib.204.1668586683092; Wed, 16 Nov 2022 00:18:03 -0800 (PST) MIME-Version: 1.0 References: <20221116023926.317352-1-yidingx.zhou@intel.com> In-Reply-To: <20221116023926.317352-1-yidingx.zhou@intel.com> From: Christian Ehrhardt Date: Wed, 16 Nov 2022 09:17:36 +0100 Message-ID: Subject: Re: [PATCH 19.11] net/iavf: add thread for event callbacks To: Yiding Zhou Cc: stable@dpdk.org Content-Type: text/plain; charset="UTF-8" X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org On Wed, Nov 16, 2022 at 3:35 AM Yiding Zhou 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 > --- > 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, > ð_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 > #include > #include > #include > @@ -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], ¬ify_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