Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <10344296be9a9e4f6f30eb2adf6fa5a3263811cc.camel@sipsolutions.net>
Subject: Re: [PATCH v2] nl80211: reset regdom when reloading regdb
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Finn Behrens <fin@nyantec.com>, linux-wireless@vger.kernel.org
Date:   Fri, 04 Dec 2020 12:39:29 +0100
In-Reply-To: <X7+F/ht8T7aGiIl7@bombur.kloenk.de> (sfid-20201126_114222_031269_4F737E8B)
References: <8c3e46ac-331b-0922-5379-0cb1ef61a83b@nyantec.com>
         <X7+F/ht8T7aGiIl7@bombur.kloenk.de> (sfid-20201126_114222_031269_4F737E8B)
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.36.5 (3.36.5-1.fc32) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 2020-11-26 at 11:39 +0100, Finn Behrens wrote:
> reset the regdom when NL80211_CMD_RELOAD_REGDB is send

Please add a bit more commit message, saying why this is needed.

>  	err = request_firmware(&fw, "regulatory.db", &reg_pdev->dev);
> -	if (err)
> +	if (err) {
> +		pr_info("failed to load regulatory.db\n");
>  		return err;
> +	}
>  
>  	if (!valid_regdb(fw->data, fw->size)) {
> +		pr_info("loaded regulatory.db is malformed or signature is missing/invalid\n");
>  		err = -ENODATA;
>  		goto out;
>  	}

These changes seem unrelated.

> @@ -1101,6 +1105,28 @@ int reg_reload_regdb(void)
>  	regdb = db;
>  	rtnl_unlock();
>  
> +	// reset regulatory

For whatever reason, the kernel doesn't use C99 comments.

> +	const struct ieee80211_regdomain *current_regdomain = NULL;

This should've given you a compiler warning that you shouldn't declare
variables in the middle of the code.

> +	current_regdomain = get_cfg80211_regdom();
> +
> +	struct regulatory_request *request = NULL;
> +
> +	request = kzalloc(sizeof(*request), GFP_KERNEL);
> +	if (!request) {
> +		err = -ENOMEM;
> +		goto out;
> +	}
> +
> +	request->wiphy_idx = WIPHY_IDX_INVALID;
> +	request->alpha2[0] = current_regdomain->alpha2[0];
> +	request->alpha2[1] = current_regdomain->alpha2[1];
> +	request->initiator = NL80211_USER_REG_HINT_USER;
> +	request->user_reg_hint_type = NL80211_USER_REG_HINT_USER;
> +	request->reload = true;
> +
> +	queue_regulatory_request(request);
> 
Why does it even need to be queued - we're in a process context where we
can sleep?

johannes