Bug 2831

Created attachment 2966 [details]
test case to reproduce

I do not think this can work, we need a re-association if we change what is supported by a station, otherwise the access point has no way to see that capabilities have changed.

I will work on a patch later this week to re-associate when changing the channel width at runtime.

I did not manage to do the same in Linux, such operation seems not permitted at runtime. In some drivers code, I saw that if the NSS and/or the MCS mask is changed, a reassociation is done. I will assume for now that we also do a reassociation if channel width is changed if it is fine for all (and I will extend to a change of the NSS).

Created attachment 3010 [details]
proposed solution

Here is a proposal, which triggers a reassociation if PHY capabilities did change.

Any feedback on the proposal?

(In reply to sebastien.deronne from comment #4)
> I did not manage to do the same in Linux, such operation seems not permitted
> at runtime. In some drivers code, I saw that if the NSS and/or the MCS mask
> is changed, a reassociation is done. I will assume for now that we also do a
> reassociation if channel width is changed if it is fine for all (and I will
> extend to a change of the NSS).

Thank you for this work. Out of curiosity, which drivers did you looked at? ath9k, ath10k?

Which implications does a re-association has for higher layers? Do we get a problem with routing for example?

(In reply to Michael Rademacher from comment #7)
> (In reply to sebastien.deronne from comment #4)
> > I did not manage to do the same in Linux, such operation seems not permitted
> > at runtime. In some drivers code, I saw that if the NSS and/or the MCS mask
> > is changed, a reassociation is done. I will assume for now that we also do a
> > reassociation if channel width is changed if it is fine for all (and I will
> > extend to a change of the NSS).
> 
> Thank you for this work. Out of curiosity, which drivers did you looked at?
> ath9k, ath10k?

If I remember well, I was looking into ath10k at that moment.

> 
> Which implications does a re-association has for higher layers? Do we get a
> problem with routing for example?

This should be quite transparent for higher layers in my opinion.

I am planning to push this one if no further comments.

(In reply to sebastien.deronne from comment #9)
> I am planning to push this one if no further comments.

I agree with the approach; two possible suggestions for improvement:

1) the new capability has no test or example code; at a minimum, can a run-time channel switch example be added and tested for regression (ideally, a new test case)
2) there is a lot of new logic in ap-wifi-mac.cc that lacks any NS_LOGging

(In reply to Tom Henderson from comment #10)
> (In reply to sebastien.deronne from comment #9)
> > I am planning to push this one if no further comments.
> 
> I agree with the approach; two possible suggestions for improvement:
> 
> 1) the new capability has no test or example code; at a minimum, can a
> run-time channel switch example be added and tested for regression (ideally,
> a new test case)

Agreed, I will add a test case.

> 2) there is a lot of new logic in ap-wifi-mac.cc that lacks any NS_LOGging

I realize a lot of logging are missing already (no indication that we receive an assoc req, a beacon, ...). Is this what you mean here (indication that a re-assoc req/response was received)?

(In reply to sebastien.deronne from comment #11)
> (In reply to Tom Henderson from comment #10)
> > (In reply to sebastien.deronne from comment #9)
> > > I am planning to push this one if no further comments.
> > 
> > I agree with the approach; two possible suggestions for improvement:
> > 
> > 1) the new capability has no test or example code; at a minimum, can a
> > run-time channel switch example be added and tested for regression (ideally,
> > a new test case)
> 
> Agreed, I will add a test case.
> 
> > 2) there is a lot of new logic in ap-wifi-mac.cc that lacks any NS_LOGging
> 
> I realize a lot of logging are missing already (no indication that we
> receive an assoc req, a beacon, ...). Is this what you mean here (indication
> that a re-assoc req/response was received)?


I suggest that at least req/response received or sent, and in the code, whenever you set 'problem' to true, you put a NS_LOG_DEBUG ("") to briefly summarize the conditions that make it true (i.e. which branch of code caused it to be set).

I am working on an updated patch.

Created attachment 3054 [details]
updated patch

I updated the patch with additional logging + a test case

I am planning to push this.

(In reply to sebastien.deronne from comment #15)
> I am planning to push this.

OK with me.

Pushed in changeset 13312:7865825507eb