| Message ID | 20230214132009.1011452-1-dev.mbornand@systemb.ch (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [v4] Set ssid when authenticating | expand |
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 <kernel@zoddo.fr> > Fixes: 7b0a0e3c3a88260b6fcb017e49f198463aa62ed1 Please add a test description to the fixes tag > Cc: linux-wireless@vger.kernel.org > Cc: stable@vger.kernel.org > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216711 > Signed-off-by: Marc Bornand <dev.mbornand@systemb.ch> > --- > 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 <kernel@zoddo.fr> > > 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 <kernel@zoddo.fr> > > > 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 <dev.mbornand@systemb.ch> 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 <kernel@zoddo.fr> > Fixes: 7b0a0e3c3a88260b6fcb017e49f198463aa62ed1 > Cc: linux-wireless@vger.kernel.org > Cc: stable@vger.kernel.org > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216711 > Signed-off-by: Marc Bornand <dev.mbornand@systemb.ch> > --- > 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.
On Wed, Feb 15, 2023 at 07:35:09AM +0200, Kalle Valo wrote: > Marc Bornand <dev.mbornand@systemb.ch> 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 <kernel@zoddo.fr> > > Fixes: 7b0a0e3c3a88260b6fcb017e49f198463aa62ed1 > > Cc: linux-wireless@vger.kernel.org > > Cc: stable@vger.kernel.org > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216711 > > Signed-off-by: Marc Bornand <dev.mbornand@systemb.ch> > > --- > > 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 <dev.mbornand@systemb.ch> writes: > On Wed, Feb 15, 2023 at 07:35:09AM +0200, Kalle Valo wrote: >> Marc Bornand <dev.mbornand@systemb.ch> 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 <kernel@zoddo.fr> >> > Fixes: 7b0a0e3c3a88260b6fcb017e49f198463aa62ed1 >> > Cc: linux-wireless@vger.kernel.org >> > Cc: stable@vger.kernel.org >> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216711 >> > Signed-off-by: Marc Bornand <dev.mbornand@systemb.ch> >> > --- >> > 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.
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)
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 <kernel@zoddo.fr> Fixes: 7b0a0e3c3a88260b6fcb017e49f198463aa62ed1 Cc: linux-wireless@vger.kernel.org Cc: stable@vger.kernel.org Link: https://bugzilla.kernel.org/show_bug.cgi?id=216711 Signed-off-by: Marc Bornand <dev.mbornand@systemb.ch> --- net/wireless/sme.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) -- 2.39.1