On 21.11.19 13:58, Stanislaw Gruszka wrote:
> On Wed, Nov 20, 2019 at 11:27:55PM +0100, Markus Theil wrote:
>>>> - }
>>>> -
>>>> - for (i = bcn_idx; i < ARRAY_SIZE(dev->beacons); i++) {
>>>> - if (!(dev->beacon_data_mask & BIT(i)))
>>>> - break;
>>>> -
>>>> - __mt76x02_mac_set_beacon(dev, i, NULL);
>>>> - }
>>>>
>>>> mt76_rmw_field(dev, MT_MAC_BSSID_DW1, MT_MAC_BSSID_DW1_MBEACON_N,
>>>> bcn_idx - 1);
>>>> +
>>>> + mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xff00 | ~dev->beacon_data_mask);
>>> I'm not sure if this is correct for multi bss.
>>>
>>> In MT7620 manual BCM_BAYPASS_MASK is described as below:
>>>
>>> "
>>> Directly bypasses the Tx Beacon frame with the specified
>>> Beacon number. Bit0=Nth Beacon, bit1=(N- 1)th Beacon,... etc.
>>> N is the number of Beacons defined in the MULTI_BCN_NUM field in the
>>> MAC_BSSID_DW1(offset: 0x1014) register.
>>> 0: Disable
>>> 1: Enable
>>> "
>>>
>>> Assuming manual is correct (it could be wrong) bypass mask should be
>>> calculated differently.
>>>
>>> Stanislaw
>>>
>> I tested the updated code with my usb nic and an mbss with 2 ap vifs.
>> Both beacons were transmitted. Maybe the manual is wrong in this place.
> If MAC_BSSID_DW1_MBEACON_N is set to 1 (2 beacons) according to manual
> bit0 is for second beacon slot and bit1 is for first beacon slot.
> Those bits are both marked at once, so no problem will happen.
>
> Problem may happen when you remove first vif/beacon. Then you will
> have bit1 marked in ->beacon_data_mask . But bit0 will be expect
> in BCM_BAYPASS_MASK by hardware (when MAC_BSSID_DW1_MBEACON_N=0)
>
> This scenarios can be extended to more vifs. So if you no longer
> use bcn_idx and use vif_idx directly to point beacon slot/address.
> (ie. before for vif_idx 0,4,6, bcn_idx were 0,1,2 now there
> will be 0,4,6). You need to specify 7 (8 beacons) in
> MT_MAC_BSSID_DW1_MBEACON_N, and set bypass mask accordingly.
> For example:
>
> mt76_rmw_field(dev, MT_MAC_BSSID_DW1, MT_MAC_BSSID_DW1_MBEACON_N, 7);
> mask = 0;
> for (i = 0; i < 8; i++)
> if (dev->beacons[i])
> mask |= BIT(7 - i);
>
> mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xff00 | ~mask);
>
> But again, this have to be tested. Ideally on mmio hardware which
> support more bssid's or on usb hardware with temporally increased
> num of bss limitation.
>
> Stanislaw
>
I'm currently fixing this. My next version will include the following for this aspect:
- clear beacon_data_mask before each round of setting beacons
- use ffz(dev->beacon_data_mask) to find next position and put next beacon in the corresponding slot
- as usb and mmio always copy the beacon, free_skb directly afterwards and drop the beacons array (the current code could leak memory for beacons in this array)
- permanently set the number of additional beacons to 7: mt76_rmw_field(dev, MT_MAC_BSSID_DW1, MT_MAC_BSSID_DW1_MBEACON_N, 7)
- updating the beacon bypass mask then becomes: mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xff00 | ~bitrev8(dev->beacon_data_mask))
When I've tested this, I'll send an updated version.
Markus
