Re: [PATCHv6 4/6] mac80211: add radar detection command/event

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

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

On Fri, Jan 18, 2013 at 11:00:11PM +0100, Johannes Berg wrote:
> On Thu, 2013-01-17 at 14:52 +0100, Simon Wunderlich wrote:
> 
> > > > +	res = drv_start_radar_detection(local, sdata, chandef);
> > > 
> > > If the vif is assigned the channel, why also pass it to the
> > > start_radar_detection command? That seems pointless, they can't be
> > > different?
> > > 
> > In the initial phase, __nl80211_set_channel() should not be able to set
> > the channel (no CAC yet), and will fail for IBSS (to be implemented later)
> > generally, at least as it's implemented now.
> > 
> > But what we can do is set the channel from mac80211 and call the driver
> > function without the channel argument, as it'll be pointless in this case.
> > Is this what you mean?
> 
> Hmm. Why are you talking about __nl80211_set_channel() now? We're now
> talking about mac80211, which can and does set the channel (chanctx,
> which may fall back to drv_config()) before calling
> drv_start_radar_detection(), so I think the latter needs no chandef
> argument?
> 

Sorry, I was confused at the point, you are right nl80211_set_channel() has
nothing to do with that. ieee80211_vif_use_channel() already sets the channel
implicitly (e.g. through ieee80211_hw_config()).

> > > Actually that raises another question: If we have "external" radar
> > > detection, say by a different NIC, then shouldn't we still ask the
> > > driver to start radar detection when using the channel? Or is that
> > > implicit, does the driver have to check?
> > 
> > That is a good question, I didn't consider that. In the "simple process"
> > we first start radar detection and start the ap on the same channel.
> > For external CAC that won't work, of course. What about mac80211 checking
> > the channel in start_ap(), and if the channel requires DFS, pass some flag
> > to the driver that radar detection should be enabled?
> 
> Yes, that makes sense. But then it would also make sense to remove the
> start_radar_detection() callback entirely, and encode all that
> information in the channel context/drv_config call?
> 
> If mac80211 gets to be responsible for it, this should totally be
> documented in the cfg80211 API though so if a full-mac driver wants to
> implement it they know what to do. I do think this is the reasonable way
> of doing it though.
> 
> Note that I'm not advocating removing the start_radar_detection() or its
> chandef from the *cfg80211* API. That is clearly needed. But in mac80211
> it seems "set this chandef with radar detection" is a better API?
> 

OK, so we keep the cfg80211 start_radar_detection() and replace the mac80211
part with respective information that radar is required. That could work,
I'll look into this.

Thanks,
	Simon

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