* parsing the multi-link element with STA profile wifi: mac80211: support MLO authentication/association with one link
@ 2022-09-28 15:33 Wen Gong
2022-09-28 19:28 ` Johannes Berg
0 siblings, 1 reply; 10+ messages in thread
From: Wen Gong @ 2022-09-28 15:33 UTC (permalink / raw)
To: Johannes Berg, linux-wireless; +Cc: ath11k
Hi Johannes,
May I know the status for below work which is written in the patch below?
I think it is needed in
ieee80211_assoc_config_link()/ieee80211_assoc_success(), right?
Extending that to multiple links will require
* work on parsing the multi-link element with STA profile properly,
including element fragmentation;
https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/commit/?id=81151ce462e5
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: parsing the multi-link element with STA profile wifi: mac80211: support MLO authentication/association with one link
2022-09-28 15:33 parsing the multi-link element with STA profile wifi: mac80211: support MLO authentication/association with one link Wen Gong
@ 2022-09-28 19:28 ` Johannes Berg
2022-09-29 1:58 ` Wen Gong
2023-04-16 2:10 ` Wen Gong
0 siblings, 2 replies; 10+ messages in thread
From: Johannes Berg @ 2022-09-28 19:28 UTC (permalink / raw)
To: Wen Gong, linux-wireless; +Cc: ath11k
On Wed, 2022-09-28 at 23:33 +0800, Wen Gong wrote:
> Hi Johannes,
>
> May I know the status for below work which is written in the patch below?
> I think it is needed in
> ieee80211_assoc_config_link()/ieee80211_assoc_success(), right?
>
It passed the plugfest last week ;-)
Yes, I need to get this posted ... but now I got another urgent thing to
look at, so it'll be some time.
johannes
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: parsing the multi-link element with STA profile wifi: mac80211: support MLO authentication/association with one link
2022-09-28 19:28 ` Johannes Berg
@ 2022-09-29 1:58 ` Wen Gong
2023-04-16 2:10 ` Wen Gong
1 sibling, 0 replies; 10+ messages in thread
From: Wen Gong @ 2022-09-29 1:58 UTC (permalink / raw)
To: Johannes Berg, linux-wireless; +Cc: ath11k
On 9/29/2022 3:28 AM, Johannes Berg wrote:
> On Wed, 2022-09-28 at 23:33 +0800, Wen Gong wrote:
>> Hi Johannes,
>>
>> May I know the status for below work which is written in the patch below?
>> I think it is needed in
>> ieee80211_assoc_config_link()/ieee80211_assoc_success(), right?
>>
> It passed the plugfest last week ;-)
it is good for that :) -)
> Yes, I need to get this posted ... but now I got another urgent thing to
> look at, so it'll be some time.
>
> johannes
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: parsing the multi-link element with STA profile wifi: mac80211: support MLO authentication/association with one link
2022-09-28 19:28 ` Johannes Berg
2022-09-29 1:58 ` Wen Gong
@ 2023-04-16 2:10 ` Wen Gong
2023-04-18 8:48 ` Johannes Berg
1 sibling, 1 reply; 10+ messages in thread
From: Wen Gong @ 2023-04-16 2:10 UTC (permalink / raw)
To: Johannes Berg, linux-wireless; +Cc: ath11k
On 9/29/2022 3:28 AM, Johannes Berg wrote:
> On Wed, 2022-09-28 at 23:33 +0800, Wen Gong wrote:
...
Hi Johannes,
The change below which added in ieee80211_rx_mgmt_assoc_resp() by
patch "wifi: mac80211: support MLO authentication/association with one
link (commit 81151ce462e5)"
maybe need refine to meet 2 links.
I hit issue that the BSS of the 2 link will always hold and never free.
My case is:
When connect with 2 links AP, the cfg80211_hold_bss() is called by
cfg80211_mlme_assoc()
for each BSS of the 2 links,
When asssocResp from AP is not success(such as status_code==1), the
ieee80211_link_data of 2nd link(sdata->link[link_id])
is NULL because ieee80211_assoc_success()->ieee80211_vif_update_links()
is not called.
Then struct cfg80211_rx_assoc_resp resp in cfg80211_rx_assoc_resp() and
struct cfg80211_connect_resp_params cr in __cfg80211_connect_result()
will only have the data of
the 1st link, and finally cfg80211_connect_result_release_bsses() only
call cfg80211_unhold_bss()
for the 1st link, then BSS of the 2nd link will never free because its
hold is awlays > 0 now.
I found it is not easy to refine it, so do you have any advise/idea?
for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
struct ieee80211_link_data *link;
link = sdata_dereference(sdata->link[link_id], sdata);
if (!link)
continue;
if (!assoc_data->link[link_id].bss)
continue;
resp.links[link_id].bss = assoc_data->link[link_id].bss;
resp.links[link_id].addr = link->conf->addr;
resp.links[link_id].status = assoc_data->link[link_id].status;
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: parsing the multi-link element with STA profile wifi: mac80211: support MLO authentication/association with one link
2023-04-16 2:10 ` Wen Gong
@ 2023-04-18 8:48 ` Johannes Berg
2023-04-18 9:02 ` Wen Gong
0 siblings, 1 reply; 10+ messages in thread
From: Johannes Berg @ 2023-04-18 8:48 UTC (permalink / raw)
To: Wen Gong, linux-wireless; +Cc: ath11k
Hi,
> My case is:
> When connect with 2 links AP, the cfg80211_hold_bss() is called by
> cfg80211_mlme_assoc()
> for each BSS of the 2 links,
>
> When asssocResp from AP is not success(such as status_code==1), the
> ieee80211_link_data of 2nd link(sdata->link[link_id])
> is NULL because ieee80211_assoc_success()->ieee80211_vif_update_links()
> is not called.
>
> Then struct cfg80211_rx_assoc_resp resp in cfg80211_rx_assoc_resp() and
> struct cfg80211_connect_resp_params cr in __cfg80211_connect_result()
> will only have the data of
> the 1st link, and finally cfg80211_connect_result_release_bsses() only
> call cfg80211_unhold_bss()
> for the 1st link, then BSS of the 2nd link will never free because its
> hold is awlays > 0 now.
Hah, yes, ouch.
> I found it is not easy to refine it, so do you have any advise/idea?
>
> for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
> struct ieee80211_link_data *link;
>
> link = sdata_dereference(sdata->link[link_id], sdata);
> if (!link)
> continue;
>
> if (!assoc_data->link[link_id].bss)
> continue;
>
> resp.links[link_id].bss = assoc_data->link[link_id].bss;
> resp.links[link_id].addr = link->conf->addr;
> resp.links[link_id].status = assoc_data->link[link_id].status;
>
But is it really so hard? We only need link for link->conf->addr, and we
can use assoc_data->link[link_id].addr for that instead, no? We need to
store those locally to avoid a use-after-free, but that's at most 15
addresses on the stack, which seems acceptable?
Oh and there's the uapsd stuff but that only matters in the success case
anyway. In fact I'm not even sure the address matters in the
unsuccessful case but we have it pretty easily.
johannes
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: parsing the multi-link element with STA profile wifi: mac80211: support MLO authentication/association with one link
2023-04-18 8:48 ` Johannes Berg
@ 2023-04-18 9:02 ` Wen Gong
2023-04-18 9:11 ` Johannes Berg
0 siblings, 1 reply; 10+ messages in thread
From: Wen Gong @ 2023-04-18 9:02 UTC (permalink / raw)
To: Johannes Berg, linux-wireless; +Cc: ath11k
On 4/18/2023 4:48 PM, Johannes Berg wrote:
> Hi,
>
>> My case is:
>> When connect with 2 links AP, the cfg80211_hold_bss() is called by
>> cfg80211_mlme_assoc()
>> for each BSS of the 2 links,
>>
>> When asssocResp from AP is not success(such as status_code==1), the
>> ieee80211_link_data of 2nd link(sdata->link[link_id])
>> is NULL because ieee80211_assoc_success()->ieee80211_vif_update_links()
>> is not called.
>>
>> Then struct cfg80211_rx_assoc_resp resp in cfg80211_rx_assoc_resp() and
>> struct cfg80211_connect_resp_params cr in __cfg80211_connect_result()
>> will only have the data of
>> the 1st link, and finally cfg80211_connect_result_release_bsses() only
>> call cfg80211_unhold_bss()
>> for the 1st link, then BSS of the 2nd link will never free because its
>> hold is awlays > 0 now.
> Hah, yes, ouch.
>
>> I found it is not easy to refine it, so do you have any advise/idea?
>>
>> for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
>> struct ieee80211_link_data *link;
>>
>> link = sdata_dereference(sdata->link[link_id], sdata);
>> if (!link)
>> continue;
>>
>> if (!assoc_data->link[link_id].bss)
>> continue;
>>
>> resp.links[link_id].bss = assoc_data->link[link_id].bss;
>> resp.links[link_id].addr = link->conf->addr;
>> resp.links[link_id].status = assoc_data->link[link_id].status;
>>
> But is it really so hard? We only need link for link->conf->addr, and we
> can use assoc_data->link[link_id].addr for that instead, no? We need to
> store those locally to avoid a use-after-free, but that's at most 15
> addresses on the stack, which seems acceptable?
>
> Oh and there's the uapsd stuff but that only matters in the success case
> anyway. In fact I'm not even sure the address matters in the
> unsuccessful case but we have it pretty easily.
>
> johannes
OK. So I guess you already have good way to refine it?
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: parsing the multi-link element with STA profile wifi: mac80211: support MLO authentication/association with one link
2023-04-18 9:02 ` Wen Gong
@ 2023-04-18 9:11 ` Johannes Berg
2023-05-11 11:01 ` Wen Gong
0 siblings, 1 reply; 10+ messages in thread
From: Johannes Berg @ 2023-04-18 9:11 UTC (permalink / raw)
To: Wen Gong, linux-wireless; +Cc: ath11k
On Tue, 2023-04-18 at 17:02 +0800, Wen Gong wrote:
> On 4/18/2023 4:48 PM, Johannes Berg wrote:
> > Hi,
> >
> > > My case is:
> > > When connect with 2 links AP, the cfg80211_hold_bss() is called by
> > > cfg80211_mlme_assoc()
> > > for each BSS of the 2 links,
> > >
> > > When asssocResp from AP is not success(such as status_code==1), the
> > > ieee80211_link_data of 2nd link(sdata->link[link_id])
> > > is NULL because ieee80211_assoc_success()->ieee80211_vif_update_links()
> > > is not called.
> > >
> > > Then struct cfg80211_rx_assoc_resp resp in cfg80211_rx_assoc_resp() and
> > > struct cfg80211_connect_resp_params cr in __cfg80211_connect_result()
> > > will only have the data of
> > > the 1st link, and finally cfg80211_connect_result_release_bsses() only
> > > call cfg80211_unhold_bss()
> > > for the 1st link, then BSS of the 2nd link will never free because its
> > > hold is awlays > 0 now.
> > Hah, yes, ouch.
> >
> > > I found it is not easy to refine it, so do you have any advise/idea?
> > >
> > > for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
> > > struct ieee80211_link_data *link;
> > >
> > > link = sdata_dereference(sdata->link[link_id], sdata);
> > > if (!link)
> > > continue;
> > >
> > > if (!assoc_data->link[link_id].bss)
> > > continue;
> > >
> > > resp.links[link_id].bss = assoc_data->link[link_id].bss;
> > > resp.links[link_id].addr = link->conf->addr;
> > > resp.links[link_id].status = assoc_data->link[link_id].status;
> > >
> > But is it really so hard? We only need link for link->conf->addr, and we
> > can use assoc_data->link[link_id].addr for that instead, no? We need to
> > store those locally to avoid a use-after-free, but that's at most 15
> > addresses on the stack, which seems acceptable?
> >
> > Oh and there's the uapsd stuff but that only matters in the success case
> > anyway. In fact I'm not even sure the address matters in the
> > unsuccessful case but we have it pretty easily.
> >
> > johannes
> OK. So I guess you already have good way to refine it?
>
No, not really, was just thinking out loud here :)
johannes
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: parsing the multi-link element with STA profile wifi: mac80211: support MLO authentication/association with one link
2023-04-18 9:11 ` Johannes Berg
@ 2023-05-11 11:01 ` Wen Gong
2023-07-26 6:30 ` Wen Gong
0 siblings, 1 reply; 10+ messages in thread
From: Wen Gong @ 2023-05-11 11:01 UTC (permalink / raw)
To: Johannes Berg, linux-wireless; +Cc: ath11k
On 4/18/2023 5:11 PM, Johannes Berg wrote:
> On Tue, 2023-04-18 at 17:02 +0800, Wen Gong wrote:
>> On 4/18/2023 4:48 PM, Johannes Berg wrote:
>>> Hi,
>>>
>>>> My case is:
>>>> When connect with 2 links AP, the cfg80211_hold_bss() is called by
>>>> cfg80211_mlme_assoc()
>>>> for each BSS of the 2 links,
>>>>
>>>> When asssocResp from AP is not success(such as status_code==1), the
>>>> ieee80211_link_data of 2nd link(sdata->link[link_id])
>>>> is NULL because ieee80211_assoc_success()->ieee80211_vif_update_links()
>>>> is not called.
>>>>
>>>> Then struct cfg80211_rx_assoc_resp resp in cfg80211_rx_assoc_resp() and
>>>> struct cfg80211_connect_resp_params cr in __cfg80211_connect_result()
>>>> will only have the data of
>>>> the 1st link, and finally cfg80211_connect_result_release_bsses() only
>>>> call cfg80211_unhold_bss()
>>>> for the 1st link, then BSS of the 2nd link will never free because its
>>>> hold is awlays > 0 now.
>>> Hah, yes, ouch.
>>>
>>>> I found it is not easy to refine it, so do you have any advise/idea?
>>>>
>>>> for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
>>>> struct ieee80211_link_data *link;
>>>>
>>>> link = sdata_dereference(sdata->link[link_id], sdata);
>>>> if (!link)
>>>> continue;
>>>>
>>>> if (!assoc_data->link[link_id].bss)
>>>> continue;
>>>>
>>>> resp.links[link_id].bss = assoc_data->link[link_id].bss;
>>>> resp.links[link_id].addr = link->conf->addr;
>>>> resp.links[link_id].status = assoc_data->link[link_id].status;
>>>>
>>> But is it really so hard? We only need link for link->conf->addr, and we
>>> can use assoc_data->link[link_id].addr for that instead, no? We need to
>>> store those locally to avoid a use-after-free, but that's at most 15
>>> addresses on the stack, which seems acceptable?
>>>
>>> Oh and there's the uapsd stuff but that only matters in the success case
>>> anyway. In fact I'm not even sure the address matters in the
>>> unsuccessful case but we have it pretty easily.
>>>
>>> johannes
>> OK. So I guess you already have good way to refine it?
>>
> No, not really, was just thinking out loud here :)
>
> johannes
Hi Johannes, if you have any patch to fix it, I can download and test.
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: parsing the multi-link element with STA profile wifi: mac80211: support MLO authentication/association with one link
2023-05-11 11:01 ` Wen Gong
@ 2023-07-26 6:30 ` Wen Gong
2023-08-11 9:19 ` Johannes Berg
0 siblings, 1 reply; 10+ messages in thread
From: Wen Gong @ 2023-07-26 6:30 UTC (permalink / raw)
To: Johannes Berg, linux-wireless; +Cc: ath11k
Hi Johannes,
I guess you have already fix it in some patch, right?
On 5/11/2023 7:01 PM, Wen Gong wrote:
> On 4/18/2023 5:11 PM, Johannes Berg wrote:
>> On Tue, 2023-04-18 at 17:02 +0800, Wen Gong wrote:
>>> On 4/18/2023 4:48 PM, Johannes Berg wrote:
>>>> Hi,
>>>>
>>>>> My case is:
>>>>> When connect with 2 links AP, the cfg80211_hold_bss() is called by
>>>>> cfg80211_mlme_assoc()
>>>>> for each BSS of the 2 links,
>>>>>
>>>>> When asssocResp from AP is not success(such as status_code==1), the
>>>>> ieee80211_link_data of 2nd link(sdata->link[link_id])
>>>>> is NULL because
>>>>> ieee80211_assoc_success()->ieee80211_vif_update_links()
>>>>> is not called.
>>>>>
>>>>> Then struct cfg80211_rx_assoc_resp resp in
>>>>> cfg80211_rx_assoc_resp() and
>>>>> struct cfg80211_connect_resp_params cr in __cfg80211_connect_result()
>>>>> will only have the data of
>>>>> the 1st link, and finally cfg80211_connect_result_release_bsses()
>>>>> only
>>>>> call cfg80211_unhold_bss()
>>>>> for the 1st link, then BSS of the 2nd link will never free because
>>>>> its
>>>>> hold is awlays > 0 now.
>>>> Hah, yes, ouch.
>>>>
>>>>> I found it is not easy to refine it, so do you have any advise/idea?
>>>>>
>>>>> for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
>>>>> struct ieee80211_link_data *link;
>>>>>
>>>>> link = sdata_dereference(sdata->link[link_id], sdata);
>>>>> if (!link)
>>>>> continue;
>>>>>
>>>>> if (!assoc_data->link[link_id].bss)
>>>>> continue;
>>>>>
>>>>> resp.links[link_id].bss = assoc_data->link[link_id].bss;
>>>>> resp.links[link_id].addr = link->conf->addr;
>>>>> resp.links[link_id].status =
>>>>> assoc_data->link[link_id].status;
>>>>>
>>>> But is it really so hard? We only need link for link->conf->addr,
>>>> and we
>>>> can use assoc_data->link[link_id].addr for that instead, no? We
>>>> need to
>>>> store those locally to avoid a use-after-free, but that's at most 15
>>>> addresses on the stack, which seems acceptable?
>>>>
>>>> Oh and there's the uapsd stuff but that only matters in the success
>>>> case
>>>> anyway. In fact I'm not even sure the address matters in the
>>>> unsuccessful case but we have it pretty easily.
>>>>
>>>> johannes
>>> OK. So I guess you already have good way to refine it?
>>>
>> No, not really, was just thinking out loud here :)
>>
>> johannes
>
> Hi Johannes, if you have any patch to fix it, I can download and test.
>
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: parsing the multi-link element with STA profile wifi: mac80211: support MLO authentication/association with one link
2023-07-26 6:30 ` Wen Gong
@ 2023-08-11 9:19 ` Johannes Berg
0 siblings, 0 replies; 10+ messages in thread
From: Johannes Berg @ 2023-08-11 9:19 UTC (permalink / raw)
To: Wen Gong, linux-wireless; +Cc: ath11k, Jeff Johnson
On Wed, 2023-07-26 at 14:30 +0800, Wen Gong wrote:
> Hi Johannes,
>
> I guess you have already fix it in some patch, right?
>
No, I don't. We've not seen this, it's not been a priority.
<rant>
Look, I told you how I think you can fix it _literally_ three months
ago. I was on vacation for pretty much exactly half of that time. And
during that time you ask if _I_ have a fix? What have you been doing for
the past three months that prevented you from fixing it?
Upstream is supposed to be a collaborative effort, not a place where
I/we do the work and you keep asking me to make the changes you need for
you!
Sure, it there's a bug, and we should probably be fixing it too, but if
that's not high on our list right now because we don't experience it,
then I don't see why you don't just send a patch!
I even get that you might not know whether something is a bug, and when
we determine that it is, how to fix it. But we cleared that up three
months ago, and all you've done in this thread is ask whether I have a
fix for you. Why?
johannes
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-08-11 9:19 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-28 15:33 parsing the multi-link element with STA profile wifi: mac80211: support MLO authentication/association with one link Wen Gong
2022-09-28 19:28 ` Johannes Berg
2022-09-29 1:58 ` Wen Gong
2023-04-16 2:10 ` Wen Gong
2023-04-18 8:48 ` Johannes Berg
2023-04-18 9:02 ` Wen Gong
2023-04-18 9:11 ` Johannes Berg
2023-05-11 11:01 ` Wen Gong
2023-07-26 6:30 ` Wen Gong
2023-08-11 9:19 ` Johannes Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).