changes since v3:
- add missing NULL check
- add missing break
changes since v2:
- The code was tottaly rewritten based on the disscution of the
v2 patch.
- the ssid is set in __cfg80211_connect_result() and only if the ssid is
not already set.
- Do not add an other ssid reset path since it is already done in
__cfg80211_disconnected()
When a connexion was established without going through
NL80211_CMD_CONNECT, the ssid was never set in the wireless_dev struct.
Now we set it in __cfg80211_connect_result() when it is not already set.
Reported-by: Yohan Prod'homme <[email protected]>
Fixes: 7b0a0e3c3a88260b6fcb017e49f198463aa62ed1
Cc: [email protected]
Cc: [email protected]
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216711
Signed-off-by: Marc Bornand <[email protected]>
---
net/wireless/sme.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index 4b5b6ee0fe01..b552d6c20a26 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -723,6 +723,7 @@ void __cfg80211_connect_result(struct net_device *dev,
bool wextev)
{
struct wireless_dev *wdev = dev->ieee80211_ptr;
+ const struct element *ssid;
const struct element *country_elem = NULL;
const u8 *country_data;
u8 country_datalen;
@@ -883,6 +884,22 @@ void __cfg80211_connect_result(struct net_device *dev,
country_data, country_datalen);
kfree(country_data);
+ if (wdev->u.client.ssid_len == 0) {
+ rcu_read_lock();
+ for_each_valid_link(cr, link) {
+ ssid = ieee80211_bss_get_elem(cr->links[link].bss,
+ WLAN_EID_SSID);
+
+ if (!ssid || ssid->datalen == 0)
+ continue;
+
+ memcpy(wdev->u.client.ssid, ssid->data, ssid->datalen);
+ wdev->u.client.ssid_len = ssid->datalen;
+ break;
+ }
+ rcu_read_unlock();
+ }
+
return;
out:
for_each_valid_link(cr, link)
--
2.39.1
On 2/14/23 16:20, Marc Bornand wrote:
> changes since v3:
> - add missing NULL check
> - add missing break
>
> changes since v2:
> - The code was tottaly rewritten based on the disscution of the
> v2 patch.
> - the ssid is set in __cfg80211_connect_result() and only if the ssid is
> not already set.
> - Do not add an other ssid reset path since it is already done in
> __cfg80211_disconnected()
>
> When a connexion was established without going through
> NL80211_CMD_CONNECT, the ssid was never set in the wireless_dev struct.
> Now we set it in __cfg80211_connect_result() when it is not already set.
A couple of small nits
>
> Reported-by: Yohan Prod'homme <[email protected]>
> Fixes: 7b0a0e3c3a88260b6fcb017e49f198463aa62ed1
Please add a test description to the fixes tag
> Cc: [email protected]
> Cc: [email protected]
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216711
> Signed-off-by: Marc Bornand <[email protected]>
> ---
> net/wireless/sme.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/net/wireless/sme.c b/net/wireless/sme.c
> index 4b5b6ee0fe01..b552d6c20a26 100644
> --- a/net/wireless/sme.c
> +++ b/net/wireless/sme.c
> @@ -723,6 +723,7 @@ void __cfg80211_connect_result(struct net_device *dev,
> bool wextev)
> {
> struct wireless_dev *wdev = dev->ieee80211_ptr;
> + const struct element *ssid;
Please use reverse xmas tree
> const struct element *country_elem = NULL;
> const u8 *country_data;
> u8 country_datalen;
> @@ -883,6 +884,22 @@ void __cfg80211_connect_result(struct net_device *dev,
> country_data, country_datalen);
> kfree(country_data);
>
> + if (wdev->u.client.ssid_len == 0) {
> + rcu_read_lock();
> + for_each_valid_link(cr, link) {
> + ssid = ieee80211_bss_get_elem(cr->links[link].bss,
> + WLAN_EID_SSID);
> +
> + if (!ssid || ssid->datalen == 0)
> + continue;
> +
> + memcpy(wdev->u.client.ssid, ssid->data, ssid->datalen);
> + wdev->u.client.ssid_len = ssid->datalen;
> + break;
> + }
> + rcu_read_unlock();
> + }
> +
> return;
> out:
> for_each_valid_link(cr, link)
> --
> 2.39.1
>
>
On Tue, Feb 14, 2023 at 04:27:27PM +0300, Denis Kirjanov wrote:
>
>
> On 2/14/23 16:20, Marc Bornand wrote:
> > changes since v3:
> > - add missing NULL check
> > - add missing break
> >
> > changes since v2:
> > - The code was tottaly rewritten based on the disscution of the
> > v2 patch.
> > - the ssid is set in __cfg80211_connect_result() and only if the ssid is
> > not already set.
> > - Do not add an other ssid reset path since it is already done in
> > __cfg80211_disconnected()
> >
> > When a connexion was established without going through
> > NL80211_CMD_CONNECT, the ssid was never set in the wireless_dev struct.
> > Now we set it in __cfg80211_connect_result() when it is not already set.
>
> A couple of small nits
>
> >
> > Reported-by: Yohan Prod'homme <[email protected]>
> > Fixes: 7b0a0e3c3a88260b6fcb017e49f198463aa62ed1
> Please add a test description to the fixes tag
What do you mean by "test description" ?
Marc
On Tue, Feb 14, 2023 at 10:01:35PM +0000, Marc Bornand wrote:
> On Tue, Feb 14, 2023 at 04:27:27PM +0300, Denis Kirjanov wrote:
> > On 2/14/23 16:20, Marc Bornand wrote:
> > > changes since v3:
> > > - add missing NULL check
> > > - add missing break
> > >
> > > changes since v2:
> > > - The code was tottaly rewritten based on the disscution of the
> > > v2 patch.
> > > - the ssid is set in __cfg80211_connect_result() and only if the ssid is
> > > not already set.
> > > - Do not add an other ssid reset path since it is already done in
> > > __cfg80211_disconnected()
> > >
> > > When a connexion was established without going through
> > > NL80211_CMD_CONNECT, the ssid was never set in the wireless_dev struct.
> > > Now we set it in __cfg80211_connect_result() when it is not already set.
> >
> > A couple of small nits
> >
> > >
> > > Reported-by: Yohan Prod'homme <[email protected]>
> > > Fixes: 7b0a0e3c3a88260b6fcb017e49f198463aa62ed1
> > Please add a test description to the fixes tag
>
> What do you mean by "test description" ?
s/s/x/ ;)
Fixes: 7b0a0e3c3a88 ("wifi: cfg80211: do some rework towards MLO link APIs")
Cheers,
Conor.
Marc Bornand <[email protected]> writes:
> changes since v3:
> - add missing NULL check
> - add missing break
>
> changes since v2:
> - The code was tottaly rewritten based on the disscution of the
> v2 patch.
> - the ssid is set in __cfg80211_connect_result() and only if the ssid is
> not already set.
> - Do not add an other ssid reset path since it is already done in
> __cfg80211_disconnected()
>
> When a connexion was established without going through
> NL80211_CMD_CONNECT, the ssid was never set in the wireless_dev struct.
> Now we set it in __cfg80211_connect_result() when it is not already set.
>
> Reported-by: Yohan Prod'homme <[email protected]>
> Fixes: 7b0a0e3c3a88260b6fcb017e49f198463aa62ed1
> Cc: [email protected]
> Cc: [email protected]
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216711
> Signed-off-by: Marc Bornand <[email protected]>
> ---
> net/wireless/sme.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
The change log ("changes since v3" etc) should be after "---" line and
the title should start with "wifi: cfg80211:". Please read the wiki link
below.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Wed, Feb 15, 2023 at 07:35:09AM +0200, Kalle Valo wrote:
> Marc Bornand <[email protected]> writes:
>
> > changes since v3:
> > - add missing NULL check
> > - add missing break
> >
> > changes since v2:
> > - The code was tottaly rewritten based on the disscution of the
> > v2 patch.
> > - the ssid is set in __cfg80211_connect_result() and only if the ssid is
> > not already set.
> > - Do not add an other ssid reset path since it is already done in
> > __cfg80211_disconnected()
> >
> > When a connexion was established without going through
> > NL80211_CMD_CONNECT, the ssid was never set in the wireless_dev struct.
> > Now we set it in __cfg80211_connect_result() when it is not already set.
> >
> > Reported-by: Yohan Prod'homme <[email protected]>
> > Fixes: 7b0a0e3c3a88260b6fcb017e49f198463aa62ed1
> > Cc: [email protected]
> > Cc: [email protected]
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216711
> > Signed-off-by: Marc Bornand <[email protected]>
> > ---
> > net/wireless/sme.c | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
>
> The change log ("changes since v3" etc) should be after "---" line and
Does it need another "---" after the change log?
something like:
"---"
"changes since v3:"
"(CHANGES)"
"---"
> the title should start with "wifi: cfg80211:". Please read the wiki link
> below.
Should i start with the version 1 with the new title?
and since i am already changing the title, the following might better
discribe the patch, or should i keep the old title after the ":" ?
[PATCH wireless] wifi: cfg80211: Set SSID if it is not already set
>
> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Marc Bornand <[email protected]> writes:
> On Wed, Feb 15, 2023 at 07:35:09AM +0200, Kalle Valo wrote:
>> Marc Bornand <[email protected]> writes:
>>
>> > changes since v3:
>> > - add missing NULL check
>> > - add missing break
>> >
>> > changes since v2:
>> > - The code was tottaly rewritten based on the disscution of the
>> > v2 patch.
>> > - the ssid is set in __cfg80211_connect_result() and only if the ssid is
>> > not already set.
>> > - Do not add an other ssid reset path since it is already done in
>> > __cfg80211_disconnected()
>> >
>> > When a connexion was established without going through
>> > NL80211_CMD_CONNECT, the ssid was never set in the wireless_dev struct.
>> > Now we set it in __cfg80211_connect_result() when it is not already set.
>> >
>> > Reported-by: Yohan Prod'homme <[email protected]>
>> > Fixes: 7b0a0e3c3a88260b6fcb017e49f198463aa62ed1
>> > Cc: [email protected]
>> > Cc: [email protected]
>> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216711
>> > Signed-off-by: Marc Bornand <[email protected]>
>> > ---
>> > net/wireless/sme.c | 17 +++++++++++++++++
>> > 1 file changed, 17 insertions(+)
>>
>> The change log ("changes since v3" etc) should be after "---" line and
>
> Does it need another "---" after the change log?
> something like:
>
> "---"
> "changes since v3:"
> "(CHANGES)"
> "---"
No need to add a second "---" line.
>> the title should start with "wifi: cfg80211:". Please read the wiki link
>> below.
>
> Should i start with the version 1 with the new title?
If you reset the version number that might confuse the reviewers, so my
recommendation is to use v5 in the next version. That makes it more
obvious that there are earlier versions available.
> and since i am already changing the title, the following might better
> discribe the patch, or should i keep the old title after the ":" ?
>
> [PATCH wireless] wifi: cfg80211: Set SSID if it is not already set
Changing the title is fine.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches