| Message ID | 1424850911-21017-4-git-send-email-marek.puzyniak@tieto.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
+Arik. It'd be nice (for me anyway) if you didn't send this in a series of other patches I don't care about - I only ever saw this due to patchwork. On Wed, 2015-02-25 at 08:55 +0100, Marek Puzyniak wrote: > Currently when TDLS station in driver goes from assoc > to authorized state it can not use rate control parameters > because rate control is not initialized yet. Some drivers > require parameters already initialized by rate control when > entering authorized state. It can be done by initializing > rate control after station transition to authorized state > but before notifying driver about that. Arik, you have a similar patch handling only NSS. Does this one look fine to you, and would it solve the problem your other patch solved? johannes
On Fri, Feb 27, 2015 at 2:50 PM, Johannes Berg <johannes@sipsolutions.net> wrote: > +Arik. > > It'd be nice (for me anyway) if you didn't send this in a series of > other patches I don't care about - I only ever saw this due to > patchwork. > > On Wed, 2015-02-25 at 08:55 +0100, Marek Puzyniak wrote: >> Currently when TDLS station in driver goes from assoc >> to authorized state it can not use rate control parameters >> because rate control is not initialized yet. Some drivers >> require parameters already initialized by rate control when >> entering authorized state. It can be done by initializing >> rate control after station transition to authorized state >> but before notifyiIEEE80211_STA_ASSOCng driver about that. > > Arik, you have a similar patch handling only NSS. Does this one look > fine to you, and would it solve the problem your other patch solved? Well currently iwlmvm requires the NSS to be set before IEEE80211_STA_ASSOC (earlier), so this doesn't help directly. I could change mvm a bit to make it work, but I don't really see a good reason for it :) The patch looks good. Shouldn't introduce new issues (at least for iwlwifi). Arik
On Sun, 2015-03-01 at 10:21 +0200, Arik Nemtsov wrote: > > Arik, you have a similar patch handling only NSS. Does this one look > > fine to you, and would it solve the problem your other patch solved? > > Well currently iwlmvm requires the NSS to be set before > IEEE80211_STA_ASSOC (earlier), so this doesn't help directly. > I could change mvm a bit to make it work, but I don't really see a > good reason for it :) > > The patch looks good. Shouldn't introduce new issues (at least for iwlwifi). Ok, thanks. Do you think it would be possible to move all of this before ASSOC? (if not, why not, and how is it that we have valid NSS if we don't have the remaining data valid?) johannes
On Tue, Mar 3, 2015 at 11:18 AM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Sun, 2015-03-01 at 10:21 +0200, Arik Nemtsov wrote: > >> > Arik, you have a similar patch handling only NSS. Does this one look >> > fine to you, and would it solve the problem your other patch solved? >> >> Well currently iwlmvm requires the NSS to be set before >> IEEE80211_STA_ASSOC (earlier), so this doesn't help directly. >> I could change mvm a bit to make it work, but I don't really see a >> good reason for it :) >> >> The patch looks good. Shouldn't introduce new issues (at least for iwlwifi). > > Ok, thanks. Do you think it would be possible to move all of this before > ASSOC? We can probably move his rate_control_rate_init() to assoc, since we have this: /* * TDLS -- everything follows authorized, but * only becoming authorized is possible, not * going back */ if (set & BIT(NL80211_STA_FLAG_AUTHORIZED)) { set |= BIT(NL80211_STA_FLAG_AUTHENTICATED) | BIT(NL80211_STA_FLAG_ASSOCIATED); mask |= BIT(NL80211_STA_FLAG_AUTHENTICATED) | BIT(NL80211_STA_FLAG_ASSOCIATED); }
On Tue, 2015-03-03 at 12:02 +0200, Arik Nemtsov wrote: > On Tue, Mar 3, 2015 at 11:18 AM, Johannes Berg > <johannes@sipsolutions.net> wrote: > > On Sun, 2015-03-01 at 10:21 +0200, Arik Nemtsov wrote: > > > >> > Arik, you have a similar patch handling only NSS. Does this one look > >> > fine to you, and would it solve the problem your other patch solved? > >> > >> Well currently iwlmvm requires the NSS to be set before > >> IEEE80211_STA_ASSOC (earlier), so this doesn't help directly. > >> I could change mvm a bit to make it work, but I don't really see a > >> good reason for it :) > >> > >> The patch looks good. Shouldn't introduce new issues (at least for iwlwifi). > > > > Ok, thanks. Do you think it would be possible to move all of this before > > ASSOC? > > We can probably move his rate_control_rate_init() to assoc, since we have this: And doing so would also address the nss problem, right? IIRC that's done in the rate control init inline. johannes
On Tue, Mar 3, 2015 at 12:06 PM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Tue, 2015-03-03 at 12:02 +0200, Arik Nemtsov wrote: >> On Tue, Mar 3, 2015 at 11:18 AM, Johannes Berg >> <johannes@sipsolutions.net> wrote: >> > On Sun, 2015-03-01 at 10:21 +0200, Arik Nemtsov wrote: >> > >> >> > Arik, you have a similar patch handling only NSS. Does this one look >> >> > fine to you, and would it solve the problem your other patch solved? >> >> >> >> Well currently iwlmvm requires the NSS to be set before >> >> IEEE80211_STA_ASSOC (earlier), so this doesn't help directly. >> >> I could change mvm a bit to make it work, but I don't really see a >> >> good reason for it :) >> >> >> >> The patch looks good. Shouldn't introduce new issues (at least for iwlwifi). >> > >> > Ok, thanks. Do you think it would be possible to move all of this before >> > ASSOC? >> >> We can probably move his rate_control_rate_init() to assoc, since we have this: > > And doing so would also address the nss problem, right? IIRC that's done > in the rate control init inline. Right. Arik
On Tue, 2015-03-03 at 12:07 +0200, Arik Nemtsov wrote: > > And doing so would also address the nss problem, right? IIRC that's done > > in the rate control init inline. > > Right. Can one of you (Marek/Arik) send a combined patch then please? :) johannes
On Wed, Mar 4, 2015 at 10:16 AM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Tue, 2015-03-03 at 12:07 +0200, Arik Nemtsov wrote: > >> > And doing so would also address the nss problem, right? IIRC that's done >> > in the rate control init inline. >> >> Right. > > Can one of you (Marek/Arik) send a combined patch then please? :) I can do it, but I'll probably only get to it next week. Arik
On 4 March 2015 at 11:04, Arik Nemtsov <arik@wizery.com> wrote:
> I can do it, but I'll probably only get to it next week.
I will try to prepare patch this week, if not I will wait for Arik's
proposition.
For ath10k rate control need to be initialised before moving to
STA_AUTHORIZED, so initialising rate control before STA_ASSOC is
perfectly fine.
Marek
Johannes Berg <johannes@sipsolutions.net> writes: > +Arik. > > It'd be nice (for me anyway) if you didn't send this in a series of > other patches I don't care about - I only ever saw this due to > patchwork. Yes, mac80211 and driver patches need to be sent in separate series. I will drop this series for now, please resend ath10k patches separetely. AND please clearly document what mac80211 patches the ath10k patches depend on.
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index dd4ff36..ae24ed3 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -983,12 +983,21 @@ static int sta_apply_auth_flags(struct ieee80211_local *local, } if (mask & BIT(NL80211_STA_FLAG_AUTHORIZED)) { - if (set & BIT(NL80211_STA_FLAG_AUTHORIZED)) + if (set & BIT(NL80211_STA_FLAG_AUTHORIZED)) { + /* + * When peer becomes authorized, init rate control as + * well. Some drivers require rate control initialized + * before drv_sta_state() is called. + */ + if (test_sta_flag(sta, WLAN_STA_TDLS_PEER)) + rate_control_rate_init(sta); + ret = sta_info_move_state(sta, IEEE80211_STA_AUTHORIZED); - else if (test_sta_flag(sta, WLAN_STA_AUTHORIZED)) + } else if (test_sta_flag(sta, WLAN_STA_AUTHORIZED)) { ret = sta_info_move_state(sta, IEEE80211_STA_ASSOC); - else + } else { ret = 0; + } if (ret) return ret; } @@ -1377,11 +1386,6 @@ static int ieee80211_change_station(struct wiphy *wiphy, if (err) goto out_err; - /* When peer becomes authorized, init rate control as well */ - if (test_sta_flag(sta, WLAN_STA_TDLS_PEER) && - test_sta_flag(sta, WLAN_STA_AUTHORIZED)) - rate_control_rate_init(sta); - mutex_unlock(&local->sta_mtx); if ((sdata->vif.type == NL80211_IFTYPE_AP ||
Currently when TDLS station in driver goes from assoc to authorized state it can not use rate control parameters because rate control is not initialized yet. Some drivers require parameters already initialized by rate control when entering authorized state. It can be done by initializing rate control after station transition to authorized state but before notifying driver about that. Signed-off-by: Marek Puzyniak <marek.puzyniak@tieto.com> --- net/mac80211/cfg.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)