From patchwork Mon Oct 18 12:31:00 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Loic Poulain X-Patchwork-Id: 515921 Delivered-To: patch@linaro.org Received: by 2002:adf:a11e:0:0:0:0:0 with SMTP id o30csp3414387wro; Mon, 18 Oct 2021 05:19:49 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxtHCLP8/9Xll+lBts0B0qyPO+Qp7+B8Srx5wx+q1RQgdQrpSekJY5daS2EB8wGZff1Gl5/ X-Received: by 2002:a05:6830:92:: with SMTP id a18mr13234706oto.307.1634559589739; Mon, 18 Oct 2021 05:19:49 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1634559589; cv=none; d=google.com; s=arc-20160816; b=pE9/K6DLT2BXNmKZ6EFpdz9wZhknnO5hA/gh7GEIWPyQRt2hXdNm0wmhRl6gVrZImE 2d9CPvNDLJ+svl5df9molzB+T4cjlL5x1Ts3rDeKkhsQSeJgIDEJyYy5Kd57/OSBA5oh koBaMcOtFKfz39fkGPcy1IjJaFfVLzItcszlyDsiLotQ1VobT4CGp1CkuC+3esVxj/9J jdXhPBKMkDjWSwrYJTtK2gn2JNfTWKVrWgsbDT5iyyl2W0GCLBTkuGuH7BUHbFZV/bsg FN4Hc5OMThMvrgS1dUMWjl3NMADjCx4ZJoJXVOxK5e+qOZzHsHDirwafngnVhVApuxod yDsw== 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=4oi7A4twaQ8Vn0/T9z/POILJXbRPoDV+W5070TZ7FvM=; b=oLuL+upLTYW7/zDuuJJdhaxsA2LDVWkAmgBgwyTHzyyQ2RaRzagWEn8EcVTbp4tOBC 4n+YAXfy649uND0bRBUf5YnM40iUzMiGys3T95iFNAqcCQmhtARgZDbT9kENFNuAg6Yq Z3FGgbXsn3bHoPtfhsVUF6m+WhawCsaDG1TlIQGaPeqcJr17iy3HjAvIhIptVpj3Ggon k+0MgbIf7IL0OIWT0r2/a0HnjKW1wRgwkJ5Iw7AIJeoDiRVH4+u4Ap9rFWZdJuTyrug+ ikx8uuK9bjOn2qiYz5bsfKa6A4n1/zKfjU6d5CGYrWtLWB3QaeTouGs54morCB+T0g1i eCCg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@lists.infradead.org header.s=bombadil.20210309 header.b=bQ4glzUR; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=xMsBXm0a; spf=neutral (google.com: 2607:7c80:54:e::133 is neither permitted nor denied by best guess record for domain of wcn36xx-bounces+patch=linaro.org@lists.infradead.org) 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 ([2607:7c80:54:e::133]) by mx.google.com with ESMTPS id n125si13079920oia.129.2021.10.18.05.19.49 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 18 Oct 2021 05:19:49 -0700 (PDT) Received-SPF: neutral (google.com: 2607:7c80:54:e::133 is neither permitted nor denied by best guess record for domain of wcn36xx-bounces+patch=linaro.org@lists.infradead.org) 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=bQ4glzUR; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=xMsBXm0a; spf=neutral (google.com: 2607:7c80:54:e::133 is neither permitted nor denied by best guess record for domain of wcn36xx-bounces+patch=linaro.org@lists.infradead.org) 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=4oi7A4twaQ8Vn0/T9z/POILJXbRPoDV+W5070TZ7FvM=; b=bQ4glzURh1IzR8 05fKxy9iIgHAxJmYfr0xuUwNxMm3WLqwMGXNDK7tnLGB8ghgIcTuYU672PHGwPFi5GDIpfKzAS6XX Hb98IP872mwWNP7cJb+DGjVl4k0km1slR8D/2gYZzNPuvYueo5GSO1KCnPLsRtggHlXOvGVLqXEUt qMFDn8nl9c60a27PcyF8AFEE5vT3Yc0MF7vbtuvvJpSyoSZAFdNISI/bb4USbN6P04K55RD1yp4Gz G5vjYz/OkzflQwGbd5T/OHas46Ka22ISzsUxdD7WGco2S9I+N4ypf4adSm0HEr3fI6H9fLUlK8zjc xDYkMOyxTuQvZE7DDohA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mcRcK-00FcUh-4i; Mon, 18 Oct 2021 12:19:48 +0000 Received: from mail-wm1-x333.google.com ([2a00:1450:4864:20::333]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mcRcG-00FcSX-74 for wcn36xx@lists.infradead.org; Mon, 18 Oct 2021 12:19:47 +0000 Received: by mail-wm1-x333.google.com with SMTP id p21so8551479wmq.1 for ; Mon, 18 Oct 2021 05:19:43 -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=Mrj3/k0aO18EsPYiCwstLG8nMGZmvk0h62LLWvj8MuM=; b=xMsBXm0aLESWeDDz5Kqb3aN+4x8vP6piEdaiQxaSmDmNHMdPFWmrPBGA/4rGlPuKl7 VuTjlMczOgT/drL7FpXM1jD4ZJLN+jdeyO+f4VgCxN7n3Lz3fWC5ys27ekvQg+5fhGEQ UhQ7CcH3uKjV0AiPe+sPkC/1xweOnkQ1lobWmfHbgoS2nS0BmwboU6SKp29LZdjhsAdd 3GM2A+mKUzXNBNquObS7kusQ2LMUOMWRN9lfOEW0vCWiT1UXrYKdlcg5K0ugOSw2QLRx spb8+2VNZP5UIHo7C2vcb9B9nem/r3RrHihTE2ji9rTp+xfAEYGRHv12yUioO+93c1aA ZCLg== 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=Mrj3/k0aO18EsPYiCwstLG8nMGZmvk0h62LLWvj8MuM=; b=x4SQLeR8w0P0lT9snplMd67rZVFQ3geRy+7vqF2SnxCdzg1+JEpOUvdOigOdGie4Vz 798XkPCk7Ejp+2E5iIhtMS6VUoY7Moske8MHzeuRlycbWrZUI+bxFDxgpMDnAeKeUqP6 RBqNdgx1HQIyaWOFhPUqQF8D5YOf8RuWJ2pYLSFRfwJ5f8V5L83FAfjOSM+pjoiOEekn /41N3L1pWihXmXVScOFfnqbnFKed/sByzRkyglV/rijT/X2vnYVYc/08Wcw94IrFWpkc EuHrS5kenmzOfoczTTRtkL+wRgxEhUtxDQPT0k1fT9e8y8rWFbd/x0IkEfay78mZ6eco j2mA== X-Gm-Message-State: AOAM531oAAcnUIV2lW4r3zMhaxKQGNmJLcQpDSCEN/XPNcgyA0LPT+qq Z4PYYcGmGI+p4AP0tUL2zyz3sGa5TXQ= X-Received: by 2002:a1c:f713:: with SMTP id v19mr29600470wmh.188.1634559582145; Mon, 18 Oct 2021 05:19:42 -0700 (PDT) Received: from localhost.localdomain ([2a01:e0a:82c:5f0:9df5:c752:530b:345b]) by smtp.gmail.com with ESMTPSA id u5sm12654865wrg.57.2021.10.18.05.19.41 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 18 Oct 2021 05:19:41 -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] wcn36xx: Fix tx_status mechanism Date: Mon, 18 Oct 2021 14:31:00 +0200 Message-Id: <1634560260-15056-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_051944_285319_67F2D9D5 X-CRM114-Status: GOOD ( 19.99 ) 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:333 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 --- drivers/net/wireless/ath/wcn36xx/dxe.c | 37 +++++++++++++-------------------- drivers/net/wireless/ath/wcn36xx/txrx.c | 30 ++++++-------------------- 2 files changed, 21 insertions(+), 46 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..ef1b133 100644 --- a/drivers/net/wireless/ath/wcn36xx/txrx.c +++ b/drivers/net/wireless/ath/wcn36xx/txrx.c @@ -612,6 +612,8 @@ int wcn36xx_start_tx(struct wcn36xx *wcn, 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 +629,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 +652,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); }