Re: [RFC v3] cfg80211: add peer measurement with FTM API

[Johannes Berg <johannes@sipsolutions.net>]

> You have no recollection what happened in the earlier versions, right? :-p

v1 was very incomplete, it didn't have any results reporting, etc.

> > v3:
> >  - add a bit to report "final" for partial results
> >  - remove list keeping etc. and just unicast out the results
> >    to the requester (big code reduction ...)
> >  - also send complete message unicast, and as a result
> >    remove the multicast group
> >  - separate out struct cfg80211_pmsr_ftm_request_peer
> >    from struct cfg80211_pmsr_request_peer
> >  - document timeout == 0 if no timeout
> >  - disallow setting timeout nl80211 attribute to 0,
> >    must not include attribute for no timeout
> 
> All these negations make my head spin (a little). Let's look at the 
> actual documentation further down...

:-)

> > +struct cfg80211_pmsr_ftm_result {
> > +	const u8 *lci;
> > +	const u8 *civicloc;
> > +	unsigned int lci_len;
> > +	unsigned int civicloc_len;
> > +	enum nl80211_peer_measurement_ftm_failure_reasons failure_reason;
> > +	u32 num_ftmr_attempts, num_ftmr_successes;
> 
> Maybe there is a good reason, but can we move the above line a bit down...

The reason was to avoid having padding for alignment.

> > +	NL80211_ATTR_TIMEOUT,
> > +
> 
> Guess you consider reuse of the TIMEOUT attribute? 

Yes, I was actually surprised we don't have one already :-)

> I checked the policy 
> definition in nl80211_policy so it disallows 0 value as mentioned in the 
> changelog. How about adding that to the documentation here, ie. "when 
> timeout attribute is not provided the timeout is disabled for the given 
> operation" or something like that.

Sure, that makes sense, will do.

johannes