Re: [RFC 09/14] mac80211: add power save support structure to mesh interface

[Johannes Berg <johannes@sipsolutions.net>]

On Fri, 2012-11-16 at 22:48 -0800, Marco Porsch wrote:

>  #ifdef CONFIG_MAC80211_MESH
> diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
> index bc3e3e1..8396602 100644
> --- a/net/mac80211/iface.c
> +++ b/net/mac80211/iface.c
> @@ -769,6 +769,9 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
>  		/* free all potentially still buffered bcast frames */
>  		local->total_ps_buffered -= skb_queue_len(&sdata->u.ap.ps.bc_buf);
>  		skb_queue_purge(&sdata->u.ap.ps.bc_buf);
> +	} else if (sdata->vif.type == NL80211_IFTYPE_MESH_POINT) {
> +		local->total_ps_buffered -= skb_queue_len(&sdata->u.mesh.ps.bc_buf);
> +		skb_queue_purge(&sdata->u.mesh.ps.bc_buf);
>  	} else if (sdata->vif.type == NL80211_IFTYPE_STATION) {
>  		ieee80211_mgd_stop(sdata);
>  	}

This will not apply, and is in the wrong place anyway.

> +	/* reset in case we have been in a mesh with PS peers before */

should amend the comment to say "and leak all the frames that might be
on the queue now.

> +	} else if (sta->sdata->vif.type == NL80211_IFTYPE_MESH_POINT) {
> +		ps = &sta->sdata->u.mesh.ps;
> +		/* TIM map only for AID <= IEEE80211_MAX_AID */

AID? Seems more like PLID to me:

> +		id = le16_to_cpu(sta->plid) % IEEE80211_MAX_AID;

> -	/* This is only necessary for stations on BSS interfaces */
> -	if (!sta->sdata->bss)
> +	/* This is only necessary for stations on BSS/MBSS interfaces */
> +	if (!sta->sdata->bss &&
> +	    !ieee80211_vif_is_mesh(&sta->sdata->vif))
>  		return false;

Instead of cluttering all the code paths like this, what if we changed

struct ieee80211_if_ap *sdata->bss

to

struct ps_data *sdata->ps


If we know the interface type is AP/AP_VLAN, we can still get the AP
sdata from that pointer:
	container_of(sdata->ps, struct ieee80211_sub_if_sdata, u.ap.ps);

This is only in a few places, and might keep all the RX/TX code simpler?
 
> +++ b/net/mac80211/tx.c
> @@ -329,6 +329,8 @@ static void purge_old_ps_buffers(struct ieee80211_local *local)
>  
>  		if (sdata->vif.type == NL80211_IFTYPE_AP)
>  			ps = &sdata->u.ap.ps;
> +		else if (sdata->vif.type == NL80211_IFTYPE_MESH_POINT)
> +			ps = &sdata->u.mesh.ps;

This could just be "ps = sdata->ps" etc.

johannes