| Message ID | 1412690018-18429-1-git-send-email-karl.beldan@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Tue, 2014-10-07 at 15:53 +0200, Karl Beldan wrote:
> From: Karl Beldan <karl.beldan@rivierawaves.com>
Is that really trivial? It seems to have some impact on the code, but I
can't right now say exactly what the impact is. Can you describe it and
say whether I should add it to mac80211 or mac80211-next? For "trivial"
I'd probably say mac80211-next, but this might be more important than
that?
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 Thu, Oct 09, 2014 at 11:06:26AM +0200, Johannes Berg wrote: > On Tue, 2014-10-07 at 15:53 +0200, Karl Beldan wrote: > > From: Karl Beldan <karl.beldan@rivierawaves.com> > > Is that really trivial? It seems to have some impact on the code, but I > can't right now say exactly what the impact is. Can you describe it and > say whether I should add it to mac80211 or mac80211-next? For "trivial" > I'd probably say mac80211-next, but this might be more important than > that? > The typo is clearly showing but the faulty behavior clearly demands more detail indeed. It affects non-(V)HT rates and can lead to selecting an rts_cts rate that is not a basic rate or way superior to the reference rate (ATM rates[0] used for the 1st attempt of the protected frame data). E.g, assuming the drivers register growing (bitrate) sorted ieee80211_rate tables, having : - rates[0].idx == d'2 and basic_rates == b'10100 will select rts_cts idx b'10011 & ~d'(BIT(2)-1), i.e. 1, likewise - rates[0].idx == d'2 and basic_rates == b'10001 will select rts_cts idx b'10000 The first is not a basic rate and the second is > rates[0]. I hope it clarifies things enough. Karl -- 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 Thu, 2014-10-09 at 16:06 +0200, Karl Beldan wrote: > On Thu, Oct 09, 2014 at 11:06:26AM +0200, Johannes Berg wrote: > > On Tue, 2014-10-07 at 15:53 +0200, Karl Beldan wrote: > > > From: Karl Beldan <karl.beldan@rivierawaves.com> > > > > Is that really trivial? It seems to have some impact on the code, but I > > can't right now say exactly what the impact is. Can you describe it and > > say whether I should add it to mac80211 or mac80211-next? For "trivial" > > I'd probably say mac80211-next, but this might be more important than > > that? > > > The typo is clearly showing but the faulty behavior clearly demands more > detail indeed. > > It affects non-(V)HT rates and can lead to selecting an rts_cts rate > that is not a basic rate or way superior to the reference rate (ATM > rates[0] used for the 1st attempt of the protected frame data). > E.g, assuming the drivers register growing (bitrate) sorted > ieee80211_rate tables, having : > - rates[0].idx == d'2 and basic_rates == b'10100 > will select rts_cts idx b'10011 & ~d'(BIT(2)-1), i.e. 1, likewise > - rates[0].idx == d'2 and basic_rates == b'10001 > will select rts_cts idx b'10000 > The first is not a basic rate and the second is > rates[0]. > > I hope it clarifies things enough. Well, I'm still not sure which tree I should put it in, I guess? 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 Thu, Oct 09, 2014 at 04:15:03PM +0200, Johannes Berg wrote: > On Thu, 2014-10-09 at 16:06 +0200, Karl Beldan wrote: > > On Thu, Oct 09, 2014 at 11:06:26AM +0200, Johannes Berg wrote: > > > On Tue, 2014-10-07 at 15:53 +0200, Karl Beldan wrote: > > > > From: Karl Beldan <karl.beldan@rivierawaves.com> > > > > > > Is that really trivial? It seems to have some impact on the code, but I > > > can't right now say exactly what the impact is. Can you describe it and > > > say whether I should add it to mac80211 or mac80211-next? For "trivial" > > > I'd probably say mac80211-next, but this might be more important than > > > that? > > > > > The typo is clearly showing but the faulty behavior clearly demands more > > detail indeed. > > > > It affects non-(V)HT rates and can lead to selecting an rts_cts rate > > that is not a basic rate or way superior to the reference rate (ATM > > rates[0] used for the 1st attempt of the protected frame data). > > E.g, assuming the drivers register growing (bitrate) sorted > > ieee80211_rate tables, having : > > - rates[0].idx == d'2 and basic_rates == b'10100 > > will select rts_cts idx b'10011 & ~d'(BIT(2)-1), i.e. 1, likewise > > - rates[0].idx == d'2 and basic_rates == b'10001 > > will select rts_cts idx b'10000 > > The first is not a basic rate and the second is > rates[0]. > > > > I hope it clarifies things enough. > > Well, I'm still not sure which tree I should put it in, I guess? > All I can say is that nor this faulty behavior nor the correspond fix are likely to cause a crash (we always tx registered rates). Karl -- 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 Thu, 2014-10-09 at 16:45 +0200, Karl Beldan wrote: > > > The typo is clearly showing but the faulty behavior clearly demands more > > > detail indeed. > > > > > > It affects non-(V)HT rates and can lead to selecting an rts_cts rate > > > that is not a basic rate or way superior to the reference rate (ATM > > > rates[0] used for the 1st attempt of the protected frame data). > > > E.g, assuming the drivers register growing (bitrate) sorted > > > ieee80211_rate tables, having : > > > - rates[0].idx == d'2 and basic_rates == b'10100 > > > will select rts_cts idx b'10011 & ~d'(BIT(2)-1), i.e. 1, likewise > > > - rates[0].idx == d'2 and basic_rates == b'10001 > > > will select rts_cts idx b'10000 > > > The first is not a basic rate and the second is > rates[0]. > > > > > > I hope it clarifies things enough. > > > > Well, I'm still not sure which tree I should put it in, I guess? > > > > All I can say is that nor this faulty behavior nor the correspond fix > are likely to cause a crash (we always tx registered rates). But it's still pretty misbehaving, no? That IMHO warrants fixing and even cc stable. 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 Thu, Oct 09, 2014 at 05:22:21PM +0200, Johannes Berg wrote: > On Thu, 2014-10-09 at 16:45 +0200, Karl Beldan wrote: > > > > > The typo is clearly showing but the faulty behavior clearly demands more > > > > detail indeed. > > > > > > > > It affects non-(V)HT rates and can lead to selecting an rts_cts rate > > > > that is not a basic rate or way superior to the reference rate (ATM > > > > rates[0] used for the 1st attempt of the protected frame data). > > > > E.g, assuming the drivers register growing (bitrate) sorted > > > > ieee80211_rate tables, having : > > > > - rates[0].idx == d'2 and basic_rates == b'10100 > > > > will select rts_cts idx b'10011 & ~d'(BIT(2)-1), i.e. 1, likewise > > > > - rates[0].idx == d'2 and basic_rates == b'10001 > > > > will select rts_cts idx b'10000 > > > > The first is not a basic rate and the second is > rates[0]. > > > > > > > > I hope it clarifies things enough. > > > > > > Well, I'm still not sure which tree I should put it in, I guess? > > > > > > > All I can say is that nor this faulty behavior nor the correspond fix > > are likely to cause a crash (we always tx registered rates). > > But it's still pretty misbehaving, no? That IMHO warrants fixing and > even cc stable. > > johannes You'd like me to reword with these details in a v2 and cc stable or ? Karl -- 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 Thu, 2014-10-09 at 18:28 +0200, Karl Beldan wrote: > > But it's still pretty misbehaving, no? That IMHO warrants fixing and > > even cc stable. > You'd like me to reword with these details in a v2 and cc stable or ? Yes please, if you don't mind. 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 Thu, Oct 09, 2014 at 05:22:21PM +0200, Johannes Berg wrote: > On Thu, 2014-10-09 at 16:45 +0200, Karl Beldan wrote: > > > > > The typo is clearly showing but the faulty behavior clearly demands more > > > > detail indeed. > > > > > > > > It affects non-(V)HT rates and can lead to selecting an rts_cts rate > > > > that is not a basic rate or way superior to the reference rate (ATM > > > > rates[0] used for the 1st attempt of the protected frame data). > > > > E.g, assuming the drivers register growing (bitrate) sorted > > > > ieee80211_rate tables, having : > > > > - rates[0].idx == d'2 and basic_rates == b'10100 > > > > will select rts_cts idx b'10011 & ~d'(BIT(2)-1), i.e. 1, likewise > > > > - rates[0].idx == d'2 and basic_rates == b'10001 > > > > will select rts_cts idx b'10000 > > > > The first is not a basic rate and the second is > rates[0]. > > > > > > > > I hope it clarifies things enough. > > > > > > Well, I'm still not sure which tree I should put it in, I guess? > > > > > > > All I can say is that nor this faulty behavior nor the correspond fix > > are likely to cause a crash (we always tx registered rates). > > But it's still pretty misbehaving, no? That IMHO warrants fixing and > even cc stable. > Also, for the record, ATM we have only one rts_cts rate. A way to handle every retry rates is still missing, along with (V)HT rates. Karl -- 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/net/mac80211/rate.c b/net/mac80211/rate.c index 8fdadfd..6081329 100644 --- a/net/mac80211/rate.c +++ b/net/mac80211/rate.c @@ -448,7 +448,7 @@ static void rate_fixup_ratelist(struct ieee80211_vif *vif, */ if (!(rates[0].flags & IEEE80211_TX_RC_MCS)) { u32 basic_rates = vif->bss_conf.basic_rates; - s8 baserate = basic_rates ? ffs(basic_rates - 1) : 0; + s8 baserate = basic_rates ? ffs(basic_rates) - 1 : 0; rate = &sband->bitrates[rates[0].idx];