DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C89C520840
Message-ID: <f1c8ce7daf62c60197da9cc62d550fdc1cac6303.camel@sipsolutions.net>
Subject: Re: [PATCH 2/3] mac80211: Implement API to configure station
 specific rssi threshold
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Tamizh chelvam <tamizhr@codeaurora.org>
Cc:     linux-wireless@vger.kernel.org
Date:   Fri, 09 Nov 2018 12:49:26 +0100
In-Reply-To: <1539626250-769-3-git-send-email-tamizhr@codeaurora.org>
References: <1539626250-769-1-git-send-email-tamizhr@codeaurora.org>
         <1539626250-769-3-git-send-email-tamizhr@codeaurora.org>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Mon, 2018-10-15 at 23:27 +0530, Tamizh chelvam wrote:
> 
> +		sta_mon_rssi_config_free(sta);
> +		sta->rssi_hyst = rssi_hyst;
> +		if (fixed_thold) {
> +			if (n_rssi_tholds > 2) {
> +				ret = -EINVAL;
> +				goto out;
> +			}

This might be slightly wrong, you free and then can still return an
error.

> +			if (n_rssi_tholds == 1) {
> +				sta->rssi_low = rssi_tholds[0];
> +				sta->rssi_high = rssi_tholds[0];
> +			} else {
> +				sta->rssi_low = rssi_tholds[0];
> +				sta->rssi_high = rssi_tholds[1];
> +			}
> +		} else {
> +			const s32 *rssi_tholds;
> +
> +			rssi_tholds = kmemdup(rssi_tholds,
> +					      n_rssi_tholds * sizeof(s32),
> +					      GFP_KERNEL);
> +			if (!rssi_tholds) {
> +				ret = -ENOMEM;
> +				goto out;
> +			}

Similarly here, I guess you should do the allocation (and other error
checking) before freeing.

> +			sta->rssi_tholds = rssi_tholds;
> +			sta->n_rssi_tholds = n_rssi_tholds;
> +			ieee80211_update_rssi_config(sta);


> +struct sta_mon_rssi_config {
> +};

Huh?


The commit log also makes it sounds like mac80211 actually *supports*
this, but clearly that's not the case. However you don't give any data
to the driver either, did you lose a patch along the way? Previously you
had patch 5 ("mac80211: Implement functionality to monitor station's
rssi cross event") and if I remember correctly I said you should squash
some, but now that's not here?

johannes