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@xxxxxxxxxxxx> > > Co-developed-by: Bo Jiao <bo.jiao@xxxxxxxxxxxx> > > Signed-off-by: Bo Jiao <bo.jiao@xxxxxxxxxxxx> > > Signed-off-by: Sujuan Chen <sujuan.chen@xxxxxxxxxxxx> > > --- > > 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