Rafael Laufer wrote: > Gábor Stefanik wrote: > >> 2009/8/21 Rafael Laufer <rlaufer@xxxxxxxxxxx>: >> >> >>> Gábor Stefanik wrote: >>> >>> >>>> 2009/8/21 Rafael Laufer <rlaufer@xxxxxxxxxxx>: >>>> >>>> >>>> >>>>> Gábor Stefanik wrote: >>>>> >>>>> >>>>> >>>>>> Maybe a new IEEE80211_TX_CTL_ or IEEE80211_TX_RC_ flag will also be >>>>>> needed, so Radiotap can indicate whether rate_control_get_rate needs >>>>>> to be called. >>>>>> >>>>>> >>>>>> >>>>> ok, I am resending the patch. I included a new flag called >>>>> IEEE80211_TX_CTL_RATE_RADIOTAP to indicate if the rate has >>>>> been set in the radiotap header. If not, then the rate control >>>>> algorithm is called. >>>>> >>>>> >>>>> >>>> Isn't it easier to check whether we already have a rate configured? >>>> (info->control.rates[0].idx is set to an invalid value before the >>>> rate_control_get_rate call AFAIK, unless you set it in the radiotap >>>> decoding function before.) >>>> >>>> >>>> >>> I guess it is also possible, but in that case you rely on the assumption >>> that the rate is invalid before rate_control_get_rate(). If in the >>> future this assumption does not hold, the code will break. If, however, >>> this is always gonna be true, I can change the code to use your >>> suggestion. Personally, I prefer to use another flag so that future >>> changes do not affect this code, but let me know what is best. >>> >>> Rafael >>> >>> >>> >> Actually, that's a good point. >> >> One thing to watch out for is that the actual rate index is not the >> only thing the rate controller sets - it is also responsible for >> things like retry count & RTS/CTS usage. Those are controlled by other >> radiotap fields. So, if any of these values is unset in radiotap, you >> will need to call rate control for them, or auto-generate them in >> other ways. Otherwise you may end up with e.g. an incorrect retry >> count. >> >> >> > This is a good point. Where is this done? Is it in the driver-specific > function? > > ref->ops <http://localhost/lxr/http/ident?v=2.6.31-rc3;i=ops>->get_rate(ref->priv <http://localhost/lxr/http/ident?v=2.6.31-rc3;i=priv>, ista, priv_sta, txrc); > > > It is strange that a function called "get_rate" would also change other > fields which are at first sight do not look related to rate. Why not > call other functions for that? What is the reasoning behind this? > Different rates have different retry counts or RTS/CTS usage? > > As far as I could tell from a quick look in the code, > rate_control_get_rate only sets the fields of info->control.rates, > except for this driver-specific function. > > If this function really does other stuff, then a simple solution is to > check if the IEEE80211_TX_CTL_RATE_RADIOTAP flag is set and, in that > case, store the value of info->control.rates[0].idx before calling > rate_control_get_rate, and restoring it afterwards. Make sense? > ops, I meant ref->ops->get_rate(ref->priv, ista, priv_sta, txrc); -- 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
