Radiotap injected rates

Radiotap injected rates

[Karl Beldan]

Hi,

Some time ago the rate selection for radiotap injected frames did not
make it essentially for mac80211 getting short in IEEE80211_TX_CTL_*s.

Would it be acceptable to replace the originally proposed:


-	if (!(tx->local->hw.flags & IEEE80211_HW_HAS_RATE_CONTROL))
+	if (!(tx->local->hw.flags & IEEE80211_HW_HAS_RATE_CONTROL) &&
+	    !(info->flags & IEEE80211_TX_CTL_NO_RC))
 		CALL_TXH(ieee80211_tx_h_rate_ctrl);

with something like :

-	if (!(tx->local->hw.flags & IEEE80211_HW_HAS_RATE_CONTROL))
+	if (!(tx->local->hw.flags & IEEE80211_HW_HAS_RATE_CONTROL) &&
+	    !(info->flags & IEEE80211_TX_CTL_INJECTED && info->control.rates[i]))
		CALL_TXH(ieee80211_tx_h_rate_ctrl);

?

 
Karl

Re: Radiotap injected rates

[Johannes Berg]

On Sun, 2013-03-17 at 22:55 +0100, Karl Beldan wrote:
> Hi,
> 
> Some time ago the rate selection for radiotap injected frames did not
> make it essentially for mac80211 getting short in IEEE80211_TX_CTL_*s.
> 
> Would it be acceptable to replace the originally proposed:
> 
> 
> -	if (!(tx->local->hw.flags & IEEE80211_HW_HAS_RATE_CONTROL))
> +	if (!(tx->local->hw.flags & IEEE80211_HW_HAS_RATE_CONTROL) &&
> +	    !(info->flags & IEEE80211_TX_CTL_NO_RC))
>  		CALL_TXH(ieee80211_tx_h_rate_ctrl);
> 
> with something like :
> 
> -	if (!(tx->local->hw.flags & IEEE80211_HW_HAS_RATE_CONTROL))
> +	if (!(tx->local->hw.flags & IEEE80211_HW_HAS_RATE_CONTROL) &&
> +	    !(info->flags & IEEE80211_TX_CTL_INJECTED && info->control.rates[i]))
> 		CALL_TXH(ieee80211_tx_h_rate_ctrl);

No, older wpa_supplicant/hostapd still use monitor interfaces for
management frame transmissions, and require rate control.

johannes

Re: Radiotap injected rates

[Karl Beldan]

On Mon, Mar 18, 2013 at 08:26:08PM +0100, Johannes Berg wrote:
> On Sun, 2013-03-17 at 22:55 +0100, Karl Beldan wrote:
> > Hi,
> > 
> > Some time ago the rate selection for radiotap injected frames did not
> > make it essentially for mac80211 getting short in IEEE80211_TX_CTL_*s.
> > 
> > Would it be acceptable to replace the originally proposed:
> > 
> > 
> > -	if (!(tx->local->hw.flags & IEEE80211_HW_HAS_RATE_CONTROL))
> > +	if (!(tx->local->hw.flags & IEEE80211_HW_HAS_RATE_CONTROL) &&
> > +	    !(info->flags & IEEE80211_TX_CTL_NO_RC))
> >  		CALL_TXH(ieee80211_tx_h_rate_ctrl);
> > 
> > with something like :
> > 
> > -	if (!(tx->local->hw.flags & IEEE80211_HW_HAS_RATE_CONTROL))
> > +	if (!(tx->local->hw.flags & IEEE80211_HW_HAS_RATE_CONTROL) &&
> > +	    !(info->flags & IEEE80211_TX_CTL_INJECTED && info->control.rates[i]))
> > 		CALL_TXH(ieee80211_tx_h_rate_ctrl);
> 
> No, older wpa_supplicant/hostapd still use monitor interfaces for
> management frame transmissions, and require rate control.
> 

Since they require rate control, I guess they don't pass any radiotap
IEEE80211_RADIOTAP_{RATE,MCS} ? In that case that wouldn't disturb them.


And, oops typo, I meant :

+	    !(info->flags & IEEE80211_TX_CTL_INJECTED && info->control.rates[0] != -1))

with info->control.rates[0] properly set in the radiotap header parsing,
instead of :

> > +	    !(info->flags & IEEE80211_TX_CTL_INJECTED && info->control.rates[i]))

If that changes anything to what you guessed.

 
Karl

Re: Radiotap injected rates

[Johannes Berg]

On Mon, 2013-03-18 at 20:54 +0100, Karl Beldan wrote:

> > No, older wpa_supplicant/hostapd still use monitor interfaces for
> > management frame transmissions, and require rate control.
> > 
> 
> Since they require rate control, I guess they don't pass any radiotap
> IEEE80211_RADIOTAP_{RATE,MCS} ? In that case that wouldn't disturb them.

Yeah, indeed, I wasn't looking right ...

> And, oops typo, I meant :
> 
> +	    !(info->flags & IEEE80211_TX_CTL_INJECTED && info->control.rates[0] != -1))
> 
> with info->control.rates[0] properly set in the radiotap header parsing,
> instead of :
> 
> > > +	    !(info->flags & IEEE80211_TX_CTL_INJECTED && info->control.rates[i]))
> 
> If that changes anything to what you guessed.

Well actually this won't work anyway because control.rates[] isn't
initialized to -1.

johannes

Re: Radiotap injected rates

[Karl Beldan]

On Fri, Mar 22, 2013 at 01:24:54PM +0100, Johannes Berg wrote:
> On Mon, 2013-03-18 at 20:54 +0100, Karl Beldan wrote:
> 
> > > No, older wpa_supplicant/hostapd still use monitor interfaces for
> > > management frame transmissions, and require rate control.
> > > 
> > 
> > Since they require rate control, I guess they don't pass any radiotap
> > IEEE80211_RADIOTAP_{RATE,MCS} ? In that case that wouldn't disturb them.
> 
> Yeah, indeed, I wasn't looking right ...
> 
> > And, oops typo, I meant :
> > 
> > +	    !(info->flags & IEEE80211_TX_CTL_INJECTED && info->control.rates[0] != -1))
> > 
1)
> > with info->control.rates[0] properly set in the radiotap header parsing,
> > instead of :
> > 
> > > > +	    !(info->flags & IEEE80211_TX_CTL_INJECTED && info->control.rates[i]))
> > 
> > If that changes anything to what you guessed.
> 
> Well actually this won't work anyway because control.rates[] isn't
> initialized to -1.
> 
Of course we would set info->control.rates[0] to -1 by default in
ieee80211_parse_tx_radiotap and set it to something else if we parse a
IEEE80211_RADIOTAP_{RATE,MCS} (i.e what I was referring to in 1b).
 
Karl

Re: Radiotap injected rates

[Johannes Berg]

On Fri, 2013-03-22 at 14:27 +0100, Karl Beldan wrote:

> > > And, oops typo, I meant :
> > > 
> > > +	    !(info->flags & IEEE80211_TX_CTL_INJECTED && info->control.rates[0] != -1))
> > > 
> > Well actually this won't work anyway because control.rates[] isn't
> > initialized to -1.
> > 
> Of course we would set info->control.rates[0] to -1 by default in
> ieee80211_parse_tx_radiotap and set it to something else if we parse a
> IEEE80211_RADIOTAP_{RATE,MCS} (i.e what I was referring to in 1b).

That still wouldn't work though, there are other paths leading to point
you're looking at. You'd have to set it always, which would probably
work but is tricky to ensure at the right points.

johannes

Re: Radiotap injected rates

[Karl Beldan]

On Fri, Mar 22, 2013 at 02:32:05PM +0100, Johannes Berg wrote:
> On Fri, 2013-03-22 at 14:27 +0100, Karl Beldan wrote:
> 
> > > > And, oops typo, I meant :
> > > > 
> > > > +	    !(info->flags & IEEE80211_TX_CTL_INJECTED && info->control.rates[0] != -1))
> > > > 
> > > Well actually this won't work anyway because control.rates[] isn't
> > > initialized to -1.
> > > 
> > Of course we would set info->control.rates[0] to -1 by default in
> > ieee80211_parse_tx_radiotap and set it to something else if we parse a
> > IEEE80211_RADIOTAP_{RATE,MCS} (i.e what I was referring to in 1b).
> 
> That still wouldn't work though, there are other paths leading to point
> you're looking at. You'd have to set it always, which would probably
> work but is tricky to ensure at the right points.
> 
The only path I could see setting IEEE80211_TX_CTL_INJECTED seems
ieee80211_monitor_start_xmit, am I missing another one ?
 
Karl

Re: Radiotap injected rates

[Johannes Berg]

On Fri, 2013-03-22 at 14:53 +0100, Karl Beldan wrote:
> On Fri, Mar 22, 2013 at 02:32:05PM +0100, Johannes Berg wrote:
> > On Fri, 2013-03-22 at 14:27 +0100, Karl Beldan wrote:
> > 
> > > > > And, oops typo, I meant :
> > > > > 
> > > > > +	    !(info->flags & IEEE80211_TX_CTL_INJECTED && info->control.rates[0] != -1))
> > > > > 
> > > > Well actually this won't work anyway because control.rates[] isn't
> > > > initialized to -1.
> > > > 
> > > Of course we would set info->control.rates[0] to -1 by default in
> > > ieee80211_parse_tx_radiotap and set it to something else if we parse a
> > > IEEE80211_RADIOTAP_{RATE,MCS} (i.e what I was referring to in 1b).
> > 
> > That still wouldn't work though, there are other paths leading to point
> > you're looking at. You'd have to set it always, which would probably
> > work but is tricky to ensure at the right points.
> > 
> The only path I could see setting IEEE80211_TX_CTL_INJECTED seems
> ieee80211_monitor_start_xmit, am I missing another one ?

No, I guess not, I'm probably not paying enough attention here ...

johannes