[RFC] ap: AP/GO interface teardown optimization

[Peer, Ilan]

Hi,

Thanks for the comments.

Ilan.

> -----Original Message-----
> From: Jouni Malinen [mailto:j at w1.fi]
> Sent: Tuesday, March 04, 2014 20:04
> To: Peer, Ilan
> Cc: hostap at lists.shmoo.com; Benji, Moshe
> Subject: Re: [RFC] ap: AP/GO interface teardown optimization
> 
> On Mon, Mar 03, 2014 at 02:08:16PM +0200, Ilan Peer wrote:
> > This commit adds an option to optimize AP teardown by leaving the
> > deletion of keys (including group keys) and stations to the driver.
> >
> > This optimization option should be used if the driver supports
> > stations and keys removal when stopping an AP.
> >
> > For example, the optimization option will always be used for cfg80211
> > drivers since cfg80211 shall always remove stations and keys when
> > stopping an AP (in order to support cases where the AP is disabled
> > without the knowledge of wpa_supplicant/hostapd).
> 
> Is that the case with all cfg80211 versions through the history? I think we
> want to enable this in general, so it sounds acceptable to follow this
> justification, but I'd like to make sure it understood if there are some older
> kernel versions where this could potentially not work.
> 

I cannot really vouch for all kernel and driver version :) ... if this patch makes sense then I'll work on adding a corresponding patch to the kernel to indicate this capability (or some similar solution).


> > diff --git a/src/ap/hostapd.c b/src/ap/hostapd.c index
> > ad1c2d0..5e39438 100644
> > --- a/src/ap/hostapd.c
> > +++ b/src/ap/hostapd.c
> > @@ -1009,6 +1013,10 @@ static int setup_interface(struct hostapd_iface
> *iface)
> >  	struct hostapd_data *hapd = iface->bss[0];
> >  	size_t i;
> >
> > +	iface->driver_ap_teardown_support =
> > +		    !!(iface->drv_flags &
> WPA_DRIVER_FLAGS_AP_TEARDOWN_SUPPORT);
> 
> What's the point in maintaining a separate copy of that bit in the exact same
> structure? I'd remove the driver_ap_teardown_support and just use
> drv_flags directly.
> 

Sure. Will fix.

> > +	iface->driver_ap_teardown = 0;
>
> Could you please clarify why driver_ap_teardown is set to 0 here? This would
> require at least a comment explaining why that is needed or maybe even
> consideration of removing this separate variable completely if it is not really
> needed.
>

It is needed since the patch introduces cases where when a station/key is removed, this variable is checked if the removal is due to a teardown flow or any other reason, and if set the station/key is not removed. Since flows that disable/enable and interface are possible, we thought that we should set the variable to zero so we do not end up with not freeing stations/keys.