Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 12 Jun 2019 12:34:57 -0400
From:   Bob Copeland <me@bobcopeland.com>
To:     Sven Eckelmann <seckelmann@datto.com>
Cc:     ath11k@lists.infradead.org, linux-wireless@vger.kernel.org
Subject: Re: [RFC PATCH v2 3/3] ath11k: register HE mesh capabilities
Message-ID: <20190612163457.GB4090@localhost>
References: <20190611180247.19524-1-sven@narfation.org>
 <20190611180247.19524-4-sven@narfation.org>
 <1919330.hVZVHELXip@bentobox>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1919330.hVZVHELXip@bentobox>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Tue, Jun 11, 2019 at 09:52:20PM +0200, Sven Eckelmann wrote:
> On Tuesday, 11 June 2019 20:02:47 CEST Sven Eckelmann wrote:
> [...]
> > ---
> > This doesn't work currently as expected. No HE rates are used between
> > the two HE mesh peers:
> [...]
> 
> There seems to be also an ordering problem. ath11k_peer_assoc_h_he is only 
> called before ieee80211_he_cap_ie_to_sta_he_cap is called. So ath11k_bss_assoc 
> will not have the information whether the remote has HE support or not.
> 
> Looks like I have adjust mesh_sta_info_init to get this somehow to 
> ath11k_peer_assoc_h_he. Maybe through ath11k_sta_rc_update but this is not 
> called by mesh_sta_info_init at the moment. Just because 
> rate_control_rate_init is called and not rate_control_rate_update.
> 
> The easiest method seems to adjust the check at the end of mesh_sta_info_init 
> to 
> 
> 	if (!test_sta_flag(sta, WLAN_STA_RATE_CONTROL) &&
> 	    !ieee80211_hw_check(&local->hw, HAS_RATE_CONTROL)) {
> 		rate_control_rate_init(sta);
> 	} else {
> 		rate_control_rate_update(local, sband, sta, changed);
> 	}

Maybe we should just do this?

diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index 76f303fda3ed..6f8bde840bb9 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -42,7 +42,7 @@ void rate_control_rate_init(struct sta_info *sta)
 	ieee80211_sta_set_rx_nss(sta);
 
 	if (!ref)
-		return;
+		goto out;
 
 	rcu_read_lock();
 
@@ -59,6 +59,7 @@ void rate_control_rate_init(struct sta_info *sta)
 			    priv_sta);
 	spin_unlock_bh(&sta->rate_ctrl_lock);
 	rcu_read_unlock();
+out:
 	set_sta_flag(sta, WLAN_STA_RATE_CONTROL);
 }


That was my intent, anyway -- that NSS always got set before
rate_control_rate_update() even if using HW rate control.

> 	if (!test_sta_flag(sta, WLAN_STA_RATE_CONTROL))
> 		rate_control_rate_init(sta);
> 
>    /* inform drivers about changes */
> 	rate_control_rate_update(local, sband, sta, changed);
> 
> Both will at least cause a call to ath11k_peer_assoc_prepare + 
> ath11k_wmi_send_peer_assoc_cmd but unfortunately the ath11k firmware hangs 
> afterwards.

I think this would be OK.

-- 
Bob Copeland %% https://bobcopeland.com/