Re: [PATCH,v2] wifi: mt76: mt7915: add wds support when wed is enabled

["Sujuan Chen (陈素娟)" <Sujuan.Chen@mediatek.com>]

On Fri, 2022-12-09 at 13:32 +0100, Felix Fietkau wrote:
> On 30.11.22 10:18, Sujuan Chen 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
> > 
> > Tested-by: Sujuan Chen <sujuan.chen@mediatek.com>
> > Co-developed-by: Bo Jiao <bo.jiao@mediatek.com>
> > Signed-off-by: Bo Jiao <bo.jiao@mediatek.com>
> > Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
> > ---
> > v2:
> >   - drop duplicate settings
> >   - reduce the patch size by redefining mt76_wcid_alloc
> > ---
> >   drivers/net/wireless/mediatek/mt76/mt76.h     |  6 +++
> >   .../net/wireless/mediatek/mt76/mt7915/main.c  | 24 +++++++++--
> >   .../net/wireless/mediatek/mt76/mt7915/mcu.c   | 13 +++++-
> >   .../net/wireless/mediatek/mt76/mt7915/mcu.h   |  1 +
> >   drivers/net/wireless/mediatek/mt76/util.c     | 40
> > +++++++++++++++++--
> >   drivers/net/wireless/mediatek/mt76/util.h     |  7 +++-
> >   6 files changed, 82 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > index c40b6098f19a..46a9e4f0396e 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > @@ -1115,6 +1122,13 @@ static void mt7915_sta_set_4addr(struct
> > ieee80211_hw *hw,
> >   	else
> >   		clear_bit(MT_WCID_FLAG_4ADDR, &msta->wcid.flags);
> >   
> > +	if (mtk_wed_device_active(&dev->mt76.mmio.wed) &&
> > +	    !is_mt7915(&dev->mt76)) {
> > +		mt7915_sta_remove(hw, vif, sta);
> > +		mt76_sta_pre_rcu_remove(hw, vif, sta);
> > +		mt7915_sta_add(hw, vif, sta);
> > +	}
> > +
> >   	mt76_connac_mcu_wtbl_update_hdr_trans(&dev->mt76, vif, sta);
> >   }
> >   
> 
> I suspect that this may a bit racy if there is concurrent tx
> activity 
> (e.g. for EAP auth). Not sure if this could cause problems for the 
> firmware or other kinds of bugs.
> 
> While my idea may need some rework of the existing functions, I think
> a 
> better flow would be:
> 
> 1. mt76_sta_pre_rcu_remove
> 2. save old wcid
> 3. mt7915_sta_add
> 4. synchronize_rcu()
> 5. remove firmware state for old wcid entry
> 
Hi Felix,

Can we modify it like this:

1. pre_sta = kzalloc(sizeof(*sta) + sizeof(*msta), GFP_KERNEL);
2. mt76_sta_pre_rcu_remove
3. memmove(old_sta, sta, sizeof(*sta) + sizeof(*msta))
4. mt7915_sta_add
5. synchronize_rcu()
6. mt7915_sta_remove(hw, vif, pre_sta)
7. kfree(pre_sta)

we can reuse mt7915_sta_remove and keep the sta info for 
mt7915_mcu_add_sta.

if save old wcid only, we need to rework __mt76_sta_remove and
mt7915_mcu_add_sta functions by add wcid as parameter.
__mt76_sta_remove is required to delete a wcid, right?

-sujuan

> - Felix