| Message ID | 1444746990-17526-1-git-send-email-sven@narfation.org (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Johannes Berg |
| Headers | show |
On Tuesday 13 October 2015 16:36:30 Sven Eckelmann wrote: > Drivers/devices without their own rate control algorithm can get the > information what rates they should use from either the radiotap header of > injected frames or from the rate control algorithm. But the parsing of the > legacy rate information from the radiotap header was removed in commit > e6a9854b05c1 ("mac80211/drivers: rewrite the rate control API"). > > The removal of this feature heavily reduced the usefulness of frame > injection when wanting to simulate specific transmission behavior. Having > rate parsing together with MCS rates and retry support allows a fine > grained selection of the tx behavior of injected frames for these kind of > tests. > > Signed-off-by: Sven Eckelmann <sven@narfation.org> > Cc: Simon Wunderlich <sw@simonwunderlich.de> > Cc: Johannes Berg <johannes@sipsolutions.net> > --- > > We have required the support for rate parsing (legacy + HT) for some tests. > It is already known that this may not be the best place to set this flag > (IEEE80211_TX_CTRL_RATE_INJECT) but the main flags field is already full. > There is also the problem that powersave could overwrite the rate control > fields - so either we disable powersave queueing or find a different > solution. > > But maybe this feature is also not wanted anymore in the mac80211 driver. > > Thanks, > Sven Sorry, this was meant as RFC but accidentally dropped the [RFC] when I regenerated the patch after fixing a part of the message. Kind regards, Sven
On Tue, 2015-10-13 at 16:36 +0200, Sven Eckelmann wrote: > > The removal of this feature heavily reduced the usefulness of frame > injection when wanting to simulate specific transmission behavior. > Having > rate parsing together with MCS rates and retry support allows a fine > grained selection of the tx behavior of injected frames for these > kind of > tests. Assuming you have hardware to do it, but that requirement seems fair. > We have required the support for rate parsing (legacy + HT) for some > tests. > It is already known that this may not be the best place to set this > flag > (IEEE80211_TX_CTRL_RATE_INJECT) but the main flags field is already > full. It seems you could perhaps put that into struct ieee80211_tx_data::flags? Or is it required somewhere outside the mac80211 processing? Otherwise I think the place is fine? What issue do you have with it? > There is also the problem that powersave could overwrite the rate > control > fields - so either we disable powersave queueing or find a different > solution. I have no idea what you mean? The flag - as you have it now - should be preserved, no? Perhaps if you moved the flag into tx_data then it wouldn't be, and that's a good argument for not moving it? > But maybe this feature is also not wanted anymore in the mac80211 > driver. Well, I'm open to adding the code if you need it. Could consider VHT as well, I guess. Adding the check to the fast-xmit path seems neither right nor necessary though, since that shouldn't get invoked for injection to start with? 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 Thursday 26 November 2015 18:25:26 Johannes Berg wrote: > > We have required the support for rate parsing (legacy + HT) for some > > tests. > > It is already known that this may not be the best place to set this > > flag > > (IEEE80211_TX_CTRL_RATE_INJECT) but the main flags field is already > > full. > > It seems you could perhaps put that into struct > ieee80211_tx_data::flags? Or is it required somewhere outside the > mac80211 processing? The flag itself has to be set when the radiotap information is available+parsed and when the actual rate information calculation should happen. Afaik the ieee80211_tx_data is always a local variable on the stack. Either from ieee80211_tx_prepare_skb, ieee80211_tx, ieee80211_xmit_fast or ieee80211_get_buffered_bc. But the parsing of the radiotap header happens before that in ieee80211_monitor_start_xmit. And after that it calls ieee80211_xmit -> ieee80211_tx. So tx_data is definitely not available when the radiotap header is parsed. > Otherwise I think the place is fine? What issue do you have with it? > > > There is also the problem that powersave could overwrite the rate > > control > > fields - so either we disable powersave queueing or find a different > > solution. > > I have no idea what you mean? The flag - as you have it now - should be > preserved, no? Perhaps if you moved the flag into tx_data then it > wouldn't be, and that's a good argument for not moving it? I was under the impression that the PS could write in part of the ieee80211_tx_info union which would be in conflict with the control part. But I've just rechecked the source and could not find anything like that. > > But maybe this feature is also not wanted anymore in the mac80211 > > driver. > > Well, I'm open to adding the code if you need it. Could consider VHT as > well, I guess. I personally don't have VHT hardware which would support injected frames with self chosen rates. So I cannot test VHT rate injection. > Adding the check to the fast-xmit path seems neither right nor > necessary though, since that shouldn't get invoked for injection to > start with? Ok, will be removed and I resent it as v2. Kind regards, Sven
On Mon, 2016-01-25 at 13:59 +0100, Sven Eckelmann wrote: > > The flag itself has to be set when the radiotap information is > available+parsed and when the actual rate information calculation > should > happen. > > Afaik the ieee80211_tx_data is always a local variable on the stack. > Either > from ieee80211_tx_prepare_skb, ieee80211_tx, ieee80211_xmit_fast or > ieee80211_get_buffered_bc. But the parsing of the radiotap header > happens > before that in ieee80211_monitor_start_xmit. And after that it calls > ieee80211_xmit -> ieee80211_tx. So tx_data is definitely not > available when > the radiotap header is parsed. Fair enough. We could put that data elsewhere, but it's probably not worth it for a simple flag. 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
diff --git a/Documentation/networking/mac80211-injection.txt b/Documentation/networking/mac80211-injection.txt index 3a93007..ec8f934 100644 --- a/Documentation/networking/mac80211-injection.txt +++ b/Documentation/networking/mac80211-injection.txt @@ -28,6 +28,23 @@ radiotap headers and used to control injection: IEEE80211_RADIOTAP_F_TX_NOACK: frame should be sent without waiting for an ACK even if it is a unicast frame + * IEEE80211_RADIOTAP_RATE + + legacy rate for the transmission (only for devices without own rate control) + + * IEEE80211_RADIOTAP_MCS + + HT rate for the transmission (only for devices without own rate control). + Also some flags are parsed + + IEEE80211_TX_RC_SHORT_GI: use short guard interval + IEEE80211_TX_RC_40_MHZ_WIDTH: send in HT40 mode + + * IEEE80211_RADIOTAP_DATA_RETRIES + + number of retries when either IEEE80211_RADIOTAP_RATE or + IEEE80211_RADIOTAP_MCS was used + The injection code can also skip all other currently defined radiotap fields facilitating replay of captured radiotap headers directly. diff --git a/include/net/mac80211.h b/include/net/mac80211.h index 4ec6fed..e1bf37d 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -689,12 +689,14 @@ enum mac80211_tx_info_flags { * protocol frame (e.g. EAP) * @IEEE80211_TX_CTRL_PS_RESPONSE: This frame is a response to a poll * frame (PS-Poll or uAPSD). + * @IEEE80211_TX_CTRL_RATE_INJECT: This frame is injected with rate information * * These flags are used in tx_info->control.flags. */ enum mac80211_tx_control_flags { IEEE80211_TX_CTRL_PORT_CTRL_PROTO = BIT(0), IEEE80211_TX_CTRL_PS_RESPONSE = BIT(1), + IEEE80211_TX_CTRL_RATE_INJECT = BIT(2), }; /* diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 464ba1a..58298c4 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -1474,7 +1474,8 @@ static int invoke_tx_handlers(struct ieee80211_tx_data *tx) CALL_TXH(ieee80211_tx_h_ps_buf); CALL_TXH(ieee80211_tx_h_check_control_port_protocol); CALL_TXH(ieee80211_tx_h_select_key); - if (!ieee80211_hw_check(&tx->local->hw, HAS_RATE_CONTROL)) + if (!ieee80211_hw_check(&tx->local->hw, HAS_RATE_CONTROL) && + !(info->control.flags & IEEE80211_TX_CTRL_RATE_INJECT)) CALL_TXH(ieee80211_tx_h_rate_ctrl); if (unlikely(info->flags & IEEE80211_TX_INTFL_RETRANSMISSION)) { @@ -1663,15 +1664,24 @@ void ieee80211_xmit(struct ieee80211_sub_if_data *sdata, ieee80211_tx(sdata, sta, skb, false); } -static bool ieee80211_parse_tx_radiotap(struct sk_buff *skb) +static bool ieee80211_parse_tx_radiotap(struct sk_buff *skb, + struct ieee80211_local *local) { struct ieee80211_radiotap_iterator iterator; struct ieee80211_radiotap_header *rthdr = (struct ieee80211_radiotap_header *) skb->data; struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); + struct ieee80211_supported_band *sband = + local->hw.wiphy->bands[info->band]; int ret = ieee80211_radiotap_iterator_init(&iterator, rthdr, skb->len, NULL); u16 txflags; + u16 rate = 0; + bool rate_found = false; + u8 rate_retries = 0; + u16 rate_flags = 0; + u8 mcs_known, mcs_flags; + int i; info->flags |= IEEE80211_TX_INTFL_DONT_ENCRYPT | IEEE80211_TX_CTL_DONTFRAG; @@ -1722,6 +1732,35 @@ static bool ieee80211_parse_tx_radiotap(struct sk_buff *skb) info->flags |= IEEE80211_TX_CTL_NO_ACK; break; + case IEEE80211_RADIOTAP_RATE: + rate = *iterator.this_arg; + rate_flags = 0; + rate_found = true; + break; + + case IEEE80211_RADIOTAP_DATA_RETRIES: + rate_retries = *iterator.this_arg; + break; + + case IEEE80211_RADIOTAP_MCS: + mcs_known = iterator.this_arg[0]; + mcs_flags = iterator.this_arg[1]; + if (!(mcs_known & IEEE80211_RADIOTAP_MCS_HAVE_MCS)) + break; + + rate_found = true; + rate = iterator.this_arg[2]; + rate_flags = IEEE80211_TX_RC_MCS; + + if (mcs_known & IEEE80211_RADIOTAP_MCS_HAVE_GI && + mcs_flags & IEEE80211_RADIOTAP_MCS_SGI) + rate_flags |= IEEE80211_TX_RC_SHORT_GI; + + if (mcs_known & IEEE80211_RADIOTAP_MCS_HAVE_BW && + mcs_flags & IEEE80211_RADIOTAP_MCS_BW_40) + rate_flags |= IEEE80211_TX_RC_40_MHZ_WIDTH; + break; + /* * Please update the file * Documentation/networking/mac80211-injection.txt @@ -1736,6 +1775,34 @@ static bool ieee80211_parse_tx_radiotap(struct sk_buff *skb) if (ret != -ENOENT) /* ie, if we didn't simply run out of fields */ return false; + if (rate_found) { + info->control.flags |= IEEE80211_TX_CTRL_RATE_INJECT; + + for (i = 0; i < IEEE80211_TX_MAX_RATES; i++) { + info->control.rates[i].idx = -1; + info->control.rates[i].flags = 0; + info->control.rates[i].count = 0; + } + + if (rate_flags & IEEE80211_TX_RC_MCS) { + info->control.rates[0].idx = rate; + } else { + for (i = 0; i < sband->n_bitrates; i++) { + if (rate * 5 != sband->bitrates[i].bitrate) + continue; + + info->control.rates[0].idx = i; + break; + } + } + + info->control.rates[0].flags = rate_flags; + if (rate_retries + 1 > local->hw.max_rate_tries) + info->control.rates[0].count = local->hw.max_rate_tries; + else + info->control.rates[0].count = rate_retries + 1; + } + /* * remove the radiotap header * iterator->_max_length was sanity-checked against @@ -1817,7 +1884,7 @@ netdev_tx_t ieee80211_monitor_start_xmit(struct sk_buff *skb, IEEE80211_TX_CTL_INJECTED; /* process and remove the injection radiotap header */ - if (!ieee80211_parse_tx_radiotap(skb)) + if (!ieee80211_parse_tx_radiotap(skb, local)) goto fail; rcu_read_lock(); @@ -2794,7 +2861,8 @@ static bool ieee80211_xmit_fast(struct ieee80211_sub_if_data *sdata, if (fast_tx->key) info->control.hw_key = &fast_tx->key->conf; - if (!ieee80211_hw_check(&local->hw, HAS_RATE_CONTROL)) { + if (!ieee80211_hw_check(&local->hw, HAS_RATE_CONTROL) && + !(info->control.flags & IEEE80211_TX_CTRL_RATE_INJECT)) { tx.skb = skb; r = ieee80211_tx_h_rate_ctrl(&tx); skb = tx.skb;
Drivers/devices without their own rate control algorithm can get the information what rates they should use from either the radiotap header of injected frames or from the rate control algorithm. But the parsing of the legacy rate information from the radiotap header was removed in commit e6a9854b05c1 ("mac80211/drivers: rewrite the rate control API"). The removal of this feature heavily reduced the usefulness of frame injection when wanting to simulate specific transmission behavior. Having rate parsing together with MCS rates and retry support allows a fine grained selection of the tx behavior of injected frames for these kind of tests. Signed-off-by: Sven Eckelmann <sven@narfation.org> Cc: Simon Wunderlich <sw@simonwunderlich.de> Cc: Johannes Berg <johannes@sipsolutions.net> --- We have required the support for rate parsing (legacy + HT) for some tests. It is already known that this may not be the best place to set this flag (IEEE80211_TX_CTRL_RATE_INJECT) but the main flags field is already full. There is also the problem that powersave could overwrite the rate control fields - so either we disable powersave queueing or find a different solution. But maybe this feature is also not wanted anymore in the mac80211 driver. Thanks, Sven --- Documentation/networking/mac80211-injection.txt | 17 ++++++ include/net/mac80211.h | 2 + net/mac80211/tx.c | 76 +++++++++++++++++++++++-- 3 files changed, 91 insertions(+), 4 deletions(-)