2015-09-01 10:14:52

by Helmut Schaa

[permalink] [raw]
Subject: [RFC 0/2] Send own beacons to monitor interfaces

Sending as RFC to gather some feedback. It can be quite useful
to see own beacons on monitor interfaces for debugging purposes.

However, since there is no tx status for beacons on most drivers
this patchset sends a copy of each beacon fetched via
ieee80211_beacon_get_tim to the monitor interfaces.

Of course, the beacon won't have proper seq numbers for example
as these are filled in in hw/fw.

Still this can be very helpful for debugging.

Helmut Schaa (2):
mac80211: Split sending tx'ed frames to monitor interfaces into its
own function
mac80211: Copy tx'ed beacons to monitor mode

net/mac80211/ieee80211_i.h | 3 ++
net/mac80211/status.c | 108 +++++++++++++++++++++++++--------------------
net/mac80211/tx.c | 10 +++++
3 files changed, 72 insertions(+), 49 deletions(-)

--
1.8.4.5



2015-09-01 10:14:54

by Helmut Schaa

[permalink] [raw]
Subject: [RFC 2/2] mac80211: Copy tx'ed beacons to monitor mode

When debugging wireless powersave issues on the AP side it's quite helpful
to see our own beacons that are transmitted by the hardware/driver. However,
this is not that easy since beacons don't pass through the regular TX queues.

Preferably drivers would call ieee80211_tx_status also for tx'ed beacons
but that's not always possible. Hence, just send a copy of each beacon
generated by ieee80211_beacon_get_tim to monitor devices when they are
getting fetched by the driver.
---
net/mac80211/tx.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 84e0e8c..f522579 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3512,6 +3512,9 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
{
struct ieee80211_mutable_offsets offs = {};
struct sk_buff *bcn = __ieee80211_beacon_get(hw, vif, &offs, false);
+ struct sk_buff *copy;
+ struct ieee80211_supported_band *sband;
+ int shift;

if (tim_offset)
*tim_offset = offs.tim_offset;
@@ -3519,6 +3522,13 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
if (tim_length)
*tim_length = offs.tim_length;

+ /* send a copy to monitor interfaces */
+ if (hw_to_local(hw)->monitors && (copy = skb_copy(bcn, GFP_ATOMIC))) {
+ shift = ieee80211_vif_get_shift(vif);
+ sband = hw->wiphy->bands[ieee80211_get_sdata_band(vif_to_sdata(vif))];
+ ieee80211_tx_monitor(hw_to_local(hw), copy, sband, 1, shift, true);
+ }
+
return bcn;
}
EXPORT_SYMBOL(ieee80211_beacon_get_tim);
--
1.8.4.5


2015-09-01 14:30:24

by Helmut Schaa

[permalink] [raw]
Subject: Re: [RFC 2/2] mac80211: Copy tx'ed beacons to monitor mode

On Tue, Sep 1, 2015 at 4:04 PM, Felix Fietkau <[email protected]> wrote:
> On 2015-09-01 12:12, Helmut Schaa wrote:
>> When debugging wireless powersave issues on the AP side it's quite helpful
>> to see our own beacons that are transmitted by the hardware/driver. However,
>> this is not that easy since beacons don't pass through the regular TX queues.
>>
>> Preferably drivers would call ieee80211_tx_status also for tx'ed beacons
>> but that's not always possible. Hence, just send a copy of each beacon
>> generated by ieee80211_beacon_get_tim to monitor devices when they are
>> getting fetched by the driver.
> How about also adding a hw flag to allow drivers to indicate that they
> submit tx status for beacons.

Makes sense, yes. Waiting for other comments before resending ...
Helmut

2015-09-01 14:04:26

by Felix Fietkau

[permalink] [raw]
Subject: Re: [RFC 2/2] mac80211: Copy tx'ed beacons to monitor mode

On 2015-09-01 12:12, Helmut Schaa wrote:
> When debugging wireless powersave issues on the AP side it's quite helpful
> to see our own beacons that are transmitted by the hardware/driver. However,
> this is not that easy since beacons don't pass through the regular TX queues.
>
> Preferably drivers would call ieee80211_tx_status also for tx'ed beacons
> but that's not always possible. Hence, just send a copy of each beacon
> generated by ieee80211_beacon_get_tim to monitor devices when they are
> getting fetched by the driver.
How about also adding a hw flag to allow drivers to indicate that they
submit tx status for beacons.

- Felix

2015-09-01 15:53:55

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC 2/2] mac80211: Copy tx'ed beacons to monitor mode

On Tue, 2015-09-01 at 12:12 +0200, Helmut Schaa wrote:
> When debugging wireless powersave issues on the AP side it's quite helpful
> to see our own beacons that are transmitted by the hardware/driver. However,
> this is not that easy since beacons don't pass through the regular TX queues.
>
> Preferably drivers would call ieee80211_tx_status also for tx'ed beacons
> but that's not always possible. Hence, just send a copy of each beacon
> generated by ieee80211_beacon_get_tim to monitor devices when they are
> getting fetched by the driver.

Generally looks fine.

> @@ -3519,6 +3522,13 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
> > > if (tim_length)
> > > > *tim_length = offs.tim_length;
>
> +> > /* send a copy to monitor interfaces */
> +> > if (hw_to_local(hw)->monitors && (copy = skb_copy(bcn, GFP_ATOMIC))) {
> +> > > shift = ieee80211_vif_get_shift(vif);
> +> > > sband = hw->wiphy->bands[ieee80211_get_sdata_band(vif_to_sdata(vif))];
> +> > > ieee80211_tx_monitor(hw_to_local(hw), copy, sband, 1, shift, true);
> +> > }


I don't really like the assignment in the if much - you could move the
variable declarations into it though.

send_to_cooked should be false, since very old versions of hostapd use
that and would really not expect the beacons there

That has me wondering if we can start to think about removing cooked
monitor mode - does anyone know if we perhaps broke it accidentally
anyway? ;-)

johannes

2015-09-01 10:14:53

by Helmut Schaa

[permalink] [raw]
Subject: [RFC 1/2] mac80211: Split sending tx'ed frames to monitor interfaces into its own function

This allows ieee80211_tx_monitor to be used directly for sending 802.11 frames
to all monitor interfaces.
---
net/mac80211/ieee80211_i.h | 3 ++
net/mac80211/status.c | 108 +++++++++++++++++++++++++--------------------
2 files changed, 62 insertions(+), 49 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 6e52659..36552de 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1641,6 +1641,9 @@ void ieee80211_purge_tx_queue(struct ieee80211_hw *hw,
struct sk_buff *
ieee80211_build_data_template(struct ieee80211_sub_if_data *sdata,
struct sk_buff *skb, u32 info_flags);
+void ieee80211_tx_monitor(struct ieee80211_local *local, struct sk_buff *skb,
+ struct ieee80211_supported_band *sband,
+ int retry_count, int shift, bool send_to_cooked);

void ieee80211_check_fast_xmit(struct sta_info *sta);
void ieee80211_check_fast_xmit_all(struct ieee80211_local *local);
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 8ba5832..98fd04c 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -668,16 +668,70 @@ void ieee80211_tx_status_noskb(struct ieee80211_hw *hw,
}
EXPORT_SYMBOL(ieee80211_tx_status_noskb);

-void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
+void ieee80211_tx_monitor(struct ieee80211_local *local, struct sk_buff *skb,
+ struct ieee80211_supported_band *sband,
+ int retry_count, int shift, bool send_to_cooked)
{
struct sk_buff *skb2;
+ struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+ struct ieee80211_sub_if_data *sdata;
+ struct net_device *prev_dev = NULL;
+ int rtap_len;
+
+ /* send frame to monitor interfaces now */
+ rtap_len = ieee80211_tx_radiotap_len(info);
+ if (WARN_ON_ONCE(skb_headroom(skb) < rtap_len)) {
+ pr_err("ieee80211_tx_status: headroom too small\n");
+ dev_kfree_skb(skb);
+ return;
+ }
+ ieee80211_add_tx_radiotap_header(local, sband, skb, retry_count,
+ rtap_len, shift);
+
+ /* XXX: is this sufficient for BPF? */
+ skb_set_mac_header(skb, 0);
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+ skb->pkt_type = PACKET_OTHERHOST;
+ skb->protocol = htons(ETH_P_802_2);
+ memset(skb->cb, 0, sizeof(skb->cb));
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(sdata, &local->interfaces, list) {
+ if (sdata->vif.type == NL80211_IFTYPE_MONITOR) {
+ if (!ieee80211_sdata_running(sdata))
+ continue;
+
+ if ((sdata->u.mntr_flags & MONITOR_FLAG_COOK_FRAMES) &&
+ !send_to_cooked)
+ continue;
+
+ if (prev_dev) {
+ skb2 = skb_clone(skb, GFP_ATOMIC);
+ if (skb2) {
+ skb2->dev = prev_dev;
+ netif_rx(skb2);
+ }
+ }
+
+ prev_dev = sdata->dev;
+ }
+ }
+ if (prev_dev) {
+ skb->dev = prev_dev;
+ netif_rx(skb);
+ skb = NULL;
+ }
+ rcu_read_unlock();
+ dev_kfree_skb(skb);
+}
+
+void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
+{
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
struct ieee80211_local *local = hw_to_local(hw);
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
__le16 fc;
struct ieee80211_supported_band *sband;
- struct ieee80211_sub_if_data *sdata;
- struct net_device *prev_dev = NULL;
struct sta_info *sta;
struct rhash_head *tmp;
int retry_count;
@@ -685,7 +739,6 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
bool send_to_cooked;
bool acked;
struct ieee80211_bar *bar;
- int rtap_len;
int shift = 0;
int tid = IEEE80211_NUM_TIDS;
const struct bucket_table *tbl;
@@ -878,51 +931,8 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
return;
}

- /* send frame to monitor interfaces now */
- rtap_len = ieee80211_tx_radiotap_len(info);
- if (WARN_ON_ONCE(skb_headroom(skb) < rtap_len)) {
- pr_err("ieee80211_tx_status: headroom too small\n");
- dev_kfree_skb(skb);
- return;
- }
- ieee80211_add_tx_radiotap_header(local, sband, skb, retry_count,
- rtap_len, shift);
-
- /* XXX: is this sufficient for BPF? */
- skb_set_mac_header(skb, 0);
- skb->ip_summed = CHECKSUM_UNNECESSARY;
- skb->pkt_type = PACKET_OTHERHOST;
- skb->protocol = htons(ETH_P_802_2);
- memset(skb->cb, 0, sizeof(skb->cb));
-
- rcu_read_lock();
- list_for_each_entry_rcu(sdata, &local->interfaces, list) {
- if (sdata->vif.type == NL80211_IFTYPE_MONITOR) {
- if (!ieee80211_sdata_running(sdata))
- continue;
-
- if ((sdata->u.mntr_flags & MONITOR_FLAG_COOK_FRAMES) &&
- !send_to_cooked)
- continue;
-
- if (prev_dev) {
- skb2 = skb_clone(skb, GFP_ATOMIC);
- if (skb2) {
- skb2->dev = prev_dev;
- netif_rx(skb2);
- }
- }
-
- prev_dev = sdata->dev;
- }
- }
- if (prev_dev) {
- skb->dev = prev_dev;
- netif_rx(skb);
- skb = NULL;
- }
- rcu_read_unlock();
- dev_kfree_skb(skb);
+ /* send to monitor interfaces */
+ ieee80211_tx_monitor(local, skb, sband, retry_count, shift, send_to_cooked);
}
EXPORT_SYMBOL(ieee80211_tx_status);

--
1.8.4.5