From patchwork Mon Oct 18 14:28:01 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Loic Poulain X-Patchwork-Id: 515924 Delivered-To: patch@linaro.org Received: by 2002:ac0:cd8c:0:0:0:0:0 with SMTP id d12csp1831189imp; Mon, 18 Oct 2021 07:22:47 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzEF7NiOQh+Tsd886flQ+uwUSfm7E0H/NRPExFZtLR5c99akxBntW+AaBwrsh2doLc5NDxF X-Received: by 2002:a05:6808:90e:: with SMTP id w14mr25871oih.131.1634566967562; Mon, 18 Oct 2021 07:22:47 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1634566967; cv=none; d=google.com; s=arc-20160816; b=G5nLYUm4m9lOREMhCyMAD6jBGt6jJOkig0LM0P6V8kzppMbHzIyKZ0HiDLptp48Pq4 e0TgkIcfgCkYPvQndyjyhElwl5xqSQTr2QN9fVlwtnyVgVv8NqXvOLI/T9901wiL4Ubj S/idyV2pyCMVzABQKvpYqactLV4u/RBWzmQ8TCLJELY8ite216NvBXFqLUJR+InvZRUu IFONjNcAGYnETQP7V8UO7/VMi19Jjcv+HkzUBK3DszIX7Ur6iM00EvhFbLQpilINGk/X SbUSpH977znw2fJsXkK1bWge5OusL/Rt8A4Rx88WziGHswsSoydH8lvuTMzsGOTiHbRW KfwQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:sender:content-transfer-encoding:mime-version :list-subscribe:list-help:list-post:list-archive:list-unsubscribe :list-id:precedence:message-id:date:subject:cc:to:from :dkim-signature:dkim-signature; bh=yabdE31r9hw3NysSR5b5n9nxsUZncDBNm6Of9gCR780=; b=sa0DmovLeL2NAJ3hbHrmBTkyHdEOvG53b48hg/2v3WZfSV2IChrmpYbw+H/8n1VKsU DEKJpmWrwJ2C7wMTVqhkb7IHKEE0fgH6NBDFFR0ilAQSC8kzh3k2vScM9S4pNRKTRKxA LBrSaoWZSwnqipFsMiqNPkTc8fb4wps9d2nZ4p1Tpp7HkxXexCgn215WOePDDwUCi8GD fRrZhJeHYk5h3oOyledUkw3YGSL3c5LyeWgJkmfEuoxmmTUI6GACZ/tIBPtK16gwQWDA gAn6cvbTa3uUcDJPRBIUsLFu2Gvo26REZKsc7i7Ft5MQodL+YfgpyNyhUj5BqGLIlflr SlwQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@lists.infradead.org header.s=bombadil.20210309 header.b=OeQVhdbD; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=FSmnVMcf; spf=pass (google.com: best guess record for domain of wcn36xx-bounces+patch=linaro.org@lists.infradead.org designates 2607:7c80:54:e::133 as permitted sender) smtp.mailfrom="wcn36xx-bounces+patch=linaro.org@lists.infradead.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from bombadil.infradead.org (bombadil.infradead.org. [2607:7c80:54:e::133]) by mx.google.com with ESMTPS id e28si14727384ook.50.2021.10.18.07.22.47 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 18 Oct 2021 07:22:47 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of wcn36xx-bounces+patch=linaro.org@lists.infradead.org designates 2607:7c80:54:e::133 as permitted sender) client-ip=2607:7c80:54:e::133; Authentication-Results: mx.google.com; dkim=pass header.i=@lists.infradead.org header.s=bombadil.20210309 header.b=OeQVhdbD; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=FSmnVMcf; spf=pass (google.com: best guess record for domain of wcn36xx-bounces+patch=linaro.org@lists.infradead.org designates 2607:7c80:54:e::133 as permitted sender) smtp.mailfrom="wcn36xx-bounces+patch=linaro.org@lists.infradead.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:List-Subscribe:List-Help: List-Post:List-Archive:List-Unsubscribe:List-Id:Message-Id:Date:Subject:Cc:To :From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=yabdE31r9hw3NysSR5b5n9nxsUZncDBNm6Of9gCR780=; b=OeQVhdbD+gPZDi yP0KDJqmbOCeGt/p+gZy6DYSkHiPTLcnIlLZqIS3RBx4S6AhZ9V80BQ9lO3+BlnIVo5hCl2Ke83P6 TdOWt9faOrsjoa2PCAnioSfIIReuvv31+VfD39+w1JnoX/kc7xXrMJEhaaxq6rFjzbxr2MiU7ubQ5 TXgYV7Vu6Not0zcnC0Sg0CrmtE4Xtv/wOcQtpH3uR5rEaQ+rmlhECHIDhnJdaBLZxVpibUjajyLKT TmXjud2g/MZE2x6sE0l6NfZS3eca/oAC0mwu84u516oi8gtcJRbzrIeuT8Jz+59ZGA0NS+5pb4o1b Rc2qreuPt4eldvqCddpQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mcTXJ-00G5D4-E8; Mon, 18 Oct 2021 14:22:45 +0000 Received: from mail-wm1-x32b.google.com ([2a00:1450:4864:20::32b]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mcTRV-00G2p1-7Q for wcn36xx@lists.infradead.org; Mon, 18 Oct 2021 14:16:47 +0000 Received: by mail-wm1-x32b.google.com with SMTP id a140-20020a1c7f92000000b0030d8315b593so10511570wmd.5 for ; Mon, 18 Oct 2021 07:16:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=LAd+S0Qnz+V8XxjM+PVfUF/cgcdX7dHn+MP2lN3vIGw=; b=FSmnVMcfCwvzrHxPNyCK6iCEYmyR46utpI4L7WYoDcKbBm+DiVvKL3/8XrgSjZyy2P kc9O9Mnd64saaasklTaWc4GVhXsNmYUCEPsAcxrxOOHKJatbqdfENrlGN0D0yq3dDoof DEqYlLp0jDQNqAm4IEwN6Sjb7/q9i7YfxHA/q4tkcN9iSNeOmmLD/4a7AcYgRqI4SJZS LQm4TFYcpWIJqgCmzQ9BljASZM9Jwzm5QK/NxpQBaOJvFgbvKG+bt2LTjg15ExK3y+q/ 2TyVT3I2iyynFQOe9BzfRTJf6HMkn9uLYMNs08dpNFQemPYVWiZ9x/aU/l28S4U5/gle eiZw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=LAd+S0Qnz+V8XxjM+PVfUF/cgcdX7dHn+MP2lN3vIGw=; b=Eo/r7+Ob8m0Y1eUpXRv7a9r5mgYETgeGTjO7hjhSpRHcHzndMaJWtDnHQLCnd2YgWO UWqmSFqgUijrvPERxOojYgSu+4NiC5sjUnS2PXu07UiXZGO1Q2A6ksP3h/r+kENqV66Y pCPqI1RheMj2Qz7mh0ouHu+3mYpuZ7qawedKJWZ0eafXSN0b7smJoXGbalM0DSakDuiF TslYJ0BFgt9ryJVjdxi9q0z7JRKwQEXvv+wlET6xrXa5/1On5kVmf+xDTDuOwLEBzdRx kSNS8/u2Q1lfs+rDabZCiACyZsfLIObiF1M3Z1JVtM3dVRUP/QaGb1sY9mflYaauP9gq b0JQ== X-Gm-Message-State: AOAM532z5lh0+yIY062gfQbDvewSmMP23x7FXgKXfz7WydAMFqyxs+gR fYfiBWRXSQQ+CTxMQin0FjioPA== X-Received: by 2002:a7b:c5d8:: with SMTP id n24mr31541462wmk.51.1634566603668; Mon, 18 Oct 2021 07:16:43 -0700 (PDT) Received: from localhost.localdomain ([2a01:e0a:82c:5f0:9df5:c752:530b:345b]) by smtp.gmail.com with ESMTPSA id v185sm18482862wme.35.2021.10.18.07.16.42 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 18 Oct 2021 07:16:43 -0700 (PDT) From: Loic Poulain To: kvalo@codeaurora.org Cc: linux-wireless@vger.kernel.org, wcn36xx@lists.infradead.org, bryan.odonoghue@linaro.org, Loic Poulain , stable@vger.kernel.org Subject: [PATCH v2] wcn36xx: Fix tx_status mechanism Date: Mon, 18 Oct 2021 16:28:01 +0200 Message-Id: <1634567281-28997-1-git-send-email-loic.poulain@linaro.org> X-Mailer: git-send-email 2.7.4 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211018_071645_416238_3B66D081 X-CRM114-Status: GOOD ( 19.96 ) X-Spam-Score: -0.2 (/) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: This change fix the TX ack mechanism in various ways: - For NO_ACK tagged packets, we don't need to way for TX_ACK indication and so are not subject to the single packet ack limitation. So we don't have to stop the tx queue, and can call the tx status ca [...] Content analysis details: (-0.2 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [2a00:1450:4864:20:0:0:0:32b listed in] [list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain X-BeenThere: wcn36xx@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: "wcn36xx" Errors-To: wcn36xx-bounces+patch=linaro.org@lists.infradead.org This change fix the TX ack mechanism in various ways: - For NO_ACK tagged packets, we don't need to way for TX_ACK indication and so are not subject to the single packet ack limitation. So we don't have to stop the tx queue, and can call the tx status callback as soon as DMA transfer has completed. - Fix skb ownership/reference. Only start status indication timeout once the DMA transfer has been completed. This avoids the skb to be both referenced in the DMA tx ring and by the tx_ack_skb pointer, preventing any use-after-free or double-free. - This adds a sanity (paranoia?) check on the skb tx ack pointer. - Resume TX queue if TX status tagged packet TX fails. Cc: stable@vger.kernel.org Fixes: fdf21cc37149 ("wcn36xx: Add TX ack support") Signed-off-by: Loic Poulain --- v2: Fix unused variable error/warning (flag) drivers/net/wireless/ath/wcn36xx/dxe.c | 37 +++++++++++++-------------------- drivers/net/wireless/ath/wcn36xx/txrx.c | 31 ++++++--------------------- 2 files changed, 21 insertions(+), 47 deletions(-) -- 2.7.4 _______________________________________________ wcn36xx mailing list wcn36xx@lists.infradead.org http://lists.infradead.org/mailman/listinfo/wcn36xx diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.c b/drivers/net/wireless/ath/wcn36xx/dxe.c index 8e1dbfd..0e0bbcd 100644 --- a/drivers/net/wireless/ath/wcn36xx/dxe.c +++ b/drivers/net/wireless/ath/wcn36xx/dxe.c @@ -403,8 +403,21 @@ static void reap_tx_dxes(struct wcn36xx *wcn, struct wcn36xx_dxe_ch *ch) dma_unmap_single(wcn->dev, ctl->desc->src_addr_l, ctl->skb->len, DMA_TO_DEVICE); info = IEEE80211_SKB_CB(ctl->skb); - if (!(info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS)) { - /* Keep frame until TX status comes */ + if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) { + if (info->flags & IEEE80211_TX_CTL_NO_ACK) { + info->flags |= IEEE80211_TX_STAT_NOACK_TRANSMITTED; + ieee80211_tx_status_irqsafe(wcn->hw, ctl->skb); + } else { + /* Wait for the TX ack indication or timeout... */ + spin_lock(&wcn->dxe_lock); + if (WARN_ON(wcn->tx_ack_skb)) + ieee80211_free_txskb(wcn->hw, wcn->tx_ack_skb); + wcn->tx_ack_skb = ctl->skb; /* Tracking ref */ + mod_timer(&wcn->tx_ack_timer, jiffies + HZ / 10); + spin_unlock(&wcn->dxe_lock); + } + /* do not free, ownership transferred to mac80211 status cb */ + } else { ieee80211_free_txskb(wcn->hw, ctl->skb); } @@ -426,7 +439,6 @@ static irqreturn_t wcn36xx_irq_tx_complete(int irq, void *dev) { struct wcn36xx *wcn = (struct wcn36xx *)dev; int int_src, int_reason; - bool transmitted = false; wcn36xx_dxe_read_register(wcn, WCN36XX_DXE_INT_SRC_RAW_REG, &int_src); @@ -466,7 +478,6 @@ static irqreturn_t wcn36xx_irq_tx_complete(int irq, void *dev) if (int_reason & (WCN36XX_CH_STAT_INT_DONE_MASK | WCN36XX_CH_STAT_INT_ED_MASK)) { reap_tx_dxes(wcn, &wcn->dxe_tx_h_ch); - transmitted = true; } } @@ -479,7 +490,6 @@ static irqreturn_t wcn36xx_irq_tx_complete(int irq, void *dev) WCN36XX_DXE_0_INT_CLR, WCN36XX_INT_MASK_CHAN_TX_L); - if (int_reason & WCN36XX_CH_STAT_INT_ERR_MASK ) { wcn36xx_dxe_write_register(wcn, WCN36XX_DXE_0_INT_ERR_CLR, @@ -507,25 +517,8 @@ static irqreturn_t wcn36xx_irq_tx_complete(int irq, void *dev) if (int_reason & (WCN36XX_CH_STAT_INT_DONE_MASK | WCN36XX_CH_STAT_INT_ED_MASK)) { reap_tx_dxes(wcn, &wcn->dxe_tx_l_ch); - transmitted = true; - } - } - - spin_lock(&wcn->dxe_lock); - if (wcn->tx_ack_skb && transmitted) { - struct ieee80211_tx_info *info = IEEE80211_SKB_CB(wcn->tx_ack_skb); - - /* TX complete, no need to wait for 802.11 ack indication */ - if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS && - info->flags & IEEE80211_TX_CTL_NO_ACK) { - info->flags |= IEEE80211_TX_STAT_NOACK_TRANSMITTED; - del_timer(&wcn->tx_ack_timer); - ieee80211_tx_status_irqsafe(wcn->hw, wcn->tx_ack_skb); - wcn->tx_ack_skb = NULL; - ieee80211_wake_queues(wcn->hw); } } - spin_unlock(&wcn->dxe_lock); return IRQ_HANDLED; } diff --git a/drivers/net/wireless/ath/wcn36xx/txrx.c b/drivers/net/wireless/ath/wcn36xx/txrx.c index 1edc703..f9d2fc3 100644 --- a/drivers/net/wireless/ath/wcn36xx/txrx.c +++ b/drivers/net/wireless/ath/wcn36xx/txrx.c @@ -608,10 +608,11 @@ int wcn36xx_start_tx(struct wcn36xx *wcn, struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data; struct wcn36xx_vif *vif_priv = NULL; struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); - unsigned long flags; bool is_low = ieee80211_is_data(hdr->frame_control); bool bcast = is_broadcast_ether_addr(hdr->addr1) || is_multicast_ether_addr(hdr->addr1); + bool ack_ind = (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) && + !(info->flags & IEEE80211_TX_CTL_NO_ACK); struct wcn36xx_tx_bd bd; int ret; @@ -627,30 +628,16 @@ int wcn36xx_start_tx(struct wcn36xx *wcn, bd.dpu_rf = WCN36XX_BMU_WQ_TX; - if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) { + if (unlikely(ack_ind)) { wcn36xx_dbg(WCN36XX_DBG_DXE, "TX_ACK status requested\n"); - spin_lock_irqsave(&wcn->dxe_lock, flags); - if (wcn->tx_ack_skb) { - spin_unlock_irqrestore(&wcn->dxe_lock, flags); - wcn36xx_warn("tx_ack_skb already set\n"); - return -EINVAL; - } - - wcn->tx_ack_skb = skb; - spin_unlock_irqrestore(&wcn->dxe_lock, flags); - /* Only one at a time is supported by fw. Stop the TX queues * until the ack status gets back. */ ieee80211_stop_queues(wcn->hw); - /* TX watchdog if no TX irq or ack indication received */ - mod_timer(&wcn->tx_ack_timer, jiffies + HZ / 10); - /* Request ack indication from the firmware */ - if (!(info->flags & IEEE80211_TX_CTL_NO_ACK)) - bd.tx_comp = 1; + bd.tx_comp = 1; } /* Data frames served first*/ @@ -664,14 +651,8 @@ int wcn36xx_start_tx(struct wcn36xx *wcn, bd.tx_bd_sign = 0xbdbdbdbd; ret = wcn36xx_dxe_tx_frame(wcn, vif_priv, &bd, skb, is_low); - if (ret && (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS)) { - /* If the skb has not been transmitted, - * don't keep a reference to it. - */ - spin_lock_irqsave(&wcn->dxe_lock, flags); - wcn->tx_ack_skb = NULL; - spin_unlock_irqrestore(&wcn->dxe_lock, flags); - + if (unlikely(ret && ack_ind)) { + /* If the skb has not been transmitted, resume TX queue */ ieee80211_wake_queues(wcn->hw); }