Message-ID: <1441122832.2441.11.camel@sipsolutions.net> (sfid-20150901_175429_186133_037A6A10)
Subject: Re: [RFC 2/2] mac80211: Copy tx'ed beacons to monitor mode
From: Johannes Berg <johannes@sipsolutions.net>
To: Helmut Schaa <helmut.schaa@googlemail.com>,
	linux-wireless@vger.kernel.org
Date: Tue, 01 Sep 2015 17:53:52 +0200
In-Reply-To: <1441102364-32516-3-git-send-email-helmut.schaa@googlemail.com> (sfid-20150901_121453_804705_AA6F5ECB)
References: <1441102364-32516-1-git-send-email-helmut.schaa@googlemail.com>
	 <1441102364-32516-3-git-send-email-helmut.schaa@googlemail.com>
	 (sfid-20150901_121453_804705_AA6F5ECB)
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Sender: linux-wireless-owner@vger.kernel.org

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