On Mon, 2007-12-10 at 22:22 +0100, Stefano Brivio wrote: > On Mon, 10 Dec 2007 21:51:07 +0100 > Mattias Nissler <mattias.nissler@xxxxxx> wrote: > > > > > On Mon, 2007-12-10 at 09:08 +0100, Stefano Brivio wrote: > > > On Sun, 09 Dec 2007 23:25:50 +0100 > > > Mattias Nissler <mattias.nissler@xxxxxx> wrote: > > > > > > > > static void *rate_control_pid_alloc(struct ieee80211_local *local) > > > > > { > > > > > struct rc_pid_info *pinfo; > > > > > + struct rc_pid_rateinfo *rinfo; > > > > > + struct ieee80211_hw_mode *mode; > > > > > + int i, j, tmp; > > > > > + bool s; > > > > > > > > > > pinfo = kmalloc(sizeof(*pinfo), GFP_ATOMIC); > > > > > + if (!pinfo) > > > > > + return NULL; > > > > > + > > > > > + mode = local->oper_hw_mode; > > > > > + rinfo = kmalloc(sizeof(*rinfo) * mode->num_rates, GFP_ATOMIC); > > > > > + if (!rinfo) { > > > > > + kfree(pinfo); > > > > > + return NULL; > > > > > + } > > > > > > > > What if the mode is changed? Have you checked the rate control algorithm > > > > gets reinitialized? If not, we're scheduling a crash here, when > > > > mode->num_rates changes. > > > > > > After a discussion on IRC with Michael (Wu), we came to the conclusion that > > > it doesn't make sense for mode->num_rates to change, because: > > > 1) if the AP drops supported rates, it'll drop the association as well, > > > then everything here will be destroyed and created again; > > > 2) that can be changed in userspace, but we couldn't figure out a scenario > > > where it would make any sense. Johannes, any comments? Wouldn't it make > > > sense to just forbid to change this in userspace? > > > > I don't agree. For example, what if you have some broken 802.11b only > > hardware that you desperately need to get going, but it freaks out on > > 802.11g encoded frames. Now if your AP is hostapd on a Linux machine, > > you'll want to change the mode. Hence, also the number of available > > rates change. > > Yes, but the association gets dropped. I'm not sure about the hostapd > implementation (will check), but APs drop association when they change > supported rates. So the per-sta rate control would obviously get reinitialized. > I didn't find anything clear about this in the 802.11 standard, though. Still, just disallowing mode switches seems odd to me. > > > Moreover, I think we can do better than just disallowing changes to the > > rate set, don't you think? > > Well, I still can't find an example of where this would be needed. Your > scenario looks like this: > 1) the AP STA periodically advertises supported rates; > 2) the non-AP STA supported rates are the intersection (as in set theory) > of the rates supported by the non-AP STA and the rate advertised by the AP > STA (in this case, assuming that the both the non-AP and the AP STAs > support 802.11b and g, rates supported by the non-AP STA are both CCK and > OFDM rates); > 3) let's say that a thunderstorm makes the air CCK-hostile, so we want to > reconfigure our AP STA to work with 802.11b only: the association will need > to be recreated (thus the rc algorithm gets reinitialized), and now rates > supported by the non-AP STA are the CCK rates only -> everything works as > expected and wanted. > > Anyway, it's not an issue at all to deal with rates changing in > rc80211-pid - I would just need to reallocate a struct and copy over some > data (but please think about this now: OFDM rates are the same for 802.11a > and 802.11g, but it really doesn't make any sense to assume that behaviour > at 2.4GHz is anywhere near to behaviour at 5GHz - so maybe I would just > reallocate the struct and that's it). But before than doing this, I wanted > to be sure that we aren't just hiding a bug in mac80211. You're right, we should find out what behaviour would be correct for the stack to implement. Who make a more educated statement? Johannes? Michael? - To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html