Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1418463 > unrolled thread

Re: brcmfmac: rework function picking free BSS index

Started byKalle Valo <kvalo@codeaurora.org>
First post2016-06-09 18:20 +0200
Last post2016-06-09 18:50 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: brcmfmac: rework function picking free BSS index Kalle Valo <kvalo@codeaurora.org> - 2016-06-09 18:20 +0200
    Re: brcmfmac: rework function picking free BSS index Kalle Valo <kvalo@codeaurora.org> - 2016-06-09 18:40 +0200
      Re: brcmfmac: rework function picking free BSS index Rafał Miłecki <zajec5@gmail.com> - 2016-06-09 18:50 +0200

#1418463 — Re: brcmfmac: rework function picking free BSS index

FromKalle Valo <kvalo@codeaurora.org>
Date2016-06-09 18:20 +0200
SubjectRe: brcmfmac: rework function picking free BSS index
Message-ID<rIaQy-hP-25@gated-at.bofh.it>
Rafał Miłecki wrote:
> The old implementation was overcomplicated and slightly bugged in some
> corner cases.
> 
> Consider following state of BSS-es (limited to 6 for simplification):
> drvr->iflist[0]: { bsscfgidx:0, ndev->name:wlan1, }
> drvr->iflist[1]:  (null)
> drvr->iflist[2]: { bsscfgidx:2, ndev->name:wlan1-1, }
> drvr->iflist[3]: { bsscfgidx:3, ndev->name:wlan1-2, }
> drvr->iflist[4]:  (null)
> drvr->iflist[5]:  (null)
> In such case the next AP interface should bsscfgidx 4 (we don't use 1 as
> it's reserved for P2P).
> 
> With old code the loop iterations were following:
> [ifidx = 0] [bsscfgidx = 2] [highest = 2]
> [ifidx = 1] [bsscfgidx = 2] [highest = 2] available = true
> [ifidx = 2] [bsscfgidx = 2] [highest = 2] bsscfgidx = highest + 1
> [ifidx = 3] [bsscfgidx = 3] [highest = 2] bsscfgidx = highest + 1
> [ifidx = 4] [bsscfgidx = 3] [highest = 2] available = true
> [ifidx = 5] [bsscfgidx = 3] [highest = 2] available = true
> There were 2 obvious problems:
> 1) Having empty BSS at index 1 was resulting in available being always
>    set to true, even if we would run out of BSS-es.
> 2) Calculated bsscfgidx was invalid (3 instead of 4) resulting in driver
>    not being able to create the 4th AP interface.
> 
> New code is simpler, placed in file where it's really used, handles
> running out of free BSS-es and allows using 4 interfaces at the same
> time. It also looks for the first free BSS instead of one after the last
> in use. It works well with current driver (which doesn't allow deleting
> interfaces) and should be future proof (if we ever allow deleting).
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>

Thanks, 1 patch applied to wireless-drivers-next.git:

d02fb8f14b2d brcmfmac: rework function picking free BSS index

-- 
Sent by pwcli
https://patchwork.kernel.org/patch/9136421/

[toc] | [next] | [standalone]


#1418479

FromKalle Valo <kvalo@codeaurora.org>
Date2016-06-09 18:40 +0200
Message-ID<rIb9U-pw-19@gated-at.bofh.it>
In reply to#1418463
Kalle Valo <kvalo@codeaurora.org> writes:

> Rafał Miłecki wrote:
>> The old implementation was overcomplicated and slightly bugged in some
>> corner cases.
>> 
>> Consider following state of BSS-es (limited to 6 for simplification):
>> drvr->iflist[0]: { bsscfgidx:0, ndev->name:wlan1, }
>> drvr->iflist[1]:  (null)
>> drvr->iflist[2]: { bsscfgidx:2, ndev->name:wlan1-1, }
>> drvr->iflist[3]: { bsscfgidx:3, ndev->name:wlan1-2, }
>> drvr->iflist[4]:  (null)
>> drvr->iflist[5]:  (null)
>> In such case the next AP interface should bsscfgidx 4 (we don't use 1 as
>> it's reserved for P2P).
>> 
>> With old code the loop iterations were following:
>> [ifidx = 0] [bsscfgidx = 2] [highest = 2]
>> [ifidx = 1] [bsscfgidx = 2] [highest = 2] available = true
>> [ifidx = 2] [bsscfgidx = 2] [highest = 2] bsscfgidx = highest + 1
>> [ifidx = 3] [bsscfgidx = 3] [highest = 2] bsscfgidx = highest + 1
>> [ifidx = 4] [bsscfgidx = 3] [highest = 2] available = true
>> [ifidx = 5] [bsscfgidx = 3] [highest = 2] available = true
>> There were 2 obvious problems:
>> 1) Having empty BSS at index 1 was resulting in available being always
>>    set to true, even if we would run out of BSS-es.
>> 2) Calculated bsscfgidx was invalid (3 instead of 4) resulting in driver
>>    not being able to create the 4th AP interface.
>> 
>> New code is simpler, placed in file where it's really used, handles
>> running out of free BSS-es and allows using 4 interfaces at the same
>> time. It also looks for the first free BSS instead of one after the last
>> in use. It works well with current driver (which doesn't allow deleting
>> interfaces) and should be future proof (if we ever allow deleting).
>> 
>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>
> Thanks, 1 patch applied to wireless-drivers-next.git:
>
> d02fb8f14b2d brcmfmac: rework function picking free BSS index

Now that patchwork.kernel.org is finally updated and I fixed my python
script, this is the first time I applied a patch using UTF-8 charset
directly from patchwork. As it's not exactly easy to get UTF-8 support
right in python, I would very much like to hear if there any issues
either with the commit below or the email above.

https://git.kernel.org/cgit/linux/kernel/git/kvalo/wireless-drivers-next.git/commit/?id=d02fb8f14b2d

-- 
Kalle Valo

[toc] | [prev] | [next] | [standalone]


#1418481

FromRafał Miłecki <zajec5@gmail.com>
Date2016-06-09 18:50 +0200
Message-ID<rIbjz-vA-5@gated-at.bofh.it>
In reply to#1418479
On 9 June 2016 at 18:30, Kalle Valo <kvalo@codeaurora.org> wrote:
> Kalle Valo <kvalo@codeaurora.org> writes:
>
>> Rafał Miłecki wrote:
>>> The old implementation was overcomplicated and slightly bugged in some
>>> corner cases.
>>>
>>> Consider following state of BSS-es (limited to 6 for simplification):
>>> drvr->iflist[0]: { bsscfgidx:0, ndev->name:wlan1, }
>>> drvr->iflist[1]:  (null)
>>> drvr->iflist[2]: { bsscfgidx:2, ndev->name:wlan1-1, }
>>> drvr->iflist[3]: { bsscfgidx:3, ndev->name:wlan1-2, }
>>> drvr->iflist[4]:  (null)
>>> drvr->iflist[5]:  (null)
>>> In such case the next AP interface should bsscfgidx 4 (we don't use 1 as
>>> it's reserved for P2P).
>>>
>>> With old code the loop iterations were following:
>>> [ifidx = 0] [bsscfgidx = 2] [highest = 2]
>>> [ifidx = 1] [bsscfgidx = 2] [highest = 2] available = true
>>> [ifidx = 2] [bsscfgidx = 2] [highest = 2] bsscfgidx = highest + 1
>>> [ifidx = 3] [bsscfgidx = 3] [highest = 2] bsscfgidx = highest + 1
>>> [ifidx = 4] [bsscfgidx = 3] [highest = 2] available = true
>>> [ifidx = 5] [bsscfgidx = 3] [highest = 2] available = true
>>> There were 2 obvious problems:
>>> 1) Having empty BSS at index 1 was resulting in available being always
>>>    set to true, even if we would run out of BSS-es.
>>> 2) Calculated bsscfgidx was invalid (3 instead of 4) resulting in driver
>>>    not being able to create the 4th AP interface.
>>>
>>> New code is simpler, placed in file where it's really used, handles
>>> running out of free BSS-es and allows using 4 interfaces at the same
>>> time. It also looks for the first free BSS instead of one after the last
>>> in use. It works well with current driver (which doesn't allow deleting
>>> interfaces) and should be future proof (if we ever allow deleting).
>>>
>>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>>
>> Thanks, 1 patch applied to wireless-drivers-next.git:
>>
>> d02fb8f14b2d brcmfmac: rework function picking free BSS index
>
> Now that patchwork.kernel.org is finally updated and I fixed my python
> script, this is the first time I applied a patch using UTF-8 charset
> directly from patchwork. As it's not exactly easy to get UTF-8 support
> right in python, I would very much like to hear if there any issues
> either with the commit below or the email above.
>
> https://git.kernel.org/cgit/linux/kernel/git/kvalo/wireless-drivers-next.git/commit/?id=d02fb8f14b2d

Look fine, thanks!

-- 
Rafał

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web