Re: [PATCH 4/5] {nl,cfg,mac}80211: finalizing mesh channel switching

[Johannes Berg <johannes@sipsolutions.net>]

On Fri, 2013-09-13 at 16:36 -0700, Chun-Yeow Yeoh wrote:
> Finalizing the requried procedures for channel switching completion and

typo: required

> also adding the function for updating the beacon and probe response frames
> with CSA and MCSP elements. Once the channel switching is completed, the
> CSA and MCSP elements are removed from the beacon or probe response frames
> as defined in the IEEE Std 802.11-2012 section 10.9.8.4.3.

I'd prefer you rewrote this to be more active ... this seems rather
observer style :)

> +int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata)
> +{
> +	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
> +	int ret = 0;
> +
> +	/* Remove the CSA and MCSP elements from the beacon */
> +	kfree(ifmsh->csa_settings);
> +	ifmsh->csa_settings = NULL;
> +	ret = ieee80211_mesh_rebuild_beacon(sdata);
> +	if (ret)
> +		return -EINVAL;

This seems totally racy with what I pointed out in the other patch ...

> +	/* Reset the TTL value and Initiator flag */
> +	ifmsh->chsw_init = false;
> +	ifmsh->chsw_ttl = 0;
> +
> +	ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON);
> +
> +	mcsa_dbg(sdata, "complete switching to center freq %d MHz",
> +		 sdata->vif.bss_conf.chandef.chan->center_freq);
> +	return ret;

return 0

> +	tmp_csa_settings = kmalloc(sizeof(struct cfg80211_csa_settings),
> +				   GFP_ATOMIC);
> +	if (!tmp_csa_settings) {
> +		mcsa_dbg(sdata, "could not allocate memory for csa beaconing");

You should never print a message for memory allocation failures, they
already give very verbose output.

> +	ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON);
> +	return ret;

return 0

johannes