[Battlemesh] BSS load element patch

[Sven Eckelmann]

On Friday 07 August 2015 12:17:00 Arthur LENA wrote:
> The QBSS load element is specified in IEEE 802.11-2012 $8.4.2.30.
> It is used to transmit population at a STA and a ratio of the
> utilization of the operating channel. This implementation fills
> the channel utilization field and the station count field. The
> available admission capacity is always set to 0.
>----

The Signed-off-by of the patch would be missing

This patch doesn't build:

   net/mac80211/main.c: In function ‘ieee80211_hw_conf_chan’:
   net/mac80211/main.c:160:7: error: ‘struct ieee80211_local’ has no member named ‘qbss_sending_interval_cnt’
     local->qbss_sending_interval_cnt = 0;

> @@ -157,6 +157,9 @@ static u32 ieee80211_hw_conf_chan(struct ieee80211_local *local)
>  		local->hw.conf.power_level = power;
>  	}
>  
> +	local->qbss_sending_interval_cnt = 0;
> +	local->qbss_last_time_busy = 0;
> +	local->qbss_chan_util = 0;
>  	return changed;
>  }
>  

The relevant variable is defined nowhere. It most likely should be changed
to

    local->qbss_interval_cnt = 0;.


> +static void ieee80211_beacon_add_qbss(struct ieee80211_sub_if_data *sdata,
> +				       struct sk_buff *skb)

The alignment is not matching the opening parenthesis (see checkpatch output).


> +	if (++local->qbss_interval_cnt >=
> +		DOT11_CHANNEL_UTILIZATION_BEACON_INTERVAL) {

Are you sure that the access to qbss_interval_cnt doesn't require locking
(or use of atomic[64]_t)?

The second line also doesn't look to be correctly aligned.

> +	*((u16 *)hdr) = cpu_to_le16(local->num_sta);

Please don't assign a __le16 to a u16


> @@ -3352,7 +3416,7 @@ __ieee80211_beacon_get(struct ieee80211_hw *hw,
>  			 */
>  			skb = dev_alloc_skb(local->tx_headroom +
>  					    beacon->head_len +
> -					    beacon->tail_len + 256 +
> +					    beacon->tail_len + 256 + 7 +
>  					    local->hw.extra_beacon_tailroom);
>  			if (!skb)
>  				goto out;

I am not sure if this is ok, but at least the comment above this alloc
is now wrong:

			/*
			 * headroom, head length,
			 * tail length and maximum TIM length
			 */

> @@ -3392,13 +3457,14 @@ __ieee80211_beacon_get(struct ieee80211_hw *hw,
>  			ieee80211_set_csa(sdata, beacon);
>  		}
>  
> -		skb = dev_alloc_skb(local->tx_headroom + beacon->head_len +
> +		skb = dev_alloc_skb(local->tx_headroom + beacon->head_len + 7 +
>  				    local->hw.extra_beacon_tailroom);
>  		if (!skb)
>  			goto out;

It should somewhere explained what this 7 is. Maybe using a define or a
similar comment as the one from the other alloc.

> +		for (band_idx = 0; band_idx < IEEE80211_NUM_BANDS; band_idx++) {
> +			sband = local->hw.wiphy->bands[band_idx];
> +			chan_idx += sband->n_channels;
> +			while (idx < chan_idx &&
> +			       current_chan != &sband->channels[idx])
> +				idx++;
> +			if (idx < chan_idx)
> +				break;
> +			band_idx++;
> +		}

Why are you increasing band_idx twice?

> +		local->qbss_chan_util = (u8)div64_u64(busy_time * 255,
> +			local->qbss_interval_cnt *
> +			sdata->vif.bss_conf.beacon_int);

Alignment is quite off here. Maybe this can be split in multiple statements
to make it more readable.

Kind regards,
	Sven
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://ml.ninux.org/pipermail/battlemesh/attachments/20150807/8327f1d0/attachment-0001.sig>