Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <1a1d993e-58cd-6f42-38c9-ea7a9fe6456f@wetzel-home.de>
Date:   Fri, 16 Sep 2022 18:38:52 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.1
Subject: Re: [PATCH] mac80211: Ensure vif queues are operational after start
To:     Ben Greear <greearb@candelatech.com>, Felix Fietkau <nbd@nbd.name>,
        linux-wireless@vger.kernel.org
Cc:     Johannes Berg <johannes@sipsolutions.net>
References: <20220915130946.302803-1-alexander@wetzel-home.de>
 <26e9ae91-8e13-df45-815c-cb45c1911032@nbd.name>
 <9e36f219-14a6-8960-a5fd-cb9f708237ed@wetzel-home.de>
 <0b342479-d04a-a45e-d63b-73eec5a1fb40@wetzel-home.de>
 <4f77c10b-a6ea-2b79-cada-deefb2a3a07b@candelatech.com>
Content-Language: en-US
From:   Alexander Wetzel <alexander@wetzel-home.de>
In-Reply-To: <4f77c10b-a6ea-2b79-cada-deefb2a3a07b@candelatech.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 15.09.22 22:39, Ben Greear wrote:

>  From reading the original patch description, it was to stop an NPE when 
> AP was stopped.  I have been testing
> this patch below and it fixes the problems I saw with multiple vdevs.  I 
> was worried that
> the code in the 'list_for_each_entry_rcu(sta, &local->sta_list, list) {' 
> might still need to run
> to keep everything in sync (and my patch allows that to happen), but I 
> do not know if that is true or not.
> 

I'm not sure if it's still save to wake the queues for the vif we are 
tearing down and assumed the intend was to skip those, too.

But it looks like all stations for the vif are deleted prior to setting 
sdata->bss = NULL, so the outcome should be the same.

Your solution removes potentially set IEEE80211_TXQ_STOP_NETIF_TX flags, 
reducing the risk that a queue restart during vif setup ends up with 
inoperable queues.
But the only way to set IEEE80211_TXQ_STOP_NETIF_TX seems to be during 
ieee80211_tx_dequeue(). Which should not be possible to be called as 
long as SDATA_STATE_RUNNING was never set for the vif.

But I'm on thin ice here:-)

Alexander