From: Karl Beldan <[email protected]>
When the 1st rate control entry is a pre-HT rate we want to set
rts_cts_rate_idx "as the fastest basic rate that is not faster than the
data rate"(code comments).
But in case some bss allowed rate indexes are lower than the lowest bss
basic rate, if the rate control selects a rate among the formers for its
1st rate control entry, rts_cts_rate_idx remains 0 and is not a basic
rate index.
This commit sets rts_cts_rate_idx to the lowest bss basic rate index in
this situation.
Note that the code assumes that lowest indexes == lowest bitrates.
Signed-off-by: Karl Beldan <[email protected]>
---
net/mac80211/tx.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index aad0bf5..c93483f 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -712,19 +712,22 @@ ieee80211_tx_h_rate_ctrl(struct ieee80211_tx_data *tx)
}
/*
- * set up the RTS/CTS rate as the fastest basic rate
- * that is not faster than the data rate
+ * Set up the RTS/CTS rate as the fastest basic rate
+ * that is not faster than the data rate unless there
+ * is no basic rate slower than the data rate, in which
+ * case we pick the slowest basic rate
*
* XXX: Should this check all retry rates?
*/
if (!(info->control.rates[0].flags & IEEE80211_TX_RC_MCS)) {
- s8 baserate = 0;
+ u32 basic_rates = tx->sdata->vif.bss_conf.basic_rates;
+ s8 baserate = basic_rates ? ffs(basic_rates - 1) : 0;
rate = &sband->bitrates[info->control.rates[0].idx];
for (i = 0; i < sband->n_bitrates; i++) {
/* must be a basic rate */
- if (!(tx->sdata->vif.bss_conf.basic_rates & BIT(i)))
+ if (!(basic_rates & BIT(i)))
continue;
/* must not be faster than the data rate */
if (sband->bitrates[i].bitrate > rate->bitrate)
--
1.8.2
On Tue, Apr 09, 2013 at 01:16:55PM +0200, Johannes Berg wrote:
> On Tue, 2013-04-09 at 13:10 +0200, Karl Beldan wrote:
>
> > > Oh, oops, confused. But then you look at 9.7.6.2 "Rate selection for
> > > control frames that initiate a TXOP", which just mandates that you use
> > > any rate that the receiver supports, so why bother doing basic rates
> > > etc. at all?
> > >
> >
> > This chapter precisely, however it reads:
> > {
> > "If a control frame other than a Basic BlockAckReq or Basic BlockAck is
> > carried in a non-HT PPDU, the transmitting STA shall transmit the frame
> > using one of the rates in the BSSBasicRateSet parameter or a rate from
> > the mandatory rate set of the attached PHY if the BSSBasicRateSet is
> > empty."
> > }
>
> Oh, right. I'll review the patch again I guess :-)
>
Ok, thanks.
Karl
On Thu, Apr 11, 2013 at 12:02:31PM +0200, Johannes Berg wrote:
> On Fri, 2013-04-05 at 12:06 +0200, Karl Beldan wrote:
> > From: Karl Beldan <[email protected]>
> >
> > When the 1st rate control entry is a pre-HT rate we want to set
> > rts_cts_rate_idx "as the fastest basic rate that is not faster than the
> > data rate"(code comments).
> > But in case some bss allowed rate indexes are lower than the lowest bss
> > basic rate, if the rate control selects a rate among the formers for its
> > 1st rate control entry, rts_cts_rate_idx remains 0 and is not a basic
> > rate index.
> > This commit sets rts_cts_rate_idx to the lowest bss basic rate index in
> > this situation.
>
> Ok after reviewing this again I applied it.
>
>
> > if (!(info->control.rates[0].flags & IEEE80211_TX_RC_MCS)) {
> > - s8 baserate = 0;
> > + u32 basic_rates = tx->sdata->vif.bss_conf.basic_rates;
> > + s8 baserate = basic_rates ? ffs(basic_rates - 1) : 0;
>
> Note that this also assumes that rate 0 is a mandatory rate, which
> presumably will always be true.
>
basic_rates == 0 should not happen so it's more a defensive fallback
in case there are no basic rates programmed (which should not happen) to
prevent out of bonds accesses via rts_cts_rate_idx.
Karl
On Fri, 2013-04-05 at 12:06 +0200, Karl Beldan wrote:
> From: Karl Beldan <[email protected]>
>
> When the 1st rate control entry is a pre-HT rate we want to set
> rts_cts_rate_idx "as the fastest basic rate that is not faster than the
> data rate"(code comments).
> But in case some bss allowed rate indexes are lower than the lowest bss
> basic rate, if the rate control selects a rate among the formers for its
> 1st rate control entry, rts_cts_rate_idx remains 0 and is not a basic
> rate index.
> This commit sets rts_cts_rate_idx to the lowest bss basic rate index in
> this situation.
I guess it's a good thing you're looking at this code. However, I'm not
sure what you're doing here is correct. In this case, the PHY mandatory
rates should be used. See 9.7.6.5.2 "Selection of a rate or MCS":
To allow the transmitting STA to calculate the contents of the
Duration/ID field, a STA responding to a received frame
transmits its control response frame at a primary rate, or at an
alternate rate, or at an MCS, as specified by the following
rules:
* If a CTS or ACK control response frame is carried in a
non-HT PPDU, the primary rate is defined to be the
highest rate in the BSSBasicRateSet parameter that is
less than or equal to the rate (or non-HT reference
rate; see 9.7.9) of the previous frame. If no rate in
the BSSBasicRateSet parameter meets these conditions,
the primary rate is defined to be the highest mandatory
rate of the attached PHY that is less than or equal to
the rate (or non-HT reference rate; see 9.7.9) of the
previous frame. The STA may select an alternate rate
according to the rules in 9.7.6.5.4. The STA shall
transmit the non-HT PPDU CTS or ACK control response
frame at either the primary rate or the alternate rate,
if one exists.
johannes
On Tue, 2013-04-09 at 12:55 +0200, Karl Beldan wrote:
> On Tue, Apr 09, 2013 at 12:29:16PM +0200, Johannes Berg wrote:
> > On Fri, 2013-04-05 at 12:06 +0200, Karl Beldan wrote:
> > > From: Karl Beldan <[email protected]>
> > >
> > > When the 1st rate control entry is a pre-HT rate we want to set
> > > rts_cts_rate_idx "as the fastest basic rate that is not faster than the
> > > data rate"(code comments).
> > > But in case some bss allowed rate indexes are lower than the lowest bss
> > > basic rate, if the rate control selects a rate among the formers for its
> > > 1st rate control entry, rts_cts_rate_idx remains 0 and is not a basic
> > > rate index.
> > > This commit sets rts_cts_rate_idx to the lowest bss basic rate index in
> > > this situation.
> >
> > I guess it's a good thing you're looking at this code. However, I'm not
> > sure what you're doing here is correct. In this case, the PHY mandatory
> > rates should be used. See 9.7.6.5.2 "Selection of a rate or MCS":
> >
> Thanks for looking at this.
>
> You are quoting the chapter for "control _response_ frames" which does
> not apply here (even CTS-to-self are not control response frames).
Oh, oops, confused. But then you look at 9.7.6.2 "Rate selection for
control frames that initiate a TXOP", which just mandates that you use
any rate that the receiver supports, so why bother doing basic rates
etc. at all?
johannes
On Tue, Apr 09, 2013 at 12:29:16PM +0200, Johannes Berg wrote:
> On Fri, 2013-04-05 at 12:06 +0200, Karl Beldan wrote:
> > From: Karl Beldan <[email protected]>
> >
> > When the 1st rate control entry is a pre-HT rate we want to set
> > rts_cts_rate_idx "as the fastest basic rate that is not faster than the
> > data rate"(code comments).
> > But in case some bss allowed rate indexes are lower than the lowest bss
> > basic rate, if the rate control selects a rate among the formers for its
> > 1st rate control entry, rts_cts_rate_idx remains 0 and is not a basic
> > rate index.
> > This commit sets rts_cts_rate_idx to the lowest bss basic rate index in
> > this situation.
>
> I guess it's a good thing you're looking at this code. However, I'm not
> sure what you're doing here is correct. In this case, the PHY mandatory
> rates should be used. See 9.7.6.5.2 "Selection of a rate or MCS":
>
Thanks for looking at this.
You are quoting the chapter for "control _response_ frames" which does
not apply here (even CTS-to-self are not control response frames).
Karl
On Fri, 2013-04-05 at 12:06 +0200, Karl Beldan wrote:
> From: Karl Beldan <[email protected]>
>
> When the 1st rate control entry is a pre-HT rate we want to set
> rts_cts_rate_idx "as the fastest basic rate that is not faster than the
> data rate"(code comments).
> But in case some bss allowed rate indexes are lower than the lowest bss
> basic rate, if the rate control selects a rate among the formers for its
> 1st rate control entry, rts_cts_rate_idx remains 0 and is not a basic
> rate index.
> This commit sets rts_cts_rate_idx to the lowest bss basic rate index in
> this situation.
Ok after reviewing this again I applied it.
> if (!(info->control.rates[0].flags & IEEE80211_TX_RC_MCS)) {
> - s8 baserate = 0;
> + u32 basic_rates = tx->sdata->vif.bss_conf.basic_rates;
> + s8 baserate = basic_rates ? ffs(basic_rates - 1) : 0;
Note that this also assumes that rate 0 is a mandatory rate, which
presumably will always be true.
johannes
On Tue, 2013-04-09 at 13:10 +0200, Karl Beldan wrote:
> > Oh, oops, confused. But then you look at 9.7.6.2 "Rate selection for
> > control frames that initiate a TXOP", which just mandates that you use
> > any rate that the receiver supports, so why bother doing basic rates
> > etc. at all?
> >
>
> This chapter precisely, however it reads:
> {
> "If a control frame other than a Basic BlockAckReq or Basic BlockAck is
> carried in a non-HT PPDU, the transmitting STA shall transmit the frame
> using one of the rates in the BSSBasicRateSet parameter or a rate from
> the mandatory rate set of the attached PHY if the BSSBasicRateSet is
> empty."
> }
Oh, right. I'll review the patch again I guess :-)
johannes
On Tue, Apr 09, 2013 at 01:05:12PM +0200, Johannes Berg wrote:
> On Tue, 2013-04-09 at 12:55 +0200, Karl Beldan wrote:
> > On Tue, Apr 09, 2013 at 12:29:16PM +0200, Johannes Berg wrote:
> > > On Fri, 2013-04-05 at 12:06 +0200, Karl Beldan wrote:
> > > > From: Karl Beldan <[email protected]>
> > > >
> > > > When the 1st rate control entry is a pre-HT rate we want to set
> > > > rts_cts_rate_idx "as the fastest basic rate that is not faster than the
> > > > data rate"(code comments).
> > > > But in case some bss allowed rate indexes are lower than the lowest bss
> > > > basic rate, if the rate control selects a rate among the formers for its
> > > > 1st rate control entry, rts_cts_rate_idx remains 0 and is not a basic
> > > > rate index.
> > > > This commit sets rts_cts_rate_idx to the lowest bss basic rate index in
> > > > this situation.
> > >
> > > I guess it's a good thing you're looking at this code. However, I'm not
> > > sure what you're doing here is correct. In this case, the PHY mandatory
> > > rates should be used. See 9.7.6.5.2 "Selection of a rate or MCS":
> > >
> > Thanks for looking at this.
> >
> > You are quoting the chapter for "control _response_ frames" which does
> > not apply here (even CTS-to-self are not control response frames).
>
> Oh, oops, confused. But then you look at 9.7.6.2 "Rate selection for
> control frames that initiate a TXOP", which just mandates that you use
> any rate that the receiver supports, so why bother doing basic rates
> etc. at all?
>
This chapter precisely, however it reads:
{
"If a control frame other than a Basic BlockAckReq or Basic BlockAck is
carried in a non-HT PPDU, the transmitting STA shall transmit the frame
using one of the rates in the BSSBasicRateSet parameter or a rate from
the mandatory rate set of the attached PHY if the BSSBasicRateSet is
empty."
}
Karl