On Wed, 2021-03-10 at 10:26 -0800, Aloka Dixit wrote:
>
> +/**
> + * enum ieee80211_bcn_tmpl_ema - EMA beacon generation type
> + * @IEEE80211_BCN_EMA_NONE: don't generate an EMA beacon.
> + * @IEEE80211_BCN_EMA_NEXT: generate the next periodicity beacon.
> + * @IEEE80211_BCN_EMA_INDEX: generate beacon by periodicity index
> + * if the value is >= this enum value.
> + */
> +enum ieee80211_bcn_tmpl_ema {
> + IEEE80211_BCN_EMA_NONE = -2,
> + IEEE80211_BCN_EMA_NEXT = -1,
> + IEEE80211_BCN_EMA_INDEX = 0,
Maybe call it _BASE instead of _INDEX, it's not really meant to be used
as is?
> +static u8 *ieee80211_copy_multiple_bssid_beacon(u8 *offset,
> + struct cfg80211_multiple_bssid_data *dest,
> + struct cfg80211_multiple_bssid_data *src)
> +{
> + int i;
> +
> + if (!dest || !src)
> + return offset;
> +
> + dest->cnt = src->cnt;
> + for (i = 0; i < dest->cnt; i++) {
> + dest->elems[i].len = src->elems[i].len;
> + dest->elems[i].data = offset;
> + memcpy(dest->elems[i].data, src->elems[i].data,
> + dest->elems[i].len);
> + offset += dest->elems[i].len;
> + }
Following my earlier question - here you just copy all the elements one
after another, as far as I can tell, so why did they need to be separate
in the first place? Might be a lot simpler everywhere if all of this was
just a single buffer, starting from the userspace API?
> @@ -4740,13 +4800,11 @@ __ieee80211_beacon_get(struct ieee80211_hw *hw,
> struct ieee80211_chanctx_conf *chanctx_conf;
> int csa_off_base = 0;
> - rcu_read_lock();
> -
> sdata = vif_to_sdata(vif);
> chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
OK, but ...
> struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
> struct ieee80211_vif *vif,
> u16 *tim_offset, u16 *tim_length)
> {
> struct ieee80211_mutable_offsets offs = {};
> - struct sk_buff *bcn = __ieee80211_beacon_get(hw, vif, &offs, false);
> + struct sk_buff *bcn = __ieee80211_beacon_get(hw, vif, &offs, false,
> + IEEE80211_BCN_EMA_NONE);
You didn't add the protection everywhere.
johannes
