Re: [PATCHv2 2/5] nl80211/cfg80211: add channel switch command

[Simon Wunderlich <simon.wunderlich@s2003.tu-chemnitz.de>]

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

On Tue, Jun 18, 2013 at 04:48:10PM +0200, Johannes Berg wrote:
> 
> > +/* struct cfg80211_csa_settings - channel switch settings
> > + *
> 
> /**
>  * struct ...
>  * ...
> 
OK

> > + * @NL80211_ATTR_CH_SWITCH_COUNT: u32 attribute specifying the number of TBTT's
> > + *	until the channel switch event.
> > + * @NL80211_ATTR_CH_SWITCH_BLOCK_TX: flag attribute specifying that transmission
> > + *	must be blocked on the current channel (before the channel switch
> > + *	operation).
> > + * @NL80211_ATTR_CSA_IES: Nested set of attributes containing the IE information
> > + *	for the time while performing a channel switch.
> > + * @NL80211_ATTR_CSA_C_OFF_BEACON: Offset of the channel switch counter
> > + *	field in the beacons tail (%NL80211_ATTR_BEACON_TAIL).
> > + * @NL80211_ATTR_CSA_C_OFF_PRESP: Offset of the channel switch counter
> > + *	field in the probe response (%NL80211_ATTR_PROBE_RESP).
> 
> Shouldn't the offset be into the CSA_IES?
> 

Hmm ... yeah you are right, that would be better. Will fix that.

> > +	struct nlattr *csa_ies[NL80211_ATTR_MAX+1];
> 
> Hmm, this doesn't seem right.
> 

Why? If it's about namespace, see below. :)

> > +	err = nla_parse_nested(csa_ies, NL80211_ATTR_MAX,
> > +				   info->attrs[NL80211_ATTR_CSA_IES],
> > +				   nl80211_policy);
> 
> (Indentation)

oops...

> 
> If you're going to use nested IEs, shouldn't you define a separate
> namespace to be used within the nesting?

The idea was to recycle the nl80211_parse_beacon() function for the CSA IEs,
but if you prefer I can define an own namespace and create a separate function
to check.

Cheers,
	Simon 


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