Search Linux Wireless

Re: vht off-by-one nss

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 15, 2013 at 12:32:55PM +0200, Johannes Berg wrote:
> On Mon, 2013-04-15 at 12:09 +0200, Karl Beldan wrote:
> > It seems to me we have to perform one of the following, otherwise one
> > driver may set negative rate indexes and iw et.al will report VHT NSSes
> > starting at 0.
> 
> Hmm, yeah, this does seem inconsistent.
> 
> > {
> > diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> > index 0dde213..2317ca9 100644
> > --- a/include/net/mac80211.h
> > +++ b/include/net/mac80211.h
> > @@ -601,8 +601,8 @@ static inline void ieee80211_rate_set_vht(struct ieee80211_tx_rate *rate,
> >  					  u8 mcs, u8 nss)
> >  {
> >  	WARN_ON(mcs & ~0xF);
> > -	WARN_ON(nss & ~0x7);
> > -	rate->idx = (nss << 4) | mcs;
> > +	WARN_ON((nss - 1) & ~0x7);
> > +	rate->idx = ((nss - 1) << 4) | mcs;
> >  }
> >  
> >  static inline u8
> > @@ -614,7 +614,7 @@ ieee80211_rate_get_vht_mcs(const struct ieee80211_tx_rate *rate)
> >  static inline u8
> >  ieee80211_rate_get_vht_nss(const struct ieee80211_tx_rate *rate)
> >  {
> > -	return rate->idx >> 4;
> > +	return (rate->idx >> 4) + 1;
> >  }
> >  
> >  /**
> > }
> 
> I think this is nicer? But it should probably have some comments.
> 
This is what I prefer too.
Ok, I'll send a patch then.


> > diff --git a/drivers/net/wireless/iwlwifi/mvm/tx.c b/drivers/net/wireless/iwlwifi/mvm/tx.c
> > index 56df249..9631391 100644
> > --- a/drivers/net/wireless/iwlwifi/mvm/tx.c
> > +++ b/drivers/net/wireless/iwlwifi/mvm/tx.c
> > @@ -545,7 +545,7 @@ static void iwl_mvm_hwrate_to_tx_control(u32 rate_n_flags,
> >  		ieee80211_rate_set_vht(
> >  			r, rate_n_flags & RATE_VHT_MCS_RATE_CODE_MSK,
> >  			((rate_n_flags & RATE_VHT_MCS_NSS_MSK) >>
> > -						RATE_VHT_MCS_NSS_POS) + 1);
> > +						RATE_VHT_MCS_NSS_POS));
> >  		r->flags |= IEEE80211_TX_RC_VHT_MCS;
> >  	} else {
> >  		r->idx = iwl_mvm_legacy_rate_to_mac80211_idx(rate_n_flags,
> > diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> > index fdd95bd..358d93c 100644
> > --- a/net/mac80211/cfg.c
> > +++ b/net/mac80211/cfg.c
> > @@ -389,7 +389,7 @@ void sta_set_rate_info_tx(struct sta_info *sta,
> >  	} else if (rate->flags & IEEE80211_TX_RC_VHT_MCS) {
> >  		rinfo->flags |= RATE_INFO_FLAGS_VHT_MCS;
> >  		rinfo->mcs = ieee80211_rate_get_vht_mcs(rate);
> > -		rinfo->nss = ieee80211_rate_get_vht_nss(rate);
> > +		rinfo->nss = ieee80211_rate_get_vht_nss(rate) + 1;
> >  	} else {
> >  		struct ieee80211_supported_band *sband;
> >  		sband = sta->local->hw.wiphy->bands[
> > }
> 
> 
> Wouldn't this one also require an update for VHT radiotap in
> net/mac80211/rx.c around line 320 (RX_FLAG_VHT)?
> 
The radiotap field is set with ieee80211_rx_status.vht_nss, so no need.

 
Karl
--
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




[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux