| Message ID | 20140106081151.GA1613@w1.fi (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Mon, 2014-01-06 at 10:11 +0200, Jouni Malinen wrote: > This allows drivers to advertise the maximum number of associated > stations they support in AP mode (including P2P GO). User space > applications can use this for cleaner way of handling the limit (e.g., > hostapd rejecting IEEE 802.11 authentication without manual > configuration of the limit) or to figure out what type of use cases can > be executed with multiple devices before trying and failing. In theory, I think this is fine. In practice, I'm not sure it really covers things too well, and some drivers like iwlwifi might end up not being able to set it. The reason is that we have a limit of ~16 stations internally, but some may be used for BSS/P2P-client interfaces and some are used internally. In addition, when TDLS will be added, concurrency might mean that we'd have to set this to a very low number although it is likely that you'd never have P2P-GO, BSS clientand TDLS all together. Since the current auth/assoc state machine is rather racy with mac80211 etc. anyway, wouldn't it be better to instead make hostapd take advantage of my kernel commit d582cffbcd04eae0bd8a83b05648bfd54bfd21c9 Author: Johannes Berg <johannes.berg@intel.com> Date: Fri Oct 26 17:53:44 2012 +0200 nl80211/mac80211: support full station state in AP mode With that, hostapd/wpa_s can add the station to the kernel, in unauthenticated state, before sending the authentication frame, and respond negatively if the addition fails. After auth frame ACK it would set to authenticated state and then after association set to associated state. This would also more accurately reflect the state to the driver, which might be helpful for some drivers. Additionally, it would go some way towards fixing the race between station addition and EAPOL RX, I believe there are some scenarios (WSC? WAPI?) where the station sends the first EAPOL(-equivalent) frame, and currently stations are added only after transmitting the assoc response, so that those frames may be dropped erroneously. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 07, 2014 at 04:54:23PM +0100, Johannes Berg wrote: > In theory, I think this is fine. In practice, I'm not sure it really > covers things too well, and some drivers like iwlwifi might end up not > being able to set it. The reason is that we have a limit of ~16 stations > internally, but some may be used for BSS/P2P-client interfaces and some > are used internally. In addition, when TDLS will be added, concurrency > might mean that we'd have to set this to a very low number although it > is likely that you'd never have P2P-GO, BSS clientand TDLS all together. I think both cases of limiting associated stations will need to be supported. It can be useful for user space to be aware of some limits and there are cases where at least 1 vs. 2 vs. 16 vs. 128 can be easily indicated even if some of the values are not accurate (e.g., that 16 may actually mean 10 in some cases, etc.). > Since the current auth/assoc state machine is rather racy with mac80211 > etc. anyway, wouldn't it be better to instead make hostapd take > advantage of my kernel commit d582cffbcd04eae0bd8a83b05648bfd54bfd21c9 > Author: Johannes Berg <johannes.berg@intel.com> > Date: Fri Oct 26 17:53:44 2012 +0200 > > nl80211/mac80211: support full station state in AP mode > > With that, hostapd/wpa_s can add the station to the kernel, in > unauthenticated state, before sending the authentication frame, and > respond negatively if the addition fails. After auth frame ACK it would > set to authenticated state and then after association set to associated > state. This would also more accurately reflect the state to the driver, > which might be helpful for some drivers. I'm trying to remember why this did not get used, but cannot really remember.. It would sound useful to support this regardless of the question of how maximum supported association state count is handled. While these do have some use cases in common, not all cases can be addressed with this, or well, cannot be addressed cleanly. I would hate to have make wpa_supplicant add a dummy station entry into the driver just to figure out whether one more station can be added.. There are use cases where an AP advertises in Beacon/Probe Response frames information about how preferable it is for stations to try to associate with it. I'd expect this area to get used more in the future, but even now, there is an example that I can give of a deployed functionality. P2P has a P2P Group Limit field which the GO uses to advertise in Beacon/Probe Response frames to indicate that no more P2P Clients can join the group. wpa_supplicant supports this functionality, but currently, requires user (or well, whoever is building the system) to configure the maximum limit. This is an extra complexity that could be easily avoided for cases where the driver were able to advertise the maximum number of associated stations. > Additionally, it would go some way towards fixing the race between > station addition and EAPOL RX, I believe there are some scenarios (WSC? > WAPI?) where the station sends the first EAPOL(-equivalent) frame, and > currently stations are added only after transmitting the assoc response, > so that those frames may be dropped erroneously. Like I said above, I see value in doing this, but that does not mean I would see that as replacing need for this patch to allow drivers to advertise the limit (if known).
On Tue, 2014-01-07 at 18:12 +0200, Jouni Malinen wrote: > On Tue, Jan 07, 2014 at 04:54:23PM +0100, Johannes Berg wrote: > > In theory, I think this is fine. In practice, I'm not sure it really > > covers things too well, and some drivers like iwlwifi might end up not > > being able to set it. The reason is that we have a limit of ~16 stations > > internally, but some may be used for BSS/P2P-client interfaces and some > > are used internally. In addition, when TDLS will be added, concurrency > > might mean that we'd have to set this to a very low number although it > > is likely that you'd never have P2P-GO, BSS clientand TDLS all together. > > I think both cases of limiting associated stations will need to be > supported. It can be useful for user space to be aware of some limits > and there are cases where at least 1 vs. 2 vs. 16 vs. 128 can be easily > indicated even if some of the values are not accurate (e.g., that 16 may > actually mean 10 in some cases, etc.). Is such a limit advertised anywhere? I can't think of a reason why a static limit would be superior to a dynamic one? Maybe if we'd want to restrict it to 8 to have enough station entries for the other use cases, but then we could do that in the code as well? Anyway, I'm not really objecting much to this, I'm just not convinced it's sufficient, and if we need a more dynamic mechanism anyway I'm not sure it really buys us much. > > nl80211/mac80211: support full station state in AP mode > I'm trying to remember why this did not get used, but cannot really > remember.. Laziness on my part. I had originally started working on this, based on some patch from TI/wizery (but which was broken in a few scenarios) and then eventually ran out of time to follow through. > It would sound useful to support this regardless of the > question of how maximum supported association state count is handled. Indeed. > While these do have some use cases in common, not all cases can be > addressed with this, or well, cannot be addressed cleanly. I would hate > to have make wpa_supplicant add a dummy station entry into the driver > just to figure out whether one more station can be added.. Why dummy? Are there cases where it needs to know whether one can be added without actually doing so? And if so, would a "heuristic" like the count really help much? > There are use cases where an AP advertises in Beacon/Probe Response > frames information about how preferable it is for stations to try to > associate with it. I'd expect this area to get used more in the future, > but even now, there is an example that I can give of a deployed > functionality. Ah, ok. > P2P has a P2P Group Limit field which the GO uses to > advertise in Beacon/Probe Response frames to indicate that no more P2P > Clients can join the group. wpa_supplicant supports this functionality, > but currently, requires user (or well, whoever is building the system) > to configure the maximum limit. This is an extra complexity that could > be easily avoided for cases where the driver were able to advertise the > maximum number of associated stations. Right, indeed. That does indicate a need for such a limit. I wonder if a more dynamic approach (such as an event saying 'I just ran out of space') would be preferable? But it'd obviously be far more complex, so may not be worth it. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 07, 2014 at 05:29:39PM +0100, Johannes Berg wrote: > Is such a limit advertised anywhere? Yes, below.. > I can't think of a reason why a > static limit would be superior to a dynamic one? Maybe if we'd want to > restrict it to 8 to have enough station entries for the other use cases, > but then we could do that in the code as well? It is not really that much of a static vs. dynamic, but more so about being able to advertise this before a station tries to start association (which, for some P2P devices, may mean having to drop another connection). > Anyway, I'm not really objecting much to this, I'm just not convinced > it's sufficient, and if we need a more dynamic mechanism anyway I'm not > sure it really buys us much. Like I said, I think both will be needed. > > P2P has a P2P Group Limit field which the GO uses to > > advertise in Beacon/Probe Response frames to indicate that no more P2P > > Clients can join the group. wpa_supplicant supports this functionality, > > but currently, requires user (or well, whoever is building the system) > > to configure the maximum limit. This is an extra complexity that could > > be easily avoided for cases where the driver were able to advertise the > > maximum number of associated stations. > > Right, indeed. That does indicate a need for such a limit. I wonder if a > more dynamic approach (such as an event saying 'I just ran out of > space') would be preferable? But it'd obviously be far more complex, so > may not be worth it. That would allow the specific (and only clearly known today, I guess) use case of P2P GO to be addressed. Or well, this would actually require even more complexity, since there would also need to be another event to indicate that more space become available (another operation stopped).. While I'm not against adding this type of functionality, I'm not sure it really would be worth the effort and it would be yet another alternative since there is already support for maximum station count in hostapd/wpa_supplicant even if that currently happens to depend on manual configuration. The case of add-sta failing will obviously need to be supported cleanly, so we (or well, I at least ;-) are pretty much stuck with having at least those two.. Adding a third one sounds a bit much.
On Tue, 2014-01-07 at 18:38 +0200, Jouni Malinen wrote: > > > P2P has a P2P Group Limit field which the GO uses to > > > advertise in Beacon/Probe Response frames to indicate that no more P2P > > > Clients can join the group. wpa_supplicant supports this functionality, > > > but currently, requires user (or well, whoever is building the system) > > > to configure the maximum limit. This is an extra complexity that could > > > be easily avoided for cases where the driver were able to advertise the > > > maximum number of associated stations. > > > > Right, indeed. That does indicate a need for such a limit. I wonder if a > > more dynamic approach (such as an event saying 'I just ran out of > > space') would be preferable? But it'd obviously be far more complex, so > > may not be worth it. > > That would allow the specific (and only clearly known today, I guess) > use case of P2P GO to be addressed. Or well, this would actually require > even more complexity, since there would also need to be another event to > indicate that more space become available (another operation stopped).. Yeah, too much complexity. > While I'm not against adding this type of functionality, I'm not sure it > really would be worth the effort and it would be yet another alternative > since there is already support for maximum station count in > hostapd/wpa_supplicant even if that currently happens to depend on > manual configuration. The case of add-sta failing will obviously need to > be supported cleanly, so we (or well, I at least ;-) are pretty much > stuck with having at least those two.. Adding a third one sounds a bit > much. I agree. I think this patch is fine, but I'd like to see this documented with the slightly relaxed semantics, i.e. documenting that it is fine in practice to set this a bit too high where it can't be known or station entries are shared. Given that is actually OK, of course, which I think we said it would be. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 3553c3a..efa7af5 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -2890,6 +2890,9 @@ struct wiphy_vendor_command { * @n_vendor_commands: number of vendor commands * @vendor_events: array of vendor events supported by the hardware * @n_vendor_events: number of vendor events + * + * @max_ap_assoc_sta: maximum number of associated stations supported in AP mode + * (including P2P GO) or 0 to indicate no such limit is advertised */ struct wiphy { /* assign these fields before you register the wiphy */ @@ -3005,6 +3008,8 @@ struct wiphy { const struct nl80211_vendor_cmd_info *vendor_events; int n_vendor_commands, n_vendor_events; + u16 max_ap_assoc_sta; + char priv[0] __aligned(NETDEV_ALIGN); }; diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index 86b8888..28d3194 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -1576,6 +1576,9 @@ enum nl80211_commands { * scans). If not included, the driver will use its default value. u32 * attribute. * + * @NL80211_ATTR_MAX_AP_ASSOC_STA: Device attribute that indicates how many + * associated stations are supported in AP mode (including P2P GO); u32. + * * @NL80211_ATTR_MAX: highest attribute number currently defined * @__NL80211_ATTR_AFTER_LAST: internal use */ @@ -1910,6 +1913,8 @@ enum nl80211_attrs { NL80211_ATTR_MIN_CHANNEL_TIME, NL80211_ATTR_MAX_CHANNEL_TIME, + NL80211_ATTR_MAX_AP_ASSOC_STA, + /* add attributes here, update the policy in nl80211.c */ __NL80211_ATTR_AFTER_LAST, diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index caf2829..2fbb84d 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -1592,6 +1592,12 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev, (nla_put_flag(msg, NL80211_ATTR_SUPPORT_5_MHZ) || nla_put_flag(msg, NL80211_ATTR_SUPPORT_10_MHZ))) goto nla_put_failure; + + if (dev->wiphy.max_ap_assoc_sta && + nla_put_u32(msg, NL80211_ATTR_MAX_AP_ASSOC_STA, + dev->wiphy.max_ap_assoc_sta)) + goto nla_put_failure; + state->split_start++; break; case 11:
This allows drivers to advertise the maximum number of associated stations they support in AP mode (including P2P GO). User space applications can use this for cleaner way of handling the limit (e.g., hostapd rejecting IEEE 802.11 authentication without manual configuration of the limit) or to figure out what type of use cases can be executed with multiple devices before trying and failing. Signed-off-by: Jouni Malinen <j@w1.fi> --- include/net/cfg80211.h | 5 +++++ include/uapi/linux/nl80211.h | 5 +++++ net/wireless/nl80211.c | 6 ++++++ 3 files changed, 16 insertions(+)