Re: [PATCH v4 2/3] mac80211/minstrel_ht: use the new rate control API

[Karl Beldan <karl.beldan@gmail.com>]

On Sun, Apr 21, 2013 at 09:57:23AM +0200, Felix Fietkau wrote:
> On 2013-04-20 7:44 PM, Karl Beldan wrote:
> > On Fri, Apr 19, 2013 at 05:28:48PM +0200, Felix Fietkau wrote:
> >> @@ -846,6 +856,8 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
> >>  
> >>  	msp->is_ht = true;
> >>  	memset(mi, 0, sizeof(*mi));
> >> +
> >> +	mi->sta = sta;
> >>  	mi->stats_update = jiffies;
> >>  
> >>  	ack_dur = ieee80211_frame_duration(sband->band, 10, 60, 1, 1);
> >> @@ -907,7 +919,6 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
> >>  	if (!n_supported)
> >>  		goto use_legacy;
> >>  
> >> -	/* init {mi,mi->groups[*]}->{max_tp_rate,max_tp_rate2,max_prob_rate} */
> >>  	minstrel_ht_update_stats(mp, mi);
> >>  
> > This time you kept minstrel_ht_update_stats but you got rid of the call
> > to minstrel_ht_update_rates you previously had.
> > If you don't minstrel_ht_update_rates, ieee80211_tx_h_rate_ctrl will
> > TX_DROP the frame until minstrel's get_sample_rate returns != -1,
> > Maybe you could pair update_stats with ht_update_rates, e.g move/call
> > ht_update_stats in ht_update_rates ?
> I really should have re-read this in a distraction free environment :)
> Looking at the code again, I see why I kept them separated. In some
> cases the rates change (when quickly switching to a different MCS group
> when the rates fail) but the statistics are not updated yet.
> Calling both functions here is the right thing to do.
> 
Good, note that minstrel ht_update_rates != its ht_rate_update and that
ht_update_stats will always be called whenever ht_update_rates is, but I
too prefer keeping them separated.
Other than that and the 'low' rates issue I sent I think I'm good.

 
Karl