Message-ID: <16a03353cee422340c8ac36240b1e088fd45802e.camel@sipsolutions.net>
Subject: Re: [v13 2/3] mac80211: MBSSID and EMA beacon handling in AP mode
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Aloka Dixit <alokad@codeaurora.org>, linux-wireless@vger.kernel.org
Date:   Fri, 26 Nov 2021 12:23:15 +0100
In-Reply-To: <20211006040938.9531-3-alokad@codeaurora.org>
References: <20211006040938.9531-1-alokad@codeaurora.org>
         <20211006040938.9531-3-alokad@codeaurora.org>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.42.1 (3.42.1-1.fc35) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 2021-10-05 at 21:09 -0700, Aloka Dixit wrote:

>  
> +static struct sk_buff *
> +ieee80211_beacon_get_ap_mbssid(struct ieee80211_hw *hw,
> +			       struct ieee80211_vif *vif,
> +			       struct ieee80211_mutable_offsets *offs,
> +			       bool is_template,
> +			       struct beacon_data *beacon,
> +			       struct ieee80211_chanctx_conf *chanctx_conf,
> +			       int ema_index,
> +			       struct list_head *ema_list)

This function is called from ieee80211_beacon_get_ap(). That's called
from __ieee80211_beacon_get(), under RCU read lock.

> +	for (i = 0; i < beacon->mbssid_ies->cnt; i++) {
> +		struct ieee80211_ema_bcns *bcn;
> +
> +		bcn = kzalloc(sizeof(*bcn), GFP_KERNEL);

Therefore, you really cannot GFP_KERNEL allocate anything. But I really
only saw this because I went back to my comments on v12 where this was
still more obvious.


Given that we're already in v13 of this patch, let's take a step back.
Much of what I've pointed out in the reviews really is automatically
testable.

Can you please add support for this to hwsim and hostapd and add a few
tests to the hostap hwsim tests? Given the complexity of this, being
able to run the tests for it myself would give me a lot more confidence
in it.

johannes