lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <49a385d0-9ffc-468f-b7de-83abfa1e18f0@nbd.name>
Date: Thu, 5 Sep 2024 14:31:58 +0200
From: Felix Fietkau <nbd@....name>
To: Shengyu Qu <wiagn233@...look.com>, lorenzo@...nel.org,
 ryder.lee@...iatek.com, shayne.chen@...iatek.com, sean.wang@...iatek.com,
 kvalo@...nel.org, matthias.bgg@...il.com,
 angelogioacchino.delregno@...labora.com, daniel@...rotopia.org,
 miriam.rachel.korenblit@...el.com, money.wang@...iatek.com,
 StanleyYP.Wang@...iatek.com, meichia.chiu@...iatek.com,
 chui-hao.chiu@...iatek.com, johannes.berg@...el.com, quic_adisi@...cinc.com,
 sujuan.chen@...iatek.com, allen.ye@...iatek.com,
 linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org
Cc: Bo Jiao <bo.jiao@...iatek.com>
Subject: Re: [PATCH v6] wifi: mt76: mt7915: add wds support when wed is
 enabled

On 05.08.24 14:43, Shengyu Qu wrote:
> The current WED only supports 256 wcid, whereas mt7986 can support up to
> 512 entries, so firmware provides a rule to get sta_info by DA when wcid
> is set to 0x3ff by txd. Also, WED provides a register to overwrite txd
> wcid, that is, wcid[9:8] can be overwritten by 0x3 and wcid[7:0] is set
> to 0xff by host driver.
> 
> However, firmware is unable to get sta_info from DA as DA != RA for
> 4addr cases, so firmware and wifi host driver both use wcid (256 - 271)
> and (768 ~ 783) for sync up to get correct sta_info.
> 
> Currently WDS+WED config is completely broken on MT7986/7981 devices if
> without this patch.
> 
> Tested-by: Sujuan Chen <sujuan.chen@...iatek.com>
> Co-developed-by: Bo Jiao <bo.jiao@...iatek.com>
> Signed-off-by: Bo Jiao <bo.jiao@...iatek.com>
> Signed-off-by: Sujuan Chen <sujuan.chen@...iatek.com>
> Signed-off-by: Shengyu Qu <wiagn233@...look.com>
> ---
> Changes since v1:
>   - Drop duplicate setting in mmio
>   - Reduce the patch size by redefining mt76_wcid_alloc
> Changes since v2:
>   - Rework wds wcid getting flow
> Changes since v3:
>   - Rebase to next-20240703
>   - Sync with downstream patch
> Changes since v4:
>   - Rebase to next-20240802
> Changes since v5:
>   - Fixed build test error reported by robot
>   - Rebase to next-20240805
> ---
>   drivers/net/wireless/mediatek/mt76/mt76.h     |  9 +++++
>   .../net/wireless/mediatek/mt76/mt7915/main.c  | 32 ++++++++++++++--
>   .../net/wireless/mediatek/mt76/mt7915/mcu.c   | 18 +++++++--
>   .../net/wireless/mediatek/mt76/mt7915/mcu.h   |  1 +
>   drivers/net/wireless/mediatek/mt76/util.c     | 37 +++++++++++++++++--
>   drivers/net/wireless/mediatek/mt76/util.h     |  7 +++-
>   6 files changed, 93 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
> index 4a58a78d5ed25..1186a4998faff 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -28,6 +28,9 @@
>   
>   #define MT76_TOKEN_FREE_THR	64
>   
> +#define MT76_WED_WDS_MIN    256
> +#define MT76_WED_WDS_MAX    272
> +
>   #define MT_QFLAG_WED_RING	GENMASK(1, 0)
>   #define MT_QFLAG_WED_TYPE	GENMASK(4, 2)
>   #define MT_QFLAG_WED		BIT(5)
> @@ -71,6 +74,12 @@ enum mt76_wed_type {
>   	MT76_WED_RRO_Q_IND,
>   };
>   
> +enum mt76_wed_state {
> +	MT76_WED_DEFAULT,
> +	MT76_WED_ACTIVE,
> +	MT76_WED_WDS_ACTIVE,
> +};
> +
>   struct mt76_bus_ops {
>   	u32 (*rr)(struct mt76_dev *dev, u32 offset);
>   	void (*wr)(struct mt76_dev *dev, u32 offset, u32 val);
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> index 049223df9beb1..dc4d87e004a0f 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> @@ -745,8 +745,15 @@ int mt7915_mac_sta_add(struct mt76_dev *mdev, struct ieee80211_vif *vif,
>   	bool ext_phy = mvif->phy != &dev->phy;
>   	int ret, idx;
>   	u32 addr;
> +	u8 flags = MT76_WED_DEFAULT;
>   
> -	idx = mt76_wcid_alloc(dev->mt76.wcid_mask, MT7915_WTBL_STA);
> +	if (mtk_wed_device_active(&dev->mt76.mmio.wed) &&
> +	    !is_mt7915(&dev->mt76)) {
> +		flags = test_bit(MT_WCID_FLAG_4ADDR, &msta->wcid.flags) ?
> +		       MT76_WED_WDS_ACTIVE : MT76_WED_ACTIVE;
> +	}
> +
> +	idx = __mt76_wcid_alloc(mdev->wcid_mask, MT7915_WTBL_STA, flags);
>   	if (idx < 0)
>   		return -ENOSPC;
>   

I'd prefer to replace the mt76_wcid_alloc flags argument with an 
explicit start offset argument.

> @@ -1201,12 +1208,27 @@ static void mt7915_sta_set_4addr(struct ieee80211_hw *hw,
>   {
>   	struct mt7915_dev *dev = mt7915_hw_dev(hw);
>   	struct mt7915_sta *msta = (struct mt7915_sta *)sta->drv_priv;
> +	int min = MT76_WED_WDS_MIN, max = MT76_WED_WDS_MAX;
>   
>   	if (enabled)
>   		set_bit(MT_WCID_FLAG_4ADDR, &msta->wcid.flags);
>   	else
>   		clear_bit(MT_WCID_FLAG_4ADDR, &msta->wcid.flags);
>   
> +	if (mtk_wed_device_active(&dev->mt76.mmio.wed) &&
> +	    !is_mt7915(&dev->mt76) &&
> +	    (msta->wcid.idx < min || msta->wcid.idx > max - 1)) {
> +		struct ieee80211_sta *pre_sta;
> +
> +		pre_sta = kzalloc(sizeof(*sta) + sizeof(*msta), GFP_KERNEL);
> +		mt76_sta_pre_rcu_remove(hw, vif, sta);
> +		memmove(pre_sta, sta, sizeof(*sta) + sizeof(*msta));
> +		mt7915_sta_add(hw, vif, sta);
> +		synchronize_rcu();
> +		mt7915_sta_remove(hw, vif, pre_sta);
> +		kfree(pre_sta);
> +	}
> +
>   	mt76_connac_mcu_wtbl_update_hdr_trans(&dev->mt76, vif, sta);
>   }
>  
In order to update the code based on my latest changes and to fix 
potential race conditions on tx/rx packets during the transition, please 
change to this order:

1. copy the sta
2. allocate a new wcid
3. change the wcid index in the copied sta to the newly allocated wcid
4. call mcu functions on the duplicate sta for creating the new sta entry.
5. use rcu_assign_pointer to point dev->wcid[new_idx] at &msta->wcid
6. swap wcid index between real sta and duplicated sta
7. rcu_assign_pointer(dev->wcid[orig_idx], NULL)
8. synchronize_rcu()
9. call mcu functions to delete the duplicate sta's entry (points to old 
wcid after the swap)
10. free the duplicated sta

This should allow mgmt tx/rx to work while the sta is being migrated to 
the new wcid entry.

- Felix

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ