Firmware 10.x supports up to 8 virtual AP interfaces
(comparing to 7 for main firmware). Previous vdev map
initialization was missing enough space for 8 + 1 vdevs
(we may spent one for mac monitor), due to wrong define used.
Use correct one - TARGET_10X_NUM_VDEVS - for 10.x firmware.
Signed-off-by: Bartosz Markowski <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 82017f5..e6c56c5 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -795,7 +795,11 @@ int ath10k_core_start(struct ath10k *ar)
if (status)
goto err_htc_stop;
- ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
+ if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
+ ar->free_vdev_map = (1 << TARGET_10X_NUM_VDEVS) - 1;
+ else
+ ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
+
INIT_LIST_HEAD(&ar->arvifs);
if (!test_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags))
--
1.8.2
On 29 May 2014 15:47, Ben Greear <[email protected]> wrote:
>
>
> On 05/29/2014 06:36 AM, Bartosz Markowski wrote:
>>
>> Firmware 10.x supports up to 8 virtual AP interfaces
>> (comparing to 7 for main firmware). Previous vdev map
>> initialization was missing enough space for 8 + 1 vdevs
>> (we may spent one for mac monitor), due to wrong define used.
>>
>> Use correct one - TARGET_10X_NUM_VDEVS - for 10.x firmware.
>
>
> You are bumping total vdevs up to 16 with that patch...have you
> actually tested that many? The stock firmware has quite a bit of
> deficiencies in the concurrency handling, at least for stations.
No, I have never tested it with so many. I'm running tests with 8 at
most. And by this patch I just aimed to support 8 VAPS we advertise in
iface_combinations to mac80211 (.max_interfaces = 8). I was managed to
get only 7 before, since 1 vdev was being allocated as monitor.
[ 219.960000] ath10k: mac monitor refs: promisc 1 monitor 0 cac 0
[ 219.960000] ath10k: mac monitor vdev 1 created
I agree the 10X_NUM_VDEVS is a bit unfortunate with its 16 value, but
that's what was there from FW APIs for a long time and I do not know
exactly if it's still valid (from firmware point of view).
> For what it's worth, my firmware will only work on stock kernels
> because I ignore the request for 16 vdevs in the firmware and knock
> it down to 8 to match the kernel driver (before your change below).
Do you encode your firmware with the FW IE (wmi-10x) also? If som then
it's a bit weird to have different firmware tracks that introduce
themselves as 10.x.
-Bartosz
On 05/29/2014 06:36 AM, Bartosz Markowski wrote:
> Firmware 10.x supports up to 8 virtual AP interfaces
> (comparing to 7 for main firmware). Previous vdev map
> initialization was missing enough space for 8 + 1 vdevs
> (we may spent one for mac monitor), due to wrong define used.
>
> Use correct one - TARGET_10X_NUM_VDEVS - for 10.x firmware.
You are bumping total vdevs up to 16 with that patch...have you
actually tested that many? The stock firmware has quite a bit of
deficiencies in the concurrency handling, at least for stations.
For what it's worth, my firmware will only work on stock kernels
because I ignore the request for 16 vdevs in the firmware and knock
it down to 8 to match the kernel driver (before your change below).
Thanks,
Ben
>
> Signed-off-by: Bartosz Markowski <[email protected]>
> ---
> drivers/net/wireless/ath/ath10k/core.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
> index 82017f5..e6c56c5 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -795,7 +795,11 @@ int ath10k_core_start(struct ath10k *ar)
> if (status)
> goto err_htc_stop;
>
> - ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
> + if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
> + ar->free_vdev_map = (1 << TARGET_10X_NUM_VDEVS) - 1;
> + else
> + ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
> +
> INIT_LIST_HEAD(&ar->arvifs);
>
> if (!test_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags))
>
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On 05/29/2014 07:42 AM, Bartosz Markowski wrote:
> On 29 May 2014 16:10, Bartosz Markowski <[email protected]> wrote:
>> On 29 May 2014 15:47, Ben Greear <[email protected]> wrote:
>>>
>>>
>>> On 05/29/2014 06:36 AM, Bartosz Markowski wrote:
>>>>
>>>> Firmware 10.x supports up to 8 virtual AP interfaces
>>>> (comparing to 7 for main firmware). Previous vdev map
>>>> initialization was missing enough space for 8 + 1 vdevs
>>>> (we may spent one for mac monitor), due to wrong define used.
>>>>
>>>> Use correct one - TARGET_10X_NUM_VDEVS - for 10.x firmware.
>>>
>>>
>>> You are bumping total vdevs up to 16 with that patch...have you
>>> actually tested that many? The stock firmware has quite a bit of
>>> deficiencies in the concurrency handling, at least for stations.
>>
>> No, I have never tested it with so many. I'm running tests with 8 at
>> most. And by this patch I just aimed to support 8 VAPS we advertise in
>> iface_combinations to mac80211 (.max_interfaces = 8). I was managed to
>> get only 7 before, since 1 vdev was being allocated as monitor.
>
> FYI: I quickly tried to extend the .max_interfaces +
> interface_combinations limit in ath10k and check with 15 VAPs (using
> hostapd, open network; on MIPS platform) and it just worked.. I did
> not perform any exploratory testing, but managed to connect a couple
> of clients to various bssids + run some sane iperf traffic etc.
Good to know that many VAPs can work...
If the OS limits are going to remain at 8, then maybe
only set the vdev mask to support 9 instead of 16
vifs to handle the monitor interface? That should make
it easier (or at least safer) to support my firmware on
un-patched kernels...
Some of the problems with running out of resources in the
firmware (which generally just asserts or crashes in that case)
are only seen under load and/or random-ish cases
in my experience. I'm all for pushing the firmware to the
max, but I suspect 16 vifs may not be that stable.
That said, I have only been testing multiple stations, so maybe it
will just work fine.
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On 29 May 2014 16:10, Bartosz Markowski <[email protected]> wrote:
> On 29 May 2014 15:47, Ben Greear <[email protected]> wrote:
>>
>>
>> On 05/29/2014 06:36 AM, Bartosz Markowski wrote:
>>>
>>> Firmware 10.x supports up to 8 virtual AP interfaces
>>> (comparing to 7 for main firmware). Previous vdev map
>>> initialization was missing enough space for 8 + 1 vdevs
>>> (we may spent one for mac monitor), due to wrong define used.
>>>
>>> Use correct one - TARGET_10X_NUM_VDEVS - for 10.x firmware.
>>
>>
>> You are bumping total vdevs up to 16 with that patch...have you
>> actually tested that many? The stock firmware has quite a bit of
>> deficiencies in the concurrency handling, at least for stations.
>
> No, I have never tested it with so many. I'm running tests with 8 at
> most. And by this patch I just aimed to support 8 VAPS we advertise in
> iface_combinations to mac80211 (.max_interfaces = 8). I was managed to
> get only 7 before, since 1 vdev was being allocated as monitor.
FYI: I quickly tried to extend the .max_interfaces +
interface_combinations limit in ath10k and check with 15 VAPs (using
hostapd, open network; on MIPS platform) and it just worked.. I did
not perform any exploratory testing, but managed to connect a couple
of clients to various bssids + run some sane iperf traffic etc.
--
Bartosz
On 2 June 2014 18:42, Kalle Valo <[email protected]> wrote:
> Bartosz Markowski <[email protected]> writes:
>
>> Firmware 10.x supports up to 8 virtual AP interfaces
>> (comparing to 7 for main firmware). Previous vdev map
>> initialization was missing enough space for 8 + 1 vdevs
>> (we may spent one for mac monitor), due to wrong define used.
>>
>> Use correct one - TARGET_10X_NUM_VDEVS - for 10.x firmware.
>>
>> Signed-off-by: Bartosz Markowski <[email protected]>
>
> So what is the actual bug you are fixing? Previously with 10.x it was
> possible to get only 7 VIFs, even though we advertised 8 to user space,
> and with your fix we get the full 8 VIFs?
For CAC, we use one VDEV to start monitor interface. In case of 10.X
firmware we advertise support up to 8 VAPs, but if we spent one for
monitor interface, only 7 left. I've noticed we fail on .add_interface
when trying to add 8th AP, here:
bit = ffs(ar->free_vdev_map);
if (bit == 0) {
ret = -EBUSY;
goto err;
}
and this lead me to initialization code for vdev_map
ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
We have an API split for main and 10.x firmware (incl. number of
vdevs, target fw is able to handle), but here we missed this split.
Ben has a valid point, the TARGET_10X_NUM_VDEVS claims to be 16, so
there's an inconsistency between what we adverts to mac in max
interfaces, but I'm not sure if this is such a big deal.
> It would be good to clear have that in the commit log so that anyone can
> understand what bug is fixed.
Do you want me to send a v2 with just an updated commit (better user
impact description)? (No patch content changes)
-Bartosz
Bartosz Markowski <[email protected]> writes:
> Firmware 10.x supports up to 8 virtual AP interfaces
> (comparing to 7 for main firmware). Previous vdev map
> initialization was missing enough space for 8 + 1 vdevs
> (we may spent one for mac monitor), due to wrong define used.
>
> Use correct one - TARGET_10X_NUM_VDEVS - for 10.x firmware.
>
> Signed-off-by: Bartosz Markowski <[email protected]>
So what is the actual bug you are fixing? Previously with 10.x it was
possible to get only 7 VIFs, even though we advertised 8 to user space,
and with your fix we get the full 8 VIFs?
It would be good to clear have that in the commit log so that anyone can
understand what bug is fixed.
--
Kalle Valo
Bartosz Markowski <[email protected]> writes:
> On 2 June 2014 18:42, Kalle Valo <[email protected]> wrote:
>
>> So what is the actual bug you are fixing? Previously with 10.x it was
>> possible to get only 7 VIFs, even though we advertised 8 to user space,
>> and with your fix we get the full 8 VIFs?
>
> For CAC, we use one VDEV to start monitor interface. In case of 10.X
> firmware we advertise support up to 8 VAPs, but if we spent one for
> monitor interface, only 7 left. I've noticed we fail on .add_interface
> when trying to add 8th AP, here:
>
> bit = ffs(ar->free_vdev_map);
> if (bit == 0) {
> ret = -EBUSY;
> goto err;
> }
>
> and this lead me to initialization code for vdev_map
>
> ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
>
> We have an API split for main and 10.x firmware (incl. number of
> vdevs, target fw is able to handle), but here we missed this split.
This is a bit too technical. Basically I need a simple description of
the bug so that both kernel and distro maintainers can quicly understand
what this fix is about. Would this be correct:
"ath10k: fix 8th virtual AP interface with DFS
Firmware 10.x supports up to 8 virtual AP interfaces, but in a DFS
channel it was possible to create only 7 interfaces as ath10k internal
creates a monitor interface for DFS. Previous vdev map initialization
was missing enough space for 8 + 1 vdevs due to wrong define used and
that's why there was no space for 8th interface. Use the correct define
TARGET_10X_NUM_VDEVS with 10.x firmware to make it possible to create
the 8th virtual interface."
> Ben has a valid point, the TARGET_10X_NUM_VDEVS claims to be 16, so
> there's an inconsistency between what we adverts to mac in max
> interfaces, but I'm not sure if this is such a big deal.
I don't see that as a problem as long as we advertise 8 to user space.
>> It would be good to clear have that in the commit log so that anyone
>> can understand what bug is fixed.
>
> Do you want me to send a v2 with just an updated commit (better user
> impact description)? (No patch content changes)
I can update the commit log, we just need to agree on the content.
--
Kalle Valo
Bartosz Markowski <[email protected]> writes:
> Firmware 10.x supports up to 8 virtual AP interfaces
> (comparing to 7 for main firmware). Previous vdev map
> initialization was missing enough space for 8 + 1 vdevs
> (we may spent one for mac monitor), due to wrong define used.
>
> Use correct one - TARGET_10X_NUM_VDEVS - for 10.x firmware.
>
> Signed-off-by: Bartosz Markowski <[email protected]>
Thanks, applied to ath-current with this commit log:
commit dfa413de1e4388818f7dcdce0a90d6212e74895b
Author: Bartosz Markowski <[email protected]>
Date: Mon Jun 2 21:19:45 2014 +0300
ath10k: fix 8th virtual AP interface with DFS
Firmware 10.x supports up to 8 virtual AP interfaces, but in a DFS
channel it was possible to create only 7 interfaces as ath10k internal
creates a monitor interface for DFS. Previous vdev map initialization
was missing enough space for 8 + 1 vdevs due to wrong define used and
that's why there was no space for 8th interface. Use the correct define
TARGET_10X_NUM_VDEVS with 10.x firmware to make it possible to create
the 8th virtual interface.
Signed-off-by: Bartosz Markowski <[email protected]>
Signed-off-by: Kalle Valo <[email protected]>
--
Kalle Valo
On 2 June 2014 19:28, Kalle Valo <[email protected]> wrote:
> Bartosz Markowski <[email protected]> writes:
>
>> On 2 June 2014 18:42, Kalle Valo <[email protected]> wrote:
>>
>>> So what is the actual bug you are fixing? Previously with 10.x it was
>>> possible to get only 7 VIFs, even though we advertised 8 to user space,
>>> and with your fix we get the full 8 VIFs?
>>
>> For CAC, we use one VDEV to start monitor interface. In case of 10.X
>> firmware we advertise support up to 8 VAPs, but if we spent one for
>> monitor interface, only 7 left. I've noticed we fail on .add_interface
>> when trying to add 8th AP, here:
>>
>> bit = ffs(ar->free_vdev_map);
>> if (bit == 0) {
>> ret = -EBUSY;
>> goto err;
>> }
>>
>> and this lead me to initialization code for vdev_map
>>
>> ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
>>
>> We have an API split for main and 10.x firmware (incl. number of
>> vdevs, target fw is able to handle), but here we missed this split.
>
> This is a bit too technical. Basically I need a simple description of
> the bug so that both kernel and distro maintainers can quicly understand
> what this fix is about. Would this be correct:
>
> "ath10k: fix 8th virtual AP interface with DFS
>
> Firmware 10.x supports up to 8 virtual AP interfaces, but in a DFS
> channel it was possible to create only 7 interfaces as ath10k internal
> creates a monitor interface for DFS. Previous vdev map initialization
> was missing enough space for 8 + 1 vdevs due to wrong define used and
> that's why there was no space for 8th interface. Use the correct define
> TARGET_10X_NUM_VDEVS with 10.x firmware to make it possible to create
> the 8th virtual interface."
>
>> Ben has a valid point, the TARGET_10X_NUM_VDEVS claims to be 16, so
>> there's an inconsistency between what we adverts to mac in max
>> interfaces, but I'm not sure if this is such a big deal.
>
> I don't see that as a problem as long as we advertise 8 to user space.
>
>>> It would be good to clear have that in the commit log so that anyone
>>> can understand what bug is fixed.
>>
>> Do you want me to send a v2 with just an updated commit (better user
>> impact description)? (No patch content changes)
>
> I can update the commit log, we just need to agree on the content.
The one you proposed looks good.
Thanks,
Bartosz