linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Simon Wunderlich <simon.wunderlich@s2003.tu-chemnitz.de>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: Simon Wunderlich <simon.wunderlich@s2003.tu-chemnitz.de>,
	linux-wireless@vger.kernel.org, linville@tuxdriver.com,
	ath9k-devel@lists.ath9k.org, rodrigue@qca.qualcomm.com,
	zefir.kurtisi@neratec.com, adrian@freebsd.org,
	kgiori@qca.qualcomm.com, mathias.kretschmer@fokus.fraunhofer.de,
	Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Subject: Re: [RFC 1/3] nl80211: add spec scan flag
Date: Wed, 28 Nov 2012 16:19:16 +0100	[thread overview]
Message-ID: <20121128151916.GA6175@pandem0nium> (raw)
In-Reply-To: <1354106616.9345.13.camel@jlt4.sipsolutions.net>

[-- Attachment #1: Type: text/plain, Size: 4569 bytes --]

Hello Johannes,

On Wed, Nov 28, 2012 at 01:43:36PM +0100, Johannes Berg wrote:
> On Wed, 2012-11-28 at 13:35 +0100, Johannes Berg wrote:
> > On Tue, 2012-11-27 at 20:01 +0100, Simon Wunderlich wrote:
> > > This flag indicates that a spectrum scan is requested, if supported.
> > 
> > That "if supported" here is pretty problematic. There's no way to know.
> > Feature flag maybe?

Hmm, I could certainly add a WIPHY_FLAG for that.

> > 
> > Also, there are scan flags now. However, I don't see that this should
> > (ab)use the scan function. It doesn't seem likely that you want to do
> > this while you're sending probe requests, etc. OTOH, it seems likely
> > you'd want identical dwell times on all channels to have comparable
> > values, which isn't the case here.

The times are not a big problem - at least for now, the spectral scan
is only done for a very short time [tm] when the channel is changed.

The main reason why I wanted to use this function is that it can be used
while operation, that is sending power save, forbidding payload tx, etc.

> > 
> > I really think you need to decouple the API for this from scanning.
> 
> And then you can also define APIs to get the data out. While I realize
> that the data is implementation-dependent, you could have an enum that
> indicates the data format and describes it:
> 
> @NL80211_SPECTRAL_DATA_ATHEROS: u8 rssi,ext_rssi,noise,fft_data[] ...
> 
> enum nl80211_spectral_data {
> 	NL80211_SPECTRAL_DATA_ATHEROS,
> 	...
> };
> 
> Then you can define a function to send the data to the userspace socket
> that asked for it (yes, if you have a separate command you can send it
> only to that app... wohoo! :P ) and don't even have to store it in the
> kernel...

Calling the spectral scan and RX the FFT data are different things, but
yes, having everything in one application would be much better than
"trigger at X, receive at Y". 
> 
> So bottom line is that I think there are two choices:
> 1) for a proof of concept, implement it in debugfs only, in ath9k only,
>    with e.g. a debugfs file that sets a flag that then triggers the
>    spectral scan when you do a scan (using sw_scan_start, config hooks)
>    --> no need to modify nl80211 at all

So the idea is something like:
 * open debugfs-file (e.g. with cat)
 * start scan "normally", without any flags
 * read debugfs results, and close it

Mhm, this is definitely possible, although not too beautiful as well. :)

It seems there is no way to trigger a scan from within ath9k/debugfs, right?

sw_scan_start() is currently undefined in ath9k, but we could add that.
By "config hooks" you mean the general config driver call to set the channel
etc?

I could also cycle channels within ath9k, but the main problem I see is that
I can't turn off TX/go into power save mode from the driver, or would you see
anything feasible? 

> 2) implement some well-defined API in nl80211, but don't tie it to
>    scanning or a debugfs implementation of getting the data out, with
>    versioning of the FFT data packets etc. so it can be updated later
>    without breaking everything

I guess if we implement this in iw, this would be done similarly to scan,
e.g. trigger the scan and wait on the socket for results? Or would we
use events instead?

This would roughly mean:
 * duplicate the mac80211 scan part for spectral scan to cycle channels/be quite
   while doing this
 * duplicate iw scan (at least some parts), the interface could look very similar:
   trigger scan, on receive scan results on another socket, wait for completion
   (although iw still has this "warning, it's racy" thing inside. :] )

This looks like a whole lot of work, and would duplicate some code which
we need for scan and spec_scan (at least it looks like it). On the plus side,
this would be easier to change in the future (e.g. adding parameters for spec
scan, etc).

> 
> This hybrid isn't a good approach at all.

I don't like hybrid approach as well, but hoped that it could be "good enough"
for a proof of concept. If someone ever implements pattern matching, the
interface will probably change again as well.

Maybe you could find this idea acceptable: Keep on using the scan command plus
some flag/and or attribute, but send "FFT result" as events or within the scan
socket instead of using debugfs. What do you think?

Thanks for your review and suggestions! I'd like to discuss the alternatives
to fully understand them and then choose one. :)

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

  reply	other threads:[~2012-11-28 15:19 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-27 19:01 [RFC 0/3] Add spectral scan support for Atheros AR92xx/AR93xx Simon Wunderlich
2012-11-27 19:01 ` [RFC 1/3] nl80211: add spec scan flag Simon Wunderlich
2012-11-28 12:35   ` Johannes Berg
2012-11-28 12:43     ` Johannes Berg
2012-11-28 15:19       ` Simon Wunderlich [this message]
2012-11-28 15:29         ` [ath9k-devel] " Malinen, Jouni
2012-11-28 16:12           ` Simon Wunderlich
2012-11-28 16:49             ` Malinen, Jouni
2012-11-28 16:57               ` Johannes Berg
2012-11-28 17:06                 ` Malinen, Jouni
2012-11-28 17:09                   ` Johannes Berg
2012-11-28 16:26         ` Johannes Berg
2012-11-28 19:39           ` Simon Wunderlich
2012-11-27 19:01 ` [RFC 2/3] mac80211: add spectral_scan function, hook it up in scanning Simon Wunderlich
2012-11-27 19:01 ` [RFC 3/3] ath9k: add spectral scan feature Simon Wunderlich
2012-12-01  4:00   ` Adrian Chadd
2012-12-05 10:40     ` Simon Wunderlich
2012-12-05 11:38       ` Felix Fietkau
2012-12-05 12:05         ` Adrian Chadd
2012-12-17 20:08           ` Adrian Chadd
2012-11-27 19:01 ` [RFC] iw: add spectral scan attribute to scan function Simon Wunderlich
2012-11-27 19:16 ` [RFC 0/3] Add spectral scan support for Atheros AR92xx/AR93xx Martin Schleier
2012-11-27 19:49   ` Simon Wunderlich
2012-11-27 20:32     ` Simon Wunderlich
2012-11-27 21:01       ` Jonathan Bither
2012-11-28 10:37         ` Simon Wunderlich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121128151916.GA6175@pandem0nium \
    --to=simon.wunderlich@s2003.tu-chemnitz.de \
    --cc=adrian@freebsd.org \
    --cc=ath9k-devel@lists.ath9k.org \
    --cc=johannes@sipsolutions.net \
    --cc=kgiori@qca.qualcomm.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=mathias.kretschmer@fokus.fraunhofer.de \
    --cc=rodrigue@qca.qualcomm.com \
    --cc=siwu@hrz.tu-chemnitz.de \
    --cc=zefir.kurtisi@neratec.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).