[patch]mac80211: add support for iwlist channel

[patch]mac80211: add support for iwlist channel

[Hong Liu]

Add supported channels info in SIOCGIWRANGE implementation.

Signed-off-by: Hong Liu <hong.liu@intel.com>

diff --git a/net/mac80211/ieee80211_ioctl.c b/net/mac80211/ieee80211_ioctl.c
index fcea8b2..3ea3ad8 100644
--- a/net/mac80211/ieee80211_ioctl.c
+++ b/net/mac80211/ieee80211_ioctl.c
@@ -1493,6 +1493,40 @@ static int ieee80211_ioctl_giwname(struct net_device *dev,
 	return 0;
 }
 
+static int is_duplicate_channel(struct ieee80211_channel *chan,
+				struct iw_freq *freq, int count)
+{
+	int i;
+
+	for (i = 0; i < count; i++, freq++)
+		if (chan->chan == freq->i)
+			return 1;
+
+	return 0;
+}
+
+static void ieee80211_get_supp_channels(struct ieee80211_local *local,
+				struct iw_range *range)
+{
+	struct ieee80211_hw_mode *mode = NULL;
+	int c = 0, j;
+
+	list_for_each_entry(mode, &local->modes_list, list) {
+		for (j = 0;
+		     j < mode->num_channels && c < IW_MAX_FREQUENCIES; j++) {
+			struct ieee80211_channel *chan = &mode->channels[j];
+
+			if (is_duplicate_channel(chan, range->freq, c))
+				continue;
+			range->freq[c].i = chan->chan;
+			range->freq[c].m = chan->freq * 100000;
+			range->freq[c].e = 1;
+			c++;
+		}
+	}
+	range->num_channels = c;
+	range->num_frequency = c;
+}
 
 static int ieee80211_ioctl_giwrange(struct net_device *dev,
 				 struct iw_request_info *info,
@@ -1533,6 +1567,8 @@ static int ieee80211_ioctl_giwrange(struct net_device *dev,
 	range->enc_capa = IW_ENC_CAPA_WPA | IW_ENC_CAPA_WPA2 |
 			  IW_ENC_CAPA_CIPHER_TKIP | IW_ENC_CAPA_CIPHER_CCMP;
 
+	ieee80211_get_supp_channels(local, range);
+
 	IW_EVENT_CAPA_SET_KERNEL(range->event_capa);
 	IW_EVENT_CAPA_SET(range->event_capa, SIOCGIWTHRSPY);
 	IW_EVENT_CAPA_SET(range->event_capa, SIOCGIWAP);

Re: [patch]mac80211: add support for iwlist channel

[Michael Wu]

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

On Monday 18 June 2007 01:47, Hong Liu wrote:
> +static int is_duplicate_channel(struct ieee80211_channel *chan,
> +				struct iw_freq *freq, int count)
> +{
> +	int i;
> +
> +	for (i = 0; i < count; i++, freq++)
> +		if (chan->chan == freq->i)
> +			return 1;
> +
> +	return 0;
> +}
> +
Currently, the only situation in which channels are duplicated is when there's 
an 11g mode registered. The scanning code in ieee80211_sta.c 
(ieee80211_sta_scan_work) checks for this specifically:

(local->hw_modes & local->enabled_modes & (1 << MODE_IEEE80211G) &&
 mode->mode == MODE_IEEE80211B)

MODE_IEEE80211B is skipped if MODE_IEEE80211G is enabled. Modes should be 
skipped instead of channels.

> +static void ieee80211_get_supp_channels(struct ieee80211_local *local,
> +				struct iw_range *range)
> +{
> +	struct ieee80211_hw_mode *mode = NULL;
> +	int c = 0, j;
> +
> +	list_for_each_entry(mode, &local->modes_list, list) {
> +		for (j = 0;
> +		     j < mode->num_channels && c < IW_MAX_FREQUENCIES; j++) {
This looks somewhat awkward. How about something like:

int j = 0;
(check if this mode is disabled or duplicates channels here)
while (j++ < mode->num_channels && c++ < IW_MAX_FREQUENCIES) {

> +			struct ieee80211_channel *chan = &mode->channels[j];
> +
> +			if (is_duplicate_channel(chan, range->freq, c))
> +				continue;
> +			range->freq[c].i = chan->chan;
> +			range->freq[c].m = chan->freq * 100000;
> +			range->freq[c].e = 1;
> +			c++;
> +		}
> +	}
> +	range->num_channels = c;
> +	range->num_frequency = c;
> +}
>
>  static int ieee80211_ioctl_giwrange(struct net_device *dev,
>  				 struct iw_request_info *info,
> @@ -1533,6 +1567,8 @@ static int ieee80211_ioctl_giwrange(struct net_device
> *dev, range->enc_capa = IW_ENC_CAPA_WPA | IW_ENC_CAPA_WPA2 |
>  			  IW_ENC_CAPA_CIPHER_TKIP | IW_ENC_CAPA_CIPHER_CCMP;
>
> +	ieee80211_get_supp_channels(local, range);
> +
Should be fine to just put all the logic here. No other code will use that 
function.

-Michael Wu

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

Re: [patch]mac80211: add support for iwlist channel

[Jiri Benc]

On Mon, 18 Jun 2007 16:47:05 +0800, Hong Liu wrote:
> +static void ieee80211_get_supp_channels(struct ieee80211_local *local,
> +				struct iw_range *range)
> +{
> +	struct ieee80211_hw_mode *mode = NULL;
> +	int c = 0, j;
> +
> +	list_for_each_entry(mode, &local->modes_list, list) {
> +		for (j = 0;
> +		     j < mode->num_channels && c < IW_MAX_FREQUENCIES; j++) {
> +			struct ieee80211_channel *chan = &mode->channels[j];
> +
> +			if (is_duplicate_channel(chan, range->freq, c))
> +				continue;
> +			range->freq[c].i = chan->chan;
> +			range->freq[c].m = chan->freq * 100000;
> +			range->freq[c].e = 1;
> +			c++;
> +		}

I don't know exactly what results are supposed to be returned by this
ioctl, but given the fact SIOCSIWFREQ allows only channels with
the IEEE80211_CHAN_W_SCAN flag set, shouldn't only channels with this
flag be returned here?

Thanks,

 Jiri

-- 
Jiri Benc
SUSE Labs

Re: [patch]mac80211: add support for iwlist channel

[Hong Liu]

On Tue, 2007-06-19 at 10:50 +0200, Jiri Benc wrote:
> On Mon, 18 Jun 2007 16:47:05 +0800, Hong Liu wrote:
> > +static void ieee80211_get_supp_channels(struct ieee80211_local *local,
> > +				struct iw_range *range)
> > +{
> > +	struct ieee80211_hw_mode *mode = NULL;
> > +	int c = 0, j;
> > +
> > +	list_for_each_entry(mode, &local->modes_list, list) {
> > +		for (j = 0;
> > +		     j < mode->num_channels && c < IW_MAX_FREQUENCIES; j++) {
> > +			struct ieee80211_channel *chan = &mode->channels[j];
> > +
> > +			if (is_duplicate_channel(chan, range->freq, c))
> > +				continue;
> > +			range->freq[c].i = chan->chan;
> > +			range->freq[c].m = chan->freq * 100000;
> > +			range->freq[c].e = 1;
> > +			c++;
> > +		}
> 
> I don't know exactly what results are supposed to be returned by this
> ioctl, but given the fact SIOCSIWFREQ allows only channels with
> the IEEE80211_CHAN_W_SCAN flag set, shouldn't only channels with this
> flag be returned here?

> Thanks,
> 
>  Jiri
> 


What is the meaning of IEEE80211_CHAN_W_SCAN flag? I can't find any
description of this flag.

Thanks,
Hong

Re: [patch]mac80211: add support for iwlist channel

[Hong Liu]

2nd try!

Add supported channels info in SIOCGIWRANGE implementation.

Signed-off-by: Hong Liu <hong.liu@intel.com>

diff --git a/net/mac80211/ieee80211_ioctl.c b/net/mac80211/ieee80211_ioctl.c
index fcea8b2..3c09a93 100644
--- a/net/mac80211/ieee80211_ioctl.c
+++ b/net/mac80211/ieee80211_ioctl.c
@@ -1500,6 +1500,9 @@ static int ieee80211_ioctl_giwrange(struct net_device *dev,
 {
 	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
 	struct iw_range *range = (struct iw_range *) extra;
+	struct ieee80211_hw_mode *mode = NULL;
+	int c = 0;
+
 
 	data->length = sizeof(struct iw_range);
 	memset(range, 0, sizeof(struct iw_range));
@@ -1533,6 +1536,28 @@ static int ieee80211_ioctl_giwrange(struct net_device *dev,
 	range->enc_capa = IW_ENC_CAPA_WPA | IW_ENC_CAPA_WPA2 |
 			  IW_ENC_CAPA_CIPHER_TKIP | IW_ENC_CAPA_CIPHER_CCMP;
 
+	list_for_each_entry(mode, &local->modes_list, list) {
+		int j = 0;
+
+		if (!(local->enabled_modes & (1 << mode->mode)) ||
+		     (local->hw_modes & local->enabled_modes &
+		      (1 << MODE_IEEE80211G) && mode->mode == MODE_IEEE80211B))
+			continue;
+
+		while (j < mode->num_channels && c < IW_MAX_FREQUENCIES) {
+			struct ieee80211_channel *chan = &mode->channels[j];
+
+			if (chan->flag & IEEE80211_CHAN_W_SCAN) {
+				range->freq[c].i = chan->chan;
+				range->freq[c].m = chan->freq * 100000;
+				range->freq[c].e = 1;
+			}
+			++j; ++c;
+		}
+	}
+	range->num_channels = c;
+	range->num_frequency = c;
+
 	IW_EVENT_CAPA_SET_KERNEL(range->event_capa);
 	IW_EVENT_CAPA_SET(range->event_capa, SIOCGIWTHRSPY);
 	IW_EVENT_CAPA_SET(range->event_capa, SIOCGIWAP);



On Mon, 2007-06-18 at 22:12 -0700, Michael Wu wrote:
> On Monday 18 June 2007 01:47, Hong Liu wrote:
> > +static int is_duplicate_channel(struct ieee80211_channel *chan,
> > +				struct iw_freq *freq, int count)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < count; i++, freq++)
> > +		if (chan->chan == freq->i)
> > +			return 1;
> > +
> > +	return 0;
> > +}
> > +
> Currently, the only situation in which channels are duplicated is when there's 
> an 11g mode registered. The scanning code in ieee80211_sta.c 
> (ieee80211_sta_scan_work) checks for this specifically:
> 
> (local->hw_modes & local->enabled_modes & (1 << MODE_IEEE80211G) &&
>  mode->mode == MODE_IEEE80211B)
> 
> MODE_IEEE80211B is skipped if MODE_IEEE80211G is enabled. Modes should be 
> skipped instead of channels.
> 
> > +static void ieee80211_get_supp_channels(struct ieee80211_local *local,
> > +				struct iw_range *range)
> > +{
> > +	struct ieee80211_hw_mode *mode = NULL;
> > +	int c = 0, j;
> > +
> > +	list_for_each_entry(mode, &local->modes_list, list) {
> > +		for (j = 0;
> > +		     j < mode->num_channels && c < IW_MAX_FREQUENCIES; j++) {
> This looks somewhat awkward. How about something like:
> 
> int j = 0;
> (check if this mode is disabled or duplicates channels here)
> while (j++ < mode->num_channels && c++ < IW_MAX_FREQUENCIES) {
> 
> > +			struct ieee80211_channel *chan = &mode->channels[j];
> > +
> > +			if (is_duplicate_channel(chan, range->freq, c))
> > +				continue;
> > +			range->freq[c].i = chan->chan;
> > +			range->freq[c].m = chan->freq * 100000;
> > +			range->freq[c].e = 1;
> > +			c++;
> > +		}
> > +	}
> > +	range->num_channels = c;
> > +	range->num_frequency = c;
> > +}
> >
> >  static int ieee80211_ioctl_giwrange(struct net_device *dev,
> >  				 struct iw_request_info *info,
> > @@ -1533,6 +1567,8 @@ static int ieee80211_ioctl_giwrange(struct net_device
> > *dev, range->enc_capa = IW_ENC_CAPA_WPA | IW_ENC_CAPA_WPA2 |
> >  			  IW_ENC_CAPA_CIPHER_TKIP | IW_ENC_CAPA_CIPHER_CCMP;
> >
> > +	ieee80211_get_supp_channels(local, range);
> > +
> Should be fine to just put all the logic here. No other code will use that 
> function.
> 
> -Michael Wu

Re: [patch]mac80211: add support for iwlist channel

[Larry Finger]

Hong Liu wrote:
> 2nd try!

The indication that this is the second try belongs in the subject. In addition, you should create a 
new message and not reply to the previous try.
> 
> Add supported channels info in SIOCGIWRANGE implementation.
> 
> Signed-off-by: Hong Liu <hong.liu@intel.com>

The next line after the last "Signed-off-by" line should be three minus signs. That line indicates 
the end of the header for the patch. Between the --- line and the diff that begins the patch, you 
can include any information that you wish to send, but does not belong in the official commit message.

> diff --git a/net/mac80211/ieee80211_ioctl.c b/net/mac80211/ieee80211_ioctl.c
> index fcea8b2..3c09a93 100644

Larry

Re: [patch]mac80211: add support for iwlist channel

[Michael Wu]

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

On Tuesday 19 June 2007 20:03, Hong Liu wrote:
> +	struct ieee80211_hw_mode *mode = NULL;
> +	int c = 0;
> +
Shouldn't add an extra empty line here.

> +			if (chan->flag & IEEE80211_CHAN_W_SCAN) {
> +				range->freq[c].i = chan->chan;
> +				range->freq[c].m = chan->freq * 100000;
> +				range->freq[c].e = 1;
> +			}
> +			++j; ++c;
c shouldn't be incremented if the channel is being skipped.

Also, two minor comments on the style since this patch needs to be revised 
anyway - using ++x instead of x++ looks a bit odd, and using 'int j' 
when 'int i' is available is also a little strange.

Patch looks good otherwise.

-Michael Wu

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

Re: [patch]mac80211: add support for iwlist channel

[Jiri Benc]

On Wed, 20 Jun 2007 10:10:25 +0800, Hong Liu wrote:
> What is the meaning of IEEE80211_CHAN_W_SCAN flag? I can't find any
> description of this flag.

It means we can tune to that channel. That way you can have all
channels the hardware support in the channel list in ieee80211_hw_mode
struct and mask out channels disallowed by the regulations.

Yes, it's misnamed. Worse, all of these IEEE80211_CHAN_W_ flags are not
documented. Patches are welcome.

Thanks,

 Jiri

-- 
Jiri Benc
SUSE Labs

Re: [patch]mac80211: add support for iwlist channel

[Jiri Benc]

On Wed, 20 Jun 2007 10:43:09 -0700, James Ketrenos wrote:
> [...]
> /**
>  * enum ieee80211_channel_flags - Channel flag bit values
>  * @IEEE80211_CHAN_W_TUNE: Hardware can tune to this channel
>  * @IEEE80211_CHAN_W_ACTIVE: Channel can be used for active scanning
>  * @IEEE80211_CHAN_W_IBSS: Hardware can act as IBSS Master
>  * @IEEE80211_CHAN_W_RADAR: Radar spectrum enforcement applies
>  * @IEEE80211_CHAN_W_TX_ALLOWED: Tx is allowed

Looks good, except...

> [...]
>  * A channel without the TX_ALLOWED bit set *can not* be used
>  * for any type of transmission and can only be used for passive
>  * listening.  This is useful for performing network audits.

I don't find this useful. The stack isn't aware of anything like this.
Do you plan to add a support for this flag? Also, cfg80211/nl80211
would need to be taught of it.

Thanks,

 Jiri

-- 
Jiri Benc
SUSE Labs

Re: [patch]mac80211: add support for iwlist channel

[Jiri Benc]

On Wed, 20 Jun 2007 11:19:55 -0700, James Ketrenos wrote:
> Is there any mechanism for documenting where we would like things
> to eventually go?  A "future" tag we can stick on things (in the
> documentation, etc.)?

Wiki at linuxwireless.org could be a good place.

 Jiri

-- 
Jiri Benc
SUSE Labs

Re: [patch]mac80211: add support for iwlist channel

[James Ketrenos]

Jiri Benc wrote:
> On Wed, 20 Jun 2007 10:10:25 +0800, Hong Liu wrote:
>> What is the meaning of IEEE80211_CHAN_W_SCAN flag? I can't find any
>> description of this flag.
> 
> It means we can tune to that channel. That way you can have all
> channels the hardware support in the channel list in ieee80211_hw_mode
> struct and mask out channels disallowed by the regulations.

The following isn't in patch form; thought I'd write it up for some
discussion purposes....

SCAN renamed TUNE, and a couple adds:

/**
 * enum ieee80211_channel_flags - Channel flag bit values
 * @IEEE80211_CHAN_W_TUNE: Hardware can tune to this channel
 * @IEEE80211_CHAN_W_ACTIVE: Channel can be used for active scanning
 * @IEEE80211_CHAN_W_IBSS: Hardware can act as IBSS Master
 * @IEEE80211_CHAN_W_RADAR: Radar spectrum enforcement applies
 * @IEEE80211_CHAN_W_TX_ALLOWED: Tx is allowed
 *
 * Any channel can be added to ieee80211_hw_mode.  If the TUNE flag
 * is not set, however, the channel will never be used.
 *
 * When the ACTIVE bit is set, probe requests can be sent
 * during scans.  If the ACTIVE bit is not set, a probe request
 * can only be sent if other 802.11 traffic is detected.
 *
 * In order to support IBSS mode, a channel must have both the
 * ACTIVE and IBSS bits set.
 *
 * On channels where RADAR is set, certain restrictions apply
 * regarding when a channel can be used.  If the client hardware
 * can detect and enforce RF silence upon radar detection, then
 * the client can transmit on RADAR channels regardless of if
 * other 802.11 RF activity exists on the channel.  If, however
 * the client hardware can not detect and enforce RF silence
 * on a radar channel, the client must "follow" another source
 * of 802.11 traffic.  This means no transmission is allowed
 * on a RADAR channel *until* traffic from a 802.11 access point
 * has been detected.  And then traffic can only be allowed so
 * long as that AP exists and is within range (so that 802.11h
 * CSA frames can be received).
 *
 * A channel without the TX_ALLOWED bit set *can not* be used
 * for any type of transmission and can only be used for passive
 * listening.  This is useful for performing network audits.
 *
 * When using the legacy wireless extension SIOGIWRANGE, only
 * channels with the TUNE and TX_ALLOWED bits will be
 * returned.  Most users of SIOGIWRANGE expect to be able to
 * use the returned results for a network association -- the
 * lack of being able to tune or transmit make a channel
 * unusable in that situation.
 */
enum ieee80211_channel_flags {
	IEEE80211_CHAN_W_TUNE = (1 << 0),
	IEEE80211_CHAN_W_ACTIVE_SCAN = (1 << 1),
	IEEE80211_CHAN_W_IBSS = (1 << 2),
	IEEE80211_CHAN_W_RADAR = (1 << 3),
	IEEE80211_CHAN_W_TX_ALLOWED = (1 << 4),
};

Re: [patch]mac80211: add support for iwlist channel

[James Ketrenos]

Jiri Benc wrote:
> On Wed, 20 Jun 2007 10:43:09 -0700, James Ketrenos wrote:
>> [...]
>> /**
>>  * enum ieee80211_channel_flags - Channel flag bit values
>>  * @IEEE80211_CHAN_W_TUNE: Hardware can tune to this channel
>>  * @IEEE80211_CHAN_W_ACTIVE: Channel can be used for active scanning
>>  * @IEEE80211_CHAN_W_IBSS: Hardware can act as IBSS Master
>>  * @IEEE80211_CHAN_W_RADAR: Radar spectrum enforcement applies
>>  * @IEEE80211_CHAN_W_TX_ALLOWED: Tx is allowed
> 
> Looks good, except...
> 
>> [...]
>>  * A channel without the TX_ALLOWED bit set *can not* be used
>>  * for any type of transmission and can only be used for passive
>>  * listening.  This is useful for performing network audits.
> 
> I don't find this useful. The stack isn't aware of anything like this.
> Do you plan to add a support for this flag? Also, cfg80211/nl80211
> would need to be taught of it.

If its not useful for anyone else, we can have it just in iwlwifi.  
I don't know if any other hardware supports tuning to channels they 
can't transmit on.

I have personally found it very useful here when using my laptop to 
troubleshoot other wireless issues in geographies where I can't 
transmit -- but I can sniff the packets to tell people what to fix.

For now, since nothing else in the stack can do anything with it,
I'd just assume yank it.  I don't have the time to implement it all
in right now.

Is there any mechanism for documenting where we would like things
to eventually go?  A "future" tag we can stick on things (in the
documentation, etc.)?

James

> 
> Thanks,
> 
>  Jiri
> 

Re: [patch]mac80211: add support for iwlist channel

[Hong Liu]

On Wed, 2007-06-20 at 00:08 -0700, Michael Wu wrote:
> On Tuesday 19 June 2007 20:03, Hong Liu wrote:
> > +	struct ieee80211_hw_mode *mode = NULL;
> > +	int c = 0;
> > +
> Shouldn't add an extra empty line here.
> 
> > +			if (chan->flag & IEEE80211_CHAN_W_SCAN) {
> > +				range->freq[c].i = chan->chan;
> > +				range->freq[c].m = chan->freq * 100000;
> > +				range->freq[c].e = 1;
> > +			}
> > +			++j; ++c;
> c shouldn't be incremented if the channel is being skipped.
> 

Oh, silly mistake. Thanks for pointing it out.

Thanks,
Hong

> Also, two minor comments on the style since this patch needs to be revised 
> anyway - using ++x instead of x++ looks a bit odd, and using 'int j' 
> when 'int i' is available is also a little strange.
> 
> Patch looks good otherwise.
> 
> -Michael Wu